convert interface address change hooks to tasks and a task_list.

this follows what's been done for detach and link state hooks, and
makes handling of hooks generally more robust.

address hooks are a bit different to detach/link state hooks in
that there's only a few things that register hooks (carp, pf, vxlan),
but a lot of places to run the hooks (lots of ipv4 and ipv6 address
configuration).

an address hook cookie was in struct pfi_kif, which is part of the
pf abi. rather than break pfctl -sI, this maintains the void * used
for the cookie and uses it to store a task, which is then used as
intended with the new api.

10 files changed, 79 insertions, 59 deletions

diff --git a/sys/net/if.c b/sys/net/if.c
index 9c029556acb..c251abd7283 100644
--- a/sys/net/if.c
+++ b/sys/net/if.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if.c,v 1.591 2019/11/07 08:03:18 dlg Exp $	*/
+/*	$OpenBSD: if.c,v 1.592 2019/11/08 07:16:29 dlg Exp $	*/
 /*	$NetBSD: if.c,v 1.35 1996/05/07 05:26:04 thorpej Exp $	*/
 
 /*
@@ -630,9 +630,7 @@ if_attach_common(struct ifnet *ifp)
 	ifp->if_iqs = ifp->if_rcv.ifiq_ifiqs;
 	ifp->if_niqs = 1;
 
-	ifp->if_addrhooks = malloc(sizeof(*ifp->if_addrhooks),
-	    M_TEMP, M_WAITOK);
-	TAILQ_INIT(ifp->if_addrhooks);
+	TAILQ_INIT(&ifp->if_addrhooks);
 	TAILQ_INIT(&ifp->if_linkstatehooks);
 	TAILQ_INIT(&ifp->if_detachhooks);
 
@@ -1046,19 +1044,18 @@ if_netisr(void *unused)
 void
 if_hooks_run(struct task_list *hooks)
 {
-	struct task *t, *nt, cursor;
+	struct task *t, *nt;
+	struct task cursor = { .t_func = NULL };
 	void (*func)(void *);
 	void *arg;
 
-	/*
-	 * holding the NET_LOCK guarantees that concurrent if_hooks_run
-	 * calls can't happen, and they therefore can't try and call
-	 * each others cursors as actual hooks.
-	 */
-	NET_ASSERT_LOCKED();
-
 	mtx_enter(&if_hooks_mtx);
 	for (t = TAILQ_FIRST(hooks); t != NULL; t = nt) {
+		while (t->t_func == NULL) { /* skip cursors */
+			t = TAILQ_NEXT(t, t_entry);
+			if (t == NULL)
+				break;
+		}
 		func = t->t_func;
 		arg = t->t_arg;
 
@@ -1177,7 +1174,7 @@ if_detach(struct ifnet *ifp)
 		}
 	}
 
-	free(ifp->if_addrhooks, M_TEMP, sizeof(*ifp->if_addrhooks));
+	KASSERT(TAILQ_EMPTY(&ifp->if_addrhooks));
 	KASSERT(TAILQ_EMPTY(&ifp->if_linkstatehooks));
 	KASSERT(TAILQ_EMPTY(&ifp->if_detachhooks));
 
@@ -3100,7 +3097,7 @@ ifnewlladdr(struct ifnet *ifp)
 		ifa = &in6ifa_ifpforlinklocal(ifp, 0)->ia_ifa;
 		if (ifa) {
 			in6_purgeaddr(ifa);
-			dohooks(ifp->if_addrhooks, 0);
+			if_hooks_run(&ifp->if_addrhooks);
 			in6_ifattach(ifp);
 		}
 	}
@@ -3114,6 +3111,28 @@ ifnewlladdr(struct ifnet *ifp)
 	splx(s);
 }
 
+void
+if_addrhook_add(struct ifnet *ifp, struct task *t)
+{
+	mtx_enter(&if_hooks_mtx);
+	TAILQ_INSERT_TAIL(&ifp->if_addrhooks, t, t_entry);
+	mtx_leave(&if_hooks_mtx);
+}
+
+void
+if_addrhook_del(struct ifnet *ifp, struct task *t)
+{
+	mtx_enter(&if_hooks_mtx);
+	TAILQ_REMOVE(&ifp->if_addrhooks, t, t_entry);
+	mtx_leave(&if_hooks_mtx);
+}
+
+void
+if_addrhooks_run(struct ifnet *ifp)
+{
+	if_hooks_run(&ifp->if_addrhooks);
+}
+
 int net_ticks;
 u_int net_livelocks;
 
