From owner-svn-src-head@FreeBSD.ORG Tue Mar 17 12:19:30 2015 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 9EC24A6D; Tue, 17 Mar 2015 12:19:30 +0000 (UTC) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:1900:2254:2068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 7FA0D676; Tue, 17 Mar 2015 12:19:30 +0000 (UTC) Received: from svn.freebsd.org ([127.0.1.70]) by svn.freebsd.org (8.14.9/8.14.9) with ESMTP id t2HCJUE5060537; Tue, 17 Mar 2015 12:19:30 GMT (envelope-from glebius@FreeBSD.org) Received: (from glebius@localhost) by svn.freebsd.org (8.14.9/8.14.9/Submit) id t2HCJTe0060534; Tue, 17 Mar 2015 12:19:29 GMT (envelope-from glebius@FreeBSD.org) Message-Id: <201503171219.t2HCJTe0060534@svn.freebsd.org> X-Authentication-Warning: svn.freebsd.org: glebius set sender to glebius@FreeBSD.org using -f From: Gleb Smirnoff Date: Tue, 17 Mar 2015 12:19:29 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r280169 - in head/sys: net netpfil/pf X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 17 Mar 2015 12:19:30 -0000 Author: glebius Date: Tue Mar 17 12:19:28 2015 New Revision: 280169 URL: https://svnweb.freebsd.org/changeset/base/280169 Log: Always lock the hash row of a source node when updating its 'states' counter. PR: 182401 Sponsored by: Nginx, Inc. Modified: head/sys/net/pfvar.h head/sys/netpfil/pf/pf.c head/sys/netpfil/pf/pf_ioctl.c Modified: head/sys/net/pfvar.h ============================================================================== --- head/sys/net/pfvar.h Tue Mar 17 11:07:59 2015 (r280168) +++ head/sys/net/pfvar.h Tue Mar 17 12:19:28 2015 (r280169) @@ -1550,7 +1550,6 @@ extern struct pf_state *pf_find_state_a extern struct pf_src_node *pf_find_src_node(struct pf_addr *, struct pf_rule *, sa_family_t, int); extern void pf_unlink_src_node(struct pf_src_node *); -extern void pf_unlink_src_node_locked(struct pf_src_node *); extern u_int pf_free_src_nodes(struct pf_src_node_list *); extern void pf_print_state(struct pf_state *); extern void pf_print_flags(u_int8_t); Modified: head/sys/netpfil/pf/pf.c ============================================================================== --- head/sys/netpfil/pf/pf.c Tue Mar 17 11:07:59 2015 (r280168) +++ head/sys/netpfil/pf/pf.c Tue Mar 17 12:19:28 2015 (r280169) @@ -649,7 +649,10 @@ pf_find_src_node(struct pf_addr *src, st ((af == AF_INET && n->addr.v4.s_addr == src->v4.s_addr) || (af == AF_INET6 && bcmp(&n->addr, src, sizeof(*src)) == 0))) break; - if (n != NULL || returnlocked == 0) + if (n != NULL) { + n->states++; + PF_HASHROW_UNLOCK(sh); + } else if (returnlocked == 0) PF_HASHROW_UNLOCK(sh); return (n); @@ -693,6 +696,7 @@ pf_insert_src_node(struct pf_src_node ** LIST_INSERT_HEAD(&sh->nodes, *sn, entry); (*sn)->creation = time_uptime; (*sn)->ruletype = rule->action; + (*sn)->states = 1; if ((*sn)->rule.ptr != NULL) counter_u64_add((*sn)->rule.ptr->src_nodes, 1); PF_HASHROW_UNLOCK(sh); @@ -709,37 +713,13 @@ pf_insert_src_node(struct pf_src_node ** } void -pf_unlink_src_node_locked(struct pf_src_node *src) +pf_unlink_src_node(struct pf_src_node *src) { -#ifdef INVARIANTS - struct pf_srchash *sh; - sh = &V_pf_srchash[pf_hashsrc(&src->addr, src->af)]; - PF_HASHROW_ASSERT(sh); -#endif + PF_HASHROW_ASSERT(&V_pf_srchash[pf_hashsrc(&src->addr, src->af)]); LIST_REMOVE(src, entry); if (src->rule.ptr) counter_u64_add(src->rule.ptr->src_nodes, -1); - counter_u64_add(V_pf_status.scounters[SCNT_SRC_NODE_REMOVALS], 1); -} - -void -pf_unlink_src_node(struct pf_src_node *src) -{ - struct pf_srchash *sh; - - sh = &V_pf_srchash[pf_hashsrc(&src->addr, src->af)]; - PF_HASHROW_LOCK(sh); - pf_unlink_src_node_locked(src); - PF_HASHROW_UNLOCK(sh); -} - -static void -pf_free_src_node(struct pf_src_node *sn) -{ - - KASSERT(sn->states == 0, ("%s: %p has refs", __func__, sn)); - uma_zfree(V_pf_sources_z, sn); } u_int @@ -749,10 +729,12 @@ pf_free_src_nodes(struct pf_src_node_lis u_int count = 0; LIST_FOREACH_SAFE(sn, head, entry, tmp) { - pf_free_src_node(sn); + uma_zfree(V_pf_sources_z, sn); count++; } + counter_u64_add(V_pf_status.scounters[SCNT_SRC_NODE_REMOVALS], count); + return (count); } @@ -1542,7 +1524,7 @@ pf_purge_expired_src_nodes() PF_HASHROW_LOCK(sh); LIST_FOREACH_SAFE(cur, &sh->nodes, entry, next) if (cur->states == 0 && cur->expire <= time_uptime) { - pf_unlink_src_node_locked(cur); + pf_unlink_src_node(cur); LIST_INSERT_HEAD(&freelist, cur, entry); } else if (cur->rule.ptr != NULL) cur->rule.ptr->rule_flag |= PFRULE_REFS; @@ -1557,27 +1539,31 @@ pf_purge_expired_src_nodes() static void pf_src_tree_remove_state(struct pf_state *s) { - u_int32_t timeout; + struct pf_src_node *sn; + struct pf_srchash *sh; + uint32_t timeout; + + timeout = s->rule.ptr->timeout[PFTM_SRC_NODE] ? + s->rule.ptr->timeout[PFTM_SRC_NODE] : + V_pf_default_rule.timeout[PFTM_SRC_NODE]; if (s->src_node != NULL) { + sn = s->src_node; + sh = &V_pf_srchash[pf_hashsrc(&sn->addr, sn->af)]; + PF_HASHROW_LOCK(sh); if (s->src.tcp_est) - --s->src_node->conn; - if (--s->src_node->states == 0) { - timeout = s->rule.ptr->timeout[PFTM_SRC_NODE]; - if (!timeout) - timeout = - V_pf_default_rule.timeout[PFTM_SRC_NODE]; - s->src_node->expire = time_uptime + timeout; - } + --sn->conn; + if (--sn->states == 0) + sn->expire = time_uptime + timeout; + PF_HASHROW_UNLOCK(sh); } if (s->nat_src_node != s->src_node && s->nat_src_node != NULL) { - if (--s->nat_src_node->states == 0) { - timeout = s->rule.ptr->timeout[PFTM_SRC_NODE]; - if (!timeout) - timeout = - V_pf_default_rule.timeout[PFTM_SRC_NODE]; - s->nat_src_node->expire = time_uptime + timeout; - } + sn = s->nat_src_node; + sh = &V_pf_srchash[pf_hashsrc(&sn->addr, sn->af)]; + PF_HASHROW_LOCK(sh); + if (--sn->states == 0) + sn->expire = time_uptime + timeout; + PF_HASHROW_UNLOCK(sh); } s->src_node = s->nat_src_node = NULL; } @@ -3563,15 +3549,12 @@ pf_create_state(struct pf_rule *r, struc s->creation = time_uptime; s->expire = time_uptime; - if (sn != NULL) { + if (sn != NULL) s->src_node = sn; - s->src_node->states++; - } if (nsn != NULL) { /* XXX We only modify one side for now. */ PF_ACPY(&nsn->raddr, &nk->addr[1], pd->af); s->nat_src_node = nsn; - s->nat_src_node->states++; } if (pd->proto == IPPROTO_TCP) { if ((pd->flags & PFDESC_TCP_NORM) && pf_normalize_tcp_init(m, @@ -3669,14 +3652,32 @@ csfailed: if (nk != NULL) uma_zfree(V_pf_state_key_z, nk); - if (sn != NULL && sn->states == 0 && sn->expire == 0) { - pf_unlink_src_node(sn); - pf_free_src_node(sn); + if (sn != NULL) { + struct pf_srchash *sh; + + sh = &V_pf_srchash[pf_hashsrc(&sn->addr, sn->af)]; + PF_HASHROW_LOCK(sh); + if (--sn->states == 0 && sn->expire == 0) { + pf_unlink_src_node(sn); + uma_zfree(V_pf_sources_z, sn); + counter_u64_add( + V_pf_status.scounters[SCNT_SRC_NODE_REMOVALS], 1); + } + PF_HASHROW_UNLOCK(sh); } - if (nsn != sn && nsn != NULL && nsn->states == 0 && nsn->expire == 0) { - pf_unlink_src_node(nsn); - pf_free_src_node(nsn); + if (nsn != sn && nsn != NULL) { + struct pf_srchash *sh; + + sh = &V_pf_srchash[pf_hashsrc(&nsn->addr, nsn->af)]; + PF_HASHROW_LOCK(sh); + if (--nsn->states == 1 && nsn->expire == 0) { + pf_unlink_src_node(nsn); + uma_zfree(V_pf_sources_z, nsn); + counter_u64_add( + V_pf_status.scounters[SCNT_SRC_NODE_REMOVALS], 1); + } + PF_HASHROW_UNLOCK(sh); } return (PF_DROP); Modified: head/sys/netpfil/pf/pf_ioctl.c ============================================================================== --- head/sys/netpfil/pf/pf_ioctl.c Tue Mar 17 11:07:59 2015 (r280168) +++ head/sys/netpfil/pf/pf_ioctl.c Tue Mar 17 12:19:28 2015 (r280169) @@ -3436,7 +3436,7 @@ pf_kill_srcnodes(struct pfioc_src_node_k &psnk->psnk_dst.addr.v.a.addr, &psnk->psnk_dst.addr.v.a.mask, &sn->raddr, sn->af)) { - pf_unlink_src_node_locked(sn); + pf_unlink_src_node(sn); LIST_INSERT_HEAD(&kill, sn, entry); sn->expire = 1; } @@ -3449,18 +3449,10 @@ pf_kill_srcnodes(struct pfioc_src_node_k PF_HASHROW_LOCK(ih); LIST_FOREACH(s, &ih->states, entry) { - if (s->src_node && s->src_node->expire == 1) { -#ifdef INVARIANTS - s->src_node->states--; -#endif + if (s->src_node && s->src_node->expire == 1) s->src_node = NULL; - } - if (s->nat_src_node && s->nat_src_node->expire == 1) { -#ifdef INVARIANTS - s->nat_src_node->states--; -#endif + if (s->nat_src_node && s->nat_src_node->expire == 1) s->nat_src_node = NULL; - } } PF_HASHROW_UNLOCK(ih); }