git: 452e6f549cdb - main - pf: Merge pf_clear_srcnodes() and pf_kill_srcnodes()

From: Kajetan Staszkiewicz <ks_at_FreeBSD.org>
Date: Tue, 12 Nov 2024 17:20:26 UTC
The branch main has been updated by ks:

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

commit 452e6f549cdb33393e05fabbaee0aed7eb744e68
Author:     Kajetan Staszkiewicz <ks@FreeBSD.org>
AuthorDate: 2024-11-12 17:17:11 +0000
Commit:     Kajetan Staszkiewicz <ks@FreeBSD.org>
CommitDate: 2024-11-12 17:17:11 +0000

    pf: Merge pf_clear_srcnodes() and pf_kill_srcnodes()
    
    The functions pf_clear_srcnodes() and pf_kill_srcnodes() serve the same
    purpose, however the former kills all source nodes while the later only
    a selected subset of them.
    
    They differ in how they reach that goal. pf_clear_srcnodes() first
    iterates over all states and detaches the source nodes from them. Then
    it iterates over all source nodes and marks them as expired leaving the
    cleanup to pf_purge_expired_src_nodes().
    
    If a new state and a new source node are created between iterating over
    all states and all source nodes, this source node will have its state
    counter set to 0 and expiry to 1, marking it as expired without properly
    detaching the state from it. Later the source node will be freed with
    the state sill pointing to it.
    
    The function pf_kill_srcnodes() performs the same operation in a safer
    manner by first marking the required source nodes as expiring and then
    iterating over all states and checking which states point to expiring
    nodes. Any source node created between iterating over states and source
    nodes will simply be ignored.
    
    Add functionality of killing all source nodes to pf_kill_srcnodes().
    Replace all calls to pf_clear_srcnodes() with a calls to
    pf_kill_srcnodes(), and remove the former.
    
    Reviewed by:            kp
    Approved by:            kp (mentor)
    Differential Revision:  https://reviews.freebsd.org/D47440
---
 sys/netpfil/pf/pf_ioctl.c | 47 ++++++++++-------------------------------------
 1 file changed, 10 insertions(+), 37 deletions(-)

diff --git a/sys/netpfil/pf/pf_ioctl.c b/sys/netpfil/pf/pf_ioctl.c
index ce28a9cd6dc1..c3f0166810ec 100644
--- a/sys/netpfil/pf/pf_ioctl.c
+++ b/sys/netpfil/pf/pf_ioctl.c
@@ -233,7 +233,6 @@ static int		 pf_clearstates_nv(struct pfioc_nv *);
 static int		 pf_getstate(struct pfioc_nv *);
 static int		 pf_getstatus(struct pfioc_nv *);
 static int		 pf_clear_tables(void);
-static void		 pf_clear_srcnodes(void);
 static void		 pf_kill_srcnodes(struct pfioc_src_node_kill *);
 static int		 pf_keepcounters(struct pfioc_nv *);
 static void		 pf_tbladdr_copyout(struct pf_addr_wrap *);
@@ -5451,8 +5450,7 @@ DIOCCHANGEADDR_error:
 	}
 
 	case DIOCCLRSRCNODES: {
-		pf_clear_srcnodes();
-		pf_purge_expired_src_nodes();
+		pf_kill_srcnodes(NULL);
 		break;
 	}
 
@@ -5927,40 +5925,11 @@ pf_clear_tables(void)
 	return (error);
 }
 
-static void
-pf_clear_srcnodes(void)
-{
-	struct pf_kstate	*s;
-	struct pf_srchash	*sh;
-	struct pf_ksrc_node	*sn;
-	int			 i;
-
-	for (i = 0; i <= V_pf_hashmask; i++) {
-		struct pf_idhash *ih = &V_pf_idhash[i];
-
-		PF_HASHROW_LOCK(ih);
-		LIST_FOREACH(s, &ih->states, entry) {
-			s->src_node = NULL;
-			s->nat_src_node = NULL;
-		}
-		PF_HASHROW_UNLOCK(ih);
-	}
-
-	for (i = 0, sh = V_pf_srchash; i <= V_pf_srchashmask;
-	    i++, sh++) {
-		PF_HASHROW_LOCK(sh);
-		LIST_FOREACH(sn, &sh->nodes, entry) {
-			sn->expire = 1;
-			sn->states = 0;
-		}
-		PF_HASHROW_UNLOCK(sh);
-	}
-}
-
 static void
 pf_kill_srcnodes(struct pfioc_src_node_kill *psnk)
 {
 	struct pf_ksrc_node_list	 kill;
+	u_int 				 killed;
 
 	LIST_INIT(&kill);
 	for (int i = 0; i <= V_pf_srchashmask; i++) {
@@ -5969,14 +5938,15 @@ pf_kill_srcnodes(struct pfioc_src_node_kill *psnk)
 
 		PF_HASHROW_LOCK(sh);
 		LIST_FOREACH_SAFE(sn, &sh->nodes, entry, tmp)
-			if (PF_MATCHA(psnk->psnk_src.neg,
+			if (psnk == NULL ||
+			    (PF_MATCHA(psnk->psnk_src.neg,
 			      &psnk->psnk_src.addr.v.a.addr,
 			      &psnk->psnk_src.addr.v.a.mask,
 			      &sn->addr, sn->af) &&
 			    PF_MATCHA(psnk->psnk_dst.neg,
 			      &psnk->psnk_dst.addr.v.a.addr,
 			      &psnk->psnk_dst.addr.v.a.mask,
-			      &sn->raddr, sn->af)) {
+			      &sn->raddr, sn->af))) {
 				pf_unlink_src_node(sn);
 				LIST_INSERT_HEAD(&kill, sn, entry);
 				sn->expire = 1;
@@ -5998,7 +5968,10 @@ pf_kill_srcnodes(struct pfioc_src_node_kill *psnk)
 		PF_HASHROW_UNLOCK(ih);
 	}
 
-	psnk->psnk_killed = pf_free_src_nodes(&kill);
+	killed = pf_free_src_nodes(&kill);
+
+	if (psnk != NULL)
+		psnk->psnk_killed = killed;
 }
 
 static int
@@ -6422,7 +6395,7 @@ shutdown_pf(void)
 
 		pf_clear_all_states();
 
-		pf_clear_srcnodes();
+		pf_kill_srcnodes(NULL);
 
 		/* status does not use malloced mem so no need to cleanup */
 		/* fingerprints and interfaces have their own cleanup code */