From owner-dev-commits-src-all@freebsd.org Tue Jun 29 07:25:00 2021 Return-Path: Delivered-To: dev-commits-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 117816450B1; Tue, 29 Jun 2021 07:25:00 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4GDbbH6rCwz4RnG; Tue, 29 Jun 2021 07:24:59 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id D1D4314D4F; Tue, 29 Jun 2021 07:24:59 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 15T7Ox9n068190; Tue, 29 Jun 2021 07:24:59 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 15T7OxIF068189; Tue, 29 Jun 2021 07:24:59 GMT (envelope-from git) Date: Tue, 29 Jun 2021 07:24:59 GMT Message-Id: <202106290724.15T7OxIF068189@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Mateusz Guzik Subject: git: 55cc305dfcad - main - pf: revert: Use counter(9) for pf_state byte/packet tracking MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: mjg X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 55cc305dfcad0ad7c4f528fa47f7473927e8223a Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-all@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for all branches of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 29 Jun 2021 07:25:00 -0000 The branch main has been updated by mjg: URL: https://cgit.FreeBSD.org/src/commit/?id=55cc305dfcad0ad7c4f528fa47f7473927e8223a commit 55cc305dfcad0ad7c4f528fa47f7473927e8223a Author: Mateusz Guzik AuthorDate: 2021-06-28 18:50:56 +0000 Commit: Mateusz Guzik CommitDate: 2021-06-29 07:24:53 +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") --- 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 2ce47f926091..c97fffea845f 100644 --- a/sys/net/pfvar.h +++ b/sys/net/pfvar.h @@ -536,8 +536,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 7579b6bbebb0..69cdc75f0f41 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; @@ -6673,8 +6652,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 7a8d12ee4c3f..928e4a1259cc 100644 --- a/sys/netpfil/pf/pf_ioctl.c +++ b/sys/netpfil/pf/pf_ioctl.c @@ -4584,12 +4584,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);