Re: pf: u_int32_t conn counter underflow in pf_src_tree_remove_state()

Hello Janak,

I was not able to reproduce the issue on my PC-engines APU2 router. I will
appreciate if you can give a try to diff below to check if it fixes the issue.
The diff is compile/regress tested only.

thanks and
regards
sashan

--------8<---------------8<---------------8<------------------8<--------
diff --git a/sys/net/pf.c b/sys/net/pf.c
index 0fd00c0dbf3..ca28eb0b98f 100644
--- a/sys/net/pf.c
+++ b/sys/net/pf.c
@@ -760,7 +760,13 @@ pf_src_connlimit(struct pf_state **stp)
 	if ((sn = pf_get_src_node((*stp), PF_SN_NONE)) == NULL)
 		return (0);
 
-	sn->conn++;
+	/*
+	 * Note: conn limit is bumped on SYN_SENT->ESTBLISHED
+	 * state transition. packet does not hold any locks
+	 * when running here, therefore atomic is needed.
+	 */
+	atomic_inc_int((int *)&sn->conn);
+
 	(*stp)->src.tcp_est = 1;
 	pf_add_threshold(&sn->conn_rate);
 
@@ -2051,10 +2057,16 @@ pf_src_tree_remove_state(struct pf_state *st)
 	u_int32_t		 timeout;
 	struct pf_sn_item	*sni;
 
+	PF_ASSERT_LOCKED();
+
 	while ((sni = SLIST_FIRST(&st->src_nodes)) != NULL) {
 		SLIST_REMOVE_HEAD(&st->src_nodes, next);
-		if (st->src.tcp_est)
+		if (st->src.tcp_est) {
+			/*
+			 * atomic not needed here, because of PF_LOCK()
+			 */
 			--sni->sn->conn;
+		}
 		if (--sni->sn->states == 0) {
 			timeout = st->rule.ptr->timeout[PFTM_SRC_NODE];
 			if (!timeout)