From owner-svn-src-all@freebsd.org Tue Jul 28 09:13:57 2015 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id E688D9AC5FF; Tue, 28 Jul 2015 09:13:57 +0000 (UTC) (envelope-from glebius@FreeBSD.org) Received: from repo.freebsd.org (repo.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 CA77AE36; Tue, 28 Jul 2015 09:13:57 +0000 (UTC) (envelope-from glebius@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.70]) by repo.freebsd.org (8.14.9/8.14.9) with ESMTP id t6S9Dvo9015416; Tue, 28 Jul 2015 09:13:57 GMT (envelope-from glebius@FreeBSD.org) Received: (from glebius@localhost) by repo.freebsd.org (8.14.9/8.14.9/Submit) id t6S9Duae015413; Tue, 28 Jul 2015 09:13:56 GMT (envelope-from glebius@FreeBSD.org) Message-Id: <201507280913.t6S9Duae015413@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: glebius set sender to glebius@FreeBSD.org using -f From: Gleb Smirnoff Date: Tue, 28 Jul 2015 09:13:56 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-10@freebsd.org Subject: svn commit: r285940 - in stable/10/sys: net netpfil/pf X-SVN-Group: stable-10 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 28 Jul 2015 09:13:58 -0000 Author: glebius Date: Tue Jul 28 09:13:55 2015 New Revision: 285940 URL: https://svnweb.freebsd.org/changeset/base/285940 Log: Merge 280169: always lock the hash row of a source node when updating its 'states' counter. PR: 182401 Modified: stable/10/sys/net/pfvar.h stable/10/sys/netpfil/pf/pf.c stable/10/sys/netpfil/pf/pf_ioctl.c Directory Properties: stable/10/ (props changed) Modified: stable/10/sys/net/pfvar.h ============================================================================== --- stable/10/sys/net/pfvar.h Tue Jul 28 09:09:01 2015 (r285939) +++ stable/10/sys/net/pfvar.h Tue Jul 28 09:13:55 2015 (r285940) @@ -1549,7 +1549,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: stable/10/sys/netpfil/pf/pf.c ============================================================================== --- stable/10/sys/netpfil/pf/pf.c Tue Jul 28 09:09:01 2015 (r285939) +++ stable/10/sys/netpfil/pf/pf.c Tue Jul 28 09:13:55 2015 (r285940) @@ -655,7 +655,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); @@ -699,6 +702,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); @@ -715,37 +719,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 @@ -755,10 +735,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); } @@ -1550,7 +1532,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; @@ -1565,27 +1547,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; } @@ -3571,15 +3557,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, @@ -3677,14 +3660,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: stable/10/sys/netpfil/pf/pf_ioctl.c ============================================================================== --- stable/10/sys/netpfil/pf/pf_ioctl.c Tue Jul 28 09:09:01 2015 (r285939) +++ stable/10/sys/netpfil/pf/pf_ioctl.c Tue Jul 28 09:13:55 2015 (r285940) @@ -3430,7 +3430,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; } @@ -3443,18 +3443,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); }