From owner-dev-commits-src-all@freebsd.org Wed Dec 23 12:03:45 2020 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 149424BD903; Wed, 23 Dec 2020 12:03:45 +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 "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4D1Bgj03y1z4mK0; Wed, 23 Dec 2020 12:03:45 +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 E927E1D7F4; Wed, 23 Dec 2020 12:03:44 +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 0BNC3inr097132; Wed, 23 Dec 2020 12:03:44 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 0BNC3ilU097131; Wed, 23 Dec 2020 12:03:44 GMT (envelope-from git) Date: Wed, 23 Dec 2020 12:03:44 GMT Message-Id: <202012231203.0BNC3ilU097131@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Kristof Provost Subject: git: 1c00efe98ed7 - pf: 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: kp X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 1c00efe98ed7d103b9684ff692ffd5e3b64d0237 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: Wed, 23 Dec 2020 12:03:45 -0000 The branch main has been updated by kp: URL: https://cgit.FreeBSD.org/src/commit/?id=1c00efe98ed7d103b9684ff692ffd5e3b64d0237 commit 1c00efe98ed7d103b9684ff692ffd5e3b64d0237 Author: Kristof Provost AuthorDate: 2020-12-23 08:37:59 +0000 Commit: Kristof Provost CommitDate: 2020-12-23 11:03:21 +0000 pf: Use counter(9) for pf_state byte/packet tracking This improves cache behaviour by not writing to the same variable from multiple cores simultaneously. pf_state is only used in the kernel, so can be safely modified. Reviewed by: Lutz Donnerhacke, philip MFC after: 1 week Sponsed by: Orange Business Services Differential Revision: https://reviews.freebsd.org/D27661 --- sys/net/pfvar.h | 4 ++-- sys/netpfil/pf/if_pfsync.c | 13 +++++++++++++ sys/netpfil/pf/pf.c | 34 ++++++++++++++++++++++++++-------- sys/netpfil/pf/pf_ioctl.c | 10 ++++++---- 4 files changed, 47 insertions(+), 14 deletions(-) diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h index bd6a8c8bd7c4..eadd3c785d73 100644 --- a/sys/net/pfvar.h +++ b/sys/net/pfvar.h @@ -740,8 +740,8 @@ struct pf_state { struct pfi_kif *rt_kif; struct pf_src_node *src_node; struct pf_src_node *nat_src_node; - u_int64_t packets[2]; - u_int64_t bytes[2]; + counter_u64_t packets[2]; + counter_u64_t bytes[2]; u_int32_t creation; u_int32_t expire; u_int32_t pfsync_time; diff --git a/sys/netpfil/pf/if_pfsync.c b/sys/netpfil/pf/if_pfsync.c index b24efe10688d..2f696698e3eb 100644 --- a/sys/netpfil/pf/if_pfsync.c +++ b/sys/netpfil/pf/if_pfsync.c @@ -507,6 +507,13 @@ pfsync_state_import(struct pfsync_state *sp, u_int8_t flags) if ((st = uma_zalloc(V_pf_state_z, M_NOWAIT | M_ZERO)) == NULL) goto cleanup; + for (int i = 0; i < 2; i++) { + st->packets[i] = counter_u64_alloc(M_NOWAIT); + st->bytes[i] = counter_u64_alloc(M_NOWAIT); + if (st->packets[i] == NULL || st->bytes[i] == NULL) + goto cleanup; + } + if ((skw = uma_zalloc(V_pf_state_key_z, M_NOWAIT)) == NULL) goto cleanup; @@ -616,6 +623,12 @@ cleanup: cleanup_state: /* pf_state_insert() frees the state keys. */ if (st) { + for (int i = 0; i < 2; i++) { + if (st->packets[i] != NULL) + counter_u64_free(st->packets[i]); + if (st->bytes[i] != NULL) + counter_u64_free(st->bytes[i]); + } if (st->dst.scrub) uma_zfree(V_pf_state_scrub_z, st->dst.scrub); if (st->src.scrub) diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c index 047ad7fc308d..3cb423d8afa7 100644 --- a/sys/netpfil/pf/pf.c +++ b/sys/netpfil/pf/pf.c @@ -1714,6 +1714,13 @@ 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++) { + if (cur->bytes[i] != NULL) + counter_u64_free(cur->bytes[i]); + if (cur->packets[i] != NULL) + 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); @@ -3704,6 +3711,16 @@ pf_create_state(struct pf_rule *r, struct pf_rule *nr, struct pf_rule *a, REASON_SET(&reason, PFRES_MEMORY); goto csfailed; } + 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); + REASON_SET(&reason, PFRES_MEMORY); + goto csfailed; + } + } s->rule.ptr = r; s->nat_rule.ptr = nr; s->anchor.ptr = a; @@ -4263,8 +4280,9 @@ 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)(*state)->packets[0], - (unsigned long long)(*state)->packets[1], + pd->p_len, ackskew, + (unsigned long long)counter_u64_fetch((*state)->packets[0]), + (unsigned long long)counter_u64_fetch((*state)->packets[1]), pd->dir == PF_IN ? "in" : "out", pd->dir == (*state)->direction ? "fwd" : "rev"); } @@ -4319,8 +4337,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)(*state)->packets[0], - (unsigned long long)(*state)->packets[1], + (unsigned long long)counter_u64_fetch((*state)->packets[0]), + (unsigned long long)counter_u64_fetch((*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", @@ -6179,8 +6197,8 @@ done: s->nat_src_node->bytes[dirndx] += pd.tot_len; } dirndx = (dir == s->direction) ? 0 : 1; - s->packets[dirndx]++; - s->bytes[dirndx] += pd.tot_len; + counter_u64_add(s->packets[dirndx], 1); + counter_u64_add(s->bytes[dirndx], pd.tot_len); } tr = r; nr = (s != NULL) ? s->nat_rule.ptr : pd.nat_rule; @@ -6575,8 +6593,8 @@ done: s->nat_src_node->bytes[dirndx] += pd.tot_len; } dirndx = (dir == s->direction) ? 0 : 1; - s->packets[dirndx]++; - s->bytes[dirndx] += pd.tot_len; + counter_u64_add(s->packets[dirndx], 1); + counter_u64_add(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 1be52e1e1a84..9e31d1c966d9 100644 --- a/sys/netpfil/pf/pf_ioctl.c +++ b/sys/netpfil/pf/pf_ioctl.c @@ -3974,10 +3974,12 @@ 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(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]); + 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]); }