Turn `pflowstats' statistics counters into per-CPU counters to make them

mpsafe.

The weird interactions around `pflow_flows' and `sc_gcounter' replaced
by simple `pflow_flows' increment. Since the flow sequence is the 32
bits integer, the `sc_gcounter' type replaced by the type of uint32_t.

ok bluhm sashan

2 files changed, 43 insertions, 20 deletions

diff --git a/sys/net/if_pflow.c b/sys/net/if_pflow.c
index 364b0a51571..4fa69b9d605 100644
--- a/sys/net/if_pflow.c
+++ b/sys/net/if_pflow.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_pflow.c,v 1.103 2023/12/11 14:25:09 mvs Exp $	*/
+/*	$OpenBSD: if_pflow.c,v 1.104 2023/12/12 00:03:31 mvs Exp $	*/
 
 /*
  * Copyright (c) 2011 Florian Obser <florian@narrans.de>
@@ -63,7 +63,22 @@
 #endif
 
 SMR_SLIST_HEAD(, pflow_softc) pflowif_list;
-struct pflowstats	 pflowstats;
+
+enum pflowstat_counters {
+	pflow_flows,
+	pflow_packets,
+	pflow_onomem,
+	pflow_oerrors,
+	pflow_ncounters,
+};
+
+struct cpumem *pflow_counters;
+
+static inline void
+pflowstat_inc(enum pflowstat_counters c)
+{
+	counters_inc(pflow_counters, c);
+}
 
 void	pflowattach(int);
 int	pflow_output(struct ifnet *ifp, struct mbuf *m, struct sockaddr *dst,
@@ -114,6 +129,7 @@ void
 pflowattach(int npflow)
 {
 	SMR_SLIST_INIT(&pflowif_list);
+	pflow_counters = counters_alloc(pflow_ncounters);
 	if_clone_attach(&pflow_cloner);
 }
 
@@ -515,7 +531,6 @@ pflowioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
 		if ((ifp->if_flags & IFF_UP) && sc->so != NULL) {
 			ifp->if_flags |= IFF_RUNNING;
 			mtx_enter(&sc->sc_mtx);
-			sc->sc_gcounter = pflowstats.pflow_flows;
 			/* send templates on startup */
 			if (sc->sc_version == PFLOW_PROTO_10)
 				pflow_sendout_ipfix_tmpl(sc);
@@ -579,7 +594,6 @@ pflowioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
 		if ((ifp->if_flags & IFF_UP) && sc->so != NULL) {
 			ifp->if_flags |= IFF_RUNNING;
 			mtx_enter(&sc->sc_mtx);
-			sc->sc_gcounter = pflowstats.pflow_flows;
 			if (sc->sc_version == PFLOW_PROTO_10)
 				pflow_sendout_ipfix_tmpl(sc);
 			mtx_leave(&sc->sc_mtx);
@@ -648,14 +662,14 @@ pflow_get_mbuf(struct pflow_softc *sc, u_int16_t set_id)
 
 	MGETHDR(m, M_DONTWAIT, MT_DATA);
 	if (m == NULL) {
-		pflowstats.pflow_onomem++;
+		pflowstat_inc(pflow_onomem);
 		return (NULL);
 	}
 
 	MCLGET(m, M_DONTWAIT);
 	if ((m->m_flags & M_EXT) == 0) {
 		m_free(m);
-		pflowstats.pflow_onomem++;
+		pflowstat_inc(pflow_onomem);
 		return (NULL);
 	}
 
@@ -900,8 +914,7 @@ copy_flow_to_m(struct pflow_flow *flow, struct pflow_softc *sc)
 	    (sc->sc_count * sizeof(struct pflow_flow)),
 	    sizeof(struct pflow_flow), flow, M_NOWAIT);
 
-	if (pflowstats.pflow_flows == sc->sc_gcounter)
-		pflowstats.pflow_flows++;
+	pflowstat_inc(pflow_flows);
 	sc->sc_gcounter++;
 	sc->sc_count++;
 
@@ -930,8 +943,7 @@ copy_flow_ipfix_4_to_m(struct pflow_ipfix_flow4 *flow, struct pflow_softc *sc)
 	    (sc->sc_count4 * sizeof(struct pflow_ipfix_flow4)),
 	    sizeof(struct pflow_ipfix_flow4), flow, M_NOWAIT);
 
