| Message ID | 20100203203929.GA6168@x200 |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
On Wed, 2010-02-03 at 22:39 +0200, Alexey Dobriyan wrote: > Jon Masters correctly points out that conntrack hash sizes > (nf_conntrack_htable_size) are global (not per-netns) and > modifiable at runtime via /sys/module/nf_conntrack/hashsize . Thanks. Jon. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Alexey Dobriyan wrote: > Jon Masters correctly points out that conntrack hash sizes > (nf_conntrack_htable_size) are global (not per-netns) and > modifiable at runtime via /sys/module/nf_conntrack/hashsize . > > Steps to reproduce: > clone(CLONE_NEWNET) > [grow /sys/module/nf_conntrack/hashsize] > exit() > > At netns exit we are going to scan random memory for conntracks to be killed. > > Apparently there is a code which deals with hashtable resize for > init_net (and it was there befode netns conntrack code), so prohibit > hashsize modification if there is more than one netns exists. > > To change hashtable sizes, you need to reload module. > > Expectation hashtable size was simply glued to a variable with no code > to rehash expectations, so it was a bug to allow writing to it. > Make "expect_hashsize" readonly. > > This is temporarily until we figure out what to do. How about alternatively moving nf_conntrack_hsize into the per-namespace struct? It doesn't look more complicated or intrusive and would allow to still change the init_net hashsize. Also seems less hackish :) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Patrick McHardy wrote: > Alexey Dobriyan wrote: >> Jon Masters correctly points out that conntrack hash sizes >> (nf_conntrack_htable_size) are global (not per-netns) and >> modifiable at runtime via /sys/module/nf_conntrack/hashsize . >> >> Steps to reproduce: >> clone(CLONE_NEWNET) >> [grow /sys/module/nf_conntrack/hashsize] >> exit() >> >> At netns exit we are going to scan random memory for conntracks to be killed. >> >> Apparently there is a code which deals with hashtable resize for >> init_net (and it was there befode netns conntrack code), so prohibit >> hashsize modification if there is more than one netns exists. >> >> To change hashtable sizes, you need to reload module. >> >> Expectation hashtable size was simply glued to a variable with no code >> to rehash expectations, so it was a bug to allow writing to it. >> Make "expect_hashsize" readonly. >> >> This is temporarily until we figure out what to do. > > How about alternatively moving nf_conntrack_hsize into the > per-namespace struct? It doesn't look more complicated or > intrusive and would allow to still change the init_net > hashsize. Also seems less hackish :) Just to avoid duplicate work, I'm currently trying that. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Alexey Dobriyan wrote: > Jon Masters correctly points out that conntrack hash sizes > (nf_conntrack_htable_size) are global (not per-netns) and > modifiable at runtime via /sys/module/nf_conntrack/hashsize . > > Steps to reproduce: > clone(CLONE_NEWNET) > [grow /sys/module/nf_conntrack/hashsize] > exit() > > At netns exit we are going to scan random memory for conntracks to be killed. > > Apparently there is a code which deals with hashtable resize for > init_net (and it was there befode netns conntrack code), so prohibit > hashsize modification if there is more than one netns exists. > > To change hashtable sizes, you need to reload module. > > Expectation hashtable size was simply glued to a variable with no code > to rehash expectations, so it was a bug to allow writing to it. > Make "expect_hashsize" readonly. I've applied the expectation part, thanks Alexey. > --- a/net/netfilter/nf_conntrack_expect.c > +++ b/net/netfilter/nf_conntrack_expect.c > @@ -569,7 +569,7 @@ static void exp_proc_remove(struct net *net) > #endif /* CONFIG_PROC_FS */ > } > > -module_param_named(expect_hashsize, nf_ct_expect_hsize, uint, 0600); > +module_param_named(expect_hashsize, nf_ct_expect_hsize, uint, 0400); > > int nf_conntrack_expect_init(struct net *net) > { > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2010-02-04 at 17:27 +0100, Patrick McHardy wrote: > Patrick McHardy wrote: > > Alexey Dobriyan wrote: > >> Jon Masters correctly points out that conntrack hash sizes > >> (nf_conntrack_htable_size) are global (not per-netns) and > >> modifiable at runtime via /sys/module/nf_conntrack/hashsize . > >> > >> Steps to reproduce: > >> clone(CLONE_NEWNET) > >> [grow /sys/module/nf_conntrack/hashsize] > >> exit() > >> > >> At netns exit we are going to scan random memory for conntracks to be killed. > >> > >> Apparently there is a code which deals with hashtable resize for > >> init_net (and it was there befode netns conntrack code), so prohibit > >> hashsize modification if there is more than one netns exists. > >> > >> To change hashtable sizes, you need to reload module. > >> > >> Expectation hashtable size was simply glued to a variable with no code > >> to rehash expectations, so it was a bug to allow writing to it. > >> Make "expect_hashsize" readonly. > >> > >> This is temporarily until we figure out what to do. > > > > How about alternatively moving nf_conntrack_hsize into the > > per-namespace struct? It doesn't look more complicated or > > intrusive and would allow to still change the init_net > > hashsize. Also seems less hackish :) > > Just to avoid duplicate work, I'm currently trying that. Bah. I already worked a set of patches to do that as I mentioned, but you've probably done it by now - can clean up and post if not :) Jon. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Jon Masters wrote: > On Thu, 2010-02-04 at 17:27 +0100, Patrick McHardy wrote: >> Patrick McHardy wrote: >>> Alexey Dobriyan wrote: >>>> Jon Masters correctly points out that conntrack hash sizes >>>> (nf_conntrack_htable_size) are global (not per-netns) and >>>> modifiable at runtime via /sys/module/nf_conntrack/hashsize . >>>> >>>> Steps to reproduce: >>>> clone(CLONE_NEWNET) >>>> [grow /sys/module/nf_conntrack/hashsize] >>>> exit() >>>> >>>> At netns exit we are going to scan random memory for conntracks to be killed. >>>> >>>> Apparently there is a code which deals with hashtable resize for >>>> init_net (and it was there befode netns conntrack code), so prohibit >>>> hashsize modification if there is more than one netns exists. >>>> >>>> To change hashtable sizes, you need to reload module. >>>> >>>> Expectation hashtable size was simply glued to a variable with no code >>>> to rehash expectations, so it was a bug to allow writing to it. >>>> Make "expect_hashsize" readonly. >>>> >>>> This is temporarily until we figure out what to do. >>> How about alternatively moving nf_conntrack_hsize into the >>> per-namespace struct? It doesn't look more complicated or >>> intrusive and would allow to still change the init_net >>> hashsize. Also seems less hackish :) >> Just to avoid duplicate work, I'm currently trying that. > > Bah. I already worked a set of patches to do that as I mentioned, but > you've probably done it by now - can clean up and post if not :) Sorry, I missed that in your mail. I'm pretty much done, will finish testing shortly. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2010-02-05 at 11:00 +0100, Patrick McHardy wrote: > Jon Masters wrote: > > On Thu, 2010-02-04 at 17:27 +0100, Patrick McHardy wrote: > >> Patrick McHardy wrote: > >>> Alexey Dobriyan wrote: > >>>> Jon Masters correctly points out that conntrack hash sizes > >>>> (nf_conntrack_htable_size) are global (not per-netns) and > >>>> modifiable at runtime via /sys/module/nf_conntrack/hashsize . > >>>> > >>>> Steps to reproduce: > >>>> clone(CLONE_NEWNET) > >>>> [grow /sys/module/nf_conntrack/hashsize] > >>>> exit() > >>>> > >>>> At netns exit we are going to scan random memory for conntracks to be killed. > >>>> > >>>> Apparently there is a code which deals with hashtable resize for > >>>> init_net (and it was there befode netns conntrack code), so prohibit > >>>> hashsize modification if there is more than one netns exists. > >>>> > >>>> To change hashtable sizes, you need to reload module. > >>>> > >>>> Expectation hashtable size was simply glued to a variable with no code > >>>> to rehash expectations, so it was a bug to allow writing to it. > >>>> Make "expect_hashsize" readonly. > >>>> > >>>> This is temporarily until we figure out what to do. > >>> How about alternatively moving nf_conntrack_hsize into the > >>> per-namespace struct? It doesn't look more complicated or > >>> intrusive and would allow to still change the init_net > >>> hashsize. Also seems less hackish :) > >> Just to avoid duplicate work, I'm currently trying that. > > > > Bah. I already worked a set of patches to do that as I mentioned, but > > you've probably done it by now - can clean up and post if not :) > > Sorry, I missed that in your mail. I'm pretty much done, will finish > testing shortly. Oh, it's cool. I hacked it together on my test box but I'm happy to go with whatever you post later, I will just try to be forthcoming next time with my bits first to save you hassle. Please do keep CCing me on these things and I'll try to test over the weekend as time permits. Jon. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Jon Masters wrote: > On Fri, 2010-02-05 at 11:00 +0100, Patrick McHardy wrote: >>>> Just to avoid duplicate work, I'm currently trying that. >>> Bah. I already worked a set of patches to do that as I mentioned, but >>> you've probably done it by now - can clean up and post if not :) >> Sorry, I missed that in your mail. I'm pretty much done, will finish >> testing shortly. > > Oh, it's cool. I hacked it together on my test box but I'm happy to go > with whatever you post later, I will just try to be forthcoming next > time with my bits first to save you hassle. Please do keep CCing me on > these things and I'll try to test over the weekend as time permits. Will do, thanks for all your help Jon. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--- a/net/netfilter/nf_conntrack_core.c +++ b/net/netfilter/nf_conntrack_core.c @@ -21,6 +21,7 @@ #include <linux/stddef.h> #include <linux/slab.h> #include <linux/random.h> +#include <linux/rtnetlink.h> #include <linux/jhash.h> #include <linux/err.h> #include <linux/percpu.h> @@ -1198,6 +1199,20 @@ int nf_conntrack_set_hashsize(const char *val, struct kernel_param *kp) if (!nf_conntrack_htable_size) return param_set_uint(val, kp); + { + struct net *net; + unsigned int nr; + + nr = 0; + rtnl_lock(); + for_each_net(net) + nr++; + rtnl_unlock(); + /* init_net always exists */ + if (nr != 1) + return -EINVAL; + } + hashsize = simple_strtoul(val, NULL, 0); if (!hashsize) return -EINVAL; --- a/net/netfilter/nf_conntrack_expect.c +++ b/net/netfilter/nf_conntrack_expect.c @@ -569,7 +569,7 @@ static void exp_proc_remove(struct net *net) #endif /* CONFIG_PROC_FS */ } -module_param_named(expect_hashsize, nf_ct_expect_hsize, uint, 0600); +module_param_named(expect_hashsize, nf_ct_expect_hsize, uint, 0400); int nf_conntrack_expect_init(struct net *net) {
Jon Masters correctly points out that conntrack hash sizes (nf_conntrack_htable_size) are global (not per-netns) and modifiable at runtime via /sys/module/nf_conntrack/hashsize . Steps to reproduce: clone(CLONE_NEWNET) [grow /sys/module/nf_conntrack/hashsize] exit() At netns exit we are going to scan random memory for conntracks to be killed. Apparently there is a code which deals with hashtable resize for init_net (and it was there befode netns conntrack code), so prohibit hashsize modification if there is more than one netns exists. To change hashtable sizes, you need to reload module. Expectation hashtable size was simply glued to a variable with no code to rehash expectations, so it was a bug to allow writing to it. Make "expect_hashsize" readonly. This is temporarily until we figure out what to do. Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com> Cc: stable@kernel.org --- net/netfilter/nf_conntrack_core.c | 15 +++++++++++++++ net/netfilter/nf_conntrack_expect.c | 2 +- 2 files changed, 16 insertions(+), 1 deletion(-) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html