diff --git a/sys/net/if_pppx.c b/sys/net/if_pppx.c
index 9485e513407..0bd206b1e8d 100644
--- a/sys/net/if_pppx.c
+++ b/sys/net/if_pppx.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_pppx.c,v 1.68 2019/06/24 13:43:19 mpi Exp $ */
+/*	$OpenBSD: if_pppx.c,v 1.69 2019/11/08 07:16:29 dlg Exp $ */
 
 /*
  * Copyright (c) 2010 Claudio Jeker <claudio@openbsd.org>
@@ -919,7 +919,7 @@ pppx_add_session(struct pppx_dev *pxd, struct pipex_session_req *req)
 		printf("pppx: unable to set addresses for %s, error=%d\n",
 		    ifp->if_xname, error);
 	} else {
-		dohooks(ifp->if_addrhooks, 0);
+		if_addrhooks_run(ifp);
 	}
 	rw_enter_write(&pppx_ifs_lk);
 	pxi->pxi_ready = 1;
diff --git a/sys/net/if_spppsubr.c b/sys/net/if_spppsubr.c
index 991f102738c..c4558db8124 100644
--- a/sys/net/if_spppsubr.c
+++ b/sys/net/if_spppsubr.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_spppsubr.c,v 1.179 2019/06/24 21:36:53 kn Exp $	*/
+/*	$OpenBSD: if_spppsubr.c,v 1.180 2019/11/08 07:16:29 dlg Exp $	*/
 /*
  * Synchronous PPP link level subroutines.
  *
@@ -4230,7 +4230,7 @@ sppp_set_ip_addrs(void *arg1)
 			}
 		}
 		if (!(error = in_ifinit(ifp, ifatoia(ifa), &new_sin, 0)))
-			dohooks(ifp->if_addrhooks, 0);
+			if_addrhooks_run(ifp);
 		if (debug && error) {
 			log(LOG_DEBUG, SPP_FMT "sppp_set_ip_addrs: in_ifinit "
 			" failed, error=%d\n", SPP_ARGS(ifp), error);
@@ -4290,7 +4290,7 @@ sppp_clear_ip_addrs(void *arg1)
 			/* replace peer addr in place */
 			dest->sin_addr.s_addr = sp->ipcp.saved_hisaddr;
 		if (!(error = in_ifinit(ifp, ifatoia(ifa), &new_sin, 0)))
