netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 net-next] net: ipv6: Add sysctl entry to disable MTU updates from RA
@ 2015-01-19 21:36 Harout Hedeshian
  2015-01-19 22:10 ` Daniel Borkmann
  2015-01-19 23:01 ` Daniel Borkmann
  0 siblings, 2 replies; 8+ messages in thread
From: Harout Hedeshian @ 2015-01-19 21:36 UTC (permalink / raw)
  To: netdev; +Cc: Harout Hedeshian

The kernel forcefully applies MTU values received in router
advertisements provided the new MTU is less than the current. This
behavior is undesirable when the user space is managing the MTU. Instead
a sysctl flag 'accept_ra_mtu' is introduced such that the user space
can control whether or not RA provided MTU updates should be applied. The
default behavior is unchanged; user space must explicitly set this flag
to 0 for RA MTUs to be ignored.

Signed-off-by: Harout Hedeshian <harouth@codeaurora.org>
---
 Documentation/networking/ip-sysctl.txt | 7 +++++++
 include/linux/ipv6.h                   | 1 +
 include/uapi/linux/ipv6.h              | 1 +
 include/uapi/linux/sysctl.h            | 1 +
 kernel/sysctl_binary.c                 | 1 +
 net/ipv6/addrconf.c                    | 9 +++++++++
 net/ipv6/ndisc.c                       | 2 +-
 7 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index 85b0221..da50faa 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -1287,6 +1287,13 @@ accept_ra_rtr_pref - BOOLEAN
 	Functional default: enabled if accept_ra is enabled.
 			    disabled if accept_ra is disabled.
 
+accept_ra_mtu - BOOLEAN
+	Apply the MTU value specified in RA option 5 (RFC4861). If
+	disabled, the MTU specified in the RA will be ignored.
+
+	Functional default: enabled if accept_ra is enabled.
+                            disabled if accept_ra is disabled.
+
 accept_redirects - BOOLEAN
 	Accept Redirects.
 
diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index c694e7b..2805062 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -52,6 +52,7 @@ struct ipv6_devconf {
 	__s32		force_tllao;
 	__s32           ndisc_notify;
 	__s32		suppress_frag_ndisc;
+	__s32		accept_ra_mtu;
 	void		*sysctl;
 };
 
diff --git a/include/uapi/linux/ipv6.h b/include/uapi/linux/ipv6.h
index 73cb02d..437a6a4 100644
--- a/include/uapi/linux/ipv6.h
+++ b/include/uapi/linux/ipv6.h
@@ -169,6 +169,7 @@ enum {
 	DEVCONF_SUPPRESS_FRAG_NDISC,
 	DEVCONF_ACCEPT_RA_FROM_LOCAL,
 	DEVCONF_USE_OPTIMISTIC,
+	DEVCONF_ACCEPT_RA_MTU,
 	DEVCONF_MAX
 };
 
diff --git a/include/uapi/linux/sysctl.h b/include/uapi/linux/sysctl.h
index 0956373..45e4fcf 100644
--- a/include/uapi/linux/sysctl.h
+++ b/include/uapi/linux/sysctl.h
@@ -570,6 +570,7 @@ enum {
 	NET_IPV6_PROXY_NDP=23,
 	NET_IPV6_ACCEPT_SOURCE_ROUTE=25,
 	NET_IPV6_ACCEPT_RA_FROM_LOCAL=26,
+	NET_IPV6_ACCEPT_RA_MTU=27,
 	__NET_IPV6_MAX
 };
 
