Convert bridge span & int. list to SLIST()

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Convert bridge span & int. list to SLIST()

Martin Pieuchot
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?

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 */
 };
 

Reply | Threaded
Open this post in threaded view
|

Re: Convert bridge span & int. list to SLIST()

Martin Pieuchot
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 */
>  };
>  
>

Reply | Threaded
Open this post in threaded view
|

Re: Convert bridge span & int. list to SLIST()

Martin Pieuchot
On 09/01/19(Wed) 10:11, Martin Pieuchot wrote:
> 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_*().

Revised diff that plugs a leak and use SLIST_REMOVE() instead of
rerolling it, as pointed out by visa@.

ok?

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 9 Jan 2019 18:10:45 -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,12 @@ 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)
- bridge_spandetach(bif);
+ }
+ while ((bif = SLIST_FIRST(&sc->sc_spanlist)) != NULL)
+ bridge_spanremove(bif);
 
  bstp_destroy(sc->sc_stp);
 
@@ -249,7 +253,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 +300,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 +341,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 +356,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 +378,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 +398,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 +408,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 +573,41 @@ 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;
+
+ SLIST_REMOVE(&sc->sc_iflist, bif, bridge_iflist, bif_next);
+ return (bridge_delete(sc, bif));
+}
+
+int
+bridge_spanremove(struct bridge_iflist *bif)
+{
+ struct bridge_softc *sc = bif->bridge_sc;
+
+ SLIST_REMOVE(&sc->sc_spanlist, bif, bridge_iflist, 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 +620,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 +635,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 +671,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 +794,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 +822,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 +1191,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 +1242,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 +1292,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 +1367,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 9 Jan 2019 17:36:42 -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 */
 };