Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH next] net: add option to not create fall-back tunnels in root-ns as well
@ 2020-08-19  0:51 Mahesh Bandewar
  2020-08-21 21:03 ` David Miller
  0 siblings, 1 reply; 5+ messages in thread
From: Mahesh Bandewar @ 2020-08-19  0:51 UTC (permalink / raw)
  To: Netdev
  Cc: David Miller, Jakub Kicinski, Eric Dumazet, Mahesh Bandewar,
	Mahesh Bandewar, Maciej Zenczykowski, Jian Yang

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(-)

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",
-- 
2.28.0.220.ged08abb693-goog


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH next] net: add option to not create fall-back tunnels in root-ns as well
  2020-08-19  0:51 [PATCH next] net: add option to not create fall-back tunnels in root-ns as well Mahesh Bandewar
@ 2020-08-21 21:03 ` David Miller
  2020-08-21 21:25   ` Maciej Żenczykowski
  0 siblings, 1 reply; 5+ messages in thread
From: David Miller @ 2020-08-21 21:03 UTC (permalink / raw)
  To: maheshb; +Cc: netdev, kuba, edumazet, mahesh, maze, jianyang

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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH next] net: add option to not create fall-back tunnels in root-ns as well
  2020-08-21 21:03 ` David Miller
@ 2020-08-21 21:25   ` Maciej Żenczykowski
  2020-08-21 21:35     ` David Miller
  0 siblings, 1 reply; 5+ messages in thread
From: Maciej Żenczykowski @ 2020-08-21 21:25 UTC (permalink / raw)
  To: David Miller
  Cc: Mahesh Bandewar, Linux NetDev, Jakub Kicinski, Eric Dumazet,
	mahesh, jianyang

> > 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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH next] net: add option to not create fall-back tunnels in root-ns as well
  2020-08-21 21:25   ` Maciej Żenczykowski
@ 2020-08-21 21:35     ` David Miller
  2020-08-22  4:18       ` Mahesh Bandewar (महेश बंडेवार)
  0 siblings, 1 reply; 5+ messages in thread
From: David Miller @ 2020-08-21 21:35 UTC (permalink / raw)
  To: maze; +Cc: maheshb, netdev, kuba, edumazet, mahesh, jianyang

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?

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH next] net: add option to not create fall-back tunnels in root-ns as well
  2020-08-21 21:35     ` David Miller
@ 2020-08-22  4:18       ` Mahesh Bandewar (महेश बंडेवार)
  0 siblings, 0 replies; 5+ messages in thread
From: Mahesh Bandewar (महेश बंडेवार) @ 2020-08-22  4:18 UTC (permalink / raw)
  To: David Miller
  Cc: Maciej Żenczykowski, linux-netdev, kuba, Eric Dumazet,
	mahesh, Jian Yang

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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-08-22  4:19 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-19  0:51 [PATCH next] net: add option to not create fall-back tunnels in root-ns as well Mahesh Bandewar
2020-08-21 21:03 ` David Miller
2020-08-21 21:25   ` Maciej Żenczykowski
2020-08-21 21:35     ` David Miller
2020-08-22  4:18       ` Mahesh Bandewar (महेश बंडेवार)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).