summaryrefslogtreecommitdiff
path: root/sys/arch
diff options
context:
space:
mode:
authorriastradh <riastradh@NetBSD.org>2021-06-16 00:21:17 +0000
committerriastradh <riastradh@NetBSD.org>2021-06-16 00:21:17 +0000
commitbbdda93be2f0bfefe91466b398fe42700a6ea284 (patch)
treee75e77046575209dbb21f73d0bc44c61ec0ec6c0 /sys/arch
parent5a96957fa793651c8e7093a5a4c2fcae87232af2 (diff)
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 (limited to 'sys/arch')
-rw-r--r--sys/arch/arm/broadcom/bcm53xx_eth.c11
-rw-r--r--sys/arch/powerpc/booke/dev/pq3etsec.c14
-rw-r--r--sys/arch/usermode/dev/if_veth.c13
3 files changed, 8 insertions, 30 deletions
diff --git a/sys/arch/arm/broadcom/bcm53xx_eth.c b/sys/arch/arm/broadcom/bcm53xx_eth.c
index bea0c1c4014..b91892cd60c 100644
--- a/sys/arch/arm/broadcom/bcm53xx_eth.c
+++ b/sys/arch/arm/broadcom/bcm53xx_eth.c
@@ -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, device_t self, void *aux)
/*
* 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, device_t self, void *aux)
return;
-fail_5:
- ifmedia_removeall(&sc->sc_media);
fail_4:
intr_disestablish(sc->sc_ih);
fail_3:
diff --git a/sys/arch/powerpc/booke/dev/pq3etsec.c b/sys/arch/powerpc/booke/dev/pq3etsec.c
index f271960a0ed..49e82d1c693 100644
--- a/sys/arch/powerpc/booke/dev/pq3etsec.c
+++ b/sys/arch/powerpc/booke/dev/pq3etsec.c
@@ -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_t self, void *aux)
/*
* 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_t self, void *aux)
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 --git a/sys/arch/usermode/dev/if_veth.c b/sys/arch/usermode/dev/if_veth.c
index a0ccd024bbb..a9488bf9c75 100644
--- a/sys/arch/usermode/dev/if_veth.c
+++ b/sys/arch/usermode/dev/if_veth.c
@@ -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>
@@ -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 self, void *opaque)
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 self, void *opaque)
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);