Always allocate per-CPU statistics counters for network interface
authormvs <mvs@openbsd.org>
Fri, 22 Dec 2023 23:01:50 +0000 (23:01 +0000)
committermvs <mvs@openbsd.org>
Fri, 22 Dec 2023 23:01:50 +0000 (23:01 +0000)
descriptor.

We have the mess in network interface statistics. Only pseudo drivers
do per-CPU counters allocation, all other network devices use the old
`if_data'. The network stack partially uses per-CPU counters and
partially use `if_data', but the protection is inconsistent: some times
counters accessed with exclusive netlock, some times with shared
netlock, some times with kernel lock, but without netlock, some times
with another locks.

To make network interfaces statistics more consistent, always allocate
per-CPU counters at interface attachment time and use it instead of
`if_data'. At this step only move counters allocation to the if_attach()
internals. The `if_data' removal will be performed with the following
diffs to make review and tests easier.

ok bluhm

22 files changed:
sys/net/if.c
sys/net/if_aggr.c
sys/net/if_bpe.c
sys/net/if_etherip.c
sys/net/if_gif.c
sys/net/if_gre.c
sys/net/if_mpe.c
sys/net/if_mpip.c
sys/net/if_mpw.c
sys/net/if_pflow.c
sys/net/if_pfsync.c
sys/net/if_pppx.c
sys/net/if_sec.c
sys/net/if_tpmr.c
sys/net/if_trunk.c
sys/net/if_tun.c
sys/net/if_var.h
sys/net/if_veb.c
sys/net/if_vlan.c
sys/net/if_vxlan.c
sys/net/if_wg.c
sys/netinet/ip_carp.c

index 89c164f..f205a67 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: if.c,v 1.711 2023/11/11 14:24:03 bluhm Exp $  */
+/*     $OpenBSD: if.c,v 1.712 2023/12/22 23:01:50 mvs Exp $    */
 /*     $NetBSD: if.c,v 1.35 1996/05/07 05:26:04 thorpej Exp $  */
 
 /*
@@ -645,6 +645,8 @@ if_attach_common(struct ifnet *ifp)
                    "%s: if_qstart not set with MPSAFE set", ifp->if_xname);
        }
 
+       ifp->if_counters = counters_alloc(ifc_ncounters);
+
        if_idxmap_alloc(ifp);
 
        ifq_init(&ifp->if_snd, ifp, 0);
@@ -1250,8 +1252,7 @@ if_detach(struct ifnet *ifp)
        /* Announce that the interface is gone. */
        rtm_ifannounce(ifp, IFAN_DEPARTURE);
 
-       if (ifp->if_counters != NULL)
-               if_counters_free(ifp);
+       counters_free(ifp->if_counters, ifc_ncounters);
 
        for (i = 0; i < ifp->if_nifqs; i++)
                ifq_destroy(ifp->if_ifqs[i]);
@@ -2770,49 +2771,28 @@ ifconf(caddr_t data)
        return (error);
 }
 