diff --git a/kernel/sysctl_binary.c b/kernel/sysctl_binary.c
index 7e7746a..16091e5 100644
--- a/kernel/sysctl_binary.c
+++ b/kernel/sysctl_binary.c
@@ -523,6 +523,7 @@ static const struct bin_table bin_net_ipv6_conf_var_table[] = {
 	{ CTL_INT,	NET_IPV6_PROXY_NDP,			"proxy_ndp" },
 	{ CTL_INT,	NET_IPV6_ACCEPT_SOURCE_ROUTE,		"accept_source_route" },
 	{ CTL_INT,	NET_IPV6_ACCEPT_RA_FROM_LOCAL,		"accept_ra_from_local" },
+	{ CTL_INT,	NET_IPV6_ACCEPT_RA_MTU,			"accept_ra_mtu" },
 	{}
 };
 
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index f7c8bbe..cdd70ed 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -201,6 +201,7 @@ static struct ipv6_devconf ipv6_devconf __read_mostly = {
 	.disable_ipv6		= 0,
 	.accept_dad		= 1,
 	.suppress_frag_ndisc	= 1,
+	.accept_ra_mtu		= 1,
 };
 
 static struct ipv6_devconf ipv6_devconf_dflt __read_mostly = {
@@ -238,6 +239,7 @@ static struct ipv6_devconf ipv6_devconf_dflt __read_mostly = {
 	.disable_ipv6		= 0,
 	.accept_dad		= 1,
 	.suppress_frag_ndisc	= 1,
+	.accept_ra_mtu		= 1,
 };
 
 /* Check if a valid qdisc is available */
@@ -5253,6 +5255,13 @@ static struct addrconf_sysctl_table
 			.proc_handler	= proc_dointvec,
 		},
 		{
+			.procname	= "accept_ra_mtu",
+			.data		= &ipv6_devconf.accept_ra_mtu,
+			.maxlen		= sizeof(int),
+			.mode		= 0644,
+			.proc_handler	= proc_dointvec,
+		},
+		{
 			/* sentinel */
 		}
 	},
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index 6828667..8a9d7c1 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -1348,7 +1348,7 @@ skip_routeinfo:
 		}
 	}
 
