replace the hooks used with if_detachhooks with a task list.

the main semantic change is that things registering detach hooks
have to allocate and set a task structure that then gets added to
the list. this means if the task is allocated up front (eg, as part
of carps softc or bridges port structure), it avoids the possibility
that adding a hook can fail. a lot of drivers weren't checking for
failure, and unwinding state in the event of failure in other parts
was error prone.

while doing this i discovered that the list operations have to be
in a particular order, but drivers weren't doing that consistently
either. this diff wraps the list ops up so you have to seriously
go out of your way to screw them up.

ive also sprinkled some NET_ASSERT_LOCKED around the list operations
so we can make sure there's no potential for the list to be corrupted,
especially while it's being run.

hrvoje popovski has tested this a bit, and some issues he discovered
have been fixed.

ok sashan@

1 files changed, 23 insertions, 7 deletions

diff --git a/sys/net/if.c b/sys/net/if.c
index 17397b68831..0f3e07a53d8 100644
--- a/sys/net/if.c
+++ b/sys/net/if.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if.c,v 1.588 2019/08/21 15:32:18 florian Exp $	*/
+/*	$OpenBSD: if.c,v 1.589 2019/11/06 03:51:26 dlg Exp $	*/
 /*	$NetBSD: if.c,v 1.35 1996/05/07 05:26:04 thorpej Exp $	*/
 
 /*
@@ -632,9 +632,7 @@ if_attach_common(struct ifnet *ifp)
 	ifp->if_linkstatehooks = malloc(sizeof(*ifp->if_linkstatehooks),
 	    M_TEMP, M_WAITOK);
 	TAILQ_INIT(ifp->if_linkstatehooks);
-	ifp->if_detachhooks = malloc(sizeof(*ifp->if_detachhooks),
-	    M_TEMP, M_WAITOK);
-	TAILQ_INIT(ifp->if_detachhooks);
+	TAILQ_INIT(&ifp->if_detachhooks);
 
 	if (ifp->if_rtrequest == NULL)
 		ifp->if_rtrequest = if_rtrequest_dummy;
@@ -1046,17 +1044,36 @@ if_netisr(void *unused)
 void
 if_deactivate(struct ifnet *ifp)
 {
-	NET_LOCK();
+	struct task *t, *nt;
+
 	/*
 	 * Call detach hooks from head to tail.  To make sure detach
 	 * hooks are executed in the reverse order they were added, all
 	 * the hooks have to be added to the head!
 	 */
-	dohooks(ifp->if_detachhooks, HOOK_REMOVE | HOOK_FREE);
 
+	NET_LOCK();
+	TAILQ_FOREACH_SAFE(t, &ifp->if_detachhooks, t_entry, nt)
+		(*t->t_func)(t->t_arg);
+
+	KASSERT(TAILQ_EMPTY(&ifp->if_detachhooks));
 	NET_UNLOCK();
 }
 
+void
+if_detachhook_add(struct ifnet *ifp, struct task *t)
+{
+	NET_ASSERT_LOCKED();
+	TAILQ_INSERT_HEAD(&ifp->if_detachhooks, t, t_entry);
+}
+
+void
+if_detachhook_del(struct ifnet *ifp, struct task *t)
+{
+	NET_ASSERT_LOCKED();
+	TAILQ_REMOVE(&ifp->if_detachhooks, t, t_entry);
+}
+
 /*
  * Detach an interface from everything in the kernel.  Also deallocate
  * private resources.
@@ -1132,7 +1149,6 @@ if_detach(struct ifnet *ifp)
 
 	free(ifp->if_addrhooks, M_TEMP, sizeof(*ifp->if_addrhooks));
 	free(ifp->if_linkstatehooks, M_TEMP, sizeof(*ifp->if_linkstatehooks));
-	free(ifp->if_detachhooks, M_TEMP, sizeof(*ifp->if_detachhooks));
 
 	for (i = 0; (dp = domains[i]) != NULL; i++) {
 		if (dp->dom_ifdetach && ifp->if_afdata[dp->dom_family])