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

2019-01-09 Thread 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 -  1.315
+++ net/if_bridge.c 9 Jan 2019 18:10:45 -
@@ -108,6 +108,8 @@ voidbridgeattach(int);
 intbridge_ioctl(struct ifnet *, u_long, caddr_t);
 void   bridge_ifdetach(void *);
 void   bridge_spandetach(void *);
+intbridge_ifremove(struct bridge_iflist *);
+intbridge_spanremove(struct bridge_iflist *);
 intbridge_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, &s

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

2019-01-09 Thread 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 -  1.315
> +++ net/if_bridge.c   12 Dec 2018 14:27:21 -
> @@ -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)
> 

Convert bridge span & int. list to SLIST()

2018-12-23 Thread 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 -  1.315
+++ net/if_bridge.c 12 Dec 2018 14:27:21 -
@@ -108,6 +108,8 @@ voidbridgeattach(int);
 intbridge_ioctl(struct ifnet *, u_long, caddr_t);
 void   bridge_ifdetach(void *);
 void   bridge_spandetach(void *);
+intbridge_ifremove(struct bridge_iflist *);
+intbridge_spanremove(struct bridge_iflist *);
 intbridge_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) {
-