git: dc1ab04e4c9e - main - pf: allow table stats clearing and reading with ruleset rlock

[Mateusz Guzik]

The branch main has been updated by mjg:

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

commit dc1ab04e4c9ede3606985e0cce1200e3060ac166
Author:     Mateusz Guzik <mjg at FreeBSD.org>
AuthorDate: 2021-07-02 12:55:57 +0000
Commit:     Mateusz Guzik <mjg at FreeBSD.org>
CommitDate: 2021-07-05 08:42:01 +0000

    pf: allow table stats clearing and reading with ruleset rlock
    
    Instead serialize against these operations with a dedicated lock.
    
    Prior to the change, When pushing 17 mln pps of traffic, calling
    DIOCRGETTSTATS in a loop would restrict throughput to about 7 mln.  With
    the change there is no slowdown.
    
    Reviewed by:    kp (previous version)
    Sponsored by:   Rubicon Communications, LLC ("Netgate")
---
 sys/net/pfvar.h           |  7 +++++++
 sys/netpfil/pf/pf.c       |  4 ++++
 sys/netpfil/pf/pf_ioctl.c | 18 ++++++++++++------
 sys/netpfil/pf/pf_table.c |  2 ++
 4 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h
index 515953c09f53..8ed998be0d78 100644
--- a/sys/net/pfvar.h
+++ b/sys/net/pfvar.h
@@ -131,10 +131,17 @@ extern struct rmlock pf_rules_lock;
 #define	PF_RULES_RUNLOCK()	rm_runlock(&pf_rules_lock, &_pf_rules_tracker)
 #define	PF_RULES_WLOCK()	rm_wlock(&pf_rules_lock)
 #define	PF_RULES_WUNLOCK()	rm_wunlock(&pf_rules_lock)
+#define	PF_RULES_WOWNED()	rm_wowned(&pf_rules_lock)
 #define	PF_RULES_ASSERT()	rm_assert(&pf_rules_lock, RA_LOCKED)
 #define	PF_RULES_RASSERT()	rm_assert(&pf_rules_lock, RA_RLOCKED)
 #define	PF_RULES_WASSERT()	rm_assert(&pf_rules_lock, RA_WLOCKED)
 
+extern struct mtx pf_table_stats_lock;
+#define	PF_TABLE_STATS_LOCK()	mtx_lock(&pf_table_stats_lock)
+#define	PF_TABLE_STATS_UNLOCK()	mtx_unlock(&pf_table_stats_lock)
+#define	PF_TABLE_STATS_OWNED()	mtx_owned(&pf_table_stats_lock)
+#define	PF_TABLE_STATS_ASSERT()	mtx_assert(&pf_rules_lock, MA_OWNED)
+
 extern struct sx pf_end_lock;
 
 #define	PF_MODVER	1
diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
index 39b032962e84..b05ea1c12285 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -211,6 +211,10 @@ struct mtx pf_unlnkdrules_mtx;
 MTX_SYSINIT(pf_unlnkdrules_mtx, &pf_unlnkdrules_mtx, "pf unlinked rules",
     MTX_DEF);
 
+struct mtx pf_table_stats_lock;
+MTX_SYSINIT(pf_table_stats_lock, &pf_table_stats_lock, "pf table stats",
+    MTX_DEF);
+
 VNET_DEFINE_STATIC(uma_zone_t,	pf_sources_z);
 #define	V_pf_sources_z	VNET(pf_sources_z)
 uma_zone_t		pf_mtag_z;
diff --git a/sys/netpfil/pf/pf_ioctl.c b/sys/netpfil/pf/pf_ioctl.c
index 9acfe56b7208..51781d488763 100644
--- a/sys/netpfil/pf/pf_ioctl.c
+++ b/sys/netpfil/pf/pf_ioctl.c
@@ -3725,10 +3725,12 @@ DIOCCHANGEADDR_error:
 			error = ENODEV;
 			break;
 		}
-		PF_RULES_WLOCK();
+		PF_TABLE_STATS_LOCK();
+		PF_RULES_RLOCK();
 		n = pfr_table_count(&io->pfrio_table, io->pfrio_flags);
 		if (n < 0) {
-			PF_RULES_WUNLOCK();
+			PF_RULES_RUNLOCK();
+			PF_TABLE_STATS_UNLOCK();
 			error = EINVAL;
 			break;
 		}
@@ -3739,12 +3741,14 @@ DIOCCHANGEADDR_error:
 		    sizeof(struct pfr_tstats), M_TEMP, M_NOWAIT);
 		if (pfrtstats == NULL) {
 			error = ENOMEM;
-			PF_RULES_WUNLOCK();
+			PF_RULES_RUNLOCK();
+			PF_TABLE_STATS_UNLOCK();
 			break;
 		}
 		error = pfr_get_tstats(&io->pfrio_table, pfrtstats,
 		    &io->pfrio_size, io->pfrio_flags | PFR_FLAG_USERIOCTL);
-		PF_RULES_WUNLOCK();
+		PF_RULES_RUNLOCK();
+		PF_TABLE_STATS_UNLOCK();
 		if (error == 0)
 			error = copyout(pfrtstats, io->pfrio_buffer, totlen);
 		free(pfrtstats, M_TEMP);
@@ -3779,10 +3783,12 @@ DIOCCHANGEADDR_error:
 			break;
 		}
 
-		PF_RULES_WLOCK();
+		PF_TABLE_STATS_LOCK();
+		PF_RULES_RLOCK();
 		error = pfr_clr_tstats(pfrts, io->pfrio_size,
 		    &io->pfrio_nzero, io->pfrio_flags | PFR_FLAG_USERIOCTL);
-		PF_RULES_WUNLOCK();
+		PF_RULES_RUNLOCK();
+		PF_TABLE_STATS_UNLOCK();
 		free(pfrts, M_TEMP);
 		break;
 	}
diff --git a/sys/netpfil/pf/pf_table.c b/sys/netpfil/pf/pf_table.c
index cd7d96eacd13..5afc90e54d7c 100644
--- a/sys/netpfil/pf/pf_table.c
+++ b/sys/netpfil/pf/pf_table.c
@@ -1864,6 +1864,8 @@ pfr_clstats_ktable(struct pfr_ktable *kt, long tzero, int recurse)
 	struct pfr_kentryworkq	 addrq;
 	int			 pfr_dir, pfr_op;
 
+	MPASS(PF_TABLE_STATS_OWNED() || PF_RULES_WOWNED());
+
 	if (recurse) {
 		pfr_enqueue_addrs(kt, &addrq, NULL, 0);
 		pfr_clstats_kentries(kt, &addrq, tzero, 0);