Date: Sat, 9 Mar 2013 13:14:16 +0100 From: =?ISO-8859-1?Q?Ermal_Lu=E7i?= <eri@freebsd.org> To: Kajetan Staszkiewicz <vegeta@tuxpowered.net> Cc: "freebsd-net@freebsd.org" <freebsd-net@freebsd.org>, "freebsd-pf@freebsd.org" <freebsd-pf@freebsd.org> Subject: Re: [patch] Source entries removing is awfully slow. Message-ID: <CAPBZQG0Jj_c-XvVJNV2S02xcitr%2Bnhs%2BmV=GjJm3YeM6iPUX7g@mail.gmail.com> In-Reply-To: <201303082151.00895.vegeta@tuxpowered.net> References: <201303081419.17743.vegeta@tuxpowered.net> <CAPBZQG2bb2xzPB2UoPUDx-ifyBdmjac6b8kV76DTPBUzLCDmJw@mail.gmail.com> <201303082151.00895.vegeta@tuxpowered.net>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --] On Fri, Mar 8, 2013 at 9:51 PM, Kajetan Staszkiewicz <vegeta@tuxpowered.net>wrote: > Dnia pi±tek, 8 marca 2013 o 21:11:43 Ermal Luçi napisa³(a): > > Is this FreeBSD 9.x or HEAD? > > I found the problem and developed the patch on 9.1. > > Can you please test this more 'beautiful' patch. Its similar to yours but also delays src state removal to the proper purge thread. Though the src node removal option through pfctl -K does a lot of job to cleanup things Still need to undertand why it takes so much time for you to loop through 500K states. The purge thread does that every tick by partitioning it to a few per time slot but still minutes is way loong. Can you please try to give a top -SH view of the time when this happens and a pfctl -vvsa output? > -- > | pozdrawiam / greetings | powered by Debian, CentOS and FreeBSD | > | Kajetan Staszkiewicz | jabber,email: vegeta()tuxpowered net | > | Vegeta | www: http://vegeta.tuxpowered.net | > `------------------------^---------------------------------------' > -- Ermal [-- Attachment #2 --] diff --git a/sys/contrib/pf/net/pf.c b/sys/contrib/pf/net/pf.c index 9fb05ae..4df40cc 100644 --- a/sys/contrib/pf/net/pf.c +++ b/sys/contrib/pf/net/pf.c @@ -720,6 +720,9 @@ pf_insert_src_node(struct pf_src_node **sn, struct pf_rule *rule, rule->max_src_conn_rate.limit, rule->max_src_conn_rate.seconds); +#ifdef __FreeBSD__ + TAILQ_INIT(&(*sn)->state_list); +#endif (*sn)->af = af; if (rule->rule_flag & PFRULE_RULESRCTRACK || rule->rpool.opts & PF_POOL_STICKYADDR) @@ -1453,6 +1456,9 @@ pf_purge_expired_src_nodes(int waslocked) #endif { struct pf_src_node *cur, *next; +#ifdef __FreeBSD__ + struct pf_state *s; +#endif int locked = waslocked; #ifdef __FreeBSD__ @@ -1486,6 +1492,12 @@ pf_purge_expired_src_nodes(int waslocked) pf_rm_rule(NULL, cur->rule.ptr); } #ifdef __FreeBSD__ + while (!TAILQ_EMPTY(&cur->state_list)) { + s = TAILQ_FIRST(&cur->state_list); + TAILQ_REMOVE(&cur->state_list, s, srcnode_link); + s->src_node = NULL; + s->nat_src_node = NULL; + } RB_REMOVE(pf_src_tree, &V_tree_src_tracking, cur); V_pf_status.scounters[SCNT_SRC_NODE_REMOVALS]++; V_pf_status.src_nodes--; @@ -1529,6 +1541,10 @@ pf_src_tree_remove_state(struct pf_state *s) #endif s->src_node->expire = time_second + timeout; } +#ifdef __FreeBSD__ + if (!TAILQ_EMPTY(&s->src_node->state_list)) + TAILQ_REMOVE(&s->src_node->state_list, s, srcnode_link); +#endif } if (s->nat_src_node != s->src_node && s->nat_src_node != NULL) { if (--s->nat_src_node->states <= 0) { @@ -1542,6 +1558,10 @@ pf_src_tree_remove_state(struct pf_state *s) #endif s->nat_src_node->expire = time_second + timeout; } +#ifdef __FreeBSD__ + if (!TAILQ_EMPTY(&s->nat_src_node->state_list)) + TAILQ_REMOVE(&s->nat_src_node->state_list, s, srcnode_link); +#endif } s->src_node = s->nat_src_node = NULL; } @@ -3949,8 +3969,18 @@ pf_create_state(struct pf_rule *r, struct pf_rule *nr, struct pf_rule *a, pool_put(&pf_state_pl, s); #endif return (PF_DROP); +#ifdef __FreeBSD__ + } else { + if (sn != NULL) + TAILQ_INSERT_HEAD(&sn->state_list, s, srcnode_link); + if (nsn != NULL) + TAILQ_INSERT_HEAD(&nsn->state_list, s, srcnode_link); + *sm = s; + } +#else } else *sm = s; +#endif pf_set_rt_ifp(s, pd->src); /* needs s->state_key set */ if (tag > 0) { diff --git a/sys/contrib/pf/net/pf_ioctl.c b/sys/contrib/pf/net/pf_ioctl.c index 3b130e5..2864e9a 100644 --- a/sys/contrib/pf/net/pf_ioctl.c +++ b/sys/contrib/pf/net/pf_ioctl.c @@ -3789,7 +3789,9 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p) case DIOCKILLSRCNODES: { struct pf_src_node *sn; +#ifndef __FreeBSD__ struct pf_state *s; +#endif struct pfioc_src_node_kill *psnk = (struct pfioc_src_node_kill *)addr; u_int killed = 0; @@ -3808,6 +3810,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p) &psnk->psnk_dst.addr.v.a.mask, &sn->raddr, sn->af)) { /* Handle state to src_node linkage */ +#ifndef __FreeBSD__ if (sn->states != 0) { RB_FOREACH(s, pf_state_tree_id, #ifdef __FreeBSD__ @@ -3822,13 +3825,16 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p) } sn->states = 0; } +#endif sn->expire = 1; killed++; } } +#if 0 if (killed > 0) pf_purge_expired_src_nodes(1); +#endif psnk->psnk_killed = killed; break; diff --git a/sys/contrib/pf/net/pfvar.h b/sys/contrib/pf/net/pfvar.h index dab70c5..e31d39d 100644 --- a/sys/contrib/pf/net/pfvar.h +++ b/sys/contrib/pf/net/pfvar.h @@ -739,6 +739,9 @@ struct pf_src_node { struct pf_addr raddr; union pf_rule_ptr rule; struct pfi_kif *kif; +#ifdef __FreeBSD__ + TAILQ_HEAD(, pf_state) state_list; +#endif u_int64_t bytes[2]; u_int64_t packets[2]; u_int32_t states; @@ -840,6 +843,9 @@ struct pf_state { TAILQ_ENTRY(pf_state) sync_list; TAILQ_ENTRY(pf_state) entry_list; +#ifdef __FreeBSD__ + TAILQ_ENTRY(pf_state) srcnode_link; +#endif RB_ENTRY(pf_state) entry_id; struct pf_state_peer src; struct pf_state_peer dst;
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAPBZQG0Jj_c-XvVJNV2S02xcitr%2Bnhs%2BmV=GjJm3YeM6iPUX7g>