From owner-svn-src-projects@FreeBSD.ORG Sun Jun 3 11:57:06 2012 Return-Path: Delivered-To: svn-src-projects@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 74D88106566C; Sun, 3 Jun 2012 11:57:06 +0000 (UTC) (envelope-from glebius@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id 5F6638FC12; Sun, 3 Jun 2012 11:57:06 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.4/8.14.4) with ESMTP id q53Bv6Do059652; Sun, 3 Jun 2012 11:57:06 GMT (envelope-from glebius@svn.freebsd.org) Received: (from glebius@localhost) by svn.freebsd.org (8.14.4/8.14.4/Submit) id q53Bv66A059650; Sun, 3 Jun 2012 11:57:06 GMT (envelope-from glebius@svn.freebsd.org) Message-Id: <201206031157.q53Bv66A059650@svn.freebsd.org> From: Gleb Smirnoff Date: Sun, 3 Jun 2012 11:57:06 +0000 (UTC) To: src-committers@freebsd.org, svn-src-projects@freebsd.org X-SVN-Group: projects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r236512 - projects/pf/head/sys/contrib/pf/net X-BeenThere: svn-src-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the src " projects" tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 03 Jun 2012 11:57:06 -0000 Author: glebius Date: Sun Jun 3 11:57:05 2012 New Revision: 236512 URL: http://svn.freebsd.org/changeset/base/236512 Log: In pf_map_addr(): - Fix "rpool->cur" update in such way, that pointer never gets a NULL value, otherwise concurrent threads may try to deref NULL. - Add a large comment explaining problems with round-robin, why most cases work, and why complicated would not. - Make some local variables more local to the case blocks, where they are used. Modified: projects/pf/head/sys/contrib/pf/net/pf_lb.c Modified: projects/pf/head/sys/contrib/pf/net/pf_lb.c ============================================================================== --- projects/pf/head/sys/contrib/pf/net/pf_lb.c Sun Jun 3 11:54:26 2012 (r236511) +++ projects/pf/head/sys/contrib/pf/net/pf_lb.c Sun Jun 3 11:57:05 2012 (r236512) @@ -350,11 +350,8 @@ int pf_map_addr(sa_family_t af, struct pf_rule *r, struct pf_addr *saddr, struct pf_addr *naddr, struct pf_addr *init_addr, struct pf_src_node **sn) { - unsigned char hash[16]; struct pf_pool *rpool = &r->rpool; - struct pf_addr *raddr = &rpool->cur->addr.v.a.addr; - struct pf_addr *rmask = &rpool->cur->addr.v.a.mask; - struct pf_pooladdr *acur = rpool->cur; + struct pf_addr *raddr = NULL, *rmask = NULL; if (*sn == NULL && r->rpool.opts & PF_POOL_STICKYADDR && (r->rpool.opts & PF_POOL_TYPEMASK) != PF_POOL_NONE) { @@ -452,10 +449,38 @@ pf_map_addr(sa_family_t af, struct pf_ru } break; case PF_POOL_SRCHASH: + { + unsigned char hash[16]; + pf_hash(saddr, (struct pf_addr *)&hash, &rpool->key, af); PF_POOLMASK(naddr, raddr, rmask, (struct pf_addr *)&hash, af); break; + } case PF_POOL_ROUNDROBIN: + { + struct pf_pooladdr *acur = rpool->cur; + + /* + * XXXGL: in the round-robin case we need to store + * the round-robin machine state in the rule, thus + * forwarding thread needs to modify rule. + * + * This is done w/o locking, because performance is assumed + * more importand than round-robin precision. + * + * In the simpliest case we just update the "rpool->cur" + * pointer. However, if pool contains tables or dynamic + * addresses, then "tblidx" is also used to store machine + * state. Since "tblidx" is int, concurrent access to it can't + * lead to inconsistence, only to lost of precision. + * + * Things get worse, if table contains not hosts, but + * prefixes. In this case counter also stores machine state, + * and for IPv6 address, counter can be updated atomically. + * Probably, using round-robin on a table containing IPv6 + * prefixes (or even IPv4) would cause a panic. + */ + if (rpool->cur->addr.type == PF_ADDR_TABLE) { if (!pfr_pool_get(rpool->cur->addr.p.tbl, &rpool->tblidx, &rpool->counter, af)) @@ -468,8 +493,10 @@ pf_map_addr(sa_family_t af, struct pf_ru goto get_addr; try_next: - if ((rpool->cur = TAILQ_NEXT(rpool->cur, entries)) == NULL) + if (TAILQ_NEXT(rpool->cur, entries) == NULL) rpool->cur = TAILQ_FIRST(&rpool->list); + else + rpool->cur = TAILQ_NEXT(rpool->cur, entries); if (rpool->cur->addr.type == PF_ADDR_TABLE) { rpool->tblidx = -1; if (pfr_pool_get(rpool->cur->addr.p.tbl, @@ -500,6 +527,7 @@ pf_map_addr(sa_family_t af, struct pf_ru PF_ACPY(init_addr, naddr, af); PF_AINC(&rpool->counter, af); break; + } } if (*sn != NULL) PF_ACPY(&(*sn)->raddr, naddr, af);