diff mbox series

[net-next] net: ipv6/addrconf: make regen_advance independent of retrans time

Message ID 20240108155347.156525-1-alexhenrie24@gmail.com (mailing list archive)
State Deferred
Delegated to: Netdev Maintainers
Headers show
Series [net-next] net: ipv6/addrconf: make regen_advance independent of retrans time | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 2946 this patch: 2946
netdev/cc_maintainers success CCed 0 of 0 maintainers
netdev/build_clang success Errors and warnings before: 1243 this patch: 1243
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 3177 this patch: 3177
netdev/checkpatch warning CHECK: spaces preferred around that '*' (ctx:VxV) WARNING: line length of 91 exceeds 80 columns
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Alex Henrie Jan. 8, 2024, 3:53 p.m. UTC 
In RFC 4941, REGEN_ADVANCE is a constant value of 5 seconds, and the RFC
does not permit the creation of temporary addresses with lifetimes
shorter than that:

> When processing a Router Advertisement with a Prefix
> Information option carrying a global scope prefix for the purposes of
> address autoconfiguration (i.e., the A bit is set), the node MUST
> perform the following steps:

> 5.  A temporary address is created only if this calculated Preferred
>     Lifetime is greater than REGEN_ADVANCE time units.

Moreover, using a non-constant regen_advance has undesirable side
effects. If regen_advance swelled above temp_prefered_lft,
ipv6_create_tempaddr would error out without creating any new address.
On my machine and network, this error happened immediately with the
preferred lifetime set to 1 second, after a few minutes with the
preferred lifetime set to 4 seconds, and not at all with the preferred
lifetime set to 5 seconds. During my investigation, I found a Stack
Exchange post from another person who seems to have had the same
problem: They stopped getting new addresses if they lowered the
preferred lifetime below 3 seconds, and they didn't really know why.

Some users want to change their IPv6 address as frequently as possible
regardless of the RFC's arbitrary minimum lifetime. For the benefit of
those users, add a regen_advance sysctl parameter that can be set to
below or above 5 seconds.

Link: https://datatracker.ietf.org/doc/html/rfc4941#section-3.3
Link: https://serverfault.com/a/1031168/310447
Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 Documentation/networking/ip-sysctl.rst | 20 +++++++++++++-------
 include/linux/ipv6.h                   |  1 +
 include/net/addrconf.h                 |  5 +++--
 net/ipv6/addrconf.c                    | 17 +++++++++++------
 4 files changed, 28 insertions(+), 15 deletions(-)

Comments

David Ahern Jan. 8, 2024, 6:52 p.m. UTC | #1 
On 1/8/24 8:53 AM, Alex Henrie wrote:
> In RFC 4941, REGEN_ADVANCE is a constant value of 5 seconds, and the RFC
> does not permit the creation of temporary addresses with lifetimes
> shorter than that:
> 
>> When processing a Router Advertisement with a Prefix
>> Information option carrying a global scope prefix for the purposes of
>> address autoconfiguration (i.e., the A bit is set), the node MUST
>> perform the following steps:
> 
>> 5.  A temporary address is created only if this calculated Preferred
>>     Lifetime is greater than REGEN_ADVANCE time units.
> 
> Moreover, using a non-constant regen_advance has undesirable side
> effects. If regen_advance swelled above temp_prefered_lft,
> ipv6_create_tempaddr would error out without creating any new address.
> On my machine and network, this error happened immediately with the
> preferred lifetime set to 1 second, after a few minutes with the
> preferred lifetime set to 4 seconds, and not at all with the preferred
> lifetime set to 5 seconds. During my investigation, I found a Stack
> Exchange post from another person who seems to have had the same
> problem: They stopped getting new addresses if they lowered the
> preferred lifetime below 3 seconds, and they didn't really know why.
> 
> Some users want to change their IPv6 address as frequently as possible
> regardless of the RFC's arbitrary minimum lifetime. For the benefit of
> those users, add a regen_advance sysctl parameter that can be set to
> below or above 5 seconds.
> 
> Link: https://datatracker.ietf.org/doc/html/rfc4941#section-3.3
> Link: https://serverfault.com/a/1031168/310447
> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
> ---
>  Documentation/networking/ip-sysctl.rst | 20 +++++++++++++-------
>  include/linux/ipv6.h                   |  1 +
>  include/net/addrconf.h                 |  5 +++--
>  net/ipv6/addrconf.c                    | 17 +++++++++++------
>  4 files changed, 28 insertions(+), 15 deletions(-)
> 

net-next is closed; please repost in 2 weeks once it has re-opened.
diff mbox series

Patch

diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst
index 7afff42612e9..0f121eda2978 100644
--- a/Documentation/networking/ip-sysctl.rst
+++ b/Documentation/networking/ip-sysctl.rst
@@ -2502,18 +2502,17 @@  use_tempaddr - INTEGER
 		* -1 (for point-to-point devices and loopback devices)
 
 temp_valid_lft - INTEGER
