git: 55cc305dfcad - main - pf: revert: Use counter(9) for pf_state byte/packet tracking

[Mateusz Guzik]

The branch main has been updated by mjg:

URL: https://cgit.FreeBSD.org/src/commit/?id=55cc305dfcad0ad7c4f528fa47f7473927e8223a

commit 55cc305dfcad0ad7c4f528fa47f7473927e8223a
Author:     Mateusz Guzik <mjg at FreeBSD.org>
AuthorDate: 2021-06-28 18:50:56 +0000
Commit:     Mateusz Guzik <mjg at FreeBSD.org>
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);