| Message ID | 20240125035710.32118-1-alexhenrie24@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net: ipv6/addrconf: make regen_advance independent of retrans time | expand |
On Wed, 2024-01-24 at 20:57 -0700, 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. RFC 4941 has been obsoleted by RFC 8981, which in turns makes REGEN_ADVANCE non constant: 3.8. Defined Protocol Parameters and Configuration Variables REGEN_ADVANCE 2 + (TEMP_IDGEN_RETRIES * DupAddrDetectTransmits * RetransTimer / 1000) > 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. I guess we can't accommodate every user desire while speaking the same protocol. Perhaps emitting a kernel message when user settings do not allow the address regeneration could be a better option? Cheers, Paolo
On Tue, Jan 30, 2024 at 3:46 AM Paolo Abeni <pabeni@redhat.com> wrote: > > On Wed, 2024-01-24 at 20:57 -0700, 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. > > RFC 4941 has been obsoleted by RFC 8981, which in turns makes > REGEN_ADVANCE non constant: > > 3.8. Defined Protocol Parameters and Configuration Variables > > REGEN_ADVANCE > 2 + (TEMP_IDGEN_RETRIES * DupAddrDetectTransmits * RetransTimer / > 1000) Ah, so that's where Linux's regen_advance formula came from! Thank you very much for pointing me to the updated RFC. However, according to the formula defined in RFC 8981, even though REGEN_ADVANCE is not a constant, it still must not be less than 2 seconds. So unfortunately, it still seems that Linux's current implementation is technically in violation of the spec because it doesn't add the 2. > > 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. > > I guess we can't accommodate every user desire while speaking the same > protocol. Linux already allows the user to reduce regen_advance to 0 by disabling duplicate address detection (setting /proc/sys/net/ipv6/conf/*/dad_transmits to 0). Disabling DAD might be a protocol violation, and setting regen_advance to 0 probably is too, but I didn't want to make it impossible because I can see people having good reasons to do both of those things. The bug I'm trying to fix is a different scenario: It happens when the user wants to rotate their IPv6 address as frequently as the protocol allows, but not so frequently as to break things like duplicate address detection. > Perhaps emitting a kernel message when user settings do not allow the > address regeneration could be a better option? The fundamental problem with emitting a warning is that when the network parameters are set, the kernel might not know that there's going to be a problem. The network could work fine for hours or days and then have a period of merely a few seconds when regen_advance swells above prefered_lft, at which point the kernel just gives up on temporary addresses. The kernel could print a warning then, but even if the user is knowledgeable enough to look at dmesg and understand the problem, unless they disable DAD to make regen_advance zero or set prefered_lft to an excessively large value, there's no way for them to pick a value for prefered_lft that is guaranteed to always be greater than regen_advance. The fact that regen_advance does not have to be a constant and there is no hard minimum means that my original patch that was in 6.7-rc1 [1] and reverted in 6.7-rc8 [2] was essentially correct, it just needs to be fixed to respect the maximums that are checked earlier in the function.[3] But if we want to add a 2-second minimum as well, I think I need to send three new patches: 1. Move the calculation of regen_advance to a helper function and add 2 to the calculated value 2. Add a regen_advance sysctl that corresponds to the number 2 in the formula, to allow changing it back to 0 or to any other value 3. Clamp preferred_lft to the minimum required as originally intended Thanks very much for the feedback and please let me know if you have any more thoughts before I get cracking. -Alex [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=629df6701c8a9172f4274af6de9dfa99e2c7ac56 [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8cdafdd94654ba418648d039c48e7a90508c1982 [3] https://lore.kernel.org/netdev/20231222234237.44823-2-alexhenrie24@gmail.com/
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,