Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src if_attach and if_initialize cannot fail, don't test return v...



details:   https://anonhg.NetBSD.org/src/rev/acab78f5a46c
branches:  trunk
changeset: 379717:acab78f5a46c
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Wed Jun 16 00:21:17 2021 +0000

description:
if_attach and if_initialize cannot fail, don't test return value

These were originally made failable back in 2017 when if_initialize
allocated a softint in every interface for link state changes, so
that it could fail gracefully instead of panicking:

https://mail-index.NetBSD.org/source-changes/2017/10/23/msg089053.html

However, this spawned many seldom- or never-tested error branches,
which are risky to have around.  And that softint in every interface
has since been replaced by a single global workqueue, because link
state changes require thread context but not low latency or high
throughput:

https://mail-index.NetBSD.org/source-changes/2020/02/06/msg113759.html

So there is no longer any reason for if_initialize to fail.  (The
subroutine if_stats_init can't fail because percpu_alloc can't fail
either.)

There is a snag: the softint_establish in if_percpuq_create could
fail, potentially leading to bad consequences later on trying to use
the softint.  This change doesn't introduce any new bugs because of
the snag -- if_percpuq_attach was already broken.  However, the snag
can be better addressed without spawning error branches, either by
using a single softint or making softints less scarce.

(Separate commit will change the signatures of if_attach and
if_initialize to return void, scheduled to ride whatever is the next
convenient kernel bump.)

Patch and testing on amd64 and evbmips64-eb by maya@; commit message
soliloquy, and compile-testing on evbppc/i386/earmv7hf, by me.

diffstat:

 share/man/man4/usbnet.4               |   6 ++----
 sys/arch/arm/broadcom/bcm53xx_eth.c   |  11 ++---------
 sys/arch/powerpc/booke/dev/pq3etsec.c |  14 +++-----------
 sys/arch/usermode/dev/if_veth.c       |  13 +++----------
 sys/dev/hyperv/if_hvn.c               |  11 +++--------
 sys/dev/ic/an.c                       |  10 +++-------
 sys/dev/ic/athn.c                     |  15 +++------------
 sys/dev/ic/atw.c                      |  11 +++--------
 sys/dev/ic/bwfm.c                     |  12 ++----------
 sys/dev/ic/bwi.c                      |  11 +++--------
 sys/dev/ic/dwc_gmac.c                 |  15 +++------------
 sys/dev/ic/malo.c                     |  17 ++++-------------
 sys/dev/ic/rt2560.c                   |  12 +++---------
 sys/dev/ic/rt2661.c                   |  12 +++---------
 sys/dev/ic/rt2860.c                   |  24 +++---------------------
 sys/dev/ic/rtw.c                      |  10 +++-------
 sys/dev/ic/wi.c                       |  15 ++++-----------
 sys/dev/pci/if_aq.c                   |  11 +++--------
 sys/dev/pci/if_iavf.c                 |  10 +++-------
 sys/dev/pci/if_ipw.c                  |  12 +++---------
 sys/dev/pci/if_iwi.c                  |  12 +++---------
 sys/dev/pci/if_iwm.c                  |  12 +++---------
 sys/dev/pci/if_iwn.c                  |  12 +++---------
 sys/dev/pci/if_ixl.c                  |  10 +++-------
 sys/dev/pci/if_rtwn.c                 |  17 +++--------------
 sys/dev/pci/if_wm.c                   |  11 +++--------
 sys/dev/pci/if_wpi.c                  |  12 +++---------
 sys/dev/pci/ixgbe/ixgbe.c             |  11 +++--------
 sys/dev/pci/ixgbe/ixv.c               |  11 +++--------
 sys/dev/pcmcia/if_malo_pcmcia.c       |  21 ++++++---------------
 sys/dev/scsipi/if_se.c                |  10 +++-------
 sys/dev/usb/if_umb.c                  |  12 +++---------
 sys/dev/usb/usbnet.c                  |  10 +++-------
 sys/net/if_arcsubr.c                  |   9 +++------
 sys/net/if_bridge.c                   |  18 +++---------------
 sys/net/if_faith.c                    |  11 +++--------
 sys/net/if_gif.c                      |   9 +++------
 sys/net/if_l2tp.c                     |   9 +++------
 sys/net/if_loop.c                     |  11 +++--------
 sys/net/if_mpls.c                     |  12 +++---------
 sys/net/if_pppoe.c                    |  11 +++--------
 sys/net/if_srt.c                      |  13 +++----------
 sys/net/if_stf.c                      |  15 +++------------
 sys/net/if_tap.c                      |  14 +++-----------
 sys/net/if_vlan.c                     |  26 +++-----------------------
 sys/net/if_wg.c                       |  10 +++-------
 sys/net/lagg/if_lagg.c                |  11 +++--------
 sys/netinet/ip_carp.c                 |  15 +++------------
 sys/rump/net/lib/libshmif/if_shmem.c  |  20 +++++---------------
 sys/rump/net/lib/libvirtif/if_virt.c  |  17 +++++------------
 50 files changed, 156 insertions(+), 488 deletions(-)

