On 23/12/18(Sun) 14:41, Martin Pieuchot wrote:
> Using SLIST() instead of TAILQ() is a step towards using lock-less
> lists.
>
> As found in my previous attempts the interface list cannot be protected
> by a mutex as long a if_enqueue() might grab the KERNEL_LOCK(). So I'm
> heading towards using SRPL_*().
>
> Ok?
Anyone?
> Index: net/if_bridge.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if_bridge.c,v
> retrieving revision 1.315
> diff -u -p -r1.315 if_bridge.c
> --- net/if_bridge.c 12 Dec 2018 14:19:15 -0000 1.315
> +++ net/if_bridge.c 12 Dec 2018 14:27:21 -0000
> @@ -108,6 +108,8 @@ void bridgeattach(int);
> int bridge_ioctl(struct ifnet *, u_long, caddr_t);
> void bridge_ifdetach(void *);
> void bridge_spandetach(void *);
> +int bridge_ifremove(struct bridge_iflist *);
> +int bridge_spanremove(struct bridge_iflist *);
> int bridge_input(struct ifnet *, struct mbuf *, void *);
> void bridge_process(struct ifnet *, struct mbuf *);
> void bridgeintr_frame(struct bridge_softc *, struct ifnet *, struct mbuf *);
> @@ -170,8 +172,8 @@ bridge_clone_create(struct if_clone *ifc
> sc->sc_brtmax = BRIDGE_RTABLE_MAX;
> sc->sc_brttimeout = BRIDGE_RTABLE_TIMEOUT;
> timeout_set(&sc->sc_brtimeout, bridge_rtage, sc);
> - TAILQ_INIT(&sc->sc_iflist);
> - TAILQ_INIT(&sc->sc_spanlist);
> + SLIST_INIT(&sc->sc_iflist);
> + SLIST_INIT(&sc->sc_spanlist);
> for (i = 0; i < BRIDGE_RTABLE_SIZE; i++)
> LIST_INIT(&sc->sc_rts[i]);
> arc4random_buf(&sc->sc_hashkey, sizeof(sc->sc_hashkey));
> @@ -216,10 +218,14 @@ bridge_clone_destroy(struct ifnet *ifp)
>
> bridge_stop(sc);
> bridge_rtflush(sc, IFBF_FLUSHALL);
> - while ((bif = TAILQ_FIRST(&sc->sc_iflist)) != NULL)
> + while ((bif = SLIST_FIRST(&sc->sc_iflist)) != NULL) {
> + SLIST_REMOVE_HEAD(&sc->sc_iflist, bif_next);
> bridge_delete(sc, bif);
> - while ((bif = TAILQ_FIRST(&sc->sc_spanlist)) != NULL)
> + }
> + while ((bif = SLIST_FIRST(&sc->sc_spanlist)) != NULL) {
> + SLIST_REMOVE_HEAD(&sc->sc_spanlist, bif_next);
> bridge_spandetach(bif);
> + }
>
> bstp_destroy(sc->sc_stp);
>
> @@ -249,7 +255,6 @@ bridge_delete(struct bridge_softc *sc, s
> hook_disestablish(bif->ifp->if_detachhooks, bif->bif_dhcookie);
>
> if_ih_remove(bif->ifp, bridge_input, NULL);
> - TAILQ_REMOVE(&sc->sc_iflist, bif, next);
> bridge_rtdelete(sc, bif->ifp, 0);
> bridge_flushrule(bif);
> free(bif, M_DEVBUF, sizeof *bif);
> @@ -297,7 +302,7 @@ bridge_ioctl(struct ifnet *ifp, u_long c
> }
>
> /* If it's in the span list, it can't be a member. */
> - TAILQ_FOREACH(bif, &sc->sc_spanlist, next) {
> + SLIST_FOREACH(bif, &sc->sc_spanlist, bif_next) {
> if (bif->ifp == ifs)
> break;
> }
> @@ -338,7 +343,7 @@ bridge_ioctl(struct ifnet *ifp, u_long c
> bif->bif_dhcookie = hook_establish(ifs->if_detachhooks, 0,
> bridge_ifdetach, bif);
> if_ih_insert(bif->ifp, bridge_input, NULL);
> - TAILQ_INSERT_TAIL(&sc->sc_iflist, bif, next);
> + SLIST_INSERT_HEAD(&sc->sc_iflist, bif, bif_next);
> break;
> case SIOCBRDGDEL:
> if ((error = suser(curproc)) != 0)
> @@ -353,7 +358,7 @@ bridge_ioctl(struct ifnet *ifp, u_long c
> error = ESRCH;
> break;
> }
> - error = bridge_delete(sc, bif);
> + error = bridge_ifremove(bif);
> break;
> case SIOCBRDGIFS:
> error = bridge_bifconf(sc, (struct ifbifconf *)data);
> @@ -375,7 +380,7 @@ bridge_ioctl(struct ifnet *ifp, u_long c
> error = EBUSY;
> break;
> }
> - TAILQ_FOREACH(bif, &sc->sc_spanlist, next) {
> + SLIST_FOREACH(bif, &sc->sc_spanlist, bif_next) {
> if (bif->ifp == ifs)
> break;
> }
> @@ -395,7 +400,7 @@ bridge_ioctl(struct ifnet *ifp, u_long c
> bridge_spandetach, bif);
> SIMPLEQ_INIT(&bif->bif_brlin);
> SIMPLEQ_INIT(&bif->bif_brlout);
> - TAILQ_INSERT_TAIL(&sc->sc_spanlist, bif, next);
> + SLIST_INSERT_HEAD(&sc->sc_spanlist, bif, bif_next);
> break;
> case SIOCBRDGDELS:
> if ((error = suser(curproc)) != 0)
> @@ -405,15 +410,15 @@ bridge_ioctl(struct ifnet *ifp, u_long c
> error = ENOENT;
> break;
> }
> - TAILQ_FOREACH(bif, &sc->sc_spanlist, next) {
> + SLIST_FOREACH(bif, &sc->sc_spanlist, bif_next) {
> if (bif->ifp == ifs)
> break;
> }
> if (bif == NULL) {
> - error = ENOENT;
> + error = ESRCH;
> break;
> }
> - bridge_spandetach(bif);
> + error = bridge_spanremove(bif);
> break;
> case SIOCBRDGGIFFLGS:
> ifs = ifunit(req->ifbr_ifsname);
> @@ -570,24 +575,66 @@ bridge_ioctl(struct ifnet *ifp, u_long c
> }
>
> /* Detach an interface from a bridge. */
> +int
> +bridge_ifremove(struct bridge_iflist *bif)
> +{
> + struct bridge_softc *sc = bif->bridge_sc;
> +
> + if (SLIST_FIRST(&sc->sc_iflist) == bif) {
> + SLIST_REMOVE_HEAD(&sc->sc_iflist, bif_next);
> + } else {
> + struct bridge_iflist *pbif;
> +
> + SLIST_FOREACH(pbif, &sc->sc_iflist, bif_next) {
> + if (SLIST_NEXT(pbif, bif_next) == bif)
> + break;
> + }
> + if (pbif == NULL)
> + return (ENOENT);
> + SLIST_REMOVE_AFTER(pbif, bif_next);
> + }
> + return (bridge_delete(sc, bif));
> +}
> +
> +int
> +bridge_spanremove(struct bridge_iflist *bif)
> +{
> + struct bridge_softc *sc = bif->bridge_sc;
> +
> + if (SLIST_FIRST(&sc->sc_spanlist) == bif) {
> + SLIST_REMOVE_HEAD(&sc->sc_spanlist, bif_next);
> + } else {
> + struct bridge_iflist *pbif;
> +
> + SLIST_FOREACH(pbif, &sc->sc_spanlist, bif_next) {
> + if (SLIST_NEXT(pbif, bif_next) == bif)
> + break;
> + }
> + if (pbif == NULL)
> + return (ENOENT);
> + SLIST_REMOVE_AFTER(pbif, bif_next);
> + }
> +
> + hook_disestablish(bif->ifp->if_detachhooks, bif->bif_dhcookie);
> + free(bif, M_DEVBUF, sizeof(*bif));
> +
> + return (0);
> +}
> +
> void
> bridge_ifdetach(void *xbif)
> {
> struct bridge_iflist *bif = xbif;
> - struct bridge_softc *sc = bif->bridge_sc;
>
> - bridge_delete(sc, bif);
> + bridge_ifremove(bif);
> }
>
> void
> bridge_spandetach(void *xbif)
> {
> struct bridge_iflist *bif = xbif;
> - struct bridge_softc *sc = bif->bridge_sc;
>
> - hook_disestablish(bif->ifp->if_detachhooks, bif->bif_dhcookie);
> - TAILQ_REMOVE(&sc->sc_spanlist, bif, next);
> - free(bif, M_DEVBUF, sizeof(*bif));
> + bridge_spanremove(bif);
> }
>
> int
> @@ -600,10 +647,10 @@ bridge_bifconf(struct bridge_softc *sc,
> int error = 0;
> struct ifbreq *breq, *breqs = NULL;
>
> - TAILQ_FOREACH(bif, &sc->sc_iflist, next)
> + SLIST_FOREACH(bif, &sc->sc_iflist, bif_next)
> total++;
>
> - TAILQ_FOREACH(bif, &sc->sc_spanlist, next)
> + SLIST_FOREACH(bif, &sc->sc_spanlist, bif_next)
> total++;
>
> if (bifc->ifbic_len == 0) {
> @@ -615,7 +662,7 @@ bridge_bifconf(struct bridge_softc *sc,
> if (breqs == NULL)
> goto done;
>
> - TAILQ_FOREACH(bif, &sc->sc_iflist, next) {
> + SLIST_FOREACH(bif, &sc->sc_iflist, bif_next) {
> if (bifc->ifbic_len < (i + 1) * sizeof(*breqs))
> break;
> breq = &breqs[i];
> @@ -651,7 +698,7 @@ bridge_bifconf(struct bridge_softc *sc,
> }
> i++;
> }
> - TAILQ_FOREACH(bif, &sc->sc_spanlist, next) {
> + SLIST_FOREACH(bif, &sc->sc_spanlist, bif_next) {
> if (bifc->ifbic_len < (i + 1) * sizeof(*breqs))
> break;
> breq = &breqs[i];
> @@ -774,7 +821,7 @@ bridge_output(struct ifnet *ifp, struct
>
> bridge_span(sc, m);
>
> - TAILQ_FOREACH(bif, &sc->sc_iflist, next) {
> + SLIST_FOREACH(bif, &sc->sc_iflist, bif_next) {
> dst_if = bif->ifp;
> if ((dst_if->if_flags & IFF_RUNNING) == 0)
> continue;
> @@ -802,7 +849,7 @@ bridge_output(struct ifnet *ifp, struct
> (m->m_flags & (M_BCAST | M_MCAST)) == 0)
> continue;
>
> - if (TAILQ_NEXT(bif, next) == NULL) {
> + if (SLIST_NEXT(bif, bif_next) == NULL) {
> used = 1;
> mc = m;
> } else {
> @@ -1171,7 +1218,7 @@ bridge_process(struct ifnet *ifp, struct
> * Unicast, make sure it's not for us.
> */
> bif0 = bif;
> - TAILQ_FOREACH(bif, &sc->sc_iflist, next) {
> + SLIST_FOREACH(bif, &sc->sc_iflist, bif_next) {
> if (bif->ifp->if_type != IFT_ETHER)
> continue;
> if (bridge_ourether(bif, eh->ether_dhost)) {
> @@ -1222,7 +1269,7 @@ bridge_broadcast(struct bridge_softc *sc
> bif = (struct bridge_iflist *)ifp->if_bridgeport;
> protected = bif->bif_protected;
>
> - TAILQ_FOREACH(bif, &sc->sc_iflist, next) {
> + SLIST_FOREACH(bif, &sc->sc_iflist, bif_next) {
> dst_if = bif->ifp;
>
> if ((dst_if->if_flags & IFF_RUNNING) == 0)
> @@ -1272,7 +1319,7 @@ bridge_broadcast(struct bridge_softc *sc
> #endif /* NMPW */
>
> /* If last one, reuse the passed-in mbuf */
> - if (TAILQ_NEXT(bif, next) == NULL) {
> + if (SLIST_NEXT(bif, bif_next) == NULL) {
> mc = m;
> used = 1;
> } else {
> @@ -1347,7 +1394,7 @@ bridge_span(struct bridge_softc *sc, str
> struct mbuf *mc;
> int error;
>
> - TAILQ_FOREACH(bif, &sc->sc_spanlist, next) {
> + SLIST_FOREACH(bif, &sc->sc_spanlist, bif_next) {
> ifp = bif->ifp;
>
> if ((ifp->if_flags & IFF_RUNNING) == 0)
> Index: net/if_bridge.h
> ===================================================================
> RCS file: /cvs/src/sys/net/if_bridge.h,v
> retrieving revision 1.58
> diff -u -p -r1.58 if_bridge.h
> --- net/if_bridge.h 7 Dec 2018 16:19:40 -0000 1.58
> +++ net/if_bridge.h 12 Dec 2018 14:26:47 -0000
> @@ -409,7 +409,7 @@ struct bstp_state {
> * Bridge interface list
> */
> struct bridge_iflist {
> - TAILQ_ENTRY(bridge_iflist) next; /* next in list */
> + SLIST_ENTRY(bridge_iflist) bif_next; /* next in list */
> struct bridge_softc *bridge_sc;
> struct bstp_port *bif_stp; /* STP port state */
> struct brl_head bif_brlin; /* input rules */
> @@ -473,8 +473,8 @@ struct bridge_softc {
> u_int64_t sc_hashkey[2]; /* siphash key */
> struct timeout sc_brtimeout; /* timeout state */
> struct bstp_state *sc_stp; /* stp state */
> - TAILQ_HEAD(, bridge_iflist) sc_iflist; /* interface list */
> - TAILQ_HEAD(, bridge_iflist) sc_spanlist; /* span ports */
> + SLIST_HEAD(, bridge_iflist) sc_iflist; /* interface list */
> + SLIST_HEAD(, bridge_iflist) sc_spanlist; /* span ports */
> LIST_HEAD(, bridge_rtnode) sc_rts[BRIDGE_RTABLE_SIZE]; /* hash
> table */
> };
>
>
