Hello, > > I think the diff below may be enough to fix this issue. It drops the SMR > critical secition around the enqueue operation but uses a reference on the > port insteadt to ensure that the device can't be removed during the > enqueue. Once the enqueue is finished we enter the SMR critical section > again and drop the reference. > > To make it clear that the SMR_TAILQ remains intact while a refcount is > held I moved refcnt_finalize() above SMR_TAILQ_REMOVE_LOCKED(). This is > not strictly needed since the next pointer remains valid up until the > smr_barrier() call but I find this a bit easier to understand. > First make sure nobody else holds a reference then remove the port from > the list. > > I currently have no test setup to verify this but I hope someone else can > give this a spin.
I agree this diff is far better than mine version. I have not realized we can actually do a dance with smr_read_enter()/smr_read_leave(). I believe it should work and fix a problem. Here is my OK (if my OK counts here). OK sashan > -- > :wq Claudio > > Index: if_veb.c > =================================================================== > RCS file: /cvs/src/sys/net/if_veb.c,v > retrieving revision 1.25 > diff -u -p -r1.25 if_veb.c > --- if_veb.c 4 Jan 2022 06:32:39 -0000 1.25 > +++ if_veb.c 12 May 2022 12:06:28 -0000 > @@ -365,7 +365,11 @@ veb_span(struct veb_softc *sc, struct mb > continue; > } > > + veb_eb_brport_take(p); > + smr_read_leave(); > if_enqueue(ifp0, m); /* XXX count error */ > + smr_read_enter(); > + veb_eb_brport_rele(p); > } > smr_read_leave(); > } > @@ -904,7 +908,12 @@ veb_broadcast(struct veb_softc *sc, stru > continue; > } > > + > + veb_eb_brport_take(tp); > + smr_read_leave(); > (*tp->p_enqueue)(ifp0, m); /* XXX count error */ > + smr_read_enter(); > + veb_eb_brport_rele(tp); > } > smr_read_leave(); > > @@ -1934,10 +1943,11 @@ veb_p_dtor(struct veb_softc *sc, struct > > port_list = &sc->sc_ports; > } > + refcnt_finalize(&p->p_refs, "vebpdtor"); > + > SMR_TAILQ_REMOVE_LOCKED(&port_list->l_list, p, p_entry); > port_list->l_count--; > > - refcnt_finalize(&p->p_refs, "vebpdtor"); > smr_barrier(); > > veb_rule_list_free(TAILQ_FIRST(&p->p_vrl));