git: 1c00efe98ed7 - pf: Use counter(9) for pf_state byte/packet tracking

Kristof Provost kp at FreeBSD.org
Wed Dec 23 12:03:45 UTC 2020


The branch main has been updated by kp:

URL: https://cgit.FreeBSD.org/src/commit/?id=1c00efe98ed7d103b9684ff692ffd5e3b64d0237

commit 1c00efe98ed7d103b9684ff692ffd5e3b64d0237
Author:     Kristof Provost <kp at FreeBSD.org>
AuthorDate: 2020-12-23 08:37:59 +0000
Commit:     Kristof Provost <kp at FreeBSD.org>
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]);
 
 }
 


More information about the dev-commits-src-all mailing list