From: David Gwynne <david@gwynne.id.au>
Subject: Re: mbuf counters splnet
To: Alexander Bluhm <bluhm@openbsd.org>
Cc: OpenBSD Tech <tech@openbsd.org>
Date: Sun, 22 Jun 2025 11:18:49 +1000



> On 22 Jun 2025, at 08:01, Alexander Bluhm <bluhm@openbsd.org> wrote:
> 
> Hi,
> 
> As dlg@ has pointed out, MP safe conters still need spl protection
> when called from interrupt context.  Some increments of mbuf counters
> have that, others do not.
> 
> I would like to introduce mbstat_inc() inline function and enum
> counter types for mbufs, like we have elsewhere.  With that splnet()
> is taken everywhere.
> 
> ok?

ok

> 
> bluhm
> 
> Index: sys/kern/kern_sysctl.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/kern/kern_sysctl.c,v
> diff -u -p -r1.478 kern_sysctl.c
> --- sys/kern/kern_sysctl.c 12 Jun 2025 20:37:58 -0000 1.478
> +++ sys/kern/kern_sysctl.c 20 Jun 2025 20:56:56 -0000
> @@ -611,25 +611,25 @@ kern_sysctl(int *name, u_int namelen, vo
> return (sysctl_rdstruct(oldp, oldlenp, newp, &bt, sizeof bt));
> }
> case KERN_MBSTAT: {
> - uint64_t counters[MBSTAT_COUNT];
> + uint64_t counters[mbs_ncounters];
> struct mbstat mbs;
> unsigned int i;
> 
> memset(&mbs, 0, sizeof(mbs));
> - counters_read(mbstat, counters, MBSTAT_COUNT, NULL);
> - for (i = 0; i < MBSTAT_TYPES; i++)
> + counters_read(mbstat, counters, mbs_ncounters, NULL);
> + for (i = 0; i < MT_NTYPES; i++)
> mbs.m_mtypes[i] = counters[i];
> -
> - mbs.m_drops = counters[MBSTAT_DROPS];
> - mbs.m_wait = counters[MBSTAT_WAIT];
> - mbs.m_drain = counters[MBSTAT_DRAIN];
> - mbs.m_defrag_alloc = counters[MBSTAT_DEFRAG_ALLOC];
> - mbs.m_prepend_alloc = counters[MBSTAT_PREPEND_ALLOC];
> - mbs.m_pullup_alloc = counters[MBSTAT_PULLUP_ALLOC];
> - mbs.m_pullup_copy = counters[MBSTAT_PULLUP_COPY];
> - mbs.m_pulldown_alloc = counters[MBSTAT_PULLDOWN_ALLOC];
> - mbs.m_pulldown_copy = counters[MBSTAT_PULLDOWN_COPY];
> -
> +#define ASSIGN(name) do { mbs.m_##name = counters[mbs_##name]; } while (0)
> + ASSIGN(drops);
> + ASSIGN(wait);
> + ASSIGN(drain);
> + ASSIGN(defrag_alloc);
> + ASSIGN(prepend_alloc);
> + ASSIGN(pullup_alloc);
> + ASSIGN(pullup_copy);
> + ASSIGN(pulldown_alloc);
> + ASSIGN(pulldown_copy);
> +#undef ASSIGN
> return (sysctl_rdstruct(oldp, oldlenp, newp,
>    &mbs, sizeof(mbs)));
> }
> Index: sys/kern/uipc_mbuf.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_mbuf.c,v
> diff -u -p -r1.298 uipc_mbuf.c
> --- sys/kern/uipc_mbuf.c 13 Jun 2025 07:05:27 -0000 1.298
> +++ sys/kern/uipc_mbuf.c 20 Jun 2025 20:56:56 -0000
> @@ -85,7 +85,6 @@
> #include <sys/socket.h>
> #include <net/if.h>
> 
> -
> #include <uvm/uvm_extern.h>
> 
> #ifdef DDB
> @@ -98,7 +97,7 @@
> #endif /* NPF > 0 */
> 
> /* mbuf stats */
> -COUNTERS_BOOT_MEMORY(mbstat_boot, MBSTAT_COUNT);
> +COUNTERS_BOOT_MEMORY(mbstat_boot, mbs_ncounters);
> struct cpumem *mbstat = COUNTERS_BOOT_INITIALIZER(mbstat_boot);
> /* mbuf pools */
> struct pool mbpool;
> @@ -201,7 +200,7 @@ mbcpuinit(void)
> {
> int i;
> 
> - mbstat = counters_alloc_ncpus(mbstat, MBSTAT_COUNT);
> + mbstat = counters_alloc_ncpus(mbstat, mbs_ncounters);
> 
> pool_cache_init(&mbpool);
> pool_cache_init(&mtagpool);
> @@ -235,7 +234,6 @@ struct mbuf *
> m_get(int nowait, int type)
> {
> struct mbuf *m;
> - int s;
> 
> KASSERT(type >= 0 && type < MT_NTYPES);
> 
> @@ -243,9 +241,7 @@ m_get(int nowait, int type)
> if (m == NULL)
> return (NULL);
> 
> - s = splnet();
> - counters_inc(mbstat, type);
> - splx(s);
> + mbstat_inc(type);
> 
> m->m_type = type;
> m->m_next = NULL;
> @@ -264,7 +260,6 @@ struct mbuf *
> m_gethdr(int nowait, int type)
> {
> struct mbuf *m;
> - int s;
> 
> KASSERT(type >= 0 && type < MT_NTYPES);
> 
> @@ -272,9 +267,7 @@ m_gethdr(int nowait, int type)
> if (m == NULL)
> return (NULL);
> 
> - s = splnet();
> - counters_inc(mbstat, type);
> - splx(s);
> + mbstat_inc(type);
> 
> m->m_type = type;
> 
> @@ -546,7 +539,7 @@ m_defrag(struct mbuf *m, int how)
> 
> KASSERT(m->m_flags & M_PKTHDR);
> 
> - counters_inc(mbstat, MBSTAT_DEFRAG_ALLOC);
> + mbstat_inc(mbs_defrag_alloc);
> if ((m0 = m_gethdr(how, m->m_type)) == NULL)
> return (ENOBUFS);
> if (m->m_pkthdr.len > MHLEN) {
> @@ -606,7 +599,7 @@ m_prepend(struct mbuf *m, int len, int h
> m->m_data -= len;
> m->m_len += len;
> } else {
> - counters_inc(mbstat, MBSTAT_PREPEND_ALLOC);
> + mbstat_inc(mbs_prepend_alloc);
> MGET(mn, how, m->m_type);
> if (mn == NULL) {
> m_freem(m);
> @@ -948,7 +941,7 @@ m_pullup(struct mbuf *m0, int len)
> m0->m_data = head;
> }
> len -= m0->m_len;
> - counters_inc(mbstat, MBSTAT_PULLUP_COPY);
> + mbstat_inc(mbs_pullup_copy);
> } else {
> /* the first mbuf is too small or read-only, make a new one */
> space = adj + len;
> @@ -959,7 +952,7 @@ m_pullup(struct mbuf *m0, int len)
> m0->m_next = m;
> m = m0;
> 
> - counters_inc(mbstat, MBSTAT_PULLUP_ALLOC);
> + mbstat_inc(mbs_pullup_alloc);
> MGET(m0, M_DONTWAIT, m->m_type);
> if (m0 == NULL)
> goto bad;
> @@ -1467,7 +1460,7 @@ m_pool_alloc(struct pool *pp, int flags,
> return (v);
> 
>  fail:
> - counters_inc(mbstat, MBSTAT_DROPS);
> + mbstat_inc(mbs_drops);
> atomic_sub_long(&mbuf_mem_alloc, pp->pr_pgsize);
> return (NULL);
> }
> Index: sys/kern/uipc_mbuf2.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_mbuf2.c,v
> diff -u -p -r1.48 uipc_mbuf2.c
> --- sys/kern/uipc_mbuf2.c 27 May 2025 07:52:49 -0000 1.48
> +++ sys/kern/uipc_mbuf2.c 20 Jun 2025 20:56:56 -0000
> @@ -118,7 +118,7 @@ m_pulldown(struct mbuf *m, int off, int 
> if (len <= n->m_len - off) {
> struct mbuf *mlast;
> 
> - counters_inc(mbstat, MBSTAT_PULLDOWN_ALLOC);
> + mbstat_inc(mbs_pulldown_alloc);
> o = m_dup1(n, off, n->m_len - off, M_DONTWAIT);
> if (o == NULL) {
> m_freem(m);
> @@ -160,7 +160,7 @@ m_pulldown(struct mbuf *m, int off, int 
> */
> if ((off == 0 || offp) && m_trailingspace(n) >= tlen &&
>    !sharedcluster) {
> - counters_inc(mbstat, MBSTAT_PULLDOWN_COPY);
> + mbstat_inc(mbs_pulldown_copy);
> m_copydata(n->m_next, 0, tlen, mtod(n, caddr_t) + n->m_len);
> n->m_len += tlen;
> m_adj(n->m_next, tlen);
> @@ -170,7 +170,7 @@ m_pulldown(struct mbuf *m, int off, int 
>    !sharedcluster && n->m_next->m_len >= tlen) {
> n->m_next->m_data -= hlen;
> n->m_next->m_len += hlen;
> - counters_inc(mbstat, MBSTAT_PULLDOWN_COPY);
> + mbstat_inc(mbs_pulldown_copy);
> memcpy(mtod(n->m_next, caddr_t), mtod(n, caddr_t) + off, hlen);
> n->m_len -= hlen;
> n = n->m_next;
> @@ -186,7 +186,7 @@ m_pulldown(struct mbuf *m, int off, int 
> m_freem(m);
> return (NULL);
> }
> - counters_inc(mbstat, MBSTAT_PULLDOWN_ALLOC);
> + mbstat_inc(mbs_pulldown_alloc);
> MGET(o, M_DONTWAIT, m->m_type);
> if (o && len > MLEN) {
> MCLGETL(o, M_DONTWAIT, len);
> Index: sys/sys/mbuf.h
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/sys/mbuf.h,v
> diff -u -p -r1.265 mbuf.h
> --- sys/sys/mbuf.h 5 Nov 2024 13:15:13 -0000 1.265
> +++ sys/sys/mbuf.h 20 Jun 2025 20:56:56 -0000
> @@ -361,17 +361,18 @@ u_int mextfree_register(void (*)(caddr_t
> /* length to m_copy to copy all */
> #define M_COPYALL 1000000000
> 
> -#define MBSTAT_TYPES MT_NTYPES
> -#define MBSTAT_DROPS (MBSTAT_TYPES + 0)
> -#define MBSTAT_WAIT (MBSTAT_TYPES + 1)
> -#define MBSTAT_DRAIN (MBSTAT_TYPES + 2)
> -#define MBSTAT_DEFRAG_ALLOC (MBSTAT_TYPES + 3)
> -#define MBSTAT_PREPEND_ALLOC (MBSTAT_TYPES + 4)
> -#define MBSTAT_PULLUP_ALLOC (MBSTAT_TYPES + 5)
> -#define MBSTAT_PULLUP_COPY (MBSTAT_TYPES + 6)
> -#define MBSTAT_PULLDOWN_ALLOC (MBSTAT_TYPES + 7)
> -#define MBSTAT_PULLDOWN_COPY (MBSTAT_TYPES + 8)
> -#define MBSTAT_COUNT (MBSTAT_TYPES + 9)
> +enum mbstat_counters {
> + mbs_drops = MT_NTYPES,
> + mbs_wait,
> + mbs_drain,
> + mbs_defrag_alloc,
> + mbs_prepend_alloc,
> + mbs_pullup_alloc,
> + mbs_pullup_copy,
> + mbs_pulldown_alloc,
> + mbs_pulldown_copy,
> + mbs_ncounters
> +};
> 
> /*
>  * Mbuf statistics.
> @@ -379,11 +380,10 @@ u_int mextfree_register(void (*)(caddr_t
>  * pool headers (mbpool and mclpool).
>  */
> struct mbstat {
> - u_long m_drops; /* times failed to find space */
> - u_long m_wait; /* times waited for space */
> - u_long m_drain; /* times drained protocols for space */
> - u_long m_mtypes[MBSTAT_TYPES];
> - /* type specific mbuf allocations */
> + u_long m_drops; /* times failed to find space */
> + u_long m_wait; /* times waited for space */
> + u_long m_drain; /* times drained protocols for space */
> + u_long m_mtypes[MT_NTYPES]; /* type specific mbuf allocations */
> u_long m_defrag_alloc;
> u_long m_prepend_alloc;
> u_long m_pullup_alloc;
> @@ -464,6 +464,16 @@ m_freemp(struct mbuf **mp)
> 
> *mp = NULL;
> return m_freem(m);
> +}
> +
> +#include <sys/percpu.h>
> +
> +static inline void
> +mbstat_inc(enum mbstat_counters c)
> +{
> + int s = splnet();
> + counters_inc(mbstat, c);
> + splx(s);
> }
> 
> /* Packet tag routines */
> Index: usr.bin/netstat/mbuf.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/usr.bin/netstat/mbuf.c,v
> diff -u -p -r1.46 mbuf.c
> --- usr.bin/netstat/mbuf.c 29 Aug 2024 10:44:40 -0000 1.46
> +++ usr.bin/netstat/mbuf.c 20 Jun 2025 20:56:36 -0000
> @@ -79,7 +79,7 @@ static struct mbtypes {
> };
> 
> int nmbtypes = sizeof(mbstat.m_mtypes) / sizeof(u_long);
> -bool seen[MBSTAT_TYPES]; /* "have we seen this type yet?" */
> +bool seen[MT_NTYPES]; /* "have we seen this type yet?" */
> 
> /*
>  * Print mbuf statistics.
> @@ -93,7 +93,7 @@ mbpr(void)
> struct mbtypes *mp;
> size_t size;
> 
> - if (nmbtypes != MBSTAT_TYPES) {
> + if (nmbtypes != MT_NTYPES) {
> fprintf(stderr,
>    "%s: unexpected change to mbstat; check source\n",
>    __progname);
>