-	if (pflowstats.pflow_flows == sc->sc_gcounter)
-		pflowstats.pflow_flows++;
+	pflowstat_inc(pflow_flows);
 	sc->sc_gcounter++;
 	sc->sc_count4++;
 
@@ -959,8 +971,7 @@ copy_flow_ipfix_6_to_m(struct pflow_ipfix_flow6 *flow, struct pflow_softc *sc)
 	    (sc->sc_count6 * sizeof(struct pflow_ipfix_flow6)),
 	    sizeof(struct pflow_ipfix_flow6), flow, M_NOWAIT);
 
-	if (pflowstats.pflow_flows == sc->sc_gcounter)
-		pflowstats.pflow_flows++;
+	pflowstat_inc(pflow_flows);
 	sc->sc_gcounter++;
 	sc->sc_count6++;
 
@@ -1116,7 +1127,7 @@ pflow_sendout_v5(struct pflow_softc *sc)
 		return (0);
 	}
 
-	pflowstats.pflow_packets++;
+	pflowstat_inc(pflow_packets);
 	h = mtod(m, struct pflow_header *);
 	h->count = htons(sc->sc_count);
 
@@ -1173,14 +1184,14 @@ pflow_sendout_ipfix(struct pflow_softc *sc, sa_family_t af)
 		return (0);
 	}
 
-	pflowstats.pflow_packets++;
+	pflowstat_inc(pflow_packets);
 	set_hdr = mtod(m, struct pflow_set_header *);
 	set_hdr->set_length = htons(set_length);
 
 	/* populate pflow_header */
 	M_PREPEND(m, sizeof(struct pflow_v10_header), M_DONTWAIT);
 	if (m == NULL) {
-		pflowstats.pflow_onomem++;
+		pflowstat_inc(pflow_onomem);
 		return (ENOBUFS);
 	}
 	h10 = mtod(m, struct pflow_v10_header *);
@@ -1217,12 +1228,12 @@ pflow_sendout_ipfix_tmpl(struct pflow_softc *sc)
 		m_freem(m);
 		return (0);
 	}
-	pflowstats.pflow_packets++;
+	pflowstat_inc(pflow_packets);
 
 	/* populate pflow_header */
 	M_PREPEND(m, sizeof(struct pflow_v10_header), M_DONTWAIT);
 	if (m == NULL) {
-		pflowstats.pflow_onomem++;
+		pflowstat_inc(pflow_onomem);
 		return (ENOBUFS);
 	}
 	h10 = mtod(m, struct pflow_v10_header *);
@@ -1262,11 +1273,23 @@ pflow_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp,
 		return (ENOTDIR);
 
 	switch (name[0]) {
-	case NET_PFLOW_STATS:
+	case NET_PFLOW_STATS: {
+		uint64_t counters[pflow_ncounters];
+		struct pflowstats pflowstats;
+
 		if (newp != NULL)
 			return (EPERM);
+
+		counters_read(pflow_counters, counters, pflow_ncounters, NULL);
+
+		pflowstats.pflow_flows = counters[pflow_flows];
+		pflowstats.pflow_packets = counters[pflow_packets];
+		pflowstats.pflow_onomem = counters[pflow_onomem];
+		pflowstats.pflow_oerrors = counters[pflow_oerrors];
+
 		return (sysctl_struct(oldp, oldlenp, newp, newlen,
 		    &pflowstats, sizeof(pflowstats)));
+	}
 	default:
 		return (EOPNOTSUPP);
 	}
diff --git a/sys/net/if_pflow.h b/sys/net/if_pflow.h
index 1aacf25206e..a4cc724d06f 100644
--- a/sys/net/if_pflow.h
+++ b/sys/net/if_pflow.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_pflow.h,v 1.21 2023/12/11 14:25:09 mvs Exp $	*/
+/*	$OpenBSD: if_pflow.h,v 1.22 2023/12/12 00:03:31 mvs Exp $	*/
 
 /*
  * Copyright (c) 2008 Henning Brauer <henning@openbsd.org>
@@ -192,7 +192,7 @@ struct pflow_softc {
 	unsigned int		 sc_maxcount;	/* [m] */
 	unsigned int		 sc_maxcount4;	/* [m] */
 	unsigned int		 sc_maxcount6;	/* [m] */
-	u_int64_t		 sc_gcounter;	/* [m] */
+	u_int32_t		 sc_gcounter;	/* [m] */
 	u_int32_t		 sc_sequence;	/* [m] */
 	struct timeout		 sc_tmo;
 	struct timeout		 sc_tmo6;