Date: Mon, 5 Jul 2021 11:33:54 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: aa9d233aadd8 - stable/13 - pf: revert: Use counter(9) for pf_state byte/packet tracking Message-ID: <202107051133.165BXsMM018103@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch stable/13 has been updated by mjg: URL: https://cgit.FreeBSD.org/src/commit/?id=aa9d233aadd840e4d671d352faf5bfc82ef8fbe7 commit aa9d233aadd840e4d671d352faf5bfc82ef8fbe7 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:32:13 +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 12ac791e3c72..b63f22e84bc0 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 3846e5335a2a..6ac19c9ad5d3 100644 --- a/sys/netpfil/pf/pf.c +++ b/sys/netpfil/pf/pf.c @@ -1739,23 +1739,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 @@ -1766,11 +1751,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); @@ -4313,9 +4293,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"); } @@ -4370,8 +4349,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", @@ -6276,8 +6255,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; @@ -6674,8 +6653,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 657c0e1945d3..49d5b2d216d2 100644 --- a/sys/netpfil/pf/pf_ioctl.c +++ b/sys/netpfil/pf/pf_ioctl.c @@ -4585,12 +4585,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?202107051133.165BXsMM018103>