All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHv2 next] net: add option to not create fall-back tunnels in root-ns as well
@ 2020-08-25 22:42 Mahesh Bandewar
  2020-08-25 22:47 ` Randy Dunlap
  0 siblings, 1 reply; 5+ messages in thread
From: Mahesh Bandewar @ 2020-08-25 22:42 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 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
+
 	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;
-- 
2.28.0.297.g1956fa8f8d-goog


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

* Re: [PATCHv2 next] net: add option to not create fall-back tunnels in root-ns as well
  2020-08-25 22:42 [PATCHv2 next] net: add option to not create fall-back tunnels in root-ns as well Mahesh Bandewar
@ 2020-08-25 22:47 ` Randy Dunlap
  2020-08-25 23:00   ` Mahesh Bandewar (महेश बंडेवार)
  0 siblings, 1 reply; 5+ messages in thread
From: Randy Dunlap @ 2020-08-25 22:47 UTC (permalink / raw)
  To: Mahesh Bandewar, Netdev
  Cc: David Miller, Jakub Kicinski, Eric Dumazet, Mahesh Bandewar,
	Maciej Zenczykowski, Jian Yang

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
>  

-- 
~Randy


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

* Re: [PATCHv2 next] net: add option to not create fall-back tunnels in root-ns as well
  2020-08-25 22:47 ` Randy Dunlap
@ 2020-08-25 23:00   ` Mahesh Bandewar (महेश बंडेवार)
  2020-08-26  0:41     ` Maciej Żenczykowski
  0 siblings, 1 reply; 5+ messages in thread
From: Mahesh Bandewar (महेश बंडेवार) @ 2020-08-25 23:00 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Netdev, David Miller, Jakub Kicinski, Eric Dumazet,
	Mahesh Bandewar, Maciej Zenczykowski, Jian Yang

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
>

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

* Re: [PATCHv2 next] net: add option to not create fall-back tunnels in root-ns as well
  2020-08-25 23:00   ` Mahesh Bandewar (महेश बंडेवार)
@ 2020-08-26  0:41     ` Maciej Żenczykowski
  2020-08-26  1:49       ` Mahesh Bandewar (महेश बंडेवार)
  0 siblings, 1 reply; 5+ messages in thread
From: Maciej Żenczykowski @ 2020-08-26  0:41 UTC (permalink / raw)
  To: Mahesh Bandewar (महेश
	बंडेवार)
  Cc: Randy Dunlap, Netdev, David Miller, Jakub Kicinski, Eric Dumazet,
	Mahesh Bandewar, Jian Yang

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.

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

* Re: [PATCHv2 next] net: add option to not create fall-back tunnels in root-ns as well
  2020-08-26  0:41     ` Maciej Żenczykowski
@ 2020-08-26  1:49       ` Mahesh Bandewar (महेश बंडेवार)
  0 siblings, 0 replies; 5+ messages in thread
From: Mahesh Bandewar (महेश बंडेवार) @ 2020-08-26  1:49 UTC (permalink / raw)
  To: Maciej Żenczykowski
  Cc: Randy Dunlap, Netdev, David Miller, Jakub Kicinski, Eric Dumazet,
	Mahesh Bandewar, Jian Yang

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.

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

end of thread, other threads:[~2020-08-26  1:49 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-25 22:42 [PATCHv2 next] net: add option to not create fall-back tunnels in root-ns as well Mahesh Bandewar
2020-08-25 22:47 ` Randy Dunlap
2020-08-25 23:00   ` Mahesh Bandewar (महेश बंडेवार)
2020-08-26  0:41     ` Maciej Żenczykowski
2020-08-26  1:49       ` Mahesh Bandewar (महेश बंडेवार)

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.