[src/trunk]: src/sys/net Introduce if_initialize and if_register as an altern...

[ozaki-r <ozaki-r%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/cc84ea97ed13
branches:  trunk
changeset: 334917:cc84ea97ed13
user:      ozaki-r <ozaki-r%NetBSD.org@localhost>
date:      Mon Dec 15 06:52:25 2014 +0000

description:
Introduce if_initialize and if_register as an alternative to if_attach

if_attach initializes an ifnet object and registers it to the system
(e.g., ifnet_list), however, if_attach doesn't complete the
initialization and the rest of it will be done by if_alloc_sadl
that is normally directly called by device drivers or called via
functions like ether_ifattach. So there is a race between
if_attach and if_alloc_sadl (A half-baked ifnet object may be
accessed, for example, via ioctl between them).

The aim of this fix is to register an initializing ifnet object
after completing its initializations. To this end, this fix
separates if_attach into an initialization part (if_initialize)
and a registration part (if_register) and call the latter after
if_alloc_sadl (ether_ifattach). So a typical usage of the two
new APIs is like this:

  if_initialize(ifp);  // was if_attach
  ether_ifattach(ifp, enaddr);
  if_register(ifp);

Nonetheless, changing every drivers to do so at once isn't
feasible. So we keep if_attach working as it used to be and
will change only some drivers that we need at this point.
Once we know the fix really works well, we'll change all
the others.

Some more information of the fix can be found here:
http://mail-index.netbsd.org/tech-kern/2014/12/10/msg018242.html

No objection on tech-kern and tech-net.

diffstat:

 sys/net/if.c |  51 +++++++++++++++++++++++++++++++++++++++------------
 sys/net/if.h |   6 ++++--
 2 files changed, 43 insertions(+), 14 deletions(-)

diffs (118 lines):

diff -r 068225de0abb -r cc84ea97ed13 sys/net/if.c
--- a/sys/net/if.c      Mon Dec 15 02:01:41 2014 +0000
+++ b/sys/net/if.c      Mon Dec 15 06:52:25 2014 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if.c,v 1.306 2014/12/14 08:57:14 martin Exp $  */
+/*     $NetBSD: if.c,v 1.307 2014/12/15 06:52:25 ozaki-r Exp $ */
 
 /*-
  * Copyright (c) 1999, 2000, 2001, 2008 The NetBSD Foundation, Inc.
@@ -90,7 +90,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if.c,v 1.306 2014/12/14 08:57:14 martin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if.c,v 1.307 2014/12/15 06:52:25 ozaki-r Exp $");
 
 #include "opt_inet.h"
 
@@ -582,19 +582,21 @@
 }
 
 /*
- * Attach an interface to the list of "active" interfaces.
+ * Initialize an interface and assign an index for it.
+ *
+ * It must be called prior to a device specific attach routine
+ * (e.g., ether_ifattach and ieee80211_ifattach) or if_alloc_sadl,
+ * and be followed by if_register:
+ *
+ *     if_initialize(ifp);
+ *     ether_ifattach(ifp, enaddr);
+ *     if_register(ifp);
  */
 void
-if_attach(ifnet_t *ifp)
+if_initialize(ifnet_t *ifp)
 {
        KASSERT(if_indexlim > 0);
        TAILQ_INIT(&ifp->if_addrlist);
-       TAILQ_INSERT_TAIL(&ifnet_list, ifp, if_list);
-
-       if (ifioctl_attach(ifp) != 0)
-               panic("%s: ifioctl_attach() failed", __func__);
-
-       if_getindex(ifp);
 
        /*
         * Link level name is allocated later by a separate call to
@@ -604,8 +606,6 @@
        if (ifp->if_snd.ifq_maxlen == 0)
                ifp->if_snd.ifq_maxlen = ifqmaxlen;
 
-       sysctl_sndq_setup(&ifp->if_sysctl_log, ifp->if_xname, &ifp->if_snd);
-
        ifp->if_broadcastaddr = 0; /* reliably crash if used uninitialized */
 
        ifp->if_link_state = LINK_STATE_UNKNOWN;
@@ -632,6 +632,20 @@
        (void)pfil_run_hooks(if_pfil,
            (struct mbuf **)PFIL_IFNET_ATTACH, ifp, PFIL_IFNET);
 
+       if_getindex(ifp);
+}
+
+/*
+ * Register an interface to the list of "active" interfaces.
+ */
+void
+if_register(ifnet_t *ifp)
+{
+       if (ifioctl_attach(ifp) != 0)
+               panic("%s: ifioctl_attach() failed", __func__);
+
+       sysctl_sndq_setup(&ifp->if_sysctl_log, ifp->if_xname, &ifp->if_snd);
+
        if (!STAILQ_EMPTY(&domains))
                if_attachdomain1(ifp);
 
@@ -645,6 +659,19 @@
                callout_setfunc(ifp->if_slowtimo_ch, if_slowtimo, ifp);
                if_slowtimo(ifp);
        }
+
+       TAILQ_INSERT_TAIL(&ifnet_list, ifp, if_list);
+}
+
+/*
+ * Deprecated. Use if_initialize and if_register instead.
+ * See the above comment of if_initialize.
+ */
+void
+if_attach(ifnet_t *ifp)
+{
+       if_initialize(ifp);
+       if_register(ifp);
 }
 
 void
diff -r 068225de0abb -r cc84ea97ed13 sys/net/if.h
--- a/sys/net/if.h      Mon Dec 15 02:01:41 2014 +0000
+++ b/sys/net/if.h      Mon Dec 15 06:52:25 2014 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if.h,v 1.183 2014/12/02 04:43:35 ozaki-r Exp $ */
+/*     $NetBSD: if.h,v 1.184 2014/12/15 06:52:25 ozaki-r Exp $ */
 
 /*-
  * Copyright (c) 1999, 2000, 2001 The NetBSD Foundation, Inc.
@@ -864,7 +864,9 @@
     const struct sockaddr_dl *);
 void   if_set_sadl(struct ifnet *, const void *, u_char, bool);
 void   if_alloc_sadl(struct ifnet *);
-void   if_attach(struct ifnet *);
+void   if_initialize(struct ifnet *);
+void   if_register(struct ifnet *);
+void   if_attach(struct ifnet *); /* Deprecated. Use if_initialize and if_register */
 void   if_attachdomain(void);
 void   if_deactivate(struct ifnet *);
 void   if_purgeaddrs(struct ifnet *, int, void (*)(struct ifaddr *));