diffs (truncated from 1953 to 300 lines):

diff -r 550ea8a967da -r acab78f5a46c share/man/man4/usbnet.4
--- a/share/man/man4/usbnet.4   Wed Jun 16 00:19:46 2021 +0000
+++ b/share/man/man4/usbnet.4   Wed Jun 16 00:21:17 2021 +0000
@@ -1,4 +1,4 @@
-.\"    $NetBSD: usbnet.4,v 1.4 2019/09/20 10:34:54 mrg Exp $
+.\"    $NetBSD: usbnet.4,v 1.5 2021/06/16 00:21:17 riastradh Exp $
 .\"
 .\" Copyright (c) 2019 Matthew R. Green
 .\" All rights reserved.
@@ -26,7 +26,7 @@
 .\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 .\" SUCH DAMAGE.
 .\"
-.Dd August 24, 2019
+.Dd June 15, 2021
 .Dt USBNET 4
 .Os
 .Sh NAME
@@ -60,8 +60,6 @@ Creation of Rx or Tx list failed, device
 Time out in transmission.
 .It "devN: pipe abort failed"
 Aborting USB pipes after watchdog timeout failed.
-.It "devN: if_initialize failed"
-Couldn't register network interface, device non-functional.
 .It "devN: couldn't establish power handler"
 Call to
 .Xr pmf_device_register 9
diff -r 550ea8a967da -r acab78f5a46c sys/arch/arm/broadcom/bcm53xx_eth.c
--- a/sys/arch/arm/broadcom/bcm53xx_eth.c       Wed Jun 16 00:19:46 2021 +0000
+++ b/sys/arch/arm/broadcom/bcm53xx_eth.c       Wed Jun 16 00:21:17 2021 +0000
@@ -35,7 +35,7 @@
 
 #include <sys/cdefs.h>
 
-__KERNEL_RCSID(1, "$NetBSD: bcm53xx_eth.c,v 1.40 2020/02/03 08:00:35 skrll Exp $");
+__KERNEL_RCSID(1, "$NetBSD: bcm53xx_eth.c,v 1.41 2021/06/16 00:21:17 riastradh Exp $");
 
 #include <sys/param.h>
 #include <sys/atomic.h>
