Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 20 Jan 2021 14:44:59 GMT
From:      Kristof Provost <kp@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: 14d531ad3f58 - stable/12 - pf: Change pf_krule counters to use counter_u64
Message-ID:  <202101201444.10KEixAe019427@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch stable/12 has been updated by kp:

URL: https://cgit.FreeBSD.org/src/commit/?id=14d531ad3f5880a375c0cab50ff8cc238099c4d6

commit 14d531ad3f5880a375c0cab50ff8cc238099c4d6
Author:     Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2020-12-05 20:41:42 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2021-01-20 14:16:05 +0000

    pf: Change pf_krule counters to use counter_u64
    
    This improves the cache behaviour of pf and results in improved
    throughput.
    
    MFC after:      2 weeks
    Sponsored by:   Orange Business Services
    Differential Revision:  https://reviews.freebsd.org/D27760
    
    (cherry picked from commit c3adacdad4d72b045058cd932351b94579cdd543)
---
 sys/net/pfvar.h           |  6 ++---
 sys/netpfil/pf/pf.c       | 32 ++++++++++++----------
 sys/netpfil/pf/pf_ioctl.c | 69 +++++++++++++++++++++++++++++++++++++----------
 sys/netpfil/pf/pf_lb.c    |  2 +-
 sys/netpfil/pf/pf_norm.c  | 18 ++++++-------
 5 files changed, 86 insertions(+), 41 deletions(-)

diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h
index 5b77b0a6c4e2..636ea8c5e02c 100644
--- a/sys/net/pfvar.h
+++ b/sys/net/pfvar.h
@@ -315,9 +315,9 @@ struct pf_krule {
 	TAILQ_ENTRY(pf_krule)	 entries;
 	struct pf_pool		 rpool;
 
-	u_int64_t		 evaluations;
-	u_int64_t		 packets[2];
-	u_int64_t		 bytes[2];
+	counter_u64_t		 evaluations;
+	counter_u64_t		 packets[2];
+	counter_u64_t		 bytes[2];
 
 	struct pfi_kif		*kif;
 	struct pf_kanchor	*anchor;
diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
index 85a6b27d7139..c2cc2ba55196 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -3538,7 +3538,7 @@ pf_test_rule(struct pf_krule **rm, struct pf_state **sm, int direction,
 	}
 
 	while (r != NULL) {
-		r->evaluations++;
+		counter_u64_add(r->evaluations, 1);
 		if (pfi_kif_match(r->kif, kif) == r->ifnot)
 			r = r->skip[PF_SKIP_IFP].ptr;
 		else if (r->direction && r->direction != direction)
@@ -3977,7 +3977,7 @@ pf_test_fragment(struct pf_krule **rm, int direction, struct pfi_kif *kif,
 
 	r = TAILQ_FIRST(pf_main_ruleset.rules[PF_RULESET_FILTER].active.ptr);
 	while (r != NULL) {
-		r->evaluations++;
+		counter_u64_add(r->evaluations, 1);
 		if (pfi_kif_match(r->kif, kif) == r->ifnot)
 			r = r->skip[PF_SKIP_IFP].ptr;
 		else if (r->direction && r->direction != direction)
@@ -6334,16 +6334,18 @@ done:
 
 	if (action == PF_PASS || r->action == PF_DROP) {
 		dirndx = (dir == PF_OUT);
-		r->packets[dirndx]++;
-		r->bytes[dirndx] += pd.tot_len;
+		counter_u64_add(r->packets[dirndx], 1);
+		counter_u64_add(r->bytes[dirndx], pd.tot_len);
 		if (a != NULL) {
-			a->packets[dirndx]++;
-			a->bytes[dirndx] += pd.tot_len;
+			counter_u64_add(a->packets[dirndx], 1);
+			counter_u64_add(a->bytes[dirndx], pd.tot_len);
 		}
 		if (s != NULL) {
 			if (s->nat_rule.ptr != NULL) {
-				s->nat_rule.ptr->packets[dirndx]++;
-				s->nat_rule.ptr->bytes[dirndx] += pd.tot_len;
+				counter_u64_add(s->nat_rule.ptr->packets[dirndx],
+				    1);
+				counter_u64_add(s->nat_rule.ptr->bytes[dirndx],
+				    pd.tot_len);
 			}
 			if (s->src_node != NULL) {
 				counter_u64_add(s->src_node->packets[dirndx],
@@ -6737,16 +6739,18 @@ done:
 
 	if (action == PF_PASS || r->action == PF_DROP) {
 		dirndx = (dir == PF_OUT);
-		r->packets[dirndx]++;
-		r->bytes[dirndx] += pd.tot_len;
+		counter_u64_add(r->packets[dirndx], 1);
+		counter_u64_add(r->bytes[dirndx], pd.tot_len);
 		if (a != NULL) {
-			a->packets[dirndx]++;
-			a->bytes[dirndx] += pd.tot_len;
+			counter_u64_add(a->packets[dirndx], 1);
+			counter_u64_add(a->bytes[dirndx], pd.tot_len);
 		}
 		if (s != NULL) {
 			if (s->nat_rule.ptr != NULL) {
-				s->nat_rule.ptr->packets[dirndx]++;
-				s->nat_rule.ptr->bytes[dirndx] += pd.tot_len;
+				counter_u64_add(s->nat_rule.ptr->packets[dirndx],
+				    1);
+				counter_u64_add(s->nat_rule.ptr->bytes[dirndx],
+				    pd.tot_len);
 			}
 			if (s->src_node != NULL) {
 				counter_u64_add(s->src_node->packets[dirndx],
diff --git a/sys/netpfil/pf/pf_ioctl.c b/sys/netpfil/pf/pf_ioctl.c
index e928c65734a7..c6d4a163fd45 100644
--- a/sys/netpfil/pf/pf_ioctl.c
+++ b/sys/netpfil/pf/pf_ioctl.c
@@ -283,6 +283,11 @@ pfattach_vnet(void)
 	V_pf_default_rule.nr = -1;
 	V_pf_default_rule.rtableid = -1;
 
+	V_pf_default_rule.evaluations = counter_u64_alloc(M_WAITOK);
+	for (int i = 0; i < 2; i++) {
+		V_pf_default_rule.packets[i] = counter_u64_alloc(M_WAITOK);
+		V_pf_default_rule.bytes[i] = counter_u64_alloc(M_WAITOK);
+	}
 	V_pf_default_rule.states_cur = counter_u64_alloc(M_WAITOK);
 	V_pf_default_rule.states_tot = counter_u64_alloc(M_WAITOK);
 	V_pf_default_rule.src_nodes = counter_u64_alloc(M_WAITOK);
@@ -462,6 +467,11 @@ pf_free_rule(struct pf_krule *rule)
 		pfi_kif_unref(rule->kif);
 	pf_kanchor_remove(rule);
 	pf_empty_pool(&rule->rpool.list);
+	counter_u64_free(rule->evaluations);
+	for (int i = 0; i < 2; i++) {
+		counter_u64_free(rule->packets[i]);
+		counter_u64_free(rule->bytes[i]);
+	}
 	counter_u64_free(rule->states_cur);
 	counter_u64_free(rule->states_tot);
 	counter_u64_free(rule->src_nodes);
@@ -1455,10 +1465,10 @@ pf_krule_to_rule(const struct pf_krule *krule, struct pf_rule *rule)
 
 	bcopy(&krule->rpool, &rule->rpool, sizeof(krule->rpool));
 
-	rule->evaluations = krule->evaluations;
+	rule->evaluations = counter_u64_fetch(krule->evaluations);
 	for (int i = 0; i < 2; i++) {
-		rule->packets[i] = krule->packets[i];
-		rule->bytes[i] = krule->bytes[i];
+		rule->packets[i] = counter_u64_fetch(krule->packets[i]);
+		rule->bytes[i] = counter_u64_fetch(krule->bytes[i]);
 	}
 
 	/* kif, anchor, overload_tbl are not copied over. */
@@ -1810,6 +1820,11 @@ pfioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flags, struct thread *td
 
 		if (rule->ifname[0])
 			kif = malloc(sizeof(*kif), PFI_MTYPE, M_WAITOK);
+		rule->evaluations = counter_u64_alloc(M_WAITOK);
+		for (int i = 0; i < 2; i++) {
+			rule->packets[i] = counter_u64_alloc(M_WAITOK);
+			rule->bytes[i] = counter_u64_alloc(M_WAITOK);
+		}
 		rule->states_cur = counter_u64_alloc(M_WAITOK);
 		rule->states_tot = counter_u64_alloc(M_WAITOK);
 		rule->src_nodes = counter_u64_alloc(M_WAITOK);
@@ -1923,8 +1938,11 @@ pfioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flags, struct thread *td
 		}
 
 		rule->rpool.cur = TAILQ_FIRST(&rule->rpool.list);
-		rule->evaluations = rule->packets[0] = rule->packets[1] =
-		    rule->bytes[0] = rule->bytes[1] = 0;
+		counter_u64_zero(rule->evaluations);
+		for (int i = 0; i < 2; i++) {
+			counter_u64_zero(rule->packets[i]);
+			counter_u64_zero(rule->bytes[i]);
+		}
 		TAILQ_INSERT_TAIL(ruleset->rules[rs_num].inactive.ptr,
 		    rule, entries);
 		ruleset->rules[rs_num].inactive.rcount++;
@@ -1934,6 +1952,11 @@ pfioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flags, struct thread *td
 #undef ERROUT
 DIOCADDRULE_error:
 		PF_RULES_WUNLOCK();
+		counter_u64_free(rule->evaluations);
+		for (int i = 0; i < 2; i++) {
+			counter_u64_free(rule->packets[i]);
+			counter_u64_free(rule->bytes[i]);
+		}
 		counter_u64_free(rule->states_cur);
 		counter_u64_free(rule->states_tot);
 		counter_u64_free(rule->src_nodes);
@@ -2019,9 +2042,11 @@ DIOCADDRULE_error:
 		pf_addr_copyout(&pr->rule.dst.addr);
 
 		if (pr->action == PF_GET_CLR_CNTR) {
-			rule->evaluations = 0;
-			rule->packets[0] = rule->packets[1] = 0;
-			rule->bytes[0] = rule->bytes[1] = 0;
+			counter_u64_zero(rule->evaluations);
+			for (int i = 0; i < 2; i++) {
+				counter_u64_zero(rule->packets[i]);
+				counter_u64_zero(rule->bytes[i]);
+			}
 			counter_u64_zero(rule->states_tot);
 		}
 		PF_RULES_WUNLOCK();
@@ -2065,6 +2090,13 @@ DIOCADDRULE_error:
 
 			if (newrule->ifname[0])
 				kif = malloc(sizeof(*kif), PFI_MTYPE, M_WAITOK);
+			newrule->evaluations = counter_u64_alloc(M_WAITOK);
+			for (int i = 0; i < 2; i++) {
+				newrule->packets[i] =
+				    counter_u64_alloc(M_WAITOK);
+				newrule->bytes[i] =
+				    counter_u64_alloc(M_WAITOK);
+			}
 			newrule->states_cur = counter_u64_alloc(M_WAITOK);
 			newrule->states_tot = counter_u64_alloc(M_WAITOK);
 			newrule->src_nodes = counter_u64_alloc(M_WAITOK);
@@ -2178,9 +2210,6 @@ DIOCADDRULE_error:
 			}
 
 			newrule->rpool.cur = TAILQ_FIRST(&newrule->rpool.list);
-			newrule->evaluations = 0;
-			newrule->packets[0] = newrule->packets[1] = 0;
-			newrule->bytes[0] = newrule->bytes[1] = 0;
 		}
 		pf_empty_pool(&V_pf_pabuf);
 
@@ -2240,6 +2269,11 @@ DIOCADDRULE_error:
 DIOCCHANGERULE_error:
 		PF_RULES_WUNLOCK();
 		if (newrule != NULL) {
+			counter_u64_free(newrule->evaluations);
+			for (int i = 0; i < 2; i++) {
+				counter_u64_free(newrule->packets[i]);
+				counter_u64_free(newrule->bytes[i]);
+			}
 			counter_u64_free(newrule->states_cur);
 			counter_u64_free(newrule->states_tot);
 			counter_u64_free(newrule->src_nodes);
@@ -2626,9 +2660,11 @@ DIOCGETSTATES_full:
 		PF_RULES_WLOCK();
 		TAILQ_FOREACH(rule,
 		    ruleset->rules[PF_RULESET_FILTER].active.ptr, entries) {
-			rule->evaluations = 0;
-			rule->packets[0] = rule->packets[1] = 0;
-			rule->bytes[0] = rule->bytes[1] = 0;
+			counter_u64_zero(rule->evaluations);
+			for (int i = 0; i < 2; i++) {
+				counter_u64_zero(rule->packets[i]);
+				counter_u64_zero(rule->bytes[i]);
+			}
 		}
 		PF_RULES_WUNLOCK();
 		break;
@@ -4643,6 +4679,11 @@ pf_unload_vnet(void)
 	uma_zdestroy(V_pf_tag_z);
 
 	/* Free counters last as we updated them during shutdown. */
+	counter_u64_free(V_pf_default_rule.evaluations);
+	for (int i = 0; i < 2; i++) {
+		counter_u64_free(V_pf_default_rule.packets[i]);
+		counter_u64_free(V_pf_default_rule.bytes[i]);
+	}
 	counter_u64_free(V_pf_default_rule.states_cur);
 	counter_u64_free(V_pf_default_rule.states_tot);
 	counter_u64_free(V_pf_default_rule.src_nodes);
diff --git a/sys/netpfil/pf/pf_lb.c b/sys/netpfil/pf/pf_lb.c
index d2bdbb6d971c..ff115f64b365 100644
--- a/sys/netpfil/pf/pf_lb.c
+++ b/sys/netpfil/pf/pf_lb.c
@@ -149,7 +149,7 @@ pf_match_translation(struct pf_pdesc *pd, struct mbuf *m, int off,
 			dst = &r->dst;
 		}
 
-		r->evaluations++;
+		counter_u64_add(r->evaluations, 1);
 		if (pfi_kif_match(r->kif, kif) == r->ifnot)
 			r = r->skip[PF_SKIP_IFP].ptr;
 		else if (r->direction && r->direction != direction)
diff --git a/sys/netpfil/pf/pf_norm.c b/sys/netpfil/pf/pf_norm.c
index 456e5184d5a5..d1a8ef37d030 100644
--- a/sys/netpfil/pf/pf_norm.c
+++ b/sys/netpfil/pf/pf_norm.c
@@ -839,7 +839,7 @@ pf_normalize_ip(struct mbuf **m0, int dir, struct pfi_kif *kif, u_short *reason,
 
 	r = TAILQ_FIRST(pf_main_ruleset.rules[PF_RULESET_SCRUB].active.ptr);
 	while (r != NULL) {
-		r->evaluations++;
+		counter_u64_add(r->evaluations, 1);
 		if (pfi_kif_match(r->kif, kif) == r->ifnot)
 			r = r->skip[PF_SKIP_IFP].ptr;
 		else if (r->direction && r->direction != dir)
@@ -866,8 +866,8 @@ pf_normalize_ip(struct mbuf **m0, int dir, struct pfi_kif *kif, u_short *reason,
 	if (r == NULL || r->action == PF_NOSCRUB)
 		return (PF_PASS);
 	else {
-		r->packets[dir == PF_OUT]++;
-		r->bytes[dir == PF_OUT] += pd->tot_len;
+		counter_u64_add(r->packets[dir == PF_OUT], 1);
+		counter_u64_add(r->bytes[dir == PF_OUT], pd->tot_len);
 	}
 
 	/* Check for illegal packets */
@@ -983,7 +983,7 @@ pf_normalize_ip6(struct mbuf **m0, int dir, struct pfi_kif *kif,
 
 	r = TAILQ_FIRST(pf_main_ruleset.rules[PF_RULESET_SCRUB].active.ptr);
 	while (r != NULL) {
-		r->evaluations++;
+		counter_u64_add(r->evaluations, 1);
 		if (pfi_kif_match(r->kif, kif) == r->ifnot)
 			r = r->skip[PF_SKIP_IFP].ptr;
 		else if (r->direction && r->direction != dir)
@@ -1009,8 +1009,8 @@ pf_normalize_ip6(struct mbuf **m0, int dir, struct pfi_kif *kif,
 	if (r == NULL || r->action == PF_NOSCRUB)
 		return (PF_PASS);
 	else {
-		r->packets[dir == PF_OUT]++;
-		r->bytes[dir == PF_OUT] += pd->tot_len;
+		counter_u64_add(r->packets[dir == PF_OUT], 1);
+		counter_u64_add(r->bytes[dir == PF_OUT], pd->tot_len);
 	}
 
 	/* Check for illegal packets */
@@ -1161,7 +1161,7 @@ pf_normalize_tcp(int dir, struct pfi_kif *kif, struct mbuf *m, int ipoff,
 
 	r = TAILQ_FIRST(pf_main_ruleset.rules[PF_RULESET_SCRUB].active.ptr);
 	while (r != NULL) {
-		r->evaluations++;
+		counter_u64_add(r->evaluations, 1);
 		if (pfi_kif_match(r->kif, kif) == r->ifnot)
 			r = r->skip[PF_SKIP_IFP].ptr;
 		else if (r->direction && r->direction != dir)
@@ -1195,8 +1195,8 @@ pf_normalize_tcp(int dir, struct pfi_kif *kif, struct mbuf *m, int ipoff,
 	if (rm == NULL || rm->action == PF_NOSCRUB)
 		return (PF_PASS);
 	else {
-		r->packets[dir == PF_OUT]++;
-		r->bytes[dir == PF_OUT] += pd->tot_len;
+		counter_u64_add(r->packets[dir == PF_OUT], 1);
+		counter_u64_add(r->bytes[dir == PF_OUT], pd->tot_len);
 	}
 
 	if (rm->rule_flag & PFRULE_REASSEMBLE_TCP)



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202101201444.10KEixAe019427>