Skip site navigation (1)Skip section navigation (2) 
Date:      Tue, 29 May 2012 12:33:09 +0000 (UTC)
From:      Gleb Smirnoff <glebius@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-projects@freebsd.org
Subject:   svn commit: r236230 - projects/pf/head/sys/contrib/pf/net
Message-ID:  <201205291233.q4TCX9xQ077265@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: glebius
Date: Tue May 29 12:33:09 2012
New Revision: 236230
URL: http://svn.freebsd.org/changeset/base/236230

Log:
  We can't trust V_pf_status.states++ and V_pf_status.states--
  in the forwarding path as soon as we get out of the pf giant lock.
  
  o Use uma_zone_get_cur() to estimate current number of states.
    Since function is considered too expensive, run it in the expiry
    thread only, and not for every state examined.
    XXX: this may make adaptive timeouts and expire times sent via
    pfsync less precise under certain circumctances.
  
  o Run state expiry 10 times more often with 10 times smaller
    chunk argument.

Modified:
  projects/pf/head/sys/contrib/pf/net/pf.c
  projects/pf/head/sys/contrib/pf/net/pfvar.h

Modified: projects/pf/head/sys/contrib/pf/net/pf.c
==============================================================================
--- projects/pf/head/sys/contrib/pf/net/pf.c	Tue May 29 11:48:53 2012	(r236229)
+++ projects/pf/head/sys/contrib/pf/net/pf.c	Tue May 29 12:33:09 2012	(r236230)
@@ -1048,7 +1048,6 @@ pf_state_insert(struct pfi_kif *kif, str
 	refcount_init(&s->refs, 2);
 
 	V_pf_status.fcounters[FCNT_STATE_INSERT]++;
-	V_pf_status.states++;
 	if (pfsync_insert_state_ptr != NULL)
 		pfsync_insert_state_ptr(s);
 
@@ -1289,7 +1288,7 @@ pf_purge_thread(void *v)
 	CURVNET_SET((struct vnet *)v);
 
 	for (;;) {
-		tsleep(pf_purge_thread, PWAIT, "pftm", 1 * hz);
+		tsleep(pf_purge_thread, PWAIT, "pftm", hz / 10);
 
 		PF_LOCK();
 
@@ -1303,9 +1302,9 @@ pf_purge_thread(void *v)
 			kproc_exit(0);
 		}
 
-		/* Process a fraction of the state table every second. */
+		/* Process 1/interval fraction of the state table every run. */
 		fullrun = pf_purge_expired_states(V_pf_hashmask /
-			    V_pf_default_rule.timeout[PFTM_INTERVAL]);
+			    (V_pf_default_rule.timeout[PFTM_INTERVAL] * 10));
 
 		/* Purge other expired types every PFTM_INTERVAL seconds. */
 		if (fullrun) {
@@ -1349,7 +1348,7 @@ pf_state_expires(const struct pf_state *
 	start = state->rule.ptr->timeout[PFTM_ADAPTIVE_START];
 	if (start) {
 		end = state->rule.ptr->timeout[PFTM_ADAPTIVE_END];
-		states = state->rule.ptr->states_cur;
+		states = state->rule.ptr->states_cur;	/* XXXGL */
 	} else {
 		start = V_pf_default_rule.timeout[PFTM_ADAPTIVE_START];
 		end = V_pf_default_rule.timeout[PFTM_ADAPTIVE_END];
@@ -1421,7 +1420,7 @@ pf_src_tree_remove_state(struct pf_state
  * called with ID hash row locked, but always returns
  * unlocked, since it needs to go through key hash locking.
  */
-void
+int
 pf_unlink_state(struct pf_state *s, u_int flags)
 {
 	struct pf_idhash *ih = &V_pf_idhash[PF_IDHASH(s)];
@@ -1457,7 +1456,7 @@ pf_unlink_state(struct pf_state *s, u_in
 	pf_detach_state(s);
 	refcount_release(&s->refs);
 
-	pf_release_state(s);
+	return (pf_release_state(s));
 }
 
 void
@@ -1475,7 +1474,6 @@ pf_free_state(struct pf_state *cur)
 	pf_normalize_tcp_cleanup(cur);
 	uma_zfree(V_pf_state_z, cur);
 	V_pf_status.fcounters[FCNT_STATE_REMOVALS]++;
-	V_pf_status.states--;
 }
 
 /*
@@ -1490,6 +1488,8 @@ pf_purge_expired_states(int maxcheck)
 	struct pf_state *s;
 	int rv = 0;
 
+	V_pf_status.states = uma_zone_get_cur(V_pf_state_z);
+
 	/*
 	 * Go through hash and unlink states that expire now.
 	 */
@@ -1506,7 +1506,8 @@ relock:
 		PF_HASHROW_LOCK(ih);
 		LIST_FOREACH(s, &ih->states, entry) {
 			if (pf_state_expires(s) <= time_uptime) {
-				pf_unlink_state(s, PF_ENTER_LOCKED);
+				V_pf_status.states -=
+				    pf_unlink_state(s, PF_ENTER_LOCKED);
 				goto relock;
 			}
 			s->rule.ptr->rule_flag |= PFRULE_REFS;
@@ -1523,6 +1524,8 @@ relock:
 		maxcheck--;
 	}
 
+	V_pf_status.states = uma_zone_get_cur(V_pf_state_z);
+
 	return (rv);
 }
 

Modified: projects/pf/head/sys/contrib/pf/net/pfvar.h
==============================================================================
--- projects/pf/head/sys/contrib/pf/net/pfvar.h	Tue May 29 11:48:53 2012	(r236229)
+++ projects/pf/head/sys/contrib/pf/net/pfvar.h	Tue May 29 12:33:09 2012	(r236230)
@@ -1757,7 +1757,7 @@ extern void			 pf_purge_thread(void *);
 extern void			 pf_intr(void *);
 extern void			 pf_purge_expired_src_nodes(void);
 
-extern void			 pf_unlink_state(struct pf_state *, u_int);
+extern int			 pf_unlink_state(struct pf_state *, u_int);
 #define	PF_ENTER_LOCKED		0x00000001
 #define	PF_RETURN_LOCKED	0x00000002
 extern int			 pf_state_insert(struct pfi_kif *,
@@ -1794,12 +1794,15 @@ pf_ref_state(struct pf_state *s)
 	refcount_acquire(&s->refs);
 }
 
-static __inline void
+static __inline int
 pf_release_state(struct pf_state *s)
 {
 
-	if (refcount_release(&s->refs))
+	if (refcount_release(&s->refs)) {
 		pf_free_state(s);
+		return (1);
+	} else
+		return (0);
 }
 
 extern struct pf_state		*pf_find_state_byid(uint64_t, uint32_t);

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