Otherwise `if_snd' could still contain packets. If any of them has
associated data with bumped reference counter, the corresponding
refcnt_finalize() will sleep forever. I don't think we need to
transmit these packets when interface became running again.
I don't like this unlocked if_flags check we have in ifq_start_task().
Guess READ_ONCE() is much better to load value.
The problem reported in the "wg destroy hangs" [1] thread is the kind of
described above. Also, current ifq_start_task() implementation denies to
introduce reference counters for wg_peer and remove this
wg_peer_destroy() hack:
NET_LOCK();
while (!ifq_empty(&sc->sc_if.if_snd)) {
NET_UNLOCK();
tsleep_nsec(sc, PWAIT, "wg_ifq", 1000);
NET_LOCK();
}
NET_UNLOCK();
1. https://marc.info/?t=169642396300001&r=1&w=2
ok?
Index: sys/net/ifq.c
===================================================================
RCS file: /cvs/src/sys/net/ifq.c,v
retrieving revision 1.51
diff -u -p -r1.51 ifq.c
--- sys/net/ifq.c 5 Oct 2023 11:08:56 -0000 1.51
+++ sys/net/ifq.c 5 Oct 2023 11:49:40 -0000
@@ -140,8 +140,11 @@ ifq_start_task(void *p)
struct ifqueue *ifq = p;
struct ifnet *ifp = ifq->ifq_if;
- if (!ISSET(ifp->if_flags, IFF_RUNNING) ||
- ifq_empty(ifq) || ifq_is_oactive(ifq))
+ if (!ISSET(READ_ONCE(ifp->if_flags), IFF_RUNNING)){
+ ifq_purge(ifq);
+ return;
+ }
+ if (ifq_empty(ifq) || ifq_is_oactive(ifq))
return;
ifp->if_qstart(ifq);
