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

diff --git a/sys/net/if_pflow.c b/sys/net/if_pflow.c
index 364b0a5..4fa69b9 100644 (file)
--- 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 1aacf25..a4cc724 100644 (file)
--- 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;