| Message ID | 20200825224208.1268641-1-maheshb@google.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | [PATCHv2,next] net: add option to not create fall-back tunnels in root-ns as well | expand |
On 8/25/20 3:42 PM, Mahesh Bandewar wrote: > The sysctl that was added earlier by commit 79134e6ce2c ("net: do > not create fallback tunnels for non-default namespaces") to create > fall-back only in root-ns. This patch enhances that behavior to provide > option not to create fallback tunnels in root-ns as well. Since modules > that create fallback tunnels could be built-in and setting the sysctl > value after booting is pointless, so added a kernel cmdline options to > change this default. The default setting is preseved for backward > compatibility. The kernel command line option of fb_tunnels=initns will > set the sysctl value to 1 and will create fallback tunnels only in initns > while kernel cmdline fb_tunnels=none will set the sysctl value to 2 and > fallback tunnels are skipped in every netns. > > Signed-off-by: Mahesh Bandewar <maheshb@google.com> > Cc: Eric Dumazet <edumazet@google.com> > Cc: Maciej Zenczykowski <maze@google.com> > Cc: Jian Yang <jianyang@google.com> > --- > v1->v2 > Removed the Kconfig option which would force rebuild and replaced with > kcmd-line option > > .../admin-guide/kernel-parameters.txt | 5 +++++ > Documentation/admin-guide/sysctl/net.rst | 20 +++++++++++++------ > include/linux/netdevice.h | 7 ++++++- > net/core/sysctl_net_core.c | 17 ++++++++++++++-- > 4 files changed, 40 insertions(+), 9 deletions(-) > > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt > index a1068742a6df..09a51598c792 100644 > --- a/Documentation/admin-guide/kernel-parameters.txt > +++ b/Documentation/admin-guide/kernel-parameters.txt > @@ -801,6 +801,11 @@ > > debug_objects [KNL] Enable object debugging > > + fb_tunnels= [NET] > + Format: { initns | none } > + See Documentation/admin-guide/sysctl/net.rst for > + fb_tunnels_only_for_init_ns > + Not at this location in this file. Entries in this file are meant to be in alphabetical order (mostly). So leave debug_objects and no_debug_objects together, and insert fb_tunnels between fail_make_request= and floppy=. Thanks. > no_debug_objects > [KNL] Disable object debugging >
On Tue, Aug 25, 2020 at 3:47 PM Randy Dunlap <rdunlap@infradead.org> wrote: > > On 8/25/20 3:42 PM, Mahesh Bandewar wrote: > > The sysctl that was added earlier by commit 79134e6ce2c ("net: do > > not create fallback tunnels for non-default namespaces") to create > > fall-back only in root-ns. This patch enhances that behavior to provide > > option not to create fallback tunnels in root-ns as well. Since modules > > that create fallback tunnels could be built-in and setting the sysctl > > value after booting is pointless, so added a kernel cmdline options to > > change this default. The default setting is preseved for backward > > compatibility. The kernel command line option of fb_tunnels=initns will > > set the sysctl value to 1 and will create fallback tunnels only in initns > > while kernel cmdline fb_tunnels=none will set the sysctl value to 2 and > > fallback tunnels are skipped in every netns. > > > > Signed-off-by: Mahesh Bandewar <maheshb@google.com> > > Cc: Eric Dumazet <edumazet@google.com> > > Cc: Maciej Zenczykowski <maze@google.com> > > Cc: Jian Yang <jianyang@google.com> > > --- > > v1->v2 > > Removed the Kconfig option which would force rebuild and replaced with > > kcmd-line option > > > > .../admin-guide/kernel-parameters.txt | 5 +++++ > > Documentation/admin-guide/sysctl/net.rst | 20 +++++++++++++------ > > include/linux/netdevice.h | 7 ++++++- > > net/core/sysctl_net_core.c | 17 ++++++++++++++-- > > 4 files changed, 40 insertions(+), 9 deletions(-) > > > > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt > > index a1068742a6df..09a51598c792 100644 > > --- a/Documentation/admin-guide/kernel-parameters.txt > > +++ b/Documentation/admin-guide/kernel-parameters.txt > > @@ -801,6 +801,11 @@ > > > > debug_objects [KNL] Enable object debugging > > > > + fb_tunnels= [NET] > > + Format: { initns | none } > > + See Documentation/admin-guide/sysctl/net.rst for > > + fb_tunnels_only_for_init_ns > > + > > Not at this location in this file. > Entries in this file are meant to be in alphabetical order (mostly). > > So leave debug_objects and no_debug_objects together, and insert fb_tunnels > between fail_make_request= and floppy=. > I see. I'll fix it in the next revision. thanks for the suggestion. --mahesh.. > Thanks. > > > no_debug_objects > > [KNL] Disable object debugging > > > > -- > ~Randy >
On Tue, Aug 25, 2020 at 4:00 PM Mahesh Bandewar (महेश बंडेवार) <maheshb@google.com> wrote: > > On Tue, Aug 25, 2020 at 3:47 PM Randy Dunlap <rdunlap@infradead.org> wrote: > > > > On 8/25/20 3:42 PM, Mahesh Bandewar wrote: > > > The sysctl that was added earlier by commit 79134e6ce2c ("net: do > > > not create fallback tunnels for non-default namespaces") to create > > > fall-back only in root-ns. This patch enhances that behavior to provide > > > option not to create fallback tunnels in root-ns as well. Since modules > > > that create fallback tunnels could be built-in and setting the sysctl > > > value after booting is pointless, so added a kernel cmdline options to > > > change this default. The default setting is preseved for backward > > > compatibility. The kernel command line option of fb_tunnels=initns will > > > set the sysctl value to 1 and will create fallback tunnels only in initns > > > while kernel cmdline fb_tunnels=none will set the sysctl value to 2 and > > > fallback tunnels are skipped in every netns. > > > > > > Signed-off-by: Mahesh Bandewar <maheshb@google.com> > > > Cc: Eric Dumazet <edumazet@google.com> > > > Cc: Maciej Zenczykowski <maze@google.com> > > > Cc: Jian Yang <jianyang@google.com> > > > --- > > > v1->v2 > > > Removed the Kconfig option which would force rebuild and replaced with > > > kcmd-line option > > > > > > .../admin-guide/kernel-parameters.txt | 5 +++++ > > > Documentation/admin-guide/sysctl/net.rst | 20 +++++++++++++------ > > > include/linux/netdevice.h | 7 ++++++- > > > net/core/sysctl_net_core.c | 17 ++++++++++++++-- > > > 4 files changed, 40 insertions(+), 9 deletions(-) > > > > > > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt > > > index a1068742a6df..09a51598c792 100644 > > > --- a/Documentation/admin-guide/kernel-parameters.txt > > > +++ b/Documentation/admin-guide/kernel-parameters.txt > > > @@ -801,6 +801,11 @@ > > > > > > debug_objects [KNL] Enable object debugging > > > > > > + fb_tunnels= [NET] > > > + Format: { initns | none } > > > + See Documentation/admin-guide/sysctl/net.rst for > > > + fb_tunnels_only_for_init_ns > > > + > > > > Not at this location in this file. > > Entries in this file are meant to be in alphabetical order (mostly). > > > > So leave debug_objects and no_debug_objects together, and insert fb_tunnels > > between fail_make_request= and floppy=. > > > I see. I'll fix it in the next revision. > thanks for the suggestion. > --mahesh.. > > > Thanks. > > > > > no_debug_objects > > > [KNL] Disable object debugging > > > > > > > -- > > ~Randy Setting it to 1 via kcmdline doesn't seem all that useful, since instead of that you can just use initrc/sysctl.conf/etc. Would it be simpler if it was just 'no_fb_tunnels' or 'no_fallback_tunnels' and the function just set the sysctl to 2 unconditionally? (ie. no =.... parsing at all) that would also be less code... btw. I also don't understand the '!IS_ENABLED(CONFIG_SYSCTL) ||' piece. Why not just delete that? This seems to force fallback tunnels if you disable CONFIG_SYSCTL... but (a) then the kcmdline option doesn't work, and (b) that should already just happen by virtue of the sysctl defaulting to 0.
On Tue, Aug 25, 2020 at 5:42 PM Maciej Żenczykowski <maze@google.com> wrote: > > On Tue, Aug 25, 2020 at 4:00 PM Mahesh Bandewar (महेश बंडेवार) > <maheshb@google.com> wrote: > > > > On Tue, Aug 25, 2020 at 3:47 PM Randy Dunlap <rdunlap@infradead.org> wrote: > > > > > > On 8/25/20 3:42 PM, Mahesh Bandewar wrote: > > > > The sysctl that was added earlier by commit 79134e6ce2c ("net: do > > > > not create fallback tunnels for non-default namespaces") to create > > > > fall-back only in root-ns. This patch enhances that behavior to provide > > > > option not to create fallback tunnels in root-ns as well. Since modules > > > > that create fallback tunnels could be built-in and setting the sysctl > > > > value after booting is pointless, so added a kernel cmdline options to > > > > change this default. The default setting is preseved for backward > > > > compatibility. The kernel command line option of fb_tunnels=initns will > > > > set the sysctl value to 1 and will create fallback tunnels only in initns > > > > while kernel cmdline fb_tunnels=none will set the sysctl value to 2 and > > > > fallback tunnels are skipped in every netns. > > > > > > > > Signed-off-by: Mahesh Bandewar <maheshb@google.com> > > > > Cc: Eric Dumazet <edumazet@google.com> > > > > Cc: Maciej Zenczykowski <maze@google.com> > > > > Cc: Jian Yang <jianyang@google.com> > > > > --- > > > > v1->v2 > > > > Removed the Kconfig option which would force rebuild and replaced with > > > > kcmd-line option > > > > > > > > .../admin-guide/kernel-parameters.txt | 5 +++++ > > > > Documentation/admin-guide/sysctl/net.rst | 20 +++++++++++++------ > > > > include/linux/netdevice.h | 7 ++++++- > > > > net/core/sysctl_net_core.c | 17 ++++++++++++++-- > > > > 4 files changed, 40 insertions(+), 9 deletions(-) > > > > > > > > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt > > > > index a1068742a6df..09a51598c792 100644 > > > > --- a/Documentation/admin-guide/kernel-parameters.txt > > > > +++ b/Documentation/admin-guide/kernel-parameters.txt > > > > @@ -801,6 +801,11 @@ > > > > > > > > debug_objects [KNL] Enable object debugging > > > > > > > > + fb_tunnels= [NET] > > > > + Format: { initns | none } > > > > + See Documentation/admin-guide/sysctl/net.rst for > > > > + fb_tunnels_only_for_init_ns > > > > + > > > > > > Not at this location in this file. > > > Entries in this file are meant to be in alphabetical order (mostly). > > > > > > So leave debug_objects and no_debug_objects together, and insert fb_tunnels > > > between fail_make_request= and floppy=. > > > > > I see. I'll fix it in the next revision. > > thanks for the suggestion. > > --mahesh.. > > > > > Thanks. > > > > > > > no_debug_objects > > > > [KNL] Disable object debugging > > > > > > > > > > -- > > > ~Randy > > Setting it to 1 via kcmdline doesn't seem all that useful, since > instead of that you can just use initrc/sysctl.conf/etc. > > Would it be simpler if it was just 'no_fb_tunnels' or > 'no_fallback_tunnels' and the function just set the sysctl to 2 > unconditionally? > (ie. no =.... parsing at all) that would also be less code... > To make it simple; all methods should be able to set all values. Otherwise I agree that it makes less sense to set value = 1 via kcmd. Also one can assign value = 2 to sysctl once kernel is booted, it may not produce desired results always but would work if you load modules after changing the sysctl value. I guess the idea here is to give user full control on what their situation is and choose the correct method for the desired end result. > btw. I also don't understand the '!IS_ENABLED(CONFIG_SYSCTL) ||' > piece. Why not just delete that? > This seems to force fallback tunnels if you disable CONFIG_SYSCTL... > but (a) then the kcmdline option doesn't work, > and (b) that should already just happen by virtue of the sysctl defaulting to 0. agreed. will remove it (!IS_ENABLED(CONFIG_SYSCTL) check) in the next revision.
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index a1068742a6df..09a51598c792 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -801,6 +801,11 @@ debug_objects [KNL] Enable object debugging + fb_tunnels= [NET] + Format: { initns | none } + See Documentation/admin-guide/sysctl/net.rst for + fb_tunnels_only_for_init_ns + no_debug_objects [KNL] Disable object debugging diff --git a/Documentation/admin-guide/sysctl/net.rst b/Documentation/admin-guide/sysctl/net.rst index 42cd04bca548..57fd6ce68fe0 100644 --- a/Documentation/admin-guide/sysctl/net.rst +++ b/Documentation/admin-guide/sysctl/net.rst @@ -300,7 +300,6 @@ Note: 0: 0 1 2 3 4 5 6 7 RSS hash key: 84:50:f4:00:a8:15:d1:a7:e9:7f:1d:60:35:c7:47:25:42:97:74:ca:56:bb:b6:a1:d8:43:e3:c9:0c:fd:17:55:c2:3a:4d:69:ed:f1:42:89 - netdev_tstamp_prequeue ---------------------- @@ -321,11 +320,20 @@ fb_tunnels_only_for_init_net ---------------------------- Controls if fallback tunnels (like tunl0, gre0, gretap0, erspan0, -sit0, ip6tnl0, ip6gre0) are automatically created when a new -network namespace is created, if corresponding tunnel is present -in initial network namespace. -If set to 1, these devices are not automatically created, and -user space is responsible for creating them if needed. +sit0, ip6tnl0, ip6gre0) are automatically created. There are 3 possibilities +(a) value = 0; respective fallback tunnels are created when module is +loaded in every net namespaces (backward compatible behavior). +(b) value = 1; [kcmd value: initns] respective fallback tunnels are +created only in init net namespace and every other net namespace will +not have them. +(c) value = 2; [kcmd value: none] fallback tunnels are not created +when a module is loaded in any of the net namespace. Setting value to +"2" is pointless after boot if these modules are built-in, so there is +a kernel command-line option that can change this default. Please refer to +Documentation/admin-guide/kernel-parameters.txt for additional details. + +Not creating fallback tunnels gives control to userspace to create +whatever is needed only and avoid creating devices which are redundant. Default : 0 (for compatibility reasons) diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index b0e303f6603f..7efcdb9ee4ff 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -640,9 +640,14 @@ struct netdev_queue { extern int sysctl_fb_tunnels_only_for_init_net; extern int sysctl_devconf_inherit_init_net; +/* + * sysctl_fb_tunnels_only_for_init_net == 0 : For all netns + * == 1 : For initns only + * == 2 : For none. + */ static inline bool net_has_fallback_tunnels(const struct net *net) { - return net == &init_net || + return (net == &init_net && sysctl_fb_tunnels_only_for_init_net == 1) || !IS_ENABLED(CONFIG_SYSCTL) || !sysctl_fb_tunnels_only_for_init_net; } diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c index 6ada114bbcca..d86d8d11cfe4 100644 --- a/net/core/sysctl_net_core.c +++ b/net/core/sysctl_net_core.c @@ -22,7 +22,7 @@ #include <net/busy_poll.h> #include <net/pkt_sched.h> -static int two __maybe_unused = 2; +static int two = 2; static int three = 3; static int min_sndbuf = SOCK_MIN_SNDBUF; static int min_rcvbuf = SOCK_MIN_RCVBUF; @@ -546,7 +546,7 @@ static struct ctl_table net_core_table[] = { .mode = 0644, .proc_handler = proc_dointvec_minmax, .extra1 = SYSCTL_ZERO, - .extra2 = SYSCTL_ONE, + .extra2 = &two, }, { .procname = "devconf_inherit_init_net", @@ -587,6 +587,19 @@ static struct ctl_table netns_core_table[] = { { } }; +static int __init fb_tunnels_only_for_init_net_sysctl_setup(char *str) +{ + /* fallback tunnels for initns only */ + if (!strncmp(str, "initns", 6)) + sysctl_fb_tunnels_only_for_init_net = 1; + /* no fallback tunnels anywhere */ + else if (!strncmp(str, "none", 4)) + sysctl_fb_tunnels_only_for_init_net = 2; + + return 1; +} +__setup("fb_tunnels=", fb_tunnels_only_for_init_net_sysctl_setup); + static __net_init int sysctl_core_net_init(struct net *net) { struct ctl_table *tbl;
The sysctl that was added earlier by commit 79134e6ce2c ("net: do not create fallback tunnels for non-default namespaces") to create fall-back only in root-ns. This patch enhances that behavior to provide option not to create fallback tunnels in root-ns as well. Since modules that create fallback tunnels could be built-in and setting the sysctl value after booting is pointless, so added a kernel cmdline options to change this default. The default setting is preseved for backward compatibility. The kernel command line option of fb_tunnels=initns will set the sysctl value to 1 and will create fallback tunnels only in initns while kernel cmdline fb_tunnels=none will set the sysctl value to 2 and fallback tunnels are skipped in every netns. Signed-off-by: Mahesh Bandewar <maheshb@google.com> Cc: Eric Dumazet <edumazet@google.com> Cc: Maciej Zenczykowski <maze@google.com> Cc: Jian Yang <jianyang@google.com> --- v1->v2 Removed the Kconfig option which would force rebuild and replaced with kcmd-line option .../admin-guide/kernel-parameters.txt | 5 +++++ Documentation/admin-guide/sysctl/net.rst | 20 +++++++++++++------ include/linux/netdevice.h | 7 ++++++- net/core/sysctl_net_core.c | 17 ++++++++++++++-- 4 files changed, 40 insertions(+), 9 deletions(-)