-void
-if_counters_alloc(struct ifnet *ifp)
-{
-       KASSERT(ifp->if_counters == NULL);
-
-       ifp->if_counters = counters_alloc(ifc_ncounters);
-}
-
-void
-if_counters_free(struct ifnet *ifp)
-{
-       KASSERT(ifp->if_counters != NULL);
-
-       counters_free(ifp->if_counters, ifc_ncounters);
-       ifp->if_counters = NULL;
-}
-
 void
 if_getdata(struct ifnet *ifp, struct if_data *data)
 {
+       uint64_t counters[ifc_ncounters];
        unsigned int i;
 
        *data = ifp->if_data;
 
-       if (ifp->if_counters != NULL) {
-               uint64_t counters[ifc_ncounters];
-
-               counters_read(ifp->if_counters, counters, nitems(counters),
-                   NULL);
-
-               data->ifi_ipackets += counters[ifc_ipackets];
-               data->ifi_ierrors += counters[ifc_ierrors];
-               data->ifi_opackets += counters[ifc_opackets];
-               data->ifi_oerrors += counters[ifc_oerrors];
-               data->ifi_collisions += counters[ifc_collisions];
-               data->ifi_ibytes += counters[ifc_ibytes];
-               data->ifi_obytes += counters[ifc_obytes];
-               data->ifi_imcasts += counters[ifc_imcasts];
-               data->ifi_omcasts += counters[ifc_omcasts];
-               data->ifi_iqdrops += counters[ifc_iqdrops];
-               data->ifi_oqdrops += counters[ifc_oqdrops];
-               data->ifi_noproto += counters[ifc_noproto];
-       }
+       counters_read(ifp->if_counters, counters, nitems(counters), NULL);
+
+       data->ifi_ipackets += counters[ifc_ipackets];
+       data->ifi_ierrors += counters[ifc_ierrors];
+       data->ifi_opackets += counters[ifc_opackets];
+       data->ifi_oerrors += counters[ifc_oerrors];
+       data->ifi_collisions += counters[ifc_collisions];
+       data->ifi_ibytes += counters[ifc_ibytes];
+       data->ifi_obytes += counters[ifc_obytes];
+       data->ifi_imcasts += counters[ifc_imcasts];
+       data->ifi_omcasts += counters[ifc_omcasts];
+       data->ifi_iqdrops += counters[ifc_iqdrops];
+       data->ifi_oqdrops += counters[ifc_oqdrops];
+       data->ifi_noproto += counters[ifc_noproto];
 
        for (i = 0; i < ifp->if_nifqs; i++) {
                struct ifqueue *ifq = ifp->if_ifqs[i];
index 5240643..5536e07 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: if_aggr.c,v 1.40 2023/05/16 14:32:54 jan Exp $ */
+/*     $OpenBSD: if_aggr.c,v 1.41 2023/12/22 23:01:50 mvs Exp $ */
 
 /*
  * Copyright (c) 2019 The University of Queensland
@@ -562,7 +562,6 @@ aggr_clone_create(struct if_clone *ifc, int unit)
        ifp->if_link_state = LINK_STATE_DOWN;
        ether_fakeaddr(ifp);
 
-       if_counters_alloc(ifp);
        if_attach(ifp);
        ether_ifattach(ifp);
 
index ec3c758..f5d4771 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: if_bpe.c,v 1.20 2023/10/27 20:56:47 jan Exp $ */
+/*     $OpenBSD: if_bpe.c,v 1.21 2023/12/22 23:01:50 mvs Exp $ */
 /*
  * Copyright (c) 2018 David Gwynne <dlg@openbsd.org>
  *
@@ -182,7 +182,6 @@ bpe_clone_create(struct if_clone *ifc, int unit)
        ifp->if_xflags = IFXF_CLONED;
        ether_fakeaddr(ifp);
 
-       if_counters_alloc(ifp);
        if_attach(ifp);
        ether_ifattach(ifp);
 
index e9b9896..2773685 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: if_etherip.c,v 1.52 2023/11/28 13:23:20 bluhm Exp $   */
+/*     $OpenBSD: if_etherip.c,v 1.53 2023/12/22 23:01:50 mvs Exp $     */
 /*
  * Copyright (c) 2015 Kazuya GODA <goda@openbsd.org>
  *
@@ -161,7 +161,6 @@ etherip_clone_create(struct if_clone *ifc, int unit)
        ifmedia_add(&sc->sc_media, IFM_ETHER | IFM_AUTO, 0, NULL);
        ifmedia_set(&sc->sc_media, IFM_ETHER | IFM_AUTO);
 
-       if_counters_alloc(ifp);
        if_attach(ifp);
        ether_ifattach(ifp);
 
index 1084251..1051247 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: if_gif.c,v 1.134 2023/11/28 13:23:20 bluhm Exp $      */
+/*     $OpenBSD: if_gif.c,v 1.135 2023/12/22 23:01:50 mvs Exp $        */
 /*     $KAME: if_gif.c,v 1.43 2001/02/20 08:51:07 itojun Exp $ */
 
 /*
@@ -176,7 +176,6 @@ gif_clone_create(struct if_clone *ifc, int unit)
 
        if_attach(ifp);
        if_alloc_sadl(ifp);
-       if_counters_alloc(ifp);
 
 #if NBPFILTER > 0
        bpfattach(&ifp->if_bpf, ifp, DLT_LOOP, sizeof(uint32_t));
index 830e5c1..967cb75 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: if_gre.c,v 1.176 2023/11/28 13:23:20 bluhm Exp $ */
+/*     $OpenBSD: if_gre.c,v 1.177 2023/12/22 23:01:50 mvs Exp $ */
 /*     $NetBSD: if_gre.c,v 1.9 1999/10/25 19:18:11 drochner Exp $ */
 
 /*
@@ -592,7 +592,6 @@ gre_clone_create(struct if_clone *ifc, int unit)
        timeout_set_proc(&sc->sc_ka_hold, gre_keepalive_hold, sc);
        sc->sc_ka_state = GRE_KA_NONE;
 
-       if_counters_alloc(ifp);
        if_attach(ifp);
        if_alloc_sadl(ifp);
 
@@ -659,7 +658,6 @@ mgre_clone_create(struct if_clone *ifc, int unit)
        sc->sc_tunnel.t_df = htons(0);
        sc->sc_tunnel.t_ecn = ECN_ALLOWED;
 
-       if_counters_alloc(ifp);
        if_attach(ifp);
        if_alloc_sadl(ifp);
 
@@ -716,7 +714,6 @@ egre_clone_create(struct if_clone *ifc, int unit)
        ifmedia_add(&sc->sc_media, IFM_ETHER | IFM_AUTO, 0, NULL);
        ifmedia_set(&sc->sc_media, IFM_ETHER | IFM_AUTO);
 
-       if_counters_alloc(ifp);
        if_attach(ifp);
        ether_ifattach(ifp);
 
@@ -794,7 +791,6 @@ nvgre_clone_create(struct if_clone *ifc, int unit)
        ifmedia_add(&sc->sc_media, IFM_ETHER | IFM_AUTO, 0, NULL);
        ifmedia_set(&sc->sc_media, IFM_ETHER | IFM_AUTO);
 
-       if_counters_alloc(ifp);
        if_attach(ifp);
        ether_ifattach(ifp);
 
@@ -858,7 +854,6 @@ eoip_clone_create(struct if_clone *ifc, int unit)
        ifmedia_add(&sc->sc_media, IFM_ETHER | IFM_AUTO, 0, NULL);
        ifmedia_set(&sc->sc_media, IFM_ETHER | IFM_AUTO);
 
-       if_counters_alloc(ifp);
        if_attach(ifp);
        ether_ifattach(ifp);
 
index 7b84d24..447570b 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: if_mpe.c,v 1.102 2022/08/29 07:51:45 bluhm Exp $ */
+/* $OpenBSD: if_mpe.c,v 1.103 2023/12/22 23:01:50 mvs Exp $ */
 
 /*
  * Copyright (c) 2008 Pierre-Yves Ritschard <pyr@spootnik.org>
@@ -119,7 +119,6 @@ mpe_clone_create(struct if_clone *ifc, int unit)
 
        if_attach(ifp);
        if_alloc_sadl(ifp);
-       if_counters_alloc(ifp);
 
 #if NBPFILTER > 0
        bpfattach(&ifp->if_bpf, ifp, DLT_LOOP, sizeof(u_int32_t));
index 4fefa75..d0c3971 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: if_mpip.c,v 1.16 2022/08/29 07:51:45 bluhm Exp $ */
+/*     $OpenBSD: if_mpip.c,v 1.17 2023/12/22 23:01:50 mvs Exp $ */
 
 /*
  * Copyright (c) 2015 Rafael Zalamena <rzalamena@openbsd.org>
@@ -121,7 +121,6 @@ mpip_clone_create(struct if_clone *ifc, int unit)
        ifp->if_hardmtu = 65535;
 
        if_attach(ifp);
-       if_counters_alloc(ifp);
        if_alloc_sadl(ifp);
 
 #if NBPFILTER > 0
index 2106c3e..2229bcf 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: if_mpw.c,v 1.63 2022/08/29 07:51:45 bluhm Exp $ */
+/*     $OpenBSD: if_mpw.c,v 1.64 2023/12/22 23:01:50 mvs Exp $ */
 
 /*
  * Copyright (c) 2015 Rafael Zalamena <rzalamena@openbsd.org>
@@ -115,7 +115,6 @@ mpw_clone_create(struct if_clone *ifc, int unit)
 
        sc->sc_dead = 0;
 
-       if_counters_alloc(ifp);
        if_attach(ifp);
        ether_ifattach(ifp);
 
index 7bc9e70..79c0f2f 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: if_pflow.c,v 1.107 2023/12/19 20:34:10 mvs Exp $      */
+/*     $OpenBSD: if_pflow.c,v 1.108 2023/12/22 23:01:50 mvs Exp $      */
 
 /*
  * Copyright (c) 2011 Florian Obser <florian@narrans.de>
@@ -279,7 +279,6 @@ pflow_clone_create(struct if_clone *ifc, int unit)
 
        task_set(&pflowif->sc_outputtask, pflow_output_process, pflowif);
 
-       if_counters_alloc(ifp);
        if_attach(ifp);
        if_alloc_sadl(ifp);
 
index 93d35ab..86664de 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: if_pfsync.c,v 1.322 2023/10/03 10:22:10 sthen Exp $   */
+/*     $OpenBSD: if_pfsync.c,v 1.323 2023/12/22 23:01:50 mvs Exp $     */
 
 /*
  * Copyright (c) 2002 Michael Shalayeff
@@ -444,7 +444,6 @@ pfsync_clone_create(struct if_clone *ifc, int unit)
 #endif
        }
 
-       if_counters_alloc(ifp);
        if_attach(ifp);
        if_alloc_sadl(ifp);
 
index 6682673..71d302d 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: if_pppx.c,v 1.126 2023/02/10 14:34:17 visa Exp $ */
+/*     $OpenBSD: if_pppx.c,v 1.127 2023/12/22 23:01:50 mvs Exp $ */
 
 /*
  * Copyright (c) 2010 Claudio Jeker <claudio@openbsd.org>
@@ -684,7 +684,6 @@ pppx_add_session(struct pppx_dev *pxd, struct pipex_session_req *req)
        ifp->if_type = IFT_PPP;
        ifp->if_softc = pxi;
        /* ifp->if_rdomain = req->pr_rdomain; */
-       if_counters_alloc(ifp);
 
        if_attach(ifp);
 
@@ -1080,7 +1079,6 @@ pppacopen(dev_t dev, int flags, int mode, struct proc *p)
        ifp->if_qstart = pppac_qstart;
        ifp->if_ioctl = pppac_ioctl;
 
-       if_counters_alloc(ifp);
        if_attach(ifp);
        if_alloc_sadl(ifp);
 
index 4c65efe..a7e85d9 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: if_sec.c,v 1.7 2023/08/15 09:46:30 dlg Exp $ */
+/*     $OpenBSD: if_sec.c,v 1.8 2023/12/22 23:01:50 mvs Exp $ */
 
 /*
  * Copyright (c) 2022 The University of Queensland
@@ -147,7 +147,6 @@ sec_clone_create(struct if_clone *ifc, int unit)
        ifp->if_ioctl = sec_ioctl;
        ifp->if_rtrequest = p2p_rtrequest;
 
-       if_counters_alloc(ifp);
        if_attach(ifp);
        if_alloc_sadl(ifp);
 
index 47bb4e5..7794dfb 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: if_tpmr.c,v 1.33 2023/05/16 14:32:54 jan Exp $ */
+/*     $OpenBSD: if_tpmr.c,v 1.34 2023/12/22 23:01:50 mvs Exp $ */
 
 /*
  * Copyright (c) 2019 The University of Queensland
@@ -168,7 +168,6 @@ tpmr_clone_create(struct if_clone *ifc, int unit)
        ifp->if_xflags = IFXF_CLONED | IFXF_MPSAFE;
        ifp->if_link_state = LINK_STATE_DOWN;
 
-       if_counters_alloc(ifp);
        if_attach(ifp);
        if_alloc_sadl(ifp);
 
index 1efc8ab..be43be6 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: if_trunk.c,v 1.152 2021/08/02 21:10:55 mvs Exp $      */
+/*     $OpenBSD: if_trunk.c,v 1.153 2023/12/22 23:01:50 mvs Exp $      */
 
 /*
  * Copyright (c) 2005, 2006, 2007 Reyk Floeter <reyk@openbsd.org>
@@ -193,7 +193,6 @@ trunk_clone_create(struct if_clone *ifc, int unit)
         * Attach as an ordinary ethernet device, children will be attached
         * as special device IFT_IEEE8023ADLAG.
         */
-       if_counters_alloc(ifp);
        if_attach(ifp);
        ether_ifattach(ifp);
 
index b297459..b4780f1 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: if_tun.c,v 1.238 2023/02/10 14:39:18 visa Exp $       */
+/*     $OpenBSD: if_tun.c,v 1.239 2023/12/22 23:01:50 mvs Exp $        */
 /*     $NetBSD: if_tun.c,v 1.24 1996/05/07 02:40:48 thorpej Exp $      */
 
 /*
@@ -246,8 +246,6 @@ tun_create(struct if_clone *ifc, int unit, int flags)
        ifp->if_hardmtu = TUNMRU;
        ifp->if_link_state = LINK_STATE_DOWN;
 
-       if_counters_alloc(ifp);
-
        if ((flags & TUN_LAYER2) == 0) {
 #if NBPFILTER > 0
                ifp->if_bpf_mtap = bpf_mtap;
index 7eb59a8..38528b2 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: if_var.h,v 1.130 2023/11/11 14:24:03 bluhm Exp $      */
+/*     $OpenBSD: if_var.h,v 1.131 2023/12/22 23:01:50 mvs Exp $        */
 /*     $NetBSD: if.h,v 1.23 1996/05/07 02:40:27 thorpej Exp $  */
 
 /*
@@ -379,9 +379,6 @@ int if_rxr_info_ioctl(struct if_rxrinfo *, u_int, struct if_rxring_info *);
 int    if_rxr_ioctl(struct if_rxrinfo *, const char *, u_int,
            struct if_rxring *);
 
-void   if_counters_alloc(struct ifnet *);
-void   if_counters_free(struct ifnet *);
-
 int    if_txhprio_l2_check(int);
 int    if_txhprio_l3_check(int);
 int    if_rxhprio_l2_check(int);
index cbdc3fb..6c1c578 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: if_veb.c,v 1.32 2023/11/23 23:45:10 dlg Exp $ */
+/*     $OpenBSD: if_veb.c,v 1.33 2023/12/22 23:01:50 mvs Exp $ */
 
 /*
  * Copyright (c) 2021 David Gwynne <dlg@openbsd.org>
@@ -314,7 +314,6 @@ veb_clone_create(struct if_clone *ifc, int unit)
        ifp->if_flags = IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST;
        ifp->if_xflags = IFXF_CLONED | IFXF_MPSAFE;
 
-       if_counters_alloc(ifp);
        if_attach(ifp);
 
        if_alloc_sadl(ifp);
@@ -2348,7 +2347,6 @@ vport_clone_create(struct if_clone *ifc, int unit)
        ifp->if_xflags = IFXF_CLONED | IFXF_MPSAFE;
        ether_fakeaddr(ifp);
 
-       if_counters_alloc(ifp);
        if_attach(ifp);
        ether_ifattach(ifp);
 
index efcdac4..1246c8c 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: if_vlan.c,v 1.216 2023/10/27 20:56:47 jan Exp $       */
+/*     $OpenBSD: if_vlan.c,v 1.217 2023/12/22 23:01:50 mvs Exp $       */
 
 /*
  * Copyright 1998 Massachusetts Institute of Technology
@@ -215,7 +215,6 @@ vlan_clone_create(struct if_clone *ifc, int unit)
        ifp->if_hardmtu = 0xffff;
        ifp->if_link_state = LINK_STATE_DOWN;
 
-       if_counters_alloc(ifp);
        if_attach(ifp);
        ether_ifattach(ifp);
        ifp->if_hdrlen = EVL_ENCAPLEN;
index 210739a..2889dce 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: if_vxlan.c,v 1.97 2023/11/29 18:46:37 denis Exp $ */
+/*     $OpenBSD: if_vxlan.c,v 1.98 2023/12/22 23:01:50 mvs Exp $ */
 
 /*
  * Copyright (c) 2021 David Gwynne <dlg@openbsd.org>
@@ -275,7 +275,6 @@ vxlan_clone_create(struct if_clone *ifc, int unit)
        ifp->if_xflags = IFXF_CLONED | IFXF_MPSAFE;
        ether_fakeaddr(ifp);
 
-       if_counters_alloc(ifp);
        if_attach(ifp);
        ether_ifattach(ifp);
 
index b6fc0c8..5752aa4 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: if_wg.c,v 1.32 2023/10/23 10:22:05 mvs Exp $ */
+/*     $OpenBSD: if_wg.c,v 1.33 2023/12/22 23:01:50 mvs Exp $ */
 
 /*
  * Copyright (C) 2015-2020 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
@@ -2693,7 +2693,6 @@ wg_clone_create(struct if_clone *ifc, int unit)
 
        if_attach(ifp);
        if_alloc_sadl(ifp);
-       if_counters_alloc(ifp);
 
 #if NBPFILTER > 0
        bpfattach(&ifp->if_bpf, ifp, DLT_LOOP, sizeof(uint32_t));
index 9aae24d..4a843fe 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ip_carp.c,v 1.358 2023/09/16 09:33:27 mpi Exp $       */
+/*     $OpenBSD: ip_carp.c,v 1.359 2023/12/22 23:01:50 mvs Exp $       */
 
 /*
  * Copyright (c) 2002 Michael Shalayeff. All rights reserved.
@@ -831,7 +831,6 @@ carp_clone_create(struct if_clone *ifc, int unit)
        ifp->if_start = carp_start;
        ifp->if_enqueue = carp_enqueue;
        ifp->if_xflags = IFXF_CLONED;
-       if_counters_alloc(ifp);
        if_attach(ifp);
        ether_ifattach(ifp);
        ifp->if_type = IFT_CARP;