-			dohooks(ifp->if_addrhooks, 0);
+			if_addrhooks_run(ifp);
 		if (debug && error) {
 			log(LOG_DEBUG, SPP_FMT "sppp_clear_ip_addrs: in_ifinit "
 			" failed, error=%d\n", SPP_ARGS(ifp), error);
diff --git a/sys/net/if_var.h b/sys/net/if_var.h
index 3787fc54e6d..7f4eeb2192a 100644
--- a/sys/net/if_var.h
+++ b/sys/net/if_var.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_var.h,v 1.102 2019/11/07 07:36:32 dlg Exp $	*/
+/*	$OpenBSD: if_var.h,v 1.103 2019/11/08 07:16:29 dlg Exp $	*/
 /*	$NetBSD: if.h,v 1.23 1996/05/07 02:40:27 thorpej Exp $	*/
 
 /*
@@ -124,7 +124,7 @@ struct ifnet {				/* and the entries */
 	TAILQ_HEAD(, ifaddr) if_addrlist; /* [N] list of addresses per if */
 	TAILQ_HEAD(, ifmaddr) if_maddrlist; /* [N] list of multicast records */
 	TAILQ_HEAD(, ifg_list) if_groups; /* [N] list of groups per if */
-	struct hook_desc_head *if_addrhooks; /* [I] address change callbacks */
+	struct task_list if_addrhooks;	/* [I] address change callbacks */
 	struct task_list if_linkstatehooks; /* [I] link change callbacks*/
 	struct task_list if_detachhooks; /* [I] detach callbacks */
 				/* [I] check or clean routes (+ or -)'d */
@@ -374,10 +374,13 @@ void	if_ih_insert(struct ifnet *, int (*)(struct ifnet *, struct mbuf *,
 void	if_ih_remove(struct ifnet *, int (*)(struct ifnet *, struct mbuf *,
 	    void *), void *);
 
-void	if_detachhook_add(struct ifnet *, struct task *);
-void	if_detachhook_del(struct ifnet *, struct task *);
+void	if_addrhook_add(struct ifnet *, struct task *);
+void	if_addrhook_del(struct ifnet *, struct task *);
+void	if_addrhooks_run(struct ifnet *);
 void	if_linkstatehook_add(struct ifnet *, struct task *);
 void	if_linkstatehook_del(struct ifnet *, struct task *);
+void	if_detachhook_add(struct ifnet *, struct task *);
+void	if_detachhook_del(struct ifnet *, struct task *);
 
 void	if_rxr_livelocked(struct if_rxring *);
 void	if_rxr_init(struct if_rxring *, u_int, u_int);
diff --git a/sys/net/if_vxlan.c b/sys/net/if_vxlan.c
index 1c4899315ee..f82a1ba9ebe 100644
--- a/sys/net/if_vxlan.c
+++ b/sys/net/if_vxlan.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_vxlan.c,v 1.75 2019/11/07 07:36:32 dlg Exp $	*/
+/*	$OpenBSD: if_vxlan.c,v 1.76 2019/11/08 07:16:29 dlg Exp $	*/
 
 /*
  * Copyright (c) 2013 Reyk Floeter <reyk@openbsd.org>
@@ -62,7 +62,7 @@ struct vxlan_softc {
 	struct ifmedia		 sc_media;
 
 	struct ip_moptions	 sc_imo;
-	void			*sc_ahcookie;
+	struct task		 sc_atask;
 	struct task		 sc_ltask;
 	struct task		 sc_dtask;
 
@@ -138,6 +138,7 @@ vxlan_clone_create(struct if_clone *ifc, int unit)
 	sc->sc_vnetid = VXLAN_VNI_UNSET;
 	sc->sc_txhprio = IFQ_TOS2PRIO(IPTOS_PREC_ROUTINE); /* 0 */
 	sc->sc_df = htons(0);
+	task_set(&sc->sc_atask, vxlan_addr_change, sc);
 	task_set(&sc->sc_ltask, vxlan_link_change, sc);
 	task_set(&sc->sc_dtask, vxlan_if_change, sc);
 	task_set(&sc->sc_sendtask, vxlan_send_dispatch, sc);
@@ -216,10 +217,7 @@ vxlan_multicast_cleanup(struct ifnet *ifp)
 
 	mifp = if_get(imo->imo_ifidx);
 	if (mifp != NULL) {
-		if (sc->sc_ahcookie != NULL) {
-			hook_disestablish(mifp->if_addrhooks, sc->sc_ahcookie);
-			sc->sc_ahcookie = NULL;
-		}
+		if_addrhook_del(mifp, &sc->sc_atask);
 		if_linkstatehook_del(mifp, &sc->sc_ltask);
 		if_detachhook_del(mifp, &sc->sc_dtask);
 
@@ -291,12 +289,9 @@ vxlan_multicast_join(struct ifnet *ifp, struct sockaddr *src,
 	 * Use interface hooks to track any changes on the interface
 	 * that is used to send out the tunnel traffic as multicast.
 	 */
+	if_addrhook_add(mifp, &sc->sc_atask);
 	if_linkstatehook_add(mifp, &sc->sc_ltask);
 	if_detachhook_add(mifp, &sc->sc_dtask);
-	if ((sc->sc_ahcookie = hook_establish(mifp->if_addrhooks,
-	    0, vxlan_addr_change, sc)) == NULL)
-		panic("%s: cannot allocate interface hook",
-		    mifp->if_xname);
 
 	return (0);
 }
diff --git a/sys/net/pf_if.c b/sys/net/pf_if.c
index 00684f80093..ba7cff1932a 100644
--- a/sys/net/pf_if.c
+++ b/sys/net/pf_if.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pf_if.c,v 1.97 2019/07/09 11:30:19 yasuoka Exp $ */
+/*	$OpenBSD: pf_if.c,v 1.98 2019/11/08 07:16:29 dlg Exp $ */
 
 /*
  * Copyright 2005 Henning Brauer <henning@openbsd.org>
@@ -235,6 +235,7 @@ void
 pfi_attach_ifnet(struct ifnet *ifp)
 {
 	struct pfi_kif		*kif;
+	struct task		*t;
 
 	pfi_initialize();
 	pfi_update++;
@@ -244,10 +245,10 @@ pfi_attach_ifnet(struct ifnet *ifp)
 	kif->pfik_ifp = ifp;
 	ifp->if_pf_kif = (caddr_t)kif;
 
-	if ((kif->pfik_ah_cookie = hook_establish(ifp->if_addrhooks, 1,
-	    pfi_kifaddr_update, kif)) == NULL)
-		panic("pfi_attach_ifnet: cannot allocate '%s' address hook",
-		    ifp->if_xname);
+	t = malloc(sizeof(*t), PFI_MTYPE, M_WAITOK);
+	task_set(t, pfi_kifaddr_update, kif);
+	if_addrhook_add(ifp, t);
+	kif->pfik_ah_cookie = t;
 
 	pfi_kif_update(kif);
 }
@@ -256,12 +257,14 @@ void
 pfi_detach_ifnet(struct ifnet *ifp)
 {
 	struct pfi_kif		*kif;
+	struct task		*t;
 
 	if ((kif = (struct pfi_kif *)ifp->if_pf_kif) == NULL)
 		return;
 
 	pfi_update++;
-	hook_disestablish(ifp->if_addrhooks, kif->pfik_ah_cookie);
+	t = kif->pfik_ah_cookie;
+	if_addrhook_del(ifp, t);
 	pfi_kif_update(kif);
 
 	kif->pfik_ifp = NULL;
diff --git a/sys/netinet/in.c b/sys/netinet/in.c
index a9a56e2b325..50e922c95a6 100644
--- a/sys/netinet/in.c
+++ b/sys/netinet/in.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: in.c,v 1.165 2019/11/07 13:25:44 bluhm Exp $	*/
+/*	$OpenBSD: in.c,v 1.166 2019/11/08 07:16:29 dlg Exp $	*/
 /*	$NetBSD: in.c,v 1.26 1996/02/13 23:41:39 christos Exp $	*/
 
 /*
@@ -390,7 +390,7 @@ in_ioctl_set_ifaddr(u_long cmd, caddr_t data, struct ifnet *ifp,
 	in_ifscrub(ifp, ia);
 	error = in_ifinit(ifp, ia, sin, newifaddr);
 	if (!error)
-		dohooks(ifp->if_addrhooks, 0);
+		if_addrhooks_run(ifp);
 
 	NET_UNLOCK();
 	return error;
@@ -503,7 +503,7 @@ in_ioctl_change_ifaddr(u_long cmd, caddr_t data, struct ifnet *ifp,
 			if (error)
 				break;
 		}
-		dohooks(ifp->if_addrhooks, 0);
+		if_addrhooks_run(ifp);
 		break;
 	    }
 	case SIOCDIFADDR:
@@ -523,7 +523,7 @@ in_ioctl_change_ifaddr(u_long cmd, caddr_t data, struct ifnet *ifp,
 		 * the scrub but before the other steps?
 		 */
 		in_purgeaddr(&ia->ia_ifa);
-		dohooks(ifp->if_addrhooks, 0);
+		if_addrhooks_run(ifp);
 		break;
 
 	default:
@@ -946,7 +946,7 @@ in_ifdetach(struct ifnet *ifp)
 		if (ifa->ifa_addr->sa_family != AF_INET)
 			continue;
 		in_purgeaddr(ifa);
-		dohooks(ifp->if_addrhooks, 0);
+		if_addrhooks_run(ifp);
 	}
 
 	if (ifp->if_xflags & IFXF_AUTOCONF4)
diff --git a/sys/netinet/ip_carp.c b/sys/netinet/ip_carp.c
index bd6d5465615..7e5bfdf4c9f 100644
--- a/sys/netinet/ip_carp.c
+++ b/sys/netinet/ip_carp.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ip_carp.c,v 1.340 2019/11/07 07:36:32 dlg Exp $	*/
+/*	$OpenBSD: ip_carp.c,v 1.341 2019/11/08 07:16:29 dlg Exp $	*/
 
 /*
  * Copyright (c) 2002 Michael Shalayeff. All rights reserved.
@@ -131,7 +131,7 @@ struct carp_softc {
 	struct arpcom sc_ac;
 #define	sc_if		sc_ac.ac_if
 #define	sc_carpdev	sc_ac.ac_if.if_carpdev
-	void *ah_cookie;
+	struct task sc_atask;
 	struct task sc_ltask;
 	struct task sc_dtask;
 	struct ip_moptions sc_imo;
@@ -808,6 +808,7 @@ carp_clone_create(struct if_clone *ifc, int unit)
 		return (ENOMEM);
 	}
 
+	task_set(&sc->sc_atask, carp_addr_updated, sc);
 	task_set(&sc->sc_ltask, carp_carpdev_state, sc);
 	task_set(&sc->sc_dtask, carpdetach, sc);
 
@@ -841,8 +842,7 @@ carp_clone_create(struct if_clone *ifc, int unit)
 	ifp->if_link_state = LINK_STATE_INVALID;
 
 	/* Hook carp_addr_updated to cope with address and route changes. */
-	sc->ah_cookie = hook_establish(sc->sc_if.if_addrhooks, 0,
-	    carp_addr_updated, sc);
+	if_addrhook_add(&sc->sc_if, &sc->sc_atask);
 
 	return (0);
 }
@@ -893,10 +893,10 @@ carp_clone_destroy(struct ifnet *ifp)
 {
 	struct carp_softc *sc = ifp->if_softc;
 
+	if_addrhook_del(&sc->sc_if, &sc->sc_atask);
+
 	NET_LOCK();
 	carpdetach(sc);
-	if (sc->ah_cookie != NULL)
-		hook_disestablish(sc->sc_if.if_addrhooks, sc->ah_cookie);
 	NET_UNLOCK();
 
 	ether_ifdetach(ifp);
diff --git a/sys/netinet6/in6.c b/sys/netinet6/in6.c
index 7e711a2c5fa..713478335be 100644
--- a/sys/netinet6/in6.c
+++ b/sys/netinet6/in6.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: in6.c,v 1.231 2019/10/22 21:40:12 bluhm Exp $	*/
+/*	$OpenBSD: in6.c,v 1.232 2019/11/08 07:16:29 dlg Exp $	*/
 /*	$KAME: in6.c,v 1.372 2004/06/14 08:14:21 itojun Exp $	*/
 
 /*
@@ -307,7 +307,7 @@ in6_ioctl_change_ifaddr(u_long cmd, caddr_t data, struct ifnet *ifp)
 			break;
 		}
 		in6_purgeaddr(&ia6->ia_ifa);
-		dohooks(ifp->if_addrhooks, 0);
+		if_addrhooks_run(ifp);
 		break;
 
 	case SIOCAIFADDR_IN6:
@@ -366,13 +366,13 @@ in6_ioctl_change_ifaddr(u_long cmd, caddr_t data, struct ifnet *ifp)
 			nd6_dad_start(&ia6->ia_ifa);
 
 		if (!newifaddr) {
-			dohooks(ifp->if_addrhooks, 0);
+			if_addrhooks_run(ifp);
 			break;
 		}
 
 		plen = in6_mask2len(&ia6->ia_prefixmask.sin6_addr, NULL);
 		if ((ifp->if_flags & IFF_LOOPBACK) || plen == 128) {
-			dohooks(ifp->if_addrhooks, 0);
+			if_addrhooks_run(ifp);
 			break;	/* No need to install a connected route. */
 		}
 
@@ -383,7 +383,7 @@ in6_ioctl_change_ifaddr(u_long cmd, caddr_t data, struct ifnet *ifp)
 			in6_purgeaddr(&ia6->ia_ifa);
 			break;
 		}
-		dohooks(ifp->if_addrhooks, 0);
+		if_addrhooks_run(ifp);
 		break;
 	}
 
