git: 860c84bc7618 - stable/14 - pf: Stop using net_epoch to synchronize access to eth rules

The branch stable/14 has been updated by markj:

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

commit 860c84bc76180910605a1794c624b55cc12926f7
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2025-02-06 14:36:20 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2025-02-21 14:41:18 +0000

    pf: Stop using net_epoch to synchronize access to eth rules
    
    Commit 20c4899a8eea4 modified pf_test_eth_rule() to not acquire the
    rules read lock, so pf_commit_eth() was changed to wait until the
    now-inactive rules are no longer in use before freeing them.  In
    particular, it uses the net_epoch to schedule callbacks once the
    inactive rules are no longer visible to packet processing threads.
    
    However, since commit 812839e5aaaf4, pf_test_eth_rule() acquires the
    rules read lock, so this deferred action is unneeded.  This patch
    reverts a portion of 20c4899a8eea4 such that we avoid using deferred
    callbacks to free inactive rules.
    
    The main motivation is performance: epoch_drain_callbacks() is quite
    slow, especially on busy systems, and its use in the DIOCXBEGIN handler
    in particular causes long stalls in relayd when reloading configuration.
    
    Reviewed by:    kp
    MFC after:      2 weeks
    Sponsored by:   Klara, Inc.
    Sponsored by:   Modirum MDPay
    Differential Revision:  https://reviews.freebsd.org/D48822
    
    (cherry picked from commit 7a66b3008693ce61957e8b2a3d99829063e1e4af)
---
 sys/net/pfvar.h           |  1 -
 sys/netpfil/pf/pf.c       |  9 +++------
 sys/netpfil/pf/pf_ioctl.c | 32 +++-----------------------------
 3 files changed, 6 insertions(+), 36 deletions(-)

diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h
index 30050896b44e..9ceed54fd52b 100644
--- a/sys/net/pfvar.h
+++ b/sys/net/pfvar.h
@@ -667,7 +667,6 @@ struct pf_keth_ruleset {
 		int			 open;
 		uint32_t		 ticket;
 	} active, inactive;
-	struct epoch_context	 epoch_ctx;
 	struct vnet		*vnet;
 	struct pf_keth_anchor	*anchor;
 };
diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
index 064277082475..b14ab91cd7aa 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -4422,11 +4422,6 @@ pf_test_eth_rule(int dir, struct pfi_kkif *kif, struct mbuf **m0)
 		return (PF_PASS);
 	}
 
-	ruleset = V_pf_keth;
-	rules = ck_pr_load_ptr(&ruleset->active.rules);
-	r = TAILQ_FIRST(rules);
-	rm = NULL;
-
 	e = mtod(m, struct ether_header *);
 	proto = ntohs(e->ether_type);
 
@@ -4463,7 +4458,9 @@ pf_test_eth_rule(int dir, struct pfi_kkif *kif, struct mbuf **m0)
 
 	PF_RULES_RLOCK();
 
-	while (r != NULL) {
+	ruleset = V_pf_keth;
+	rules = atomic_load_ptr(&ruleset->active.rules);
+	for (r = TAILQ_FIRST(rules), rm = NULL; r != NULL;) {
 		counter_u64_add(r->evaluations, 1);
 		SDT_PROBE2(pf, eth, test_rule, test, r->nr, r);
 
diff --git a/sys/netpfil/pf/pf_ioctl.c b/sys/netpfil/pf/pf_ioctl.c
index e67a0ddadfb7..d95f36d06ee3 100644
--- a/sys/netpfil/pf/pf_ioctl.c
+++ b/sys/netpfil/pf/pf_ioctl.c
@@ -106,7 +106,6 @@ static void		 pf_empty_kpool(struct pf_kpalist *);
 static int		 pfioctl(struct cdev *, u_long, caddr_t, int,
 			    struct thread *);
 static int		 pf_begin_eth(uint32_t *, const char *);
-static void		 pf_rollback_eth_cb(struct epoch_context *);
 static int		 pf_rollback_eth(uint32_t, const char *);
 static int		 pf_commit_eth(uint32_t, const char *);
 static void		 pf_free_eth_rule(struct pf_keth_rule *);
@@ -782,23 +781,6 @@ pf_begin_eth(uint32_t *ticket, const char *anchor)
 	return (0);
 }
 
-static void
-pf_rollback_eth_cb(struct epoch_context *ctx)
-{
-	struct pf_keth_ruleset *rs;
-
-	rs = __containerof(ctx, struct pf_keth_ruleset, epoch_ctx);
-
-	CURVNET_SET(rs->vnet);
-
-	PF_RULES_WLOCK();
-	pf_rollback_eth(rs->inactive.ticket,
-	    rs->anchor ? rs->anchor->path : "");
-	PF_RULES_WUNLOCK();
-
-	CURVNET_RESTORE();
-}
-
 static int
 pf_rollback_eth(uint32_t ticket, const char *anchor)
 {
@@ -892,15 +874,12 @@ pf_commit_eth(uint32_t ticket, const char *anchor)
 	pf_eth_calc_skip_steps(rs->inactive.rules);
 
 	rules = rs->active.rules;
-	ck_pr_store_ptr(&rs->active.rules, rs->inactive.rules);
+	atomic_store_ptr(&rs->active.rules, rs->inactive.rules);
 	rs->inactive.rules = rules;
 	rs->inactive.ticket = rs->active.ticket;
 
-	/* Clean up inactive rules (i.e. previously active rules), only when
-	 * we're sure they're no longer used. */
-	NET_EPOCH_CALL(pf_rollback_eth_cb, &rs->epoch_ctx);
-
-	return (0);
+	return (pf_rollback_eth(rs->inactive.ticket,
+	    rs->anchor ? rs->anchor->path : ""));
 }
 
 #ifdef ALTQ
@@ -5208,8 +5187,6 @@ DIOCCHANGEADDR_error:
 			free(ioes, M_TEMP);
 			break;
 		}
-		/* Ensure there's no more ethernet rules to clean up. */
-		NET_EPOCH_DRAIN_CALLBACKS();
 		PF_RULES_WLOCK();
 		for (i = 0, ioe = ioes; i < io->size; i++, ioe++) {
 			ioe->anchor[sizeof(ioe->anchor) - 1] = '\0';
@@ -6852,9 +6829,6 @@ pf_unload_vnet(void)
 	shutdown_pf();
 	PF_RULES_WUNLOCK();
 
-	/* Make sure we've cleaned up ethernet rules before we continue. */
-	NET_EPOCH_DRAIN_CALLBACKS();
-
 	ret = swi_remove(V_pf_swi_cookie);
 	MPASS(ret == 0);
 	ret = intr_event_destroy(V_pf_swi_ie);