On 27/05/17(Sat) 17:33, Alexandr Nedvedicky wrote:
> Hello,
>
> On Fri, May 26, 2017 at 04:54:57PM +0200, Martin Pieuchot wrote:
> > The global list of softc is *not* accessed in the input path, so it
> > doesn't need splnet().
> >
> > ioctl(2) handlers are already executed with the NET_LOCK() held, so
> > splnet() is superfluous.
>
> changes look good to me, but still have a question:
>
> what is the plan for splnet()/splx() in trunk_init() and trunk_stop()
> functions. It seems to me they are executed through ioctl(2) only,
> so we can also let them go. or you want to leave it for another day?
You're right! Plus these functions are only present for lacp and just
schedule a timeout.
So we can get rid of them as well. Updated diff below.
Index: net/if_trunk.c
===================================================================
RCS file: /cvs/src/sys/net/if_trunk.c,v
retrieving revision 1.129
diff -u -p -r1.129 if_trunk.c
--- net/if_trunk.c 22 Jan 2017 10:17:39 -0000 1.129
+++ net/if_trunk.c 27 May 2017 18:43:57 -0000
@@ -210,19 +210,15 @@ trunk_clone_destroy(struct ifnet *ifp)
{
struct trunk_softc *tr = (struct trunk_softc *)ifp->if_softc;
struct trunk_port *tp;
- int error, s;
+ int error;
/* Remove any multicast groups that we may have joined. */
trunk_ether_purgemulti(tr);
- s = splnet();
-
/* Shutdown and remove trunk ports, return on error */
while ((tp = SLIST_FIRST(&tr->tr_ports)) != NULL) {
- if ((error = trunk_port_destroy(tp)) != 0) {
- splx(s);
+ if ((error = trunk_port_destroy(tp)) != 0)
return (error);
- }
}
ifmedia_delete_instance(&tr->tr_media, IFM_INST_ANY);
@@ -232,8 +228,6 @@ trunk_clone_destroy(struct ifnet *ifp)
SLIST_REMOVE(&trunk_list, tr, trunk_softc, tr_entries);
free(tr, M_DEVBUF, sizeof *tr);
- splx(s);
-
return (0);
}
@@ -474,9 +468,7 @@ trunk_port_ioctl(struct ifnet *ifp, u_lo
struct trunk_reqport *rp = (struct trunk_reqport *)data;
struct trunk_softc *tr;
struct trunk_port *tp = NULL;
- int s, error = 0;
-
- s = splnet();
+ int error = 0;
/* Should be checked by the caller */
if (ifp->if_type != IFT_IEEE8023ADLAG ||
@@ -512,12 +504,9 @@ trunk_port_ioctl(struct ifnet *ifp, u_lo
goto fallback;
}
- splx(s);
return (error);
fallback:
- splx(s);
-
if (tp != NULL)
error = (*tp->tp_ioctl)(ifp, cmd, data);
@@ -616,9 +605,7 @@ trunk_ioctl(struct ifnet *ifp, u_long cm
struct ifreq *ifr = (struct ifreq *)data;
struct trunk_port *tp;
struct ifnet *tpif;
- int s, i, error = 0;
-
- s = splnet();
+ int i, error = 0;
bzero(&rpbuf, sizeof(rpbuf));
@@ -768,7 +755,6 @@ trunk_ioctl(struct ifnet *ifp, u_long cm
}
out:
- splx(s);
return (error);
}
@@ -1014,32 +1000,22 @@ void
trunk_init(struct ifnet *ifp)
{
struct trunk_softc *tr = (struct trunk_softc *)ifp->if_softc;
- int s;
-
- s = splnet();
ifp->if_flags |= IFF_RUNNING;
if (tr->tr_init != NULL)
(*tr->tr_init)(tr);
-
- splx(s);
}
void
trunk_stop(struct ifnet *ifp)
{
struct trunk_softc *tr = (struct trunk_softc *)ifp->if_softc;
- int s;
-
- s = splnet();
ifp->if_flags &= ~IFF_RUNNING;
if (tr->tr_stop != NULL)
(*tr->tr_stop)(tr);
-
- splx(s);
}
int
