| Message ID | 20200819005123.1867051-1-maheshb@google.com |
|---|---|
| State | Rejected |
| Delegated to: | David Miller |
| Headers | show |
| Series | [next] net: add option to not create fall-back tunnels in root-ns as well | expand |
From: Mahesh Bandewar <maheshb@google.com> Date: Tue, 18 Aug 2020 17:51:23 -0700 > 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 config option which defaults > to zero (to preserve backward compatibility) but also takes values "1" and > "2" which don't create fallback tunnels in non-root namespaces > only and no-where respectively. > > Signed-off-by: Mahesh Bandewar <maheshb@google.com> ... > +config SYSCTL_FB_TUNNEL ... > -int sysctl_fb_tunnels_only_for_init_net __read_mostly = 0; > +int sysctl_fb_tunnels_only_for_init_net __read_mostly = CONFIG_SYSCTL_FB_TUNNEL; I can't allow this. This requires a kernel rebuild when none is really necessary. You're also forcing distributions to make a choice they have no place making at all. You have two ways to handle this situation already: 1) Kernel command line 2) initrd I'm not allowing to add a third. And if I had, then that sets precedence and others will want to do this as well for their favorite sysctl that has implications as soon as modules get loaded.
> > 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 config option which defaults > > to zero (to preserve backward compatibility) but also takes values "1" and > > "2" which don't create fallback tunnels in non-root namespaces > > only and no-where respectively. > > > > Signed-off-by: Mahesh Bandewar <maheshb@google.com> > ... > > +config SYSCTL_FB_TUNNEL > ... > > -int sysctl_fb_tunnels_only_for_init_net __read_mostly = 0; > > +int sysctl_fb_tunnels_only_for_init_net __read_mostly = CONFIG_SYSCTL_FB_TUNNEL; > > I can't allow this. This requires a kernel rebuild when none is > really necessary. You're also forcing distributions to make a choice > they have no place making at all. > > You have two ways to handle this situation already: > > 1) Kernel command line > > 2) initrd > > I'm not allowing to add a third. And if I had, then that sets > precedence and others will want to do this as well for their > favorite sysctl that has implications as soon as modules get > loaded. I don't think initrd works for things built into the kernel, since it runs too late - after kernel init is done. So only the kernel command line method is viable. If no kernel command line option is specified, should the default be to maintain compatibility, or do you think it's okay to make the default be no extra interfaces? They can AFAICT always be added manually via 'ip link add' netlink commands.
From: Maciej Żenczykowski <maze@google.com> Date: Fri, 21 Aug 2020 14:25:20 -0700 > If no kernel command line option is specified, should the default > be to maintain compatibility, or do you think it's okay to make > the default be no extra interfaces? They can AFAICT always be added > manually via 'ip link add' netlink commands. You can't change current default behavior, so the answer should be obvious right?
On Fri, Aug 21, 2020 at 2:35 PM David Miller <davem@davemloft.net> wrote: > > From: Maciej Żenczykowski <maze@google.com> > Date: Fri, 21 Aug 2020 14:25:20 -0700 > > > If no kernel command line option is specified, should the default > > be to maintain compatibility, or do you think it's okay to make > > the default be no extra interfaces? They can AFAICT always be added > > manually via 'ip link add' netlink commands. > > You can't change current default behavior, so the answer should be > obvious right? Yes, I don't want to change the default behavior that's why I kept the default value = 0, but yes this config-option would force one to rebuild the kernel. OK, I'll respin it with kcmdline option instead of config option maintaining the default behavior. thanks.
diff --git a/Documentation/admin-guide/sysctl/net.rst b/Documentation/admin-guide/sysctl/net.rst index 42cd04bca548..aa1f5727d291 100644 --- a/Documentation/admin-guide/sysctl/net.rst +++ b/Documentation/admin-guide/sysctl/net.rst @@ -321,13 +321,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. - -Default : 0 (for compatibility reasons) +sit0, ip6tnl0, ip6gre0) are automatically created. There are 3 +possibiltieis. +(a) value = 0; respective fallback tunnels are created when module is +loaded in every net namespaces (backward compatible behavior). +(b) value = 1; respective fallback tunnels are created only in root +net namespace and every other net namespace will not have them. +(c) value = 2; fallback tunnels are not created when a module is +loaded in any of the net namespace. + +Not creating fallback tunnels gives control to userspace to create +whatever is needed and avoid creating devices which are not used. + +Default: The value of this sysctl is set via config item SYSCTL_FB_TUNNEL +and is set to "0" by default. (for compatibility reasons) devconf_inherit_init_net ------------------------ diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index b0e303f6603f..327a302c8c26 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/Kconfig b/net/Kconfig index 3831206977a1..a57671e8a324 100644 --- a/net/Kconfig +++ b/net/Kconfig @@ -460,6 +460,17 @@ config ETHTOOL_NETLINK netlink. It provides better extensibility and some new features, e.g. notification messages. +config SYSCTL_FB_TUNNEL + int "Value for sysctl_fb_tunnels_only_for_init_net" + range 0 2 + default 0 + help + A sysctl value for sysctl_fb_tunnels_only_for_init_net. The value "0" + is for backward compatibility and creates fall-back tunnels in root-ns + as well as any newly created net namespaces. The value "1" restricts + this these fallback tunnels to only root-ns while value "2" does not + create these tunnels anywhere. + endif # if NET # Used by archs to tell that they support BPF JIT compiler plus which flavour. diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c index 6ada114bbcca..06b98cb2e21d 100644 --- a/net/core/sysctl_net_core.c +++ b/net/core/sysctl_net_core.c @@ -32,7 +32,7 @@ static long long_max __maybe_unused = LONG_MAX; static int net_msg_warn; /* Unused, but still a sysctl */ -int sysctl_fb_tunnels_only_for_init_net __read_mostly = 0; +int sysctl_fb_tunnels_only_for_init_net __read_mostly = CONFIG_SYSCTL_FB_TUNNEL; EXPORT_SYMBOL(sysctl_fb_tunnels_only_for_init_net); /* 0 - Keep current behavior: @@ -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",
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 config option which defaults to zero (to preserve backward compatibility) but also takes values "1" and "2" which don't create fallback tunnels in non-root namespaces only and no-where respectively. 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> --- Documentation/admin-guide/sysctl/net.rst | 21 ++++++++++++++------- include/linux/netdevice.h | 7 ++++++- net/Kconfig | 11 +++++++++++ net/core/sysctl_net_core.c | 4 ++-- 4 files changed, 33 insertions(+), 10 deletions(-)