On Wed, Dec 27, 2017 at 04:55:49PM +0100, Martin Pieuchot wrote:
> Updated diff below.
OK bluhm@
> Index: net/if.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if.c,v
> retrieving revision 1.531
> diff -u -p -r1.531 if.c
> --- net/if.c 15 Dec 2017 01:37:30 -0000 1.531
> +++ net/if.c 27 Dec 2017 15:54:18 -0000
> @@ -1854,13 +1854,12 @@ ifioctl(struct socket *so, u_long cmd, c
> oif_flags = ifp->if_flags;
> oif_xflags = ifp->if_xflags;
>
> - NET_LOCK();
> -
> switch (cmd) {
> case SIOCIFAFATTACH:
> case SIOCIFAFDETACH:
> if ((error = suser(p, 0)) != 0)
> break;
> + NET_LOCK();
> switch (ifar->ifar_af) {
> case AF_INET:
> /* attach is a noop for AF_INET */
> @@ -1878,22 +1877,21 @@ ifioctl(struct socket *so, u_long cmd, c
> default:
> error = EAFNOSUPPORT;
> }
> + NET_UNLOCK();
> break;
>
> case SIOCSIFFLAGS:
> if ((error = suser(p, 0)) != 0)
> break;
>
> + NET_LOCK();
> ifp->if_flags = (ifp->if_flags & IFF_CANTCHANGE) |
> (ifr->ifr_flags & ~IFF_CANTCHANGE);
>
> error = (*ifp->if_ioctl)(ifp, cmd, data);
> if (error != 0) {
> ifp->if_flags = oif_flags;
> - break;
> - }
> -
> - if (ISSET(oif_flags ^ ifp->if_flags, IFF_UP)) {
> + } else if (ISSET(oif_flags ^ ifp->if_flags, IFF_UP)) {
> s = splnet();
> if (ISSET(ifp->if_flags, IFF_UP))
> if_up(ifp);
> @@ -1901,17 +1899,21 @@ ifioctl(struct socket *so, u_long cmd, c
> if_down(ifp);
> splx(s);
> }
> + NET_UNLOCK();
> break;
>
> case SIOCSIFXFLAGS:
> if ((error = suser(p, 0)) != 0)
> break;
>
> + NET_LOCK();
> #ifdef INET6
> if (ISSET(ifr->ifr_flags, IFXF_AUTOCONF6)) {
> error = in6_ifattach(ifp);
> - if (error != 0)
> + if (error != 0) {
> + NET_UNLOCK();
> break;
> + }
> }
> #endif /* INET6 */
>
> @@ -1942,8 +1944,6 @@ ifioctl(struct socket *so, u_long cmd, c
> ifp->if_xflags |= IFXF_WOL;
> error = ifp->if_wol(ifp, 1);
> splx(s);
> - if (error)
> - break;
> }
> if (ISSET(ifp->if_xflags, IFXF_WOL) &&
> !ISSET(ifr->ifr_flags, IFXF_WOL)) {
> @@ -1951,8 +1951,6 @@ ifioctl(struct socket *so, u_long cmd, c
> ifp->if_xflags &= ~IFXF_WOL;
> error = ifp->if_wol(ifp, 0);
> splx(s);
> - if (error)
> - break;
> }
> } else if (ISSET(ifr->ifr_flags, IFXF_WOL)) {
> ifr->ifr_flags &= ~IFXF_WOL;
> @@ -1960,20 +1958,26 @@ ifioctl(struct socket *so, u_long cmd, c
> }
> #endif
>
> - ifp->if_xflags = (ifp->if_xflags & IFXF_CANTCHANGE) |
> - (ifr->ifr_flags & ~IFXF_CANTCHANGE);
> + if (error == 0)
> + ifp->if_xflags = (ifp->if_xflags & IFXF_CANTCHANGE) |
> + (ifr->ifr_flags & ~IFXF_CANTCHANGE);
> + NET_UNLOCK();
> break;
>
> case SIOCSIFMETRIC:
> if ((error = suser(p, 0)) != 0)
> break;
> + NET_LOCK();
> ifp->if_metric = ifr->ifr_metric;
> + NET_UNLOCK();
> break;
>
> case SIOCSIFMTU:
> if ((error = suser(p, 0)) != 0)
> break;
> + NET_LOCK();
> error = (*ifp->if_ioctl)(ifp, cmd, data);
> + NET_UNLOCK();
> if (!error)
> rtm_ifchg(ifp);
> break;
> @@ -2013,27 +2017,34 @@ ifioctl(struct socket *so, u_long cmd, c
> case SIOCSIFRDOMAIN:
> if ((error = suser(p, 0)) != 0)
> break;
> + NET_LOCK();
> error = if_setrdomain(ifp, ifr->ifr_rdomainid);
> + NET_UNLOCK();
> break;
>
> case SIOCAIFGROUP:
> if ((error = suser(p, 0)))
> break;
> - if ((error = if_addgroup(ifp, ifgr->ifgr_group)))
> - break;
> - error = (*ifp->if_ioctl)(ifp, cmd, data);
> - if (error == ENOTTY)
> - error = 0;
> + NET_LOCK();
> + error = if_addgroup(ifp, ifgr->ifgr_group);
> + if (error == 0) {
> + error = (*ifp->if_ioctl)(ifp, cmd, data);
> + if (error == ENOTTY)
> + error = 0;
> + }
> + NET_UNLOCK();
> break;
>
> case SIOCDIFGROUP:
> if ((error = suser(p, 0)))
> break;
> + NET_LOCK();
> error = (*ifp->if_ioctl)(ifp, cmd, data);
> if (error == ENOTTY)
> error = 0;
> if (error == 0)
> error = if_delgroup(ifp, ifgr->ifgr_group);
> + NET_UNLOCK();
> break;
>
> case SIOCSIFLLADDR:
> @@ -2045,6 +2056,7 @@ ifioctl(struct socket *so, u_long cmd, c
> error = EINVAL;
> break;
> }
> + NET_LOCK();
> switch (ifp->if_type) {
> case IFT_ETHER:
> case IFT_CARP:
> @@ -2063,6 +2075,7 @@ ifioctl(struct socket *so, u_long cmd, c
>
> if (error == 0)
> ifnewlladdr(ifp);
> + NET_UNLOCK();
> break;
>
> case SIOCSIFLLPRIO:
> @@ -2072,7 +2085,9 @@ ifioctl(struct socket *so, u_long cmd, c
> error = EINVAL;
> break;
> }
> + NET_LOCK();
> ifp->if_llprio = ifr->ifr_llprio;
> + NET_UNLOCK();
> break;
>
> case SIOCDIFPHYADDR:
> @@ -2090,11 +2105,13 @@ ifioctl(struct socket *so, u_long cmd, c
> break;
> /* FALLTHROUGH */
> default:
> + NET_LOCK();
> error = ((*so->so_proto->pr_usrreq)(so, PRU_CONTROL,
> (struct mbuf *) cmd, (struct mbuf *) data,
> (struct mbuf *) ifp, p));
> if (error == EOPNOTSUPP)
> error = ((*ifp->if_ioctl)(ifp, cmd, data));
> + NET_UNLOCK();
> break;
> }
>
> @@ -2103,8 +2120,6 @@ ifioctl(struct socket *so, u_long cmd, c
>
> if (((oif_flags ^ ifp->if_flags) & IFF_UP) != 0)
> getmicrotime(&ifp->if_lastchange);
> -
> - NET_UNLOCK();
>
> return (error);
> }
> Index: net/if_var.h
> ===================================================================
> RCS file: /cvs/src/sys/net/if_var.h,v
> retrieving revision 1.85
> diff -u -p -r1.85 if_var.h
> --- net/if_var.h 15 Dec 2017 01:37:30 -0000 1.85
> +++ net/if_var.h 27 Dec 2017 15:39:36 -0000
> @@ -91,7 +91,14 @@ struct if_clone {
>
> #define IF_CLONE_INITIALIZER(name, create, destroy)
> \
> { { 0 }, name, sizeof(name) - 1, create, destroy }
> -
> +/*
> + * Locks used to protect struct members in this file:
> + * I immutable after creation
> + * d protection left do the driver
> + * c only used in ioctl or routing socket contexts (kernel lock)
> + * k kernel lock
> + * N net lock
> + */
> /*
> * Structure defining a queue for a network interface.
> *
> @@ -102,17 +109,17 @@ TAILQ_HEAD(ifnet_head, ifnet); /* the a
> struct ifnet { /* and the entries */
> void *if_softc; /* lower-level data for this if */
> struct refcnt if_refcnt;
> - TAILQ_ENTRY(ifnet) if_list; /* all struct ifnets are chained */
> - TAILQ_HEAD(, ifaddr) if_addrlist; /* linked list of addresses per if */
> - TAILQ_HEAD(, ifmaddr) if_maddrlist; /* list of multicast records */
> - TAILQ_HEAD(, ifg_list) if_groups; /* linked list of groups per if */
> - struct hook_desc_head *if_addrhooks; /* address change callbacks */
> - struct hook_desc_head *if_linkstatehooks; /* link change callbacks */
> - struct hook_desc_head *if_detachhooks; /* detach callbacks */
> - /* check or clean routes (+ or -)'d */
> + TAILQ_ENTRY(ifnet) if_list; /* [k] all struct ifnets are chained */
> + TAILQ_HEAD(, ifaddr) if_addrlist; /* [N] list of addresses per if */
> + TAILQ_HEAD(, ifmaddr) if_maddrlist; /* [N] list of multicast records */
> + TAILQ_HEAD(, ifg_list) if_groups; /* [N] list of groups per if */
> + struct hook_desc_head *if_addrhooks; /* [I] address change callbacks */
> + struct hook_desc_head *if_linkstatehooks; /* [I] link change callbacks*/
> + struct hook_desc_head *if_detachhooks; /* [I] detach callbacks */
> + /* [I] check or clean routes (+ or -)'d */
> void (*if_rtrequest)(struct ifnet *, int, struct rtentry *);
> - char if_xname[IFNAMSIZ]; /* external name (name + unit) */
> - int if_pcount; /* number of promiscuous listeners */
> + char if_xname[IFNAMSIZ]; /* [I] external name (name + unit) */
> + int if_pcount; /* [k] # of promiscuous listeners */
> caddr_t if_bpf; /* packet filter structure */
> caddr_t if_bridgeport; /* used by bridge ports */
> caddr_t if_switchport; /* used by switch ports */
> @@ -125,48 +132,43 @@ struct ifnet { /* and the
> entries */
> } if_carp_ptr;
> #define if_carp if_carp_ptr.carp_s
> #define if_carpdev if_carp_ptr.carp_d
> - unsigned int if_index; /* numeric abbreviation for this if */
> + unsigned int if_index; /* [I] unique index for this if */
> short if_timer; /* time 'til if_watchdog called */
> - unsigned short if_flags; /* up/down, broadcast, etc. */
> - int if_xflags; /* extra softnet flags */
> + unsigned short if_flags; /* [N] up/down, broadcast, etc. */
> + int if_xflags; /* [N] extra softnet flags */
> struct if_data if_data; /* stats and other data about if */
> - u_int32_t if_hardmtu; /* maximum MTU device supports */
> - char if_description[IFDESCRSIZE]; /* interface description */
> - u_short if_rtlabelid; /* next route label */
> - u_int8_t if_priority;
> - u_int8_t if_llprio; /* link layer priority */
> - struct timeout *if_slowtimo; /* watchdog timeout */
> - struct task *if_watchdogtask; /* watchdog task */
> - struct task *if_linkstatetask; /* task to do route updates */
> + uint32_t if_hardmtu; /* [d] maximum MTU device supports */
> + char if_description[IFDESCRSIZE]; /* [c] interface description */
> + u_short if_rtlabelid; /* [c] next route label */
> + uint8_t if_priority; /* [c] route priority offset */
> + uint8_t if_llprio; /* [N] link layer priority */
> + struct timeout *if_slowtimo; /* [I] watchdog timeout */
> + struct task *if_watchdogtask; /* [I] watchdog task */
> + struct task *if_linkstatetask; /* [I] task to do route updates */
>
> /* procedure handles */
> SRPL_HEAD(, ifih) if_inputs; /* input routines (dequeue) */
> -
> - /* output routine (enqueue) */
> int (*if_output)(struct ifnet *, struct mbuf *, struct sockaddr *,
> - struct rtentry *);
> -
> + struct rtentry *); /* output routine (enqueue) */
> /* link level output function */
> int (*if_ll_output)(struct ifnet *, struct mbuf *,
> struct sockaddr *, struct rtentry *);
> - /* initiate output routine */
> - void (*if_start)(struct ifnet *);
> - /* ioctl routine */
> - int (*if_ioctl)(struct ifnet *, u_long, caddr_t);
> - /* timer routine */
> - void (*if_watchdog)(struct ifnet *);
> - int (*if_wol)(struct ifnet *, int);
> + void (*if_start)(struct ifnet *); /* initiate output */
> + int (*if_ioctl)(struct ifnet *, u_long, caddr_t); /* ioctl hook */
> + void (*if_watchdog)(struct ifnet *); /* timer routine */
> + int (*if_wol)(struct ifnet *, int); /* WoL routine **/
>
> + /* queues */
> struct ifqueue if_snd; /* transmit queue */
> - struct ifqueue **if_ifqs; /* pointer to an array of sndqs */
> + struct ifqueue **if_ifqs; /* [I] pointer to an array of sndqs */
> void (*if_qstart)(struct ifqueue *);
> - unsigned int if_nifqs;
> + unsigned int if_nifqs; /* [I] number of output queues */
>
> struct ifiqueue if_rcv; /* rx/input queue */
> - struct ifiqueue **if_iqs; /* pointer to the array of iqs */
> - unsigned int if_niqs;
> + struct ifiqueue **if_iqs; /* [I] pointer to the array of iqs */
> + unsigned int if_niqs; /* [I] number of input queues */
>
> - struct sockaddr_dl *if_sadl; /* pointer to our sockaddr_dl */
> + struct sockaddr_dl *if_sadl; /* [N] pointer to our sockaddr_dl */
>
> void *if_afdata[AF_MAX];
> };
> @@ -189,7 +191,7 @@ struct ifnet { /* and the
> entries */
> #define if_iqdrops if_data.ifi_iqdrops
> #define if_oqdrops if_data.ifi_oqdrops
> #define if_noproto if_data.ifi_noproto
> -#define if_lastchange if_data.ifi_lastchange
> +#define if_lastchange if_data.ifi_lastchange /* [c] last op. state
> change */
> #define if_capabilities if_data.ifi_capabilities
> #define if_rdomain if_data.ifi_rdomain
>
