Date: Mon, 5 Jul 2021 12:02:02 GMT From: Mateusz Guzik <mjg@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org Subject: git: 3f136d9fea86 - stable/12 - pf: revert: Use counter(9) for pf_state byte/packet tracking Message-ID: <202107051202.165C22vf055522@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch stable/12 has been updated by mjg: URL: https://cgit.FreeBSD.org/src/commit/?id=3f136d9fea869515900bcda1ef6533f5804d819c commit 3f136d9fea869515900bcda1ef6533f5804d819c Author: Mateusz Guzik <mjg@FreeBSD.org> AuthorDate: 2021-06-28 18:50:56 +0000 Commit: Mateusz Guzik <mjg@FreeBSD.org> CommitDate: 2021-07-05 11:38:08 +0000 pf: revert: Use counter(9) for pf_state byte/packet tracking stats are not shared and consequently per-CPU counters only waste memory. No slowdown was measured when passing over 20M pps. Reviewed by: kp Sponsored by: Rubicon Communications, LLC ("Netgate") (cherry picked from commit 55cc305dfcad0ad7c4f528fa47f7473927e8223a) --- sys/net/pfvar.h | 4 ++-- sys/netpfil/pf/pf.c | 39 +++++++++------------------------------ sys/netpfil/pf/pf_ioctl.c | 10 ++++------ sys/netpfil/pf/pf_nv.c | 4 ++-- 4 files changed, 17 insertions(+), 40 deletions(-) diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h index 4f11ff9137d8..c11455837072 100644 --- a/sys/net/pfvar.h +++ b/sys/net/pfvar.h @@ -533,8 +533,8 @@ struct pf_state { struct pfi_kkif *rt_kif; struct pf_ksrc_node *src_node; struct pf_ksrc_node *nat_src_node; - counter_u64_t packets[2]; - counter_u64_t bytes[2]; + u_int64_t packets[2]; + u_int64_t bytes[2]; u_int32_t creation; u_int32_t expire; u_int32_t pfsync_time; diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c index c76207d1e7f1..6edbd60b8d07 100644 --- a/sys/netpfil/pf/pf.c +++ b/sys/netpfil/pf/pf.c @@ -1737,23 +1737,8 @@ pf_unlink_state(struct pf_state *s, u_int flags) struct pf_state * pf_alloc_state(int flags) { - struct pf_state *s; - - s = uma_zalloc(V_pf_state_z, flags | M_ZERO); - if (__predict_false(s == NULL)) - return (NULL); - - for (int i = 0; i < 2; i++) { - s->bytes[i] = counter_u64_alloc(M_NOWAIT); - s->packets[i] = counter_u64_alloc(M_NOWAIT); - - if (s->bytes[i] == NULL || s->packets[i] == NULL) { - pf_free_state(s); - return (NULL); - } - } - return (s); + return (uma_zalloc(V_pf_state_z, flags | M_ZERO)); } void @@ -1764,11 +1749,6 @@ pf_free_state(struct pf_state *cur) KASSERT(cur->timeout == PFTM_UNLINKED, ("%s: timeout %u", __func__, cur->timeout)); - for (int i = 0; i < 2; i++) { - counter_u64_free(cur->bytes[i]); - counter_u64_free(cur->packets[i]); - } - pf_normalize_tcp_cleanup(cur); uma_zfree(V_pf_state_z, cur); counter_u64_add(V_pf_status.fcounters[FCNT_STATE_REMOVALS], 1); @@ -4317,9 +4297,8 @@ pf_tcp_track_full(struct pf_state_peer *src, struct pf_state_peer *dst, pf_print_flags(th->th_flags); printf(" seq=%u (%u) ack=%u len=%u ackskew=%d " "pkts=%llu:%llu dir=%s,%s\n", seq, orig_seq, ack, - pd->p_len, ackskew, - (unsigned long long)counter_u64_fetch((*state)->packets[0]), - (unsigned long long)counter_u64_fetch((*state)->packets[1]), + pd->p_len, ackskew, (unsigned long long)(*state)->packets[0], + (unsigned long long)(*state)->packets[1], pd->dir == PF_IN ? "in" : "out", pd->dir == (*state)->direction ? "fwd" : "rev"); } @@ -4374,8 +4353,8 @@ pf_tcp_track_full(struct pf_state_peer *src, struct pf_state_peer *dst, printf(" seq=%u (%u) ack=%u len=%u ackskew=%d " "pkts=%llu:%llu dir=%s,%s\n", seq, orig_seq, ack, pd->p_len, ackskew, - (unsigned long long)counter_u64_fetch((*state)->packets[0]), - (unsigned long long)counter_u64_fetch((*state)->packets[1]), + (unsigned long long)(*state)->packets[0], + (unsigned long long)(*state)->packets[1], pd->dir == PF_IN ? "in" : "out", pd->dir == (*state)->direction ? "fwd" : "rev"); printf("pf: State failure on: %c %c %c %c | %c %c\n", @@ -6363,8 +6342,8 @@ done: pd.tot_len); } dirndx = (dir == s->direction) ? 0 : 1; - counter_u64_add(s->packets[dirndx], 1); - counter_u64_add(s->bytes[dirndx], pd.tot_len); + s->packets[dirndx]++; + s->bytes[dirndx] += pd.tot_len; } tr = r; nr = (s != NULL) ? s->nat_rule.ptr : pd.nat_rule; @@ -6764,8 +6743,8 @@ done: pd.tot_len); } dirndx = (dir == s->direction) ? 0 : 1; - counter_u64_add(s->packets[dirndx], 1); - counter_u64_add(s->bytes[dirndx], pd.tot_len); + s->packets[dirndx]++; + s->bytes[dirndx] += pd.tot_len; } tr = r; nr = (s != NULL) ? s->nat_rule.ptr : pd.nat_rule; diff --git a/sys/netpfil/pf/pf_ioctl.c b/sys/netpfil/pf/pf_ioctl.c index 3d5ae03b5a11..ce7a8d1dc582 100644 --- a/sys/netpfil/pf/pf_ioctl.c +++ b/sys/netpfil/pf/pf_ioctl.c @@ -4652,12 +4652,10 @@ pfsync_state_export(struct pfsync_state *sp, struct pf_state *st) else sp->nat_rule = htonl(st->nat_rule.ptr->nr); - pf_state_counter_hton(counter_u64_fetch(st->packets[0]), - sp->packets[0]); - pf_state_counter_hton(counter_u64_fetch(st->packets[1]), - sp->packets[1]); - pf_state_counter_hton(counter_u64_fetch(st->bytes[0]), sp->bytes[0]); - pf_state_counter_hton(counter_u64_fetch(st->bytes[1]), sp->bytes[1]); + pf_state_counter_hton(st->packets[0], sp->packets[0]); + pf_state_counter_hton(st->packets[1], sp->packets[1]); + pf_state_counter_hton(st->bytes[0], sp->bytes[0]); + pf_state_counter_hton(st->bytes[1], sp->bytes[1]); } diff --git a/sys/netpfil/pf/pf_nv.c b/sys/netpfil/pf/pf_nv.c index 31943ba69687..553290c88586 100644 --- a/sys/netpfil/pf/pf_nv.c +++ b/sys/netpfil/pf/pf_nv.c @@ -982,9 +982,9 @@ pf_state_to_nvstate(const struct pf_state *s) for (int i = 0; i < 2; i++) { nvlist_append_number_array(nvl, "packets", - counter_u64_fetch(s->packets[i])); + s->packets[i]); nvlist_append_number_array(nvl, "bytes", - counter_u64_fetch(s->bytes[i])); + s->bytes[i]); } nvlist_add_number(nvl, "creatorid", s->creatorid);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202107051202.165C22vf055522>