>Number: 184003 >Category: kern >Synopsis: On state creation src_node is looked up twice. >Confidential: no >Severity: non-critical >Priority: low >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Fri Nov 15 15:50:00 UTC 2013 >Closed-Date: >Last-Modified: >Originator: Kajetan Staszkiewicz >Release: 9.1, 10.0-BETA >Organization: InnoGames GmbH >Environment: FreeBSD xxxxxxx 10.0-BETA3 FreeBSD 10.0-BETA3 #3 75c5288(lookup-src_node-only-once)-dirty: Fri Nov 15 15:31:21 CET 2013 root@lbdevel100:/usr/obj/mnt/builder/freebsd.git/sys/IGLB amd64 >Description: When a new state is created, pf_insert_src_node is called which tries to find an existing src_node or creates a new one if none matching is found. Later, when pf_set_rt_ifp (and pf_map_addr) is called, a search for src_node is performed again, even though matching (found or new) src_node is already known. >How-To-Repeat: Have your FreeBSD-based loadbalancer under a SYN DDoS attack, observe 2x more src_node lookups than incoming SYN packets. >Fix: Do not call pf_find_src_node in pf_map_addr if source_node is given.
The attached patch is for FreeBSD 10.0-BETA3 and was not yet tested under bigger load, although the same approach works well for FreeBSD 9.1. I can provide the 9.1 patch too if requested. Patch attached with submission follows: # When a new state is created, pf_insert_src_node is called which tries to # find an existing src_node or creates a new one if none matching is found. # Later, when pf_set_rt_ifp (and pf_map_addr) is called, a search for src_node # is performed again, even though matching (found or new) src_node is already # known. # # Do not call pf_find_src_node in pf_map_addr if source_node is given. # # [email protected] # Work sponsored by InnoGames GmbH # diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c index 59a349d..c58438a 100644 --- a/sys/netpfil/pf/pf.c +++ b/sys/netpfil/pf/pf.c @@ -268,7 +268,8 @@ static u_int16_t pf_get_mss(struct mbuf *, int, u_int16_t, static u_int16_t pf_calc_mss(struct pf_addr *, sa_family_t, int, u_int16_t); static int pf_set_rt_ifp(struct pf_state *, - struct pf_addr *, sa_family_t af); + struct pf_addr *, sa_family_t af, + struct pf_src_node **sn); static int pf_check_proto_cksum(struct mbuf *, int, int, u_int8_t, sa_family_t); static void pf_print_state_parts(struct pf_state *, @@ -2930,10 +2931,10 @@ pf_calc_mss(struct pf_addr *addr, sa_family_t af, int rtableid, u_int16_t offer) } static int -pf_set_rt_ifp(struct pf_state *s, struct pf_addr *saddr, sa_family_t af) +pf_set_rt_ifp(struct pf_state *s, struct pf_addr *saddr, sa_family_t af, + struct pf_src_node **sn) { struct pf_rule *r = s->rule.ptr; - struct pf_src_node *sn = NULL; int map_status = 0; s->rt_kif = NULL; @@ -2942,13 +2943,13 @@ pf_set_rt_ifp(struct pf_state *s, struct pf_addr *saddr, sa_family_t af) switch (af) { #ifdef INET case AF_INET: - map_status = pf_map_addr(AF_INET, r, saddr, &s->rt_addr, NULL, &sn); + map_status = pf_map_addr(AF_INET, r, saddr, &s->rt_addr, NULL, sn); s->rt_kif = r->rpool.cur->kif; break; #endif /* INET */ #ifdef INET6 case AF_INET6: - map_status = pf_map_addr(AF_INET6, r, saddr, &s->rt_addr, NULL, &sn); + map_status = pf_map_addr(AF_INET6, r, saddr, &s->rt_addr, NULL, sn); s->rt_kif = r->rpool.cur->kif; break; #endif /* INET6 */ @@ -3523,7 +3524,7 @@ pf_create_state(struct pf_rule *r, struct pf_rule *nr, struct pf_rule *a, remove the state and drop the packet. It makes no sense forwarding it if redirection mapping has faied. Do it before setting timeouts, csfailed fails otherwise. */ - if (pf_set_rt_ifp(s, pd->src, pd->af)) { + if (pf_set_rt_ifp(s, pd->src, pd->af, &sn)) { REASON_SET(&reason, PFRES_MAPFAILED); pf_src_tree_remove_state(s); STATE_DEC_COUNTERS(s); diff --git a/sys/netpfil/pf/pf_lb.c b/sys/netpfil/pf/pf_lb.c index f870bf4..137f1de 100644 --- a/sys/netpfil/pf/pf_lb.c +++ b/sys/netpfil/pf/pf_lb.c @@ -308,22 +308,31 @@ pf_map_addr(sa_family_t af, struct pf_rule *r, struct pf_addr *saddr, struct pf_pool *rpool = &r->rpool; struct pf_addr *raddr = NULL, *rmask = NULL; + /* Try to find a src_node if none was given and this + is a sticky-address rule. */ if (*sn == NULL && r->rpool.opts & PF_POOL_STICKYADDR && (r->rpool.opts & PF_POOL_TYPEMASK) != PF_POOL_NONE) { *sn = pf_find_src_node(saddr, r, af, 0); - if (*sn != NULL && !PF_AZERO(&(*sn)->raddr, af)) { - PF_ACPY(naddr, &(*sn)->raddr, af); - if (V_pf_status.debug >= PF_DEBUG_MISC) { - printf("pf_map_addr: src tracking maps "); - pf_print_host(saddr, 0, af); - printf(" to "); - pf_print_host(naddr, 0, af); - printf("\n"); - } - return (0); + } + + /* If a src_node was found or explicitly given and it has a non-zero + route address, use this address. A zeroed address is found if the + src node was created just a moment ago in pf_create_state and it + needs to be filled in with routing decission calculated here. */ + if (*sn != NULL && !PF_AZERO(&(*sn)->raddr, af)) { + PF_ACPY(naddr, &(*sn)->raddr, af); + if (V_pf_status.debug >= PF_DEBUG_MISC) { + printf("pf_map_addr: src tracking maps "); + pf_print_host(saddr, 0, af); + printf(" to "); + pf_print_host(naddr, 0, af); + printf("\n"); } + return (0); } + /* Find the route using chosen algorithm. Store the found route + in src_node if it was given or found. */ if (rpool->cur->addr.type == PF_ADDR_NOROUTE) return (1); if (rpool->cur->addr.type == PF_ADDR_DYNIFTL) { >Release-Note: >Audit-Trail: >Unformatted: _______________________________________________ [email protected] mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-bugs To unsubscribe, send any mail to "[email protected]"