@@ -407,12 +407,7 @@ bcmeth_ccb_attach(device_t parent, devic
        /*
         * Attach the interface.
         */
-       error = if_initialize(ifp);
-       if (error != 0) {
-               aprint_error_dev(sc->sc_dev, "if_initialize failed(%d)\n",
-                   error);
-               goto fail_5;
-       }
+       if_initialize(ifp);
        ether_ifattach(ifp, sc->sc_enaddr);
        if_register(ifp);
 
@@ -433,8 +428,6 @@ bcmeth_ccb_attach(device_t parent, devic
 
        return;
 
-fail_5:
-       ifmedia_removeall(&sc->sc_media);
 fail_4:
        intr_disestablish(sc->sc_ih);
 fail_3:
diff -r 550ea8a967da -r acab78f5a46c sys/arch/powerpc/booke/dev/pq3etsec.c
--- a/sys/arch/powerpc/booke/dev/pq3etsec.c     Wed Jun 16 00:19:46 2021 +0000
+++ b/sys/arch/powerpc/booke/dev/pq3etsec.c     Wed Jun 16 00:21:17 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: pq3etsec.c,v 1.54 2021/04/24 23:36:46 thorpej Exp $    */
+/*     $NetBSD: pq3etsec.c,v 1.55 2021/06/16 00:21:18 riastradh Exp $  */
 /*-
  * Copyright (c) 2010, 2011 The NetBSD Foundation, Inc.
  * All rights reserved.
@@ -35,7 +35,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: pq3etsec.c,v 1.54 2021/04/24 23:36:46 thorpej Exp $");
+__KERNEL_RCSID(0, "$NetBSD: pq3etsec.c,v 1.55 2021/06/16 00:21:18 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_inet.h"
@@ -804,12 +804,7 @@ pq3etsec_attach(device_t parent, device_
        /*
         * Attach the interface.
         */
-       error = if_initialize(ifp);
-       if (error != 0) {
-               aprint_error_dev(sc->sc_dev, "if_initialize failed(%d)\n",
-                   error);
-               goto fail_10;
-       }
+       if_initialize(ifp);
        pq3etsec_sysctl_setup(NULL, sc);
        if_attach(ifp);
        if_deferred_start_init(ifp, NULL);
@@ -840,9 +835,6 @@ pq3etsec_attach(device_t parent, device_
            NULL, xname, "mii ticks");
        return;
 
-fail_10:
-       ifmedia_removeall(&mii->mii_media);
-       mii_detach(mii, sc->sc_phy_addr, MII_OFFSET_ANY);
 fail_9:
        softint_disestablish(sc->sc_soft_ih);
 fail_8:
diff -r 550ea8a967da -r acab78f5a46c sys/arch/usermode/dev/if_veth.c
--- a/sys/arch/usermode/dev/if_veth.c   Wed Jun 16 00:19:46 2021 +0000
+++ b/sys/arch/usermode/dev/if_veth.c   Wed Jun 16 00:21:17 2021 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: if_veth.c,v 1.14 2020/02/05 07:18:16 skrll Exp $ */
+/* $NetBSD: if_veth.c,v 1.15 2021/06/16 00:21:18 riastradh Exp $ */
 
 /*-
  * Copyright (c) 2011 Jared D. McNeill <jmcneill%invisible.ca@localhost>
@@ -27,7 +27,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_veth.c,v 1.14 2020/02/05 07:18:16 skrll Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_veth.c,v 1.15 2021/06/16 00:21:18 riastradh Exp $");
 
 #include <sys/param.h>
 #include <sys/proc.h>
@@ -102,7 +102,6 @@ veth_attach(device_t parent, device_t se
        struct veth_softc *sc = device_private(self);
        struct thunkbus_attach_args *taa = opaque;
        struct ifnet *ifp = &sc->sc_ec.ec_if;
-       int rv;
 
        sc->sc_dev = self;
 
@@ -138,13 +137,7 @@ veth_attach(device_t parent, device_t se
        IFQ_SET_MAXLEN(&ifp->if_snd, IFQ_MAXLEN);
        IFQ_SET_READY(&ifq->if_snd);
 
-       rv = if_initialize(ifp);
-       if (rv != 0) {
-               aprint_error_dev(self, "if_initialize failed(%d)\n", rv);
-               thunk_close(sc->sc_tapfd);
-               pmf_device_deregister(self);
-               return; /* Error */
-       }
+       if_initialize(ifp);
        ether_ifattach(ifp, sc->sc_eaddr);
        if_register(ifp);
 
diff -r 550ea8a967da -r acab78f5a46c sys/dev/hyperv/if_hvn.c
--- a/sys/dev/hyperv/if_hvn.c   Wed Jun 16 00:19:46 2021 +0000
+++ b/sys/dev/hyperv/if_hvn.c   Wed Jun 16 00:21:17 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if_hvn.c,v 1.20 2021/01/29 04:38:49 nonaka Exp $       */
+/*     $NetBSD: if_hvn.c,v 1.21 2021/06/16 00:21:18 riastradh Exp $    */
 /*     $OpenBSD: if_hvn.c,v 1.39 2018/03/11 14:31:34 mikeb Exp $       */
 
 /*-
@@ -35,7 +35,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_hvn.c,v 1.20 2021/01/29 04:38:49 nonaka Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_hvn.c,v 1.21 2021/06/16 00:21:18 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_inet.h"
@@ -243,7 +243,6 @@ hvn_attach(device_t parent, device_t sel
        struct vmbus_attach_args *aa = aux;
        struct ifnet *ifp = SC2IFP(sc);
        uint8_t enaddr[ETHER_ADDR_LEN];
-       int error;
 
        sc->sc_dev = self;
        sc->sc_vmbus = (struct vmbus_softc *)device_private(parent);
@@ -302,11 +301,7 @@ hvn_attach(device_t parent, device_t sel
        ifmedia_add(&sc->sc_media, IFM_ETHER | IFM_MANUAL, 0, NULL);
        ifmedia_set(&sc->sc_media, IFM_ETHER | IFM_MANUAL);
 
-       error = if_initialize(ifp);
-       if (error) {
-               aprint_error_dev(self, "if_initialize failed(%d)\n", error);
-               goto fail3;
-       }
+       if_initialize(ifp);
        sc->sc_ipq = if_percpuq_create(ifp);
        if_deferred_start_init(ifp, NULL);
 
diff -r 550ea8a967da -r acab78f5a46c sys/dev/ic/an.c
--- a/sys/dev/ic/an.c   Wed Jun 16 00:19:46 2021 +0000
+++ b/sys/dev/ic/an.c   Wed Jun 16 00:21:17 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: an.c,v 1.74 2021/06/11 05:00:41 jdc Exp $      */
+/*     $NetBSD: an.c,v 1.75 2021/06/16 00:21:18 riastradh Exp $        */
 /*
  * Copyright (c) 1997, 1998, 1999
  *     Bill Paul <wpaul%ctr.columbia.edu@localhost>.  All rights reserved.
@@ -77,7 +77,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: an.c,v 1.74 2021/06/11 05:00:41 jdc Exp $");
+__KERNEL_RCSID(0, "$NetBSD: an.c,v 1.75 2021/06/16 00:21:18 riastradh Exp $");
 
 
 #include <sys/param.h>
@@ -315,11 +315,7 @@ an_attach(struct an_softc *sc)
        /*
         * Call MI attach routine.
         */
-       rv = if_initialize(ifp);
-       if (rv != 0) {
-               aprint_error_dev(sc->sc_dev, "if_initialize failed(%d)\n", rv);
-               goto fail_2;
-       }
+       if_initialize(ifp);
        ieee80211_ifattach(ic);
        ifp->if_percpuq = if_percpuq_create(ifp);
        if_register(ifp);
diff -r 550ea8a967da -r acab78f5a46c sys/dev/ic/athn.c
--- a/sys/dev/ic/athn.c Wed Jun 16 00:19:46 2021 +0000
+++ b/sys/dev/ic/athn.c Wed Jun 16 00:21:17 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: athn.c,v 1.24 2020/11/15 12:33:53 mlelstv Exp $        */
+/*     $NetBSD: athn.c,v 1.25 2021/06/16 00:21:18 riastradh Exp $      */
 /*     $OpenBSD: athn.c,v 1.83 2014/07/22 13:12:11 mpi Exp $   */
 
 /*-
@@ -23,7 +23,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: athn.c,v 1.24 2020/11/15 12:33:53 mlelstv Exp $");
+__KERNEL_RCSID(0, "$NetBSD: athn.c,v 1.25 2021/06/16 00:21:18 riastradh Exp $");
 
 #ifndef _MODULE
 #include "athn_usb.h"          /* for NATHN_USB */
@@ -348,16 +348,7 @@ athn_attach(struct athn_softc *sc)
        IFQ_SET_READY(&ifp->if_snd);
        memcpy(ifp->if_xname, device_xname(sc->sc_dev), IFNAMSIZ);
 
-       error = if_initialize(ifp);
-       if (error != 0) {
-               aprint_error_dev(sc->sc_dev, "if_initialize failed(%d)\n",
-                   error);
-               pmf_event_deregister(sc->sc_dev, PMFE_RADIO_OFF,
-                   athn_pmf_wlan_off, false);
-               callout_destroy(&sc->sc_scan_to);
-               callout_destroy(&sc->sc_calib_to);
-               return error;
-       }
+       if_initialize(ifp);
        ieee80211_ifattach(ic);
        /* Use common softint-based if_input */
        ifp->if_percpuq = if_percpuq_create(ifp);
diff -r 550ea8a967da -r acab78f5a46c sys/dev/ic/atw.c
--- a/sys/dev/ic/atw.c  Wed Jun 16 00:19:46 2021 +0000
+++ b/sys/dev/ic/atw.c  Wed Jun 16 00:21:17 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: atw.c,v 1.170 2020/01/29 14:09:58 thorpej Exp $  */
+/*     $NetBSD: atw.c,v 1.171 2021/06/16 00:21:18 riastradh Exp $  */
 
 /*-
  * Copyright (c) 1998, 1999, 2000, 2002, 2003, 2004 The NetBSD Foundation, Inc.
@@ -34,7 +34,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: atw.c,v 1.170 2020/01/29 14:09:58 thorpej Exp $");
+__KERNEL_RCSID(0, "$NetBSD: atw.c,v 1.171 2021/06/16 00:21:18 riastradh Exp $");
 
 
 #include <sys/param.h>
@@ -780,12 +780,7 @@ atw_attach(struct atw_softc *sc)
         * Call MI attach routines.
         */
 
-       error = if_initialize(ifp);
-       if (error != 0) {
-               aprint_error_dev(sc->sc_dev, "if_initialize failed(%d)\n",
-                   error);
-               goto fail_5;
-       }
+       if_initialize(ifp);
        ieee80211_ifattach(ic);
        /* Use common softint-based if_input */
        ifp->if_percpuq = if_percpuq_create(ifp);
diff -r 550ea8a967da -r acab78f5a46c sys/dev/ic/bwfm.c
--- a/sys/dev/ic/bwfm.c Wed Jun 16 00:19:46 2021 +0000
+++ b/sys/dev/ic/bwfm.c Wed Jun 16 00:21:17 2021 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: bwfm.c,v 1.30 2021/04/13 04:13:52 mrg Exp $ */
+/* $NetBSD: bwfm.c,v 1.31 2021/06/16 00:21:18 riastradh Exp $ */
 /* $OpenBSD: bwfm.c,v 1.5 2017/10/16 22:27:16 patrick Exp $ */
 /*
  * Copyright (c) 2010-2016 Broadcom Corporation
@@ -422,15 +422,7 @@ bwfm_attach(struct bwfm_softc *sc)
        IFQ_SET_READY(&ifp->if_snd);
        memcpy(ifp->if_xname, DEVNAME(sc), IFNAMSIZ);
 
-       error = if_initialize(ifp);
-       if (error != 0) {
-               printf("%s: if_initialize failed(%d)\n", DEVNAME(sc), error);
-               pool_cache_destroy(sc->sc_freetask);



Home | Main Index | Thread Index | Old Index