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>