On Mon, Nov 28, 2016 at 03:24:56PM +0100, Martin Pieuchot wrote:
> This remove all but one splsoftnet() recursions when creating or
> destroying cloned interfaces.
>
> Note that I have to introduce some extra splsoftnet()/splx() dances in
> tun(4) and tap(4) create functions.
switchopen() also calls if_clone_create(). I think you need the same
dance there.
> ok?
otherwise OK bluhm@
>
> Index: net/if.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if.c,v
> retrieving revision 1.463
> diff -u -p -r1.463 if.c
> --- net/if.c 28 Nov 2016 11:18:02 -0000 1.463
> +++ net/if.c 28 Nov 2016 14:16:03 -0000
> @@ -133,6 +133,8 @@
> void if_attachsetup(struct ifnet *);
> void if_attachdomain(struct ifnet *);
> void if_attach_common(struct ifnet *);
> +int if_setrdomain(struct ifnet *, int);
> +void if_slowtimo(void *);
>
> void if_detached_start(struct ifnet *);
> int if_detached_ioctl(struct ifnet *, u_long, caddr_t);
> @@ -402,6 +404,8 @@ if_attachsetup(struct ifnet *ifp)
> {
> unsigned long ifidx;
>
> + splsoftassert(IPL_SOFTNET);
> +
> TAILQ_INIT(&ifp->if_groups);
>
> if_addgroup(ifp, IFG_ALL);
> @@ -501,17 +505,25 @@ if_attachdomain(struct ifnet *ifp)
> void
> if_attachhead(struct ifnet *ifp)
> {
> + int s;
> +
> + s = splsoftnet();
> if_attach_common(ifp);
> TAILQ_INSERT_HEAD(&ifnet, ifp, if_list);
> if_attachsetup(ifp);
> + splx(s);
> }
>
> void
> if_attach(struct ifnet *ifp)
> {
> + int s;
> +
> + s = splsoftnet();
> if_attach_common(ifp);
> TAILQ_INSERT_TAIL(&ifnet, ifp, if_list);
> if_attachsetup(ifp);
> + splx(s);
> }
>
> void
> @@ -1036,7 +1048,9 @@ if_clone_create(const char *name, int rd
> {
> struct if_clone *ifc;
> struct ifnet *ifp;
> - int unit, ret, s;
> + int unit, ret;
> +
> + splsoftassert(IPL_SOFTNET);
>
> ifc = if_clone_lookup(name, &unit);
> if (ifc == NULL)
> @@ -1045,9 +1059,7 @@ if_clone_create(const char *name, int rd
> if (ifunit(name) != NULL)
> return (EEXIST);
>
> - s = splsoftnet();
> ret = (*ifc->ifc_create)(ifc, unit);
> - splx(s);
>
> if (ret != 0 || (ifp = ifunit(name)) == NULL)
> return (ret);
> @@ -1067,7 +1079,8 @@ if_clone_destroy(const char *name)
> {
> struct if_clone *ifc;
> struct ifnet *ifp;
> - int error, s;
> +
> + splsoftassert(IPL_SOFTNET);
>
> ifc = if_clone_lookup(name, NULL);
> if (ifc == NULL)
> @@ -1081,15 +1094,13 @@ if_clone_destroy(const char *name)
> return (EOPNOTSUPP);
>
> if (ifp->if_flags & IFF_UP) {
> + int s;
> s = splnet();
> if_down(ifp);
> splx(s);
> }
>
> - s = splsoftnet();
> - error = (*ifc->ifc_destroy)(ifp);
> - splx(s);
> - return (error);
> + return ((*ifc->ifc_destroy)(ifp));
> }
>
> /*
> @@ -1594,7 +1605,7 @@ int
> if_setrdomain(struct ifnet *ifp, int rdomain)
> {
> struct ifreq ifr;
> - int s, error;
> + int error;
>
> if (rdomain < 0 || rdomain > RT_TABLEID_MAX)
> return (EINVAL);
> @@ -1618,11 +1629,8 @@ if_setrdomain(struct ifnet *ifp, int rdo
> if (error && (ifp != loifp || error != EEXIST))
> return (error);
>
> -
> - s = splsoftnet();
> if ((error = rtable_add(rdomain)) == 0)
> rtable_l2set(rdomain, rdomain, loifp->if_index);
> - splx(s);
> if (error) {
> if_clone_destroy(loifname);
> return (error);
> @@ -1638,6 +1646,8 @@ if_setrdomain(struct ifnet *ifp, int rdo
> /* remove all routing entries when switching domains */
> /* XXX this is a bit ugly */
> if (rdomain != ifp->if_rdomain) {
> + int s;
> +
> s = splnet();
> /*
> * We are tearing down the world.
> Index: net/if.h
> ===================================================================
> RCS file: /cvs/src/sys/net/if.h,v
> retrieving revision 1.180
> diff -u -p -r1.180 if.h
> --- net/if.h 27 Oct 2016 03:00:35 -0000 1.180
> +++ net/if.h 28 Nov 2016 14:16:03 -0000
> @@ -467,7 +467,6 @@ void if_detach(struct ifnet *);
> void if_down(struct ifnet *);
> void if_downall(void);
> void if_link_state_change(struct ifnet *);
> -void if_slowtimo(void *);
> void if_up(struct ifnet *);
> int ifconf(u_long, caddr_t);
> void ifinit(void);
> @@ -485,7 +484,6 @@ void if_congestion(void);
> int if_congested(void);
> __dead void unhandled_af(int);
> int if_setlladdr(struct ifnet *, const uint8_t *);
> -int if_setrdomain(struct ifnet *, int);
>
> #endif /* _KERNEL */
>
> Index: net/if_tun.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if_tun.c,v
> retrieving revision 1.169
> diff -u -p -r1.169 if_tun.c
> --- net/if_tun.c 4 Sep 2016 15:46:39 -0000 1.169
> +++ net/if_tun.c 28 Nov 2016 14:16:03 -0000
> @@ -312,14 +312,17 @@ int
> tunopen(dev_t dev, int flag, int mode, struct proc *p)
> {
> struct tun_softc *tp;
> - int error;
> + unsigned int rdomain = rtable_l2(p->p_p->ps_rtableid);
>
> if ((tp = tun_lookup(minor(dev))) == NULL) { /* create on demand */
> char xname[IFNAMSIZ];
> + int s, error;
>
> snprintf(xname, sizeof(xname), "%s%d", "tun", minor(dev));
> - if ((error = if_clone_create(xname,
> - rtable_l2(p->p_p->ps_rtableid))) != 0)
> + s = splsoftnet();
> + error = if_clone_create(xname, rdomain);
> + splx(s);
> + if (error != 0)
> return (error);
>
> if ((tp = tun_lookup(minor(dev))) == NULL)
> @@ -334,14 +337,17 @@ int
> tapopen(dev_t dev, int flag, int mode, struct proc *p)
> {
> struct tun_softc *tp;
> - int error;
> + unsigned int rdomain = rtable_l2(p->p_p->ps_rtableid);
>
> if ((tp = tap_lookup(minor(dev))) == NULL) { /* create on demand */
> char xname[IFNAMSIZ];
> + int s, error;
>
> snprintf(xname, sizeof(xname), "%s%d", "tap", minor(dev));
> - if ((error = if_clone_create(xname,
> - rtable_l2(p->p_p->ps_rtableid))) != 0)
> + s = splsoftnet();
> + error = if_clone_create(xname, rdomain);
> + splx(s);
> + if (error != 0)
> return (error);
>
> if ((tp = tap_lookup(minor(dev))) == NULL)
> @@ -403,7 +409,7 @@ tapclose(dev_t dev, int flag, int mode,
> int
> tun_dev_close(struct tun_softc *tp, int flag, int mode, struct proc *p)
> {
> - int s;
> + int s, error = 0;
> struct ifnet *ifp;
>
> ifp = &tp->tun_if;
> @@ -420,14 +426,16 @@ tun_dev_close(struct tun_softc *tp, int
>
> TUNDEBUG(("%s: closed\n", ifp->if_xname));
>
> - if (!(tp->tun_flags & TUN_STAYUP))
> - return (if_clone_destroy(ifp->if_xname));
> - else {
> + if (!(tp->tun_flags & TUN_STAYUP)) {
> + s = splsoftnet();
> + error = if_clone_destroy(ifp->if_xname);
> + splx(s);
> + } else {
> tp->tun_pgid = 0;
> selwakeup(&tp->tun_rsel);
> }
>
> - return (0);
> + return (error);
> }
>
> int
