From 1c00efe98ed7d103b9684ff692ffd5e3b64d0237 Mon Sep 17 00:00:00 2001 From: Kristof Provost Date: Wed, 23 Dec 2020 09:37:59 +0100 Subject: 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(-) (limited to 'sys') diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h index bd6a8c8bd7c41..eadd3c785d738 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 b24efe10688de..2f696698e3eb9 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 047ad7fc308d7..3cb423d8afa76 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 1be52e1e1a847..9e31d1c966d99 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]); } -- cgit v1.2.3