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

4 files changed, 47 insertions, 14 deletions

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]);
 
 }