-	if (ndopts.nd_opts_mtu) {
+	if (ndopts.nd_opts_mtu && in6_dev->cnf.accept_ra_mtu) {
 		__be32 n;
 		u32 mtu;
 
-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

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

* Re: [PATCH v2 net-next] net: ipv6: Add sysctl entry to disable MTU updates from RA
  2015-01-19 21:36 [PATCH v2 net-next] net: ipv6: Add sysctl entry to disable MTU updates from RA Harout Hedeshian
@ 2015-01-19 22:10 ` Daniel Borkmann
  2015-01-19 22:42   ` Harout Hedeshian
  2015-01-19 23:01 ` Daniel Borkmann
  1 sibling, 1 reply; 8+ messages in thread
From: Daniel Borkmann @ 2015-01-19 22:10 UTC (permalink / raw)
  To: Harout Hedeshian; +Cc: netdev

On 01/19/2015 10:36 PM, Harout Hedeshian wrote:
...
> diff --git a/include/uapi/linux/sysctl.h b/include/uapi/linux/sysctl.h
> index 0956373..45e4fcf 100644
> --- a/include/uapi/linux/sysctl.h
> +++ b/include/uapi/linux/sysctl.h
> @@ -570,6 +570,7 @@ enum {
>   	NET_IPV6_PROXY_NDP=23,
>   	NET_IPV6_ACCEPT_SOURCE_ROUTE=25,
>   	NET_IPV6_ACCEPT_RA_FROM_LOCAL=26,
> +	NET_IPV6_ACCEPT_RA_MTU=27,
>   	__NET_IPV6_MAX
>   };
>
> diff --git a/kernel/sysctl_binary.c b/kernel/sysctl_binary.c
> index 7e7746a..16091e5 100644
> --- a/kernel/sysctl_binary.c
> +++ b/kernel/sysctl_binary.c
> @@ -523,6 +523,7 @@ static const struct bin_table bin_net_ipv6_conf_var_table[] = {
>   	{ CTL_INT,	NET_IPV6_PROXY_NDP,			"proxy_ndp" },
>   	{ CTL_INT,	NET_IPV6_ACCEPT_SOURCE_ROUTE,		"accept_source_route" },
>   	{ CTL_INT,	NET_IPV6_ACCEPT_RA_FROM_LOCAL,		"accept_ra_from_local" },
> +	{ CTL_INT,	NET_IPV6_ACCEPT_RA_MTU,			"accept_ra_mtu" },
>   	{}
>   };

Hm, afaik, the binary sysctl interface is deprecated and nothing
should be adding entries there anymore. I believe you copied this
over from commit d9333196572 ("ipv6: Allow accepting RA from local
IP addresses.")? :/

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

* RE: [PATCH v2 net-next] net: ipv6: Add sysctl entry to disable MTU updates from RA
  2015-01-19 22:10 ` Daniel Borkmann
@ 2015-01-19 22:42   ` Harout Hedeshian
  2015-01-19 23:01     ` Daniel Borkmann
  2015-01-20  0:29     ` David Miller
  0 siblings, 2 replies; 8+ messages in thread
From: Harout Hedeshian @ 2015-01-19 22:42 UTC (permalink / raw)
  To: 'Daniel Borkmann'; +Cc: netdev



> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-
> owner@vger.kernel.org] On Behalf Of Daniel Borkmann
> Sent: Monday, January 19, 2015 3:11 PM
> To: Harout Hedeshian
> Cc: netdev@vger.kernel.org
> Subject: Re: [PATCH v2 net-next] net: ipv6: Add sysctl entry to disable
MTU
> updates from RA
> 
> On 01/19/2015 10:36 PM, Harout Hedeshian wrote:
> ...
> > diff --git a/include/uapi/linux/sysctl.h b/include/uapi/linux/sysctl.h
> > index 0956373..45e4fcf 100644
> > --- a/include/uapi/linux/sysctl.h
> > +++ b/include/uapi/linux/sysctl.h
> > @@ -570,6 +570,7 @@ enum {
> >   	NET_IPV6_PROXY_NDP=23,
> >   	NET_IPV6_ACCEPT_SOURCE_ROUTE=25,
> >   	NET_IPV6_ACCEPT_RA_FROM_LOCAL=26,
> > +	NET_IPV6_ACCEPT_RA_MTU=27,
> >   	__NET_IPV6_MAX
> >   };
> >
> > diff --git a/kernel/sysctl_binary.c b/kernel/sysctl_binary.c index
> > 7e7746a..16091e5 100644
> > --- a/kernel/sysctl_binary.c
> > +++ b/kernel/sysctl_binary.c
> > @@ -523,6 +523,7 @@ static const struct bin_table
> bin_net_ipv6_conf_var_table[] = {
> >   	{ CTL_INT,	NET_IPV6_PROXY_NDP,
> 	"proxy_ndp" },
> >   	{ CTL_INT,	NET_IPV6_ACCEPT_SOURCE_ROUTE,
> 	"accept_source_route" },
> >   	{ CTL_INT,	NET_IPV6_ACCEPT_RA_FROM_LOCAL,
> 	"accept_ra_from_local" },
> > +	{ CTL_INT,	NET_IPV6_ACCEPT_RA_MTU,
> 	"accept_ra_mtu" },
> >   	{}
> >   };
> 
> Hm, afaik, the binary sysctl interface is deprecated and nothing should be
> adding entries there anymore. I believe you copied this over from commit
> d9333196572 ("ipv6: Allow accepting RA from local IP addresses.")? :/

Hi Daniel,

Indeed, the patch is based on previous modifications to add sysctl/proc
entries for IPv6 parameters. It is done this way to maintain consistency. Do
you think we need a v3 without the changes to the sysctl_binary.c and
corresponding changes in includes/uapi?

d9333196572 ("ipv6: Allow accepting RA from local IP addresses.") is as
recent as June 2014.

Thanks,
Harout

--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux
Foundation Collaborative Project

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

* Re: [PATCH v2 net-next] net: ipv6: Add sysctl entry to disable MTU updates from RA
  2015-01-19 21:36 [PATCH v2 net-next] net: ipv6: Add sysctl entry to disable MTU updates from RA Harout Hedeshian
  2015-01-19 22:10 ` Daniel Borkmann
@ 2015-01-19 23:01 ` Daniel Borkmann
  2015-01-20 16:48   ` Harout Hedeshian
  1 sibling, 1 reply; 8+ messages in thread
From: Daniel Borkmann @ 2015-01-19 23:01 UTC (permalink / raw)
  To: Harout Hedeshian; +Cc: netdev

On 01/19/2015 10:36 PM, Harout Hedeshian wrote:
...
> diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
> index 85b0221..da50faa 100644
> --- a/Documentation/networking/ip-sysctl.txt
> +++ b/Documentation/networking/ip-sysctl.txt
> @@ -1287,6 +1287,13 @@ accept_ra_rtr_pref - BOOLEAN
>   	Functional default: enabled if accept_ra is enabled.
>   			    disabled if accept_ra is disabled.
>
> +accept_ra_mtu - BOOLEAN
> +	Apply the MTU value specified in RA option 5 (RFC4861). If
> +	disabled, the MTU specified in the RA will be ignored.
> +
> +	Functional default: enabled if accept_ra is enabled.
> +                            disabled if accept_ra is disabled.

Nit: please indent correctly with tab here.

> +
...
> diff --git a/include/uapi/linux/ipv6.h b/include/uapi/linux/ipv6.h
> index 73cb02d..437a6a4 100644
> --- a/include/uapi/linux/ipv6.h
> +++ b/include/uapi/linux/ipv6.h
> @@ -169,6 +169,7 @@ enum {
>   	DEVCONF_SUPPRESS_FRAG_NDISC,
>   	DEVCONF_ACCEPT_RA_FROM_LOCAL,
>   	DEVCONF_USE_OPTIMISTIC,
> +	DEVCONF_ACCEPT_RA_MTU,
>   	DEVCONF_MAX
>   };

You also need a corresponding ipv6_store_devconf() entry, otherwise
netlink dumps will always see this setting as disabled.

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

* Re: [PATCH v2 net-next] net: ipv6: Add sysctl entry to disable MTU updates from RA
  2015-01-19 22:42   ` Harout Hedeshian
@ 2015-01-19 23:01     ` Daniel Borkmann
  2015-01-20  0:29     ` David Miller
  1 sibling, 0 replies; 8+ messages in thread
From: Daniel Borkmann @ 2015-01-19 23:01 UTC (permalink / raw)
  To: Harout Hedeshian; +Cc: netdev

On 01/19/2015 11:42 PM, Harout Hedeshian wrote:
...
> Indeed, the patch is based on previous modifications to add sysctl/proc
> entries for IPv6 parameters. It is done this way to maintain consistency. Do
> you think we need a v3 without the changes to the sysctl_binary.c and
> corresponding changes in includes/uapi?

Yep, please.

> d9333196572 ("ipv6: Allow accepting RA from local IP addresses.") is as
> recent as June 2014.

Yeah, that looks wrong unfortunately.

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

* Re: [PATCH v2 net-next] net: ipv6: Add sysctl entry to disable MTU updates from RA
  2015-01-19 22:42   ` Harout Hedeshian
  2015-01-19 23:01     ` Daniel Borkmann
@ 2015-01-20  0:29     ` David Miller
  1 sibling, 0 replies; 8+ messages in thread
From: David Miller @ 2015-01-20  0:29 UTC (permalink / raw)
  To: harouth; +Cc: dborkman, netdev

From: "Harout Hedeshian" <harouth@codeaurora.org>
Date: Mon, 19 Jan 2015 15:42:08 -0700

> Indeed, the patch is based on previous modifications to add sysctl/proc
> entries for IPv6 parameters. It is done this way to maintain consistency. Do
> you think we need a v3 without the changes to the sysctl_binary.c and
> corresponding changes in includes/uapi?

Probably, yes.

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

* RE: [PATCH v2 net-next] net: ipv6: Add sysctl entry to disable MTU updates from RA
  2015-01-19 23:01 ` Daniel Borkmann
@ 2015-01-20 16:48   ` Harout Hedeshian
  2015-01-20 16:56     ` Daniel Borkmann
  0 siblings, 1 reply; 8+ messages in thread
From: Harout Hedeshian @ 2015-01-20 16:48 UTC (permalink / raw)
  To: 'Daniel Borkmann'; +Cc: netdev

> > diff --git a/include/uapi/linux/ipv6.h b/include/uapi/linux/ipv6.h
> > index 73cb02d..437a6a4 100644
> > --- a/include/uapi/linux/ipv6.h
> > +++ b/include/uapi/linux/ipv6.h
> > @@ -169,6 +169,7 @@ enum {
> >   	DEVCONF_SUPPRESS_FRAG_NDISC,
> >   	DEVCONF_ACCEPT_RA_FROM_LOCAL,
> >   	DEVCONF_USE_OPTIMISTIC,
> > +	DEVCONF_ACCEPT_RA_MTU,
> >   	DEVCONF_MAX
> >   };
> 
> You also need a corresponding ipv6_store_devconf() entry, otherwise
> netlink dumps will always see this setting as disabled.

I added the following:
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index cdd70ed..5743293 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -4378,6 +4378,7 @@ static inline void ipv6_store_devconf(struct
ipv6_devconf *cnf,
        array[DEVCONF_NDISC_NOTIFY] = cnf->ndisc_notify;
        array[DEVCONF_SUPPRESS_FRAG_NDISC] = cnf->suppress_frag_ndisc;
        array[DEVCONF_ACCEPT_RA_FROM_LOCAL] = cnf->accept_ra_from_local;
+       array[DEVCONF_ACCEPT_RA_MTU] = cnf->accept_ra_mtu;
 }

I will upload v3 with the following changes:
 - Fix tab indentation in documentation
 - Remove changes from sysctl_binary.c and sysctl.h
 - Fixed netlink dumps

-- Sanity test results below --

As a quick test, I wrote a very simple utility to dump out the raw contents
of a RTM_NEWLINK event and tested it out on a UML instance by assigning some
easily identifiable (but meaningless) value:

root@debian-wheezy-template:/mnt/host# ./a.out &
root@debian-wheezy-template:/mnt/host# cd /proc/sys/net/ipv6/conf/eth0
root@debian-wheezy-template:/proc/sys/net/ipv6/conf/eth0# echo 90 >
accept_ra_mtu #should print the letter 'Z'
root@debian-wheezy-template:/proc/sys/net/ipv6/conf/eth0# ifconfig eth0 up
&& ifconfig eth0 down
Read 1152 bytes from the socket
           0  1  2  3  4  5  6  7  8  9  A  B  C  D  E  F
...<truncated>...
00000020| 09 00 03 00 65 74 68 30 00 00 00 00 08 00 0D 00 | ....eth0........
...<truncated>...
00000300| 00 00 00 00 00 00 00 00 5A 00 00 00 24 01 03 00 | ........Z...$...
root@debian-wheezy-template:/proc/sys/net/ipv6/conf/eth0# echo 89 >
accept_ra_mtu # should print the letter 'Y'
root@debian-wheezy-template:/proc/sys/net/ipv6/conf/eth0# ifconfig eth0 up
&& ifconfig eth0 down
Read 1152 bytes from the socket
           0  1  2  3  4  5  6  7  8  9  A  B  C  D  E  F
...<truncated>...
00000020| 09 00 03 00 65 74 68 30 00 00 00 00 08 00 0D 00 | ....eth0........
...<truncated>...
00000300| 00 00 00 00 00 00 00 00 59 00 00 00 24 01 03 00 | ........Y...$...


Thanks,
Harout

--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux
Foundation Collaborative Project

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

* Re: [PATCH v2 net-next] net: ipv6: Add sysctl entry to disable MTU updates from RA
  2015-01-20 16:48   ` Harout Hedeshian
@ 2015-01-20 16:56     ` Daniel Borkmann
  0 siblings, 0 replies; 8+ messages in thread
From: Daniel Borkmann @ 2015-01-20 16:56 UTC (permalink / raw)
  To: Harout Hedeshian; +Cc: netdev

On 01/20/2015 05:48 PM, Harout Hedeshian wrote:
...
> I will upload v3 with the following changes:
>   - Fix tab indentation in documentation
>   - Remove changes from sysctl_binary.c and sysctl.h
>   - Fixed netlink dumps

Sounds good, thanks!

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

end of thread, other threads:[~2015-01-20 16:56 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-19 21:36 [PATCH v2 net-next] net: ipv6: Add sysctl entry to disable MTU updates from RA Harout Hedeshian
2015-01-19 22:10 ` Daniel Borkmann
2015-01-19 22:42   ` Harout Hedeshian
2015-01-19 23:01     ` Daniel Borkmann
2015-01-20  0:29     ` David Miller
2015-01-19 23:01 ` Daniel Borkmann
2015-01-20 16:48   ` Harout Hedeshian
2015-01-20 16:56     ` Daniel Borkmann

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