Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 20 Jan 2021 14:45:02 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: efd9d47d0b35 - stable/12 - pf: Convert pfi_kkif to use counter_u64
Message-ID:  <202101201445.10KEj2E5019495@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=efd9d47d0b358c829bfc12fda2cc5826e73e5974

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

    pf: Convert pfi_kkif to use counter_u64
    
    Improve caching behaviour by using counter_u64 rather than variables
    shared between cores.
    
    The result of converting all counters to counter(9) (i.e. this full
    patch series) is a significant improvement in throughput. As tested by
    olivier@, on Intel Xeon E5-2697Av4 (16Cores, 32 threads) hardware with
    Mellanox ConnectX-4 MCX416A-CCAT (100GBase-SR4) nics we see:
    
    x FreeBSD 20201223: inet packets-per-second
    + FreeBSD 20201223 with pf patches: inet packets-per-second
    +--------------------------------------------------------------------------+
    |                                                                        + |
    | xx                                                                     + |
    |xxx                                                                    +++|
    ||A|                                                                       |
    |                                                                       |A||
    +--------------------------------------------------------------------------+
        N           Min           Max        Median           Avg        Stddev
    x   5       9216962       9526356       9343902     9371057.6     116720.36
    +   5      19427190      19698400      19502922      19546509     109084.92
    Difference at 95.0% confidence
            1.01755e+07 +/- 164756
            108.584% +/- 2.9359%
            (Student's t, pooled s = 112967)
    
    Reviewed by:    philip
    MFC after:      2 weeks
    Sponsored by:   Orange Business Services
    Differential Revision:  https://reviews.freebsd.org/D27763
    
    (cherry picked from commit 5a3b9507d784aaa6a7ce35432b2111a7eec12cba)
---
 sys/net/pfvar.h        |  5 ++--
 sys/netpfil/pf/pf.c    | 12 ++++++----
 sys/netpfil/pf/pf_if.c | 65 +++++++++++++++++++++++++++++++++++++++++---------
 3 files changed, 65 insertions(+), 17 deletions(-)

diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h
index a28483fa9814..dcc5bf51fdf6 100644
--- a/sys/net/pfvar.h
+++ b/sys/net/pfvar.h
@@ -879,8 +879,8 @@ struct pfi_kkif {
 	} _pfik_glue;
 #define	pfik_tree	_pfik_glue._pfik_tree
 #define	pfik_list	_pfik_glue._pfik_list
-	u_int64_t			 pfik_packets[2][2][2];
-	u_int64_t			 pfik_bytes[2][2][2];
+	counter_u64_t			 pfik_packets[2][2][2];
+	counter_u64_t			 pfik_bytes[2][2][2];
 	u_int32_t			 pfik_tzero;
 	u_int				 pfik_flags;
 	struct ifnet			*pfik_ifp;
@@ -1666,6 +1666,7 @@ struct pf_state_key	*pf_state_key_clone(struct pf_state_key *);
 
 struct pfi_kkif		*pf_kkif_create(int);
 void			 pf_kkif_free(struct pfi_kkif *);
+void			 pf_kkif_zero(struct pfi_kkif *);
 #endif /* _KERNEL */
 
 #endif /* _NET_PFVAR_H_ */
diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
index f1c26342577f..d4101aab9332 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -6329,8 +6329,10 @@ done:
 		    (s == NULL));
 	}
 
-	kif->pfik_bytes[0][dir == PF_OUT][action != PF_PASS] += pd.tot_len;
-	kif->pfik_packets[0][dir == PF_OUT][action != PF_PASS]++;
+	counter_u64_add(kif->pfik_bytes[0][dir == PF_OUT][action != PF_PASS],
+	    pd.tot_len);
+	counter_u64_add(kif->pfik_packets[0][dir == PF_OUT][action != PF_PASS],
+	    1);
 
 	if (action == PF_PASS || r->action == PF_DROP) {
 		dirndx = (dir == PF_OUT);
@@ -6734,8 +6736,10 @@ done:
 		    &pd, (s == NULL));
 	}
 
