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));
