Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 14 Jul 2021 14:54:41 GMT
From:      Mateusz Guzik <mjg@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: a65a22739870 - stable/13 - pf: allow table stats clearing and reading with ruleset rlock
Message-ID:  <202107141454.16EEsfhO098918@gitrepo.freebsd.org>

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

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

commit a65a227398703c23168120723694f04c1a71737f
Author:     Mateusz Guzik <mjg@FreeBSD.org>
AuthorDate: 2021-07-02 12:55:57 +0000
Commit:     Mateusz Guzik <mjg@FreeBSD.org>
CommitDate: 2021-07-14 14:50:12 +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")
    
    (cherry picked from commit dc1ab04e4c9ede3606985e0cce1200e3060ac166)
---
 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 3485592ffec7..0e53c92fcd85 100644
--- a/sys/net/pfvar.h
+++ b/sys/net/pfvar.h
@@ -128,10 +128,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 04a62535f461..a33854a207aa 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 4eebc8a43de8..694134c6c663 100644
--- a/sys/netpfil/pf/pf_ioctl.c
+++ b/sys/netpfil/pf/pf_ioctl.c
@@ -3726,10 +3726,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;
 		}
@@ -3740,12 +3742,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);
@@ -3780,10 +3784,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);



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