diff --git a/sys/netinet6/in6_ifattach.c b/sys/netinet6/in6_ifattach.c
index 87aac0e1378..9e3232452de 100644
--- a/sys/netinet6/in6_ifattach.c
+++ b/sys/netinet6/in6_ifattach.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: in6_ifattach.c,v 1.114 2019/08/21 15:32:18 florian Exp $	*/
+/*	$OpenBSD: in6_ifattach.c,v 1.115 2019/11/08 07:16:29 dlg Exp $	*/
 /*	$KAME: in6_ifattach.c,v 1.124 2001/07/18 08:32:51 jinmei Exp $	*/
 
 /*
@@ -289,7 +289,7 @@ in6_ifattach_linklocal(struct ifnet *ifp, struct in6_addr *ifid)
 		nd6_dad_start(&ia6->ia_ifa);
 
 	if (ifp->if_flags & IFF_LOOPBACK) {
-		dohooks(ifp->if_addrhooks, 0);
+		if_addrhooks_run(ifp);
 		return (0); /* No need to install a connected route. */
 	}
 
@@ -303,7 +303,7 @@ in6_ifattach_linklocal(struct ifnet *ifp, struct in6_addr *ifid)
 		in6_purgeaddr(&ia6->ia_ifa);
 		return (error);
 	}
-	dohooks(ifp->if_addrhooks, 0);
+	if_addrhooks_run(ifp);
 
 	return (0);
 }
@@ -419,7 +419,7 @@ in6_ifdetach(struct ifnet *ifp)
 		if (ifa->ifa_addr->sa_family != AF_INET6)
 			continue;
 		in6_purgeaddr(ifa);
-		dohooks(ifp->if_addrhooks, 0);
+		if_addrhooks_run(ifp);
 	}
 
 	/*