-	kif->pfik_bytes[1][dir == PF_OUT][action != PF_PASS] += pd.tot_len;
-	kif->pfik_packets[1][dir == PF_OUT][action != PF_PASS]++;
+	counter_u64_add(kif->pfik_bytes[1][dir == PF_OUT][action != PF_PASS],
+	    pd.tot_len);
+	counter_u64_add(kif->pfik_packets[1][dir == PF_OUT][action != PF_PASS],
+	    1);
 
 	if (action == PF_PASS || r->action == PF_DROP) {
 		dirndx = (dir == PF_OUT);
diff --git a/sys/netpfil/pf/pf_if.c b/sys/netpfil/pf/pf_if.c
index 3230a3e424f6..9ca404174cca 100644
--- a/sys/netpfil/pf/pf_if.c
+++ b/sys/netpfil/pf/pf_if.c
@@ -199,7 +199,26 @@ pf_kkif_create(int flags)
 {
 	struct pfi_kkif *kif;
 
-	kif = malloc(sizeof(*kif), PFI_MTYPE, flags);
+	kif = malloc(sizeof(*kif), PFI_MTYPE, flags | M_ZERO);
+	if (! kif)
+		return (kif);
+
+	for (int i = 0; i < 2; i++) {
+		for (int j = 0; j < 2; j++) {
+			for (int k = 0; k < 2; k++) {
+				kif->pfik_packets[i][j][k] =
+				    counter_u64_alloc(flags);
+				kif->pfik_bytes[i][j][k] =
+				    counter_u64_alloc(flags);
+
+				if (! kif->pfik_packets[i][j][k] ||
+				    ! kif->pfik_bytes[i][j][k]) {
+					pf_kkif_free(kif);
+					return (NULL);
+				}
+			}
+		}
+	}
 
 	return (kif);
 }
@@ -210,9 +229,35 @@ pf_kkif_free(struct pfi_kkif *kif)
 	if (! kif)
 		return;
 
+	for (int i = 0; i < 2; i++) {
+		for (int j = 0; j < 2; j++) {
+			for (int k = 0; k < 2; k++) {
+				if (kif->pfik_packets[i][j][k])
+					counter_u64_free(kif->pfik_packets[i][j][k]);
+				if (kif->pfik_bytes[i][j][k])
+					counter_u64_free(kif->pfik_bytes[i][j][k]);
+			}
+		}
+	}
+
 	free(kif, PFI_MTYPE);
 }
 
+void
+pf_kkif_zero(struct pfi_kkif *kif)
+{
+
+	for (int i = 0; i < 2; i++) {
+		for (int j = 0; j < 2; j++) {
+			for (int k = 0; k < 2; k++) {
+				counter_u64_zero(kif->pfik_packets[i][j][k]);
+				counter_u64_zero(kif->pfik_bytes[i][j][k]);
+			}
+		}
+	}
+	kif->pfik_tzero = time_second;
+}
+
 struct pfi_kkif *
 pfi_kkif_find(const char *kif_name)
 {
@@ -240,7 +285,7 @@ pfi_kkif_attach(struct pfi_kkif *kif, const char *kif_name)
 		return (kif1);
 	}
 
-	bzero(kif, sizeof(*kif));
+	pf_kkif_zero(kif);
 	strlcpy(kif->pfik_name, kif_name, sizeof(kif->pfik_name));
 	/*
 	 * It seems that the value of time_second is in unintialzied state
@@ -339,7 +384,7 @@ pfi_attach_ifnet(struct ifnet *ifp)
 {
 	struct pfi_kkif *kif;
 
-	kif = malloc(sizeof(*kif), PFI_MTYPE, M_WAITOK);
+	kif = pf_kkif_create(M_WAITOK);
 
 	PF_RULES_WLOCK();
 	V_pfi_update++;
@@ -359,7 +404,7 @@ pfi_attach_ifgroup(struct ifg_group *ifg)
 {
 	struct pfi_kkif *kif;
 
-	kif = malloc(sizeof(*kif), PFI_MTYPE, M_WAITOK);
+	kif = pf_kkif_create(M_WAITOK);
 
 	PF_RULES_WLOCK();
 	V_pfi_update++;
@@ -740,18 +785,16 @@ pfi_update_status(const char *name, struct pf_status *pfs)
 
 		/* just clear statistics */
 		if (pfs == NULL) {
-			bzero(p->pfik_packets, sizeof(p->pfik_packets));
-			bzero(p->pfik_bytes, sizeof(p->pfik_bytes));
-			p->pfik_tzero = time_second;
+			pf_kkif_zero(p);
 			continue;
 		}
 		for (i = 0; i < 2; i++)
 			for (j = 0; j < 2; j++)
 				for (k = 0; k < 2; k++) {
 					pfs->pcounters[i][j][k] +=
-						p->pfik_packets[i][j][k];
+					    counter_u64_fetch(p->pfik_packets[i][j][k]);
 					pfs->bcounters[i][j] +=
-						p->pfik_bytes[i][j][k];
+					    counter_u64_fetch(p->pfik_bytes[i][j][k]);
 				}
 	}
 }
@@ -766,9 +809,9 @@ pf_kkif_to_kif(const struct pfi_kkif *kkif, struct pfi_kif *kif)
 		for (int j = 0; j < 2; j++) {
 			for (int k = 0; k < 2; k++) {
 				kif->pfik_packets[i][j][k] =
-				    kkif->pfik_packets[i][j][k];
+				    counter_u64_fetch(kkif->pfik_packets[i][j][k]);
 				kif->pfik_bytes[i][j][k] =
-				    kkif->pfik_bytes[i][j][k];
+				    counter_u64_fetch(kkif->pfik_bytes[i][j][k]);
 			}
 		}
 	}



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