Date: Tue, 28 Jul 2015 09:13:56 +0000 (UTC) From: Gleb Smirnoff <glebius@FreeBSD.org> 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 Message-ID: <201507280913.t6S9Duae015413@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
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); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201507280913.t6S9Duae015413>