Simplify and somewhat redesign interaction between pf_purge_thread() and

pf_purge_expired_states().

Now pf purging daemon stores the current hash table index on stack
in pf_purge_thread(), and supplies it to next iteration of
pf_purge_expired_states(). The latter returns new index back.

The important change is that whenever pf_purge_expired_states() wraps
around the array it returns immediately. This makes our knowledge about
status of states expiry run more consistent. Prior to this change it
could happen that n-th run stopped on i-th entry, and returned (1) as
full run complete, then next (n+1) full run stopped on j-th entry, where
j < i, and that broke the mark-and-sweep algorythm that saves references
rules. A referenced rule was freed, and this later lead to a crash.

1 files changed, 15 insertions, 18 deletions

diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
index 1f17ab5..0eacc5a 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -282,7 +282,7 @@ static int		 pf_src_connlimit(struct pf_state **);
 static void		 pf_overload_task(void *c, int pending);
 static int		 pf_insert_src_node(struct pf_src_node **,
 			    struct pf_rule *, struct pf_addr *, sa_family_t);
-static int		 pf_purge_expired_states(int);
+static u_int		 pf_purge_expired_states(u_int, int);
 static void		 pf_purge_unlinked_rules(void);
 static int		 pf_mtag_init(void *, int, int);
 static void		 pf_mtag_free(struct m_tag *);
@@ -1307,7 +1307,7 @@ pf_intr(void *v)
 void
 pf_purge_thread(void *v)
 {
-	int fullrun;
+	u_int idx = 0;
 
 	CURVNET_SET((struct vnet *)v);
 
@@ -1329,7 +1329,7 @@ pf_purge_thread(void *v)
 			/*
 			 * Now purge everything.
 			 */
-			pf_purge_expired_states(V_pf_hashmask + 1);
+			pf_purge_expired_states(0, V_pf_hashmask);
 			pf_purge_expired_fragments();
 			pf_purge_expired_src_nodes();
 
@@ -1352,11 +1352,11 @@ pf_purge_thread(void *v)
 		PF_RULES_RUNLOCK();
 
 		/* Process 1/interval fraction of the state table every run. */
-		fullrun = pf_purge_expired_states(V_pf_hashmask /
+		idx = pf_purge_expired_states(idx, V_pf_hashmask /
 			    (V_pf_default_rule.timeout[PFTM_INTERVAL] * 10));
 
 		/* Purge other expired types every PFTM_INTERVAL seconds. */
-		if (fullrun) {
+		if (idx == 0) {
 			/*
 			 * Order is important:
 			 * - states and src nodes reference rules
@@ -1533,14 +1533,11 @@ pf_free_state(struct pf_state *cur)
 /*
  * Called only from pf_purge_thread(), thus serialized.
  */
-static int
-pf_purge_expired_states(int maxcheck)
+static u_int
+pf_purge_expired_states(u_int i, int maxcheck)
 {
-	static u_int i = 0;
-
 	struct pf_idhash *ih;
 	struct pf_state *s;
-	int rv = 0;
 
 	V_pf_status.states = uma_zone_get_cur(V_pf_state_z);
 
@@ -1549,12 +1546,6 @@ pf_purge_expired_states(int maxcheck)
 	 */
 	while (maxcheck > 0) {
 
-		/* Wrap to start of hash when we hit the end. */
-		if (i > V_pf_hashmask) {
-			i = 0;
-			rv = 1;
-		}
-
 		ih = &V_pf_idhash[i];
 relock:
 		PF_HASHROW_LOCK(ih);
@@ -1574,13 +1565,19 @@ relock:
 				s->rt_kif->pfik_flags |= PFI_IFLAG_REFS;
 		}
 		PF_HASHROW_UNLOCK(ih);
-		i++;
+
+		/* Return when we hit end of hash. */
+		if (++i > V_pf_hashmask) {
+			V_pf_status.states = uma_zone_get_cur(V_pf_state_z);
+			return (0);
+		}
+
 		maxcheck--;
 	}
 
 	V_pf_status.states = uma_zone_get_cur(V_pf_state_z);
 
-	return (rv);
+	return (i);
 }
 
 static void