[PATCH net-next v2] net: add sysctl to disable rfc4862 5.5.3e lifetime handling

[PATCH net-next v2] net: add sysctl to disable rfc4862 5.5.3e lifetime handling

[Patrick Rohr]

This change adds a sysctl to opt-out of RFC4862 section 5.5.3e's valid
lifetime derivation mechanism.

RFC4862 section 5.5.3e prescribes that the valid lifetime in a Router
Advertisement PIO shall be ignored if it less than 2 hours and to reset
the lifetime of the corresponding address to 2 hours. An in-progress
6man draft (see draft-ietf-6man-slaac-renum-07 section 4.2) is currently
looking to remove this mechanism. While this draft has not been moving
particularly quickly for other reasons, there is widespread consensus on
section 4.2 which updates RFC4862 section 5.5.3e.

Cc: Maciej Żenczykowski <maze@google.com>
Cc: Lorenzo Colitti <lorenzo@google.com>
Cc: Jen Linkova <furry@google.com>
Signed-off-by: Patrick Rohr <prohr@google.com>
---
 Documentation/networking/ip-sysctl.rst | 11 ++++++++
 include/linux/ipv6.h                   |  1 +
 net/ipv6/addrconf.c                    | 38 +++++++++++++++++---------
 3 files changed, 37 insertions(+), 13 deletions(-)

diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst
index a66054d0763a..7f21877e3f78 100644
--- a/Documentation/networking/ip-sysctl.rst
+++ b/Documentation/networking/ip-sysctl.rst
@@ -2304,6 +2304,17 @@ accept_ra_pinfo - BOOLEAN
 		- enabled if accept_ra is enabled.
 		- disabled if accept_ra is disabled.
 
+ra_pinfo_rfc4862_5_5_3e - BOOLEAN
+	Use RFC4862 Section 5.5.3e to determine the valid lifetime of
+	an address matching a prefix sent in a Router Advertisement
+	Prefix Information Option.
+
+	- If enabled, RFC4862 section 5.5.3e is used to determine
+	  the valid lifetime of the address.
+	- If disabled, the PIO valid lifetime will always be honored.
+
+	Default: 1
+
 accept_ra_rt_info_min_plen - INTEGER
 	Minimum prefix length of Route Information in RA.
 
diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index 5883551b1ee8..f90cf8835ed4 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -35,6 +35,7 @@ struct ipv6_devconf {
 	__s32		accept_ra_min_hop_limit;
 	__s32		accept_ra_min_lft;
 	__s32		accept_ra_pinfo;
+	__s32		ra_pinfo_rfc4862_5_5_3e;
 	__s32		ignore_routes_with_linkdown;
 #ifdef CONFIG_IPV6_ROUTER_PREF
 	__s32		accept_ra_rtr_pref;
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 47d1dd8501b7..1ac23a37e8eb 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -204,6 +204,7 @@ static struct ipv6_devconf ipv6_devconf __read_mostly = {
 	.accept_ra_min_hop_limit= 1,
 	.accept_ra_min_lft	= 0,
 	.accept_ra_pinfo	= 1,
+	.ra_pinfo_rfc4862_5_5_3e = 1,
 #ifdef CONFIG_IPV6_ROUTER_PREF
 	.accept_ra_rtr_pref	= 1,
 	.rtr_probe_interval	= 60 * HZ,
@@ -265,6 +266,7 @@ static struct ipv6_devconf ipv6_devconf_dflt __read_mostly = {
 	.accept_ra_min_hop_limit= 1,
 	.accept_ra_min_lft	= 0,
 	.accept_ra_pinfo	= 1,
+	.ra_pinfo_rfc4862_5_5_3e = 1,
 #ifdef CONFIG_IPV6_ROUTER_PREF
 	.accept_ra_rtr_pref	= 1,
 	.rtr_probe_interval	= 60 * HZ,
@@ -2657,22 +2659,23 @@ int addrconf_prefix_rcv_add_addr(struct net *net, struct net_device *dev,
 			stored_lft = ifp->valid_lft - (now - ifp->tstamp) / HZ;
 		else
 			stored_lft = 0;
-		if (!create && stored_lft) {
+
+		/* RFC4862 Section 5.5.3e:
+		 * "Note that the preferred lifetime of the
+		 *  corresponding address is always reset to
+		 *  the Preferred Lifetime in the received
+		 *  Prefix Information option, regardless of
+		 *  whether the valid lifetime is also reset or
+		 *  ignored."
+		 *
+		 * So we should always update prefered_lft here.
+		 */
+		update_lft = !create && stored_lft;
+
+		if (update_lft && in6_dev->cnf.ra_pinfo_rfc4862_5_5_3e) {
 			const u32 minimum_lft = min_t(u32,
 				stored_lft, MIN_VALID_LIFETIME);
 			valid_lft = max(valid_lft, minimum_lft);
-
-			/* RFC4862 Section 5.5.3e:
-			 * "Note that the preferred lifetime of the
-			 *  corresponding address is always reset to
-			 *  the Preferred Lifetime in the received
-			 *  Prefix Information option, regardless of
-			 *  whether the valid lifetime is also reset or
-			 *  ignored."
-			 *
-			 * So we should always update prefered_lft here.
-			 */
-			update_lft = 1;
 		}
 
 		if (update_lft) {
@@ -6846,6 +6849,15 @@ static const struct ctl_table addrconf_sysctl[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec,
 	},
+	{
+		.procname	= "ra_pinfo_rfc4862_5_5_3e",
+		.data		= &ipv6_devconf.ra_pinfo_rfc4862_5_5_3e,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= SYSCTL_ZERO,
+		.extra2		= SYSCTL_ONE,
+	},
 #ifdef CONFIG_IPV6_ROUTER_PREF
 	{
 		.procname	= "accept_ra_rtr_pref",
-- 
2.42.0.283.g2d96d420d3-goog

Re: [PATCH net-next v2] net: add sysctl to disable rfc4862 5.5.3e lifetime handling

[Jiri Pirko]

Tue, Sep 12, 2023 at 03:44:25PM CEST, prohr@google.com wrote:
>This change adds a sysctl to opt-out of RFC4862 section 5.5.3e's valid
>lifetime derivation mechanism.
>
>RFC4862 section 5.5.3e prescribes that the valid lifetime in a Router
>Advertisement PIO shall be ignored if it less than 2 hours and to reset
>the lifetime of the corresponding address to 2 hours. An in-progress
>6man draft (see draft-ietf-6man-slaac-renum-07 section 4.2) is currently
>looking to remove this mechanism. While this draft has not been moving
>particularly quickly for other reasons, there is widespread consensus on
>section 4.2 which updates RFC4862 section 5.5.3e.
>
>Cc: Maciej Żenczykowski <maze@google.com>
>Cc: Lorenzo Colitti <lorenzo@google.com>
>Cc: Jen Linkova <furry@google.com>
>Signed-off-by: Patrick Rohr <prohr@google.com>
>---
> Documentation/networking/ip-sysctl.rst | 11 ++++++++
> include/linux/ipv6.h                   |  1 +
> net/ipv6/addrconf.c                    | 38 +++++++++++++++++---------
> 3 files changed, 37 insertions(+), 13 deletions(-)
>
>diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst
>index a66054d0763a..7f21877e3f78 100644
>--- a/Documentation/networking/ip-sysctl.rst
>+++ b/Documentation/networking/ip-sysctl.rst
>@@ -2304,6 +2304,17 @@ accept_ra_pinfo - BOOLEAN
> 		- enabled if accept_ra is enabled.
> 		- disabled if accept_ra is disabled.
> 
>+ra_pinfo_rfc4862_5_5_3e - BOOLEAN

This is very odd sysctl name.


>+	Use RFC4862 Section 5.5.3e to determine the valid lifetime of
>+	an address matching a prefix sent in a Router Advertisement
>+	Prefix Information Option.
>+
>+	- If enabled, RFC4862 section 5.5.3e is used to determine
>+	  the valid lifetime of the address.
>+	- If disabled, the PIO valid lifetime will always be honored.

Can't you reverse the logic and call it something like:
ra_honor_pio_lifetime
?

Re: [PATCH net-next v2] net: add sysctl to disable rfc4862 5.5.3e lifetime handling

[Lorenzo Colitti]

On Wed, Sep 13, 2023 at 5:12 PM Jiri Pirko <jiri@resnulli.us> wrote:
> >+      - If enabled, RFC4862 section 5.5.3e is used to determine
> >+        the valid lifetime of the address.
> >+      - If disabled, the PIO valid lifetime will always be honored.
>
> Can't you reverse the logic and call it something like:
> ra_honor_pio_lifetime

Maybe accept_ra_pinfo_low_lifetime ? Consistent with the existing
accept_ra_pinfo which controls whether PIOs are accepted.

Re: [PATCH net-next v2] net: add sysctl to disable rfc4862 5.5.3e lifetime handling

[Maciej Żenczykowski]

On Wed, Sep 13, 2023 at 1:50 AM Lorenzo Colitti <lorenzo@google.com> wrote:
> On Wed, Sep 13, 2023 at 5:12 PM Jiri Pirko <jiri@resnulli.us> wrote:
> > >+      - If enabled, RFC4862 section 5.5.3e is used to determine
> > >+        the valid lifetime of the address.
> > >+      - If disabled, the PIO valid lifetime will always be honored.
> >
> > Can't you reverse the logic and call it something like:
> > ra_honor_pio_lifetime
>
> Maybe accept_ra_pinfo_low_lifetime ? Consistent with the existing
> accept_ra_pinfo which controls whether PIOs are accepted.

accept_ra... is about whether to accept or drop/ignore an RA or
portion there-of.
We considered it and decided it was inappropriate here, as this new
sysctl doesn't change drop/ignore.

As such it should be named ra_...

ra_honor_pio_lifetime or ra_honor_pio_lft has the problem of seeming
to be a lifetime (ie. seconds) and not a boolean,
but does look much better...  (maybe using _lifetime instead of _lft
makes it sufficiently different from the existing _lft sysctls that it
being a boolean is ok?)

...or... perhaps we do actually make it an actual number of seconds,

ra_pio_min_valid_lft, and we default it to MIN_VALID_LIFETIME,
then I believe a value of 0 would get the desired behaviour...

Re: [PATCH net-next v2] net: add sysctl to disable rfc4862 5.5.3e lifetime handling

[Lorenzo Colitti]

On Thu, Sep 14, 2023 at 2:20 AM Maciej Żenczykowski <maze@google.com> wrote:
>
> > Maybe accept_ra_pinfo_low_lifetime ? Consistent with the existing
> > accept_ra_pinfo which controls whether PIOs are accepted.
>
> accept_ra... is about whether to accept or drop/ignore an RA or
> portion there-of.

Not only. For example, accept_ra_rt_info_min_plen and
accept_ra_rt_info_max_plen control which prefix lengths will be
accepted in RIOs.

If we want to make it an integer, then we could call it
accept_ra_pinfo_min_valid_lft.