From ccf5da69583c0d4369ab3dc89805c858d4b2e8dc Mon Sep 17 00:00:00 2001 From: mvs Date: Tue, 12 Dec 2023 00:03:31 +0000 Subject: [PATCH] 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 --- sys/net/if_pflow.c | 59 ++++++++++++++++++++++++++++++++-------------- sys/net/if_pflow.h | 4 ++-- 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 @@ -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 @@ -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; -- 2.20.1