-	valid lifetime (in seconds) for temporary addresses. If less than the
-	minimum required lifetime (typically 5 seconds), temporary addresses
-	will not be created.
+	valid lifetime (in seconds) for temporary addresses. If temp_valid_lft
+	is less than or equal to regen_advance, temporary addresses will not be
+	created.
 
 	Default: 172800 (2 days)
 
 temp_prefered_lft - INTEGER
 	Preferred lifetime (in seconds) for temporary addresses. If
-	temp_prefered_lft is less than the minimum required lifetime (typically
-	5 seconds), temporary addresses will not be created. If
-	temp_prefered_lft is greater than temp_valid_lft, the preferred lifetime
-	is temp_valid_lft.
+	temp_prefered_lft is less than or equal to regen_advance, temporary
+	addresses will not be created. If temp_prefered_lft is greater than
+	temp_valid_lft, the preferred lifetime is temp_valid_lft.
 
 	Default: 86400 (1 day)
 
@@ -2535,6 +2534,13 @@  max_desync_factor - INTEGER
 
 	Default: 600
 
+regen_advance - INTEGER
+
+	How far in advance (in seconds) to create a new temporary address before
+	the current one is deprecated.
+
+	Default: 5
+
 regen_max_retry - INTEGER
 	Number of attempts before give up attempting to generate
 	valid temporary addresses.
diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index 5e605e384aac..1ff10ef9abb6 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -27,6 +27,7 @@  struct ipv6_devconf {
 	__s32		use_tempaddr;
 	__s32		temp_valid_lft;
 	__s32		temp_prefered_lft;
+	__s32		regen_advance;
 	__s32		regen_max_retry;
 	__s32		max_desync_factor;
 	__s32		max_addresses;
diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index 61ebe723ee4d..b8f9d88959c7 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -8,8 +8,9 @@ 
 
 #define MIN_VALID_LIFETIME		(2*3600)	/* 2 hours */
 
-#define TEMP_VALID_LIFETIME		(7*86400)
-#define TEMP_PREFERRED_LIFETIME		(86400)
+#define TEMP_VALID_LIFETIME		(7*86400)	/* 1 week */
+#define TEMP_PREFERRED_LIFETIME		(86400)		/* 24 hours */
+#define REGEN_ADVANCE			(5)		/* 5 seconds */
 #define REGEN_MAX_RETRY			(3)
 #define MAX_DESYNC_FACTOR		(600)
 
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 733ace18806c..047ac97ae3c8 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -195,6 +195,7 @@  static struct ipv6_devconf ipv6_devconf __read_mostly = {
 	.use_tempaddr		= 0,
 	.temp_valid_lft		= TEMP_VALID_LIFETIME,
 	.temp_prefered_lft	= TEMP_PREFERRED_LIFETIME,
+	.regen_advance		= REGEN_ADVANCE,
 	.regen_max_retry	= REGEN_MAX_RETRY,
 	.max_desync_factor	= MAX_DESYNC_FACTOR,
 	.max_addresses		= IPV6_MAX_ADDRESSES,
@@ -257,6 +258,7 @@  static struct ipv6_devconf ipv6_devconf_dflt __read_mostly = {
 	.use_tempaddr		= 0,
 	.temp_valid_lft		= TEMP_VALID_LIFETIME,
 	.temp_prefered_lft	= TEMP_PREFERRED_LIFETIME,
+	.regen_advance		= REGEN_ADVANCE,
 	.regen_max_retry	= REGEN_MAX_RETRY,
 	.max_desync_factor	= MAX_DESYNC_FACTOR,
 	.max_addresses		= IPV6_MAX_ADDRESSES,
@@ -1372,9 +1374,7 @@  static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, bool block)
 
 	age = (now - ifp->tstamp) / HZ;
 
-	regen_advance = idev->cnf.regen_max_retry *
-			idev->cnf.dad_transmits *
-			max(NEIGH_VAR(idev->nd_parms, RETRANS_TIME), HZ/100) / HZ;
+	regen_advance = idev->cnf.regen_advance;
 
 	/* recalculate max_desync_factor each time and update
 	 * idev->desync_factor if it's larger
@@ -4577,9 +4577,7 @@  static void addrconf_verify_rtnl(struct net *net)
 			    !ifp->regen_count && ifp->ifpub) {
 				/* This is a non-regenerated temporary addr. */
 
-				unsigned long regen_advance = ifp->idev->cnf.regen_max_retry *
-					ifp->idev->cnf.dad_transmits *
-					max(NEIGH_VAR(ifp->idev->nd_parms, RETRANS_TIME), HZ/100) / HZ;
+				unsigned long regen_advance = ifp->idev->cnf.regen_advance;
 
 				if (age + regen_advance >= ifp->prefered_lft) {
 					struct inet6_ifaddr *ifpub = ifp->ifpub;
@@ -6789,6 +6787,13 @@  static const struct ctl_table addrconf_sysctl[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec,
 	},
+	{
+		.procname	= "regen_advance",
+		.data		= &ipv6_devconf.regen_advance,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec,
+	},
 	{
 		.procname	= "regen_max_retry",
 		.data		= &ipv6_devconf.regen_max_retry,