* [PATCH net-next 1/3] net: ipv6/addrconf: ensure that regen_advance is at least 2 seconds
@ 2024-02-09 6:10 Alex Henrie
2024-02-09 6:10 ` [PATCH net-next 2/3] net: ipv6/addrconf: introduce a regen_min_advance sysctl Alex Henrie
` (3 more replies)
0 siblings, 4 replies; 16+ messages in thread
From: Alex Henrie @ 2024-02-09 6:10 UTC (permalink / raw)
To: netdev, dan, bagasdotme, davem, dsahern, edumazet, kuba, pabeni,
jikos
Cc: Alex Henrie
RFC 8981 defines REGEN_ADVANCE as follows:
REGEN_ADVANCE = 2 + (TEMP_IDGEN_RETRIES * DupAddrDetectTransmits * RetransTimer / 1000)
Thus, allowing it to be less than 2 seconds is technically a protocol
violation.
Link: https://datatracker.ietf.org/doc/html/rfc8981#name-defined-protocol-parameters
Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
net/ipv6/addrconf.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index d63f5d063f07..99a3ab6ec9d2 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1331,6 +1331,13 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
in6_ifa_put(ifp);
}
+static unsigned long ipv6_get_regen_advance(struct inet6_dev *idev)
+{
+ return 2 + idev->cnf.regen_max_retry *
+ idev->cnf.dad_transmits *
+ max(NEIGH_VAR(idev->nd_parms, RETRANS_TIME), HZ/100) / HZ;
+}
+
static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, bool block)
{
struct inet6_dev *idev = ifp->idev;
@@ -1372,9 +1379,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 = ipv6_get_regen_advance(idev);
/* recalculate max_desync_factor each time and update
* idev->desync_factor if it's larger
@@ -4577,9 +4582,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 = ipv6_get_regen_advance(ifp->idev);
if (age + regen_advance >= ifp->prefered_lft) {
struct inet6_ifaddr *ifpub = ifp->ifpub;
--
2.43.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH net-next 2/3] net: ipv6/addrconf: introduce a regen_min_advance sysctl 2024-02-09 6:10 [PATCH net-next 1/3] net: ipv6/addrconf: ensure that regen_advance is at least 2 seconds Alex Henrie @ 2024-02-09 6:10 ` Alex Henrie 2024-02-13 15:36 ` David Ahern 2024-02-09 6:10 ` [PATCH net-next 3/3] net: ipv6/addrconf: clamp preferred_lft to the minimum required Alex Henrie ` (2 subsequent siblings) 3 siblings, 1 reply; 16+ messages in thread From: Alex Henrie @ 2024-02-09 6:10 UTC (permalink / raw) To: netdev, dan, bagasdotme, davem, dsahern, edumazet, kuba, pabeni, jikos Cc: Alex Henrie In RFC 8981, REGEN_ADVANCE cannot be less than 2 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 prefix for the purposes of > address autoconfiguration (i.e., the A bit is set), the host MUST > perform the following steps: > 5. A temporary address is created only if this calculated preferred > lifetime is greater than REGEN_ADVANCE time units. However, 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_min_advance sysctl parameter that can be set to below or above 2 seconds. Link: https://datatracker.ietf.org/doc/html/rfc8981 Signed-off-by: Alex Henrie <alexhenrie24@gmail.com> --- Documentation/networking/ip-sysctl.rst | 8 ++++++++ include/linux/ipv6.h | 1 + include/net/addrconf.h | 5 +++-- net/ipv6/addrconf.c | 11 ++++++++++- 4 files changed, 22 insertions(+), 3 deletions(-) diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst index 7afff42612e9..fcd6aa71b4fa 100644 --- a/Documentation/networking/ip-sysctl.rst +++ b/Documentation/networking/ip-sysctl.rst @@ -2535,6 +2535,14 @@ max_desync_factor - INTEGER Default: 600 +regen_min_advance - INTEGER + How far in advance (in seconds), at minimum, to create a new temporary + address before the current one is deprecated. This value is added to + the amount of time that may be required for duplicate address detection + to detemine when to create a new address. + + Default: 2 + 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..ef3aa060a289 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_min_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..30d6f1e84e46 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_MIN_ADVANCE (2) /* 2 seconds */ #define REGEN_MAX_RETRY (3) #define MAX_DESYNC_FACTOR (600) diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c index 99a3ab6ec9d2..0b78ffc101ef 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_min_advance = REGEN_MIN_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_min_advance = REGEN_MIN_ADVANCE, .regen_max_retry = REGEN_MAX_RETRY, .max_desync_factor = MAX_DESYNC_FACTOR, .max_addresses = IPV6_MAX_ADDRESSES, @@ -1333,7 +1335,7 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp) static unsigned long ipv6_get_regen_advance(struct inet6_dev *idev) { - return 2 + idev->cnf.regen_max_retry * + return idev->cnf.regen_min_advance + idev->cnf.regen_max_retry * idev->cnf.dad_transmits * max(NEIGH_VAR(idev->nd_parms, RETRANS_TIME), HZ/100) / HZ; } @@ -6792,6 +6794,13 @@ static const struct ctl_table addrconf_sysctl[] = { .mode = 0644, .proc_handler = proc_dointvec, }, + { + .procname = "regen_min_advance", + .data = &ipv6_devconf.regen_min_advance, + .maxlen = sizeof(int), + .mode = 0644, + .proc_handler = proc_dointvec, + }, { .procname = "regen_max_retry", .data = &ipv6_devconf.regen_max_retry, -- 2.43.0 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH net-next 2/3] net: ipv6/addrconf: introduce a regen_min_advance sysctl 2024-02-09 6:10 ` [PATCH net-next 2/3] net: ipv6/addrconf: introduce a regen_min_advance sysctl Alex Henrie @ 2024-02-13 15:36 ` David Ahern 0 siblings, 0 replies; 16+ messages in thread From: David Ahern @ 2024-02-13 15:36 UTC (permalink / raw) To: Alex Henrie, netdev, dan, bagasdotme, davem, edumazet, kuba, pabeni, jikos On 2/8/24 11:10 PM, Alex Henrie wrote: > diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst > index 7afff42612e9..fcd6aa71b4fa 100644 > --- a/Documentation/networking/ip-sysctl.rst > +++ b/Documentation/networking/ip-sysctl.rst > @@ -2535,6 +2535,14 @@ max_desync_factor - INTEGER > > Default: 600 > > +regen_min_advance - INTEGER > + How far in advance (in seconds), at minimum, to create a new temporary > + address before the current one is deprecated. This value is added to > + the amount of time that may be required for duplicate address detection > + to detemine when to create a new address. s/detemine/determine/ Add some more comments here - e.g., RFC 8981 recommends a minimum of no less than 2 seconds which is the default. > + > + Default: 2 > + > regen_max_retry - INTEGER > Number of attempts before give up attempting to generate > valid temporary addresses. ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH net-next 3/3] net: ipv6/addrconf: clamp preferred_lft to the minimum required 2024-02-09 6:10 [PATCH net-next 1/3] net: ipv6/addrconf: ensure that regen_advance is at least 2 seconds Alex Henrie 2024-02-09 6:10 ` [PATCH net-next 2/3] net: ipv6/addrconf: introduce a regen_min_advance sysctl Alex Henrie @ 2024-02-09 6:10 ` Alex Henrie 2024-02-13 10:13 ` Paolo Abeni 2024-02-13 15:35 ` [PATCH net-next 1/3] net: ipv6/addrconf: ensure that regen_advance is at least 2 seconds David Ahern 2024-02-14 6:26 ` [PATCH net-next v2 0/3] net: ipv6/addrconf: ensure that temporary addresses' preferred lifetimes are long enough Alex Henrie 3 siblings, 1 reply; 16+ messages in thread From: Alex Henrie @ 2024-02-09 6:10 UTC (permalink / raw) To: netdev, dan, bagasdotme, davem, dsahern, edumazet, kuba, pabeni, jikos Cc: Alex Henrie If the preferred lifetime was less than the minimum required lifetime, 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 5 seconds or less, after a few minutes with the preferred lifetime set to 6 seconds, and not at all with the preferred lifetime set to 7 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. The preferred lifetime is a preference, not a hard requirement. The kernel does not strictly forbid new connections on a deprecated address, nor does it guarantee that the address will be disposed of the instant its total valid lifetime expires. So rather than disable IPv6 privacy extensions altogether if the minimum required lifetime swells above the preferred lifetime, it is more in keeping with the user's intent to increase the temporary address's lifetime to the minimum necessary for the current network conditions. With these fixes, setting the preferred lifetime to 5 or 6 seconds "just works" because the extra fraction of a second is practically unnoticeable. It's even possible to reduce the time before deprecation to 1 or 2 seconds by setting /proc/sys/net/ipv6/conf/*/regen_min_advance and /proc/sys/net/ipv6/conf/*/dad_transmits to 0. I realize that that is a pretty niche use case, but I know at least one person who would gladly sacrifice performance and convenience to be sure that they are getting the maximum possible level of privacy. Link: https://serverfault.com/a/1031168/310447 Signed-off-by: Alex Henrie <alexhenrie24@gmail.com> --- net/ipv6/addrconf.c | 43 ++++++++++++++++++++++++++++++++++--------- 1 file changed, 34 insertions(+), 9 deletions(-) diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c index 0b78ffc101ef..8d3023e54822 100644 --- a/net/ipv6/addrconf.c +++ b/net/ipv6/addrconf.c @@ -1347,6 +1347,7 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, bool block) unsigned long regen_advance; unsigned long now = jiffies; s32 cnf_temp_preferred_lft; + u32 if_public_preferred_lft; struct inet6_ifaddr *ift; struct ifa6_config cfg; long max_desync_factor; @@ -1401,11 +1402,13 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, bool block) } } + if_public_preferred_lft = ifp->prefered_lft; + memset(&cfg, 0, sizeof(cfg)); cfg.valid_lft = min_t(__u32, ifp->valid_lft, idev->cnf.temp_valid_lft + age); cfg.preferred_lft = cnf_temp_preferred_lft + age - idev->desync_factor; - cfg.preferred_lft = min_t(__u32, ifp->prefered_lft, cfg.preferred_lft); + cfg.preferred_lft = min_t(__u32, if_public_preferred_lft, cfg.preferred_lft); cfg.preferred_lft = min_t(__u32, cfg.valid_lft, cfg.preferred_lft); cfg.plen = ifp->prefix_len; @@ -1414,19 +1417,41 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, bool block) write_unlock_bh(&idev->lock); - /* A temporary address is created only if this calculated Preferred - * Lifetime is greater than REGEN_ADVANCE time units. In particular, - * an implementation must not create a temporary address with a zero - * Preferred Lifetime. + /* From RFC 4941: + * + * A temporary address is created only if this calculated Preferred + * Lifetime is greater than REGEN_ADVANCE time units. In + * particular, an implementation must not create a temporary address + * with a zero Preferred Lifetime. + * + * ... + * + * When creating a temporary address, the lifetime values MUST be + * derived from the corresponding prefix as follows: + * + * ... + * + * * Its Preferred Lifetime is the lower of the Preferred Lifetime + * of the public address or TEMP_PREFERRED_LIFETIME - + * DESYNC_FACTOR. + * + * To comply with the RFC's requirements, clamp the preferred lifetime + * to a minimum of regen_advance, unless that would exceed valid_lft or + * ifp->prefered_lft. + * * Use age calculation as in addrconf_verify to avoid unnecessary * temporary addresses being generated. */ age = (now - tmp_tstamp + ADDRCONF_TIMER_FUZZ_MINUS) / HZ; if (cfg.preferred_lft <= regen_advance + age) { - in6_ifa_put(ifp); - in6_dev_put(idev); - ret = -1; - goto out; + cfg.preferred_lft = regen_advance + age + 1; + if (cfg.preferred_lft > cfg.valid_lft || + cfg.preferred_lft > if_public_preferred_lft) { + in6_ifa_put(ifp); + in6_dev_put(idev); + ret = -1; + goto out; + } } cfg.ifa_flags = IFA_F_TEMPORARY; -- 2.43.0 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH net-next 3/3] net: ipv6/addrconf: clamp preferred_lft to the minimum required 2024-02-09 6:10 ` [PATCH net-next 3/3] net: ipv6/addrconf: clamp preferred_lft to the minimum required Alex Henrie @ 2024-02-13 10:13 ` Paolo Abeni 2024-02-13 15:40 ` David Ahern 0 siblings, 1 reply; 16+ messages in thread From: Paolo Abeni @ 2024-02-13 10:13 UTC (permalink / raw) To: dsahern; +Cc: edumazet, kuba, jikos, Alex Henrie, netdev, dan, bagasdotme, davem On Thu, 2024-02-08 at 23:10 -0700, Alex Henrie wrote: > If the preferred lifetime was less than the minimum required lifetime, > 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 5 seconds or less, after a few minutes with > the preferred lifetime set to 6 seconds, and not at all with the > preferred lifetime set to 7 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. > > The preferred lifetime is a preference, not a hard requirement. The > kernel does not strictly forbid new connections on a deprecated address, > nor does it guarantee that the address will be disposed of the instant > its total valid lifetime expires. So rather than disable IPv6 privacy > extensions altogether if the minimum required lifetime swells above the > preferred lifetime, it is more in keeping with the user's intent to > increase the temporary address's lifetime to the minimum necessary for > the current network conditions. > > With these fixes, setting the preferred lifetime to 5 or 6 seconds "just > works" because the extra fraction of a second is practically > unnoticeable. It's even possible to reduce the time before deprecation > to 1 or 2 seconds by setting /proc/sys/net/ipv6/conf/*/regen_min_advance > and /proc/sys/net/ipv6/conf/*/dad_transmits to 0. I realize that that is > a pretty niche use case, but I know at least one person who would gladly > sacrifice performance and convenience to be sure that they are getting > the maximum possible level of privacy. > > Link: https://serverfault.com/a/1031168/310447 > Signed-off-by: Alex Henrie <alexhenrie24@gmail.com> > --- > net/ipv6/addrconf.c | 43 ++++++++++++++++++++++++++++++++++--------- > 1 file changed, 34 insertions(+), 9 deletions(-) > > diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c > index 0b78ffc101ef..8d3023e54822 100644 > --- a/net/ipv6/addrconf.c > +++ b/net/ipv6/addrconf.c > @@ -1347,6 +1347,7 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, bool block) > unsigned long regen_advance; > unsigned long now = jiffies; > s32 cnf_temp_preferred_lft; > + u32 if_public_preferred_lft; [only if a repost is needed for some other reason] please respect the reverse x-mas tree above. > struct inet6_ifaddr *ift; > struct ifa6_config cfg; > long max_desync_factor; > @@ -1401,11 +1402,13 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, bool block) > } > } > > + if_public_preferred_lft = ifp->prefered_lft; > + > memset(&cfg, 0, sizeof(cfg)); > cfg.valid_lft = min_t(__u32, ifp->valid_lft, > idev->cnf.temp_valid_lft + age); > cfg.preferred_lft = cnf_temp_preferred_lft + age - idev->desync_factor; > - cfg.preferred_lft = min_t(__u32, ifp->prefered_lft, cfg.preferred_lft); > + cfg.preferred_lft = min_t(__u32, if_public_preferred_lft, cfg.preferred_lft); > cfg.preferred_lft = min_t(__u32, cfg.valid_lft, cfg.preferred_lft); > > cfg.plen = ifp->prefix_len; > @@ -1414,19 +1417,41 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, bool block) > > write_unlock_bh(&idev->lock); > > - /* A temporary address is created only if this calculated Preferred > - * Lifetime is greater than REGEN_ADVANCE time units. In particular, > - * an implementation must not create a temporary address with a zero > - * Preferred Lifetime. > + /* From RFC 4941: > + * > + * A temporary address is created only if this calculated Preferred > + * Lifetime is greater than REGEN_ADVANCE time units. In > + * particular, an implementation must not create a temporary address > + * with a zero Preferred Lifetime. > + * > + * ... > + * > + * When creating a temporary address, the lifetime values MUST be > + * derived from the corresponding prefix as follows: > + * > + * ... > + * > + * * Its Preferred Lifetime is the lower of the Preferred Lifetime > + * of the public address or TEMP_PREFERRED_LIFETIME - > + * DESYNC_FACTOR. > + * > + * To comply with the RFC's requirements, clamp the preferred lifetime > + * to a minimum of regen_advance, unless that would exceed valid_lft or > + * ifp->prefered_lft. > + * > * Use age calculation as in addrconf_verify to avoid unnecessary > * temporary addresses being generated. > */ > age = (now - tmp_tstamp + ADDRCONF_TIMER_FUZZ_MINUS) / HZ; > if (cfg.preferred_lft <= regen_advance + age) { > - in6_ifa_put(ifp); > - in6_dev_put(idev); > - ret = -1; > - goto out; > + cfg.preferred_lft = regen_advance + age + 1; > + if (cfg.preferred_lft > cfg.valid_lft || > + cfg.preferred_lft > if_public_preferred_lft) { > + in6_ifa_put(ifp); > + in6_dev_put(idev); > + ret = -1; > + goto out; > + } > } > > cfg.ifa_flags = IFA_F_TEMPORARY; The above sounds reasonable to me, but I would appreciate a couple of additional eyeballs on it. @David, could you please have a look? Thanks, Paolo ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next 3/3] net: ipv6/addrconf: clamp preferred_lft to the minimum required 2024-02-13 10:13 ` Paolo Abeni @ 2024-02-13 15:40 ` David Ahern 2024-02-13 21:07 ` Dan Moulding 0 siblings, 1 reply; 16+ messages in thread From: David Ahern @ 2024-02-13 15:40 UTC (permalink / raw) To: Paolo Abeni Cc: edumazet, kuba, jikos, Alex Henrie, netdev, dan, bagasdotme, davem On 2/13/24 3:13 AM, Paolo Abeni wrote: >> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c >> index 0b78ffc101ef..8d3023e54822 100644 >> --- a/net/ipv6/addrconf.c >> +++ b/net/ipv6/addrconf.c >> @@ -1347,6 +1347,7 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, bool block) >> unsigned long regen_advance; >> unsigned long now = jiffies; >> s32 cnf_temp_preferred_lft; >> + u32 if_public_preferred_lft; > > [only if a repost is needed for some other reason] please respect the > reverse x-mas tree above. > >> struct inet6_ifaddr *ift; >> struct ifa6_config cfg; >> long max_desync_factor; >> @@ -1401,11 +1402,13 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, bool block) >> } >> } >> >> + if_public_preferred_lft = ifp->prefered_lft; >> + >> memset(&cfg, 0, sizeof(cfg)); >> cfg.valid_lft = min_t(__u32, ifp->valid_lft, >> idev->cnf.temp_valid_lft + age); >> cfg.preferred_lft = cnf_temp_preferred_lft + age - idev->desync_factor; >> - cfg.preferred_lft = min_t(__u32, ifp->prefered_lft, cfg.preferred_lft); >> + cfg.preferred_lft = min_t(__u32, if_public_preferred_lft, cfg.preferred_lft); >> cfg.preferred_lft = min_t(__u32, cfg.valid_lft, cfg.preferred_lft); >> >> cfg.plen = ifp->prefix_len; >> @@ -1414,19 +1417,41 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, bool block) >> >> write_unlock_bh(&idev->lock); >> >> - /* A temporary address is created only if this calculated Preferred >> - * Lifetime is greater than REGEN_ADVANCE time units. In particular, >> - * an implementation must not create a temporary address with a zero >> - * Preferred Lifetime. >> + /* From RFC 4941: >> + * >> + * A temporary address is created only if this calculated Preferred >> + * Lifetime is greater than REGEN_ADVANCE time units. In >> + * particular, an implementation must not create a temporary address >> + * with a zero Preferred Lifetime. >> + * >> + * ... >> + * >> + * When creating a temporary address, the lifetime values MUST be >> + * derived from the corresponding prefix as follows: >> + * >> + * ... >> + * >> + * * Its Preferred Lifetime is the lower of the Preferred Lifetime >> + * of the public address or TEMP_PREFERRED_LIFETIME - >> + * DESYNC_FACTOR. >> + * >> + * To comply with the RFC's requirements, clamp the preferred lifetime >> + * to a minimum of regen_advance, unless that would exceed valid_lft or >> + * ifp->prefered_lft. >> + * >> * Use age calculation as in addrconf_verify to avoid unnecessary >> * temporary addresses being generated. >> */ >> age = (now - tmp_tstamp + ADDRCONF_TIMER_FUZZ_MINUS) / HZ; >> if (cfg.preferred_lft <= regen_advance + age) { >> - in6_ifa_put(ifp); >> - in6_dev_put(idev); >> - ret = -1; >> - goto out; >> + cfg.preferred_lft = regen_advance + age + 1; >> + if (cfg.preferred_lft > cfg.valid_lft || >> + cfg.preferred_lft > if_public_preferred_lft) { >> + in6_ifa_put(ifp); >> + in6_dev_put(idev); >> + ret = -1; >> + goto out; >> + } >> } >> >> cfg.ifa_flags = IFA_F_TEMPORARY; > > The above sounds reasonable to me, but I would appreciate a couple of > additional eyeballs on it. @David, could you please have a look? > I went through the set this past weekend along with the earlier thread with the problem Dan mentioned. I think the logic is ok. Dan: did you get a chance to test this set? Reviewed-by: David Ahern <dsahern@kernel.org> ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next 3/3] net: ipv6/addrconf: clamp preferred_lft to the minimum required 2024-02-13 15:40 ` David Ahern @ 2024-02-13 21:07 ` Dan Moulding 0 siblings, 0 replies; 16+ messages in thread From: Dan Moulding @ 2024-02-13 21:07 UTC (permalink / raw) To: dsahern Cc: alexhenrie24, bagasdotme, dan, davem, edumazet, jikos, kuba, netdev, pabeni > I went through the set this past weekend along with the earlier thread > with the problem Dan mentioned. I think the logic is ok. Dan: did you > get a chance to test this set? I just applied it on top of v6.7.4 and I do not see any issues. It handled the router advertisements and prefix deprecation just fine, AFAICT (it generated just one temporary address as I would have expected, instead of thousands of temporary addresses with very short lifetimes). Cheers, -- Dan ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next 1/3] net: ipv6/addrconf: ensure that regen_advance is at least 2 seconds 2024-02-09 6:10 [PATCH net-next 1/3] net: ipv6/addrconf: ensure that regen_advance is at least 2 seconds Alex Henrie 2024-02-09 6:10 ` [PATCH net-next 2/3] net: ipv6/addrconf: introduce a regen_min_advance sysctl Alex Henrie 2024-02-09 6:10 ` [PATCH net-next 3/3] net: ipv6/addrconf: clamp preferred_lft to the minimum required Alex Henrie @ 2024-02-13 15:35 ` David Ahern 2024-02-14 6:26 ` [PATCH net-next v2 0/3] net: ipv6/addrconf: ensure that temporary addresses' preferred lifetimes are long enough Alex Henrie 3 siblings, 0 replies; 16+ messages in thread From: David Ahern @ 2024-02-13 15:35 UTC (permalink / raw) To: Alex Henrie, netdev, dan, bagasdotme, davem, edumazet, kuba, pabeni, jikos On 2/8/24 11:10 PM, Alex Henrie wrote: > RFC 8981 defines REGEN_ADVANCE as follows: > > REGEN_ADVANCE = 2 + (TEMP_IDGEN_RETRIES * DupAddrDetectTransmits * RetransTimer / 1000) > > Thus, allowing it to be less than 2 seconds is technically a protocol > violation. > > Link: https://datatracker.ietf.org/doc/html/rfc8981#name-defined-protocol-parameters > Signed-off-by: Alex Henrie <alexhenrie24@gmail.com> > --- > net/ipv6/addrconf.c | 15 +++++++++------ > 1 file changed, 9 insertions(+), 6 deletions(-) > Reviewed-by: David Ahern <dsahern@kernel.org> ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH net-next v2 0/3] net: ipv6/addrconf: ensure that temporary addresses' preferred lifetimes are long enough 2024-02-09 6:10 [PATCH net-next 1/3] net: ipv6/addrconf: ensure that regen_advance is at least 2 seconds Alex Henrie ` (2 preceding siblings ...) 2024-02-13 15:35 ` [PATCH net-next 1/3] net: ipv6/addrconf: ensure that regen_advance is at least 2 seconds David Ahern @ 2024-02-14 6:26 ` Alex Henrie 2024-02-14 6:26 ` [PATCH net-next v2 1/3] net: ipv6/addrconf: ensure that regen_advance is at least 2 seconds Alex Henrie ` (3 more replies) 3 siblings, 4 replies; 16+ messages in thread From: Alex Henrie @ 2024-02-14 6:26 UTC (permalink / raw) To: netdev, dan, bagasdotme, davem, dsahern, edumazet, kuba, pabeni, jikos Cc: Alex Henrie v2 corrects and updates the documentation for these features. Changes from v1: - Update the typical minimum lifetime stated in the documentation, and make it a range to emphasize the variability - Fix spelling of "determine" in the documentation - Mention RFC 8981's requirements in the documentation - Arrange variables in "reverse Christmas tree" - Update documentation of what happens if temp_prefered_lft is less than the minimum required lifetime Thanks to David, Paolo, and Dan for your feedback. Alex Henrie (3): net: ipv6/addrconf: ensure that regen_advance is at least 2 seconds net: ipv6/addrconf: introduce a regen_min_advance sysctl net: ipv6/addrconf: clamp preferred_lft to the minimum required Documentation/networking/ip-sysctl.rst | 14 +++++- include/linux/ipv6.h | 1 + include/net/addrconf.h | 5 +- net/ipv6/addrconf.c | 67 ++++++++++++++++++++------ 4 files changed, 68 insertions(+), 19 deletions(-) Range-diff against v1: 1: 95ff3ac2f7a9 ! 1: 6978ee9a6d9e net: ipv6/addrconf: ensure that regen_advance is at least 2 seconds @@ Commit message Link: https://datatracker.ietf.org/doc/html/rfc8981#name-defined-protocol-parameters Signed-off-by: Alex Henrie <alexhenrie24@gmail.com> + ## Documentation/networking/ip-sysctl.rst ## +@@ Documentation/networking/ip-sysctl.rst: use_tempaddr - INTEGER + + temp_valid_lft - INTEGER + valid lifetime (in seconds) for temporary addresses. If less than the +- minimum required lifetime (typically 5 seconds), temporary addresses ++ minimum required lifetime (typically 5-7 seconds), temporary addresses + will not be created. + + Default: 172800 (2 days) +@@ Documentation/networking/ip-sysctl.rst: temp_valid_lft - INTEGER + 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 ++ 5-7 seconds), temporary addresses will not be created. If + temp_prefered_lft is greater than temp_valid_lft, the preferred lifetime + is temp_valid_lft. + + ## net/ipv6/addrconf.c ## @@ net/ipv6/addrconf.c: static void ipv6_del_addr(struct inet6_ifaddr *ifp) in6_ifa_put(ifp); 2: c7f773887259 ! 2: e2b3623db770 net: ipv6/addrconf: introduce a regen_min_advance sysctl @@ Documentation/networking/ip-sysctl.rst: max_desync_factor - INTEGER + How far in advance (in seconds), at minimum, to create a new temporary + address before the current one is deprecated. This value is added to + the amount of time that may be required for duplicate address detection -+ to detemine when to create a new address. ++ to determine when to create a new address. Linux permits setting this ++ value to less than the default of 2 seconds, but a value less than 2 ++ does not conform to RFC 8981. + + Default: 2 + 3: b4e3dc5b3479 ! 3: 422f2a0a209e net: ipv6/addrconf: clamp preferred_lft to the minimum required @@ Commit message Link: https://serverfault.com/a/1031168/310447 Signed-off-by: Alex Henrie <alexhenrie24@gmail.com> + ## Documentation/networking/ip-sysctl.rst ## +@@ Documentation/networking/ip-sysctl.rst: temp_valid_lft - INTEGER + temp_prefered_lft - INTEGER + Preferred lifetime (in seconds) for temporary addresses. If + temp_prefered_lft is less than the minimum required lifetime (typically +- 5-7 seconds), temporary addresses will not be created. If ++ 5-7 seconds), the preferred lifetime is the minimum required. If + temp_prefered_lft is greater than temp_valid_lft, the preferred lifetime + is temp_valid_lft. + + ## net/ipv6/addrconf.c ## @@ net/ipv6/addrconf.c: static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, bool block) + unsigned long tmp_tstamp, age; unsigned long regen_advance; unsigned long now = jiffies; - s32 cnf_temp_preferred_lft; + u32 if_public_preferred_lft; + s32 cnf_temp_preferred_lft; struct inet6_ifaddr *ift; struct ifa6_config cfg; - long max_desync_factor; @@ net/ipv6/addrconf.c: static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, bool block) } } -- 2.43.1 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH net-next v2 1/3] net: ipv6/addrconf: ensure that regen_advance is at least 2 seconds 2024-02-14 6:26 ` [PATCH net-next v2 0/3] net: ipv6/addrconf: ensure that temporary addresses' preferred lifetimes are long enough Alex Henrie @ 2024-02-14 6:26 ` Alex Henrie 2024-02-14 15:38 ` David Ahern 2024-02-14 6:26 ` [PATCH net-next v2 2/3] net: ipv6/addrconf: introduce a regen_min_advance sysctl Alex Henrie ` (2 subsequent siblings) 3 siblings, 1 reply; 16+ messages in thread From: Alex Henrie @ 2024-02-14 6:26 UTC (permalink / raw) To: netdev, dan, bagasdotme, davem, dsahern, edumazet, kuba, pabeni, jikos Cc: Alex Henrie RFC 8981 defines REGEN_ADVANCE as follows: REGEN_ADVANCE = 2 + (TEMP_IDGEN_RETRIES * DupAddrDetectTransmits * RetransTimer / 1000) Thus, allowing it to be less than 2 seconds is technically a protocol violation. Link: https://datatracker.ietf.org/doc/html/rfc8981#name-defined-protocol-parameters Signed-off-by: Alex Henrie <alexhenrie24@gmail.com> --- Documentation/networking/ip-sysctl.rst | 4 ++-- net/ipv6/addrconf.c | 15 +++++++++------ 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst index 7afff42612e9..458305931345 100644 --- a/Documentation/networking/ip-sysctl.rst +++ b/Documentation/networking/ip-sysctl.rst @@ -2503,7 +2503,7 @@ use_tempaddr - INTEGER temp_valid_lft - INTEGER valid lifetime (in seconds) for temporary addresses. If less than the - minimum required lifetime (typically 5 seconds), temporary addresses + minimum required lifetime (typically 5-7 seconds), temporary addresses will not be created. Default: 172800 (2 days) @@ -2511,7 +2511,7 @@ temp_valid_lft - INTEGER 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 + 5-7 seconds), temporary addresses will not be created. If temp_prefered_lft is greater than temp_valid_lft, the preferred lifetime is temp_valid_lft. diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c index ca1b719323c0..68516493404a 100644 --- a/net/ipv6/addrconf.c +++ b/net/ipv6/addrconf.c @@ -1339,6 +1339,13 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp) in6_ifa_put(ifp); } +static unsigned long ipv6_get_regen_advance(struct inet6_dev *idev) +{ + return 2 + idev->cnf.regen_max_retry * + idev->cnf.dad_transmits * + max(NEIGH_VAR(idev->nd_parms, RETRANS_TIME), HZ/100) / HZ; +} + static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, bool block) { struct inet6_dev *idev = ifp->idev; @@ -1380,9 +1387,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 = ipv6_get_regen_advance(idev); /* recalculate max_desync_factor each time and update * idev->desync_factor if it's larger @@ -4595,9 +4600,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 = ipv6_get_regen_advance(ifp->idev); if (age + regen_advance >= ifp->prefered_lft) { struct inet6_ifaddr *ifpub = ifp->ifpub; -- 2.43.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH net-next v2 1/3] net: ipv6/addrconf: ensure that regen_advance is at least 2 seconds 2024-02-14 6:26 ` [PATCH net-next v2 1/3] net: ipv6/addrconf: ensure that regen_advance is at least 2 seconds Alex Henrie @ 2024-02-14 15:38 ` David Ahern 0 siblings, 0 replies; 16+ messages in thread From: David Ahern @ 2024-02-14 15:38 UTC (permalink / raw) To: Alex Henrie, netdev, dan, bagasdotme, davem, edumazet, kuba, pabeni, jikos On 2/13/24 11:26 PM, Alex Henrie wrote: > RFC 8981 defines REGEN_ADVANCE as follows: > > REGEN_ADVANCE = 2 + (TEMP_IDGEN_RETRIES * DupAddrDetectTransmits * RetransTimer / 1000) > > Thus, allowing it to be less than 2 seconds is technically a protocol > violation. > > Link: https://datatracker.ietf.org/doc/html/rfc8981#name-defined-protocol-parameters > Signed-off-by: Alex Henrie <alexhenrie24@gmail.com> > --- > Documentation/networking/ip-sysctl.rst | 4 ++-- > net/ipv6/addrconf.c | 15 +++++++++------ > 2 files changed, 11 insertions(+), 8 deletions(-) > Alex: if no changes are made to a specific patch, then you should keep any reviewed by tags on the next version. Reviewed-by: David Ahern <dsahern@kernel.org> ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH net-next v2 2/3] net: ipv6/addrconf: introduce a regen_min_advance sysctl 2024-02-14 6:26 ` [PATCH net-next v2 0/3] net: ipv6/addrconf: ensure that temporary addresses' preferred lifetimes are long enough Alex Henrie 2024-02-14 6:26 ` [PATCH net-next v2 1/3] net: ipv6/addrconf: ensure that regen_advance is at least 2 seconds Alex Henrie @ 2024-02-14 6:26 ` Alex Henrie 2024-02-14 15:39 ` David Ahern 2024-02-14 6:26 ` [PATCH net-next v2 3/3] net: ipv6/addrconf: clamp preferred_lft to the minimum required Alex Henrie 2024-02-15 14:50 ` [PATCH net-next v2 0/3] net: ipv6/addrconf: ensure that temporary addresses' preferred lifetimes are long enough patchwork-bot+netdevbpf 3 siblings, 1 reply; 16+ messages in thread From: Alex Henrie @ 2024-02-14 6:26 UTC (permalink / raw) To: netdev, dan, bagasdotme, davem, dsahern, edumazet, kuba, pabeni, jikos Cc: Alex Henrie In RFC 8981, REGEN_ADVANCE cannot be less than 2 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 prefix for the purposes of > address autoconfiguration (i.e., the A bit is set), the host MUST > perform the following steps: > 5. A temporary address is created only if this calculated preferred > lifetime is greater than REGEN_ADVANCE time units. However, 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_min_advance sysctl parameter that can be set to below or above 2 seconds. Link: https://datatracker.ietf.org/doc/html/rfc8981 Signed-off-by: Alex Henrie <alexhenrie24@gmail.com> --- Documentation/networking/ip-sysctl.rst | 10 ++++++++++ include/linux/ipv6.h | 1 + include/net/addrconf.h | 5 +++-- net/ipv6/addrconf.c | 11 ++++++++++- 4 files changed, 24 insertions(+), 3 deletions(-) diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst index 458305931345..407d917d1a36 100644 --- a/Documentation/networking/ip-sysctl.rst +++ b/Documentation/networking/ip-sysctl.rst @@ -2535,6 +2535,16 @@ max_desync_factor - INTEGER Default: 600 +regen_min_advance - INTEGER + How far in advance (in seconds), at minimum, to create a new temporary + address before the current one is deprecated. This value is added to + the amount of time that may be required for duplicate address detection + to determine when to create a new address. Linux permits setting this + value to less than the default of 2 seconds, but a value less than 2 + does not conform to RFC 8981. + + Default: 2 + 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..ef3aa060a289 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_min_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..30d6f1e84e46 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_MIN_ADVANCE (2) /* 2 seconds */ #define REGEN_MAX_RETRY (3) #define MAX_DESYNC_FACTOR (600) diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c index 68516493404a..9af56b73d08c 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_min_advance = REGEN_MIN_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_min_advance = REGEN_MIN_ADVANCE, .regen_max_retry = REGEN_MAX_RETRY, .max_desync_factor = MAX_DESYNC_FACTOR, .max_addresses = IPV6_MAX_ADDRESSES, @@ -1341,7 +1343,7 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp) static unsigned long ipv6_get_regen_advance(struct inet6_dev *idev) { - return 2 + idev->cnf.regen_max_retry * + return idev->cnf.regen_min_advance + idev->cnf.regen_max_retry * idev->cnf.dad_transmits * max(NEIGH_VAR(idev->nd_parms, RETRANS_TIME), HZ/100) / HZ; } @@ -6819,6 +6821,13 @@ static const struct ctl_table addrconf_sysctl[] = { .mode = 0644, .proc_handler = proc_dointvec, }, + { + .procname = "regen_min_advance", + .data = &ipv6_devconf.regen_min_advance, + .maxlen = sizeof(int), + .mode = 0644, + .proc_handler = proc_dointvec, + }, { .procname = "regen_max_retry", .data = &ipv6_devconf.regen_max_retry, -- 2.43.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH net-next v2 2/3] net: ipv6/addrconf: introduce a regen_min_advance sysctl 2024-02-14 6:26 ` [PATCH net-next v2 2/3] net: ipv6/addrconf: introduce a regen_min_advance sysctl Alex Henrie @ 2024-02-14 15:39 ` David Ahern 0 siblings, 0 replies; 16+ messages in thread From: David Ahern @ 2024-02-14 15:39 UTC (permalink / raw) To: Alex Henrie, netdev, dan, bagasdotme, davem, edumazet, kuba, pabeni, jikos On 2/13/24 11:26 PM, Alex Henrie wrote: > In RFC 8981, REGEN_ADVANCE cannot be less than 2 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 prefix for the purposes of >> address autoconfiguration (i.e., the A bit is set), the host MUST >> perform the following steps: > >> 5. A temporary address is created only if this calculated preferred >> lifetime is greater than REGEN_ADVANCE time units. > > However, 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_min_advance sysctl parameter that > can be set to below or above 2 seconds. > > Link: https://datatracker.ietf.org/doc/html/rfc8981 > Signed-off-by: Alex Henrie <alexhenrie24@gmail.com> > --- > Documentation/networking/ip-sysctl.rst | 10 ++++++++++ > include/linux/ipv6.h | 1 + > include/net/addrconf.h | 5 +++-- > net/ipv6/addrconf.c | 11 ++++++++++- > 4 files changed, 24 insertions(+), 3 deletions(-) > Reviewed-by: David Ahern <dsahern@kernel.org> ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH net-next v2 3/3] net: ipv6/addrconf: clamp preferred_lft to the minimum required 2024-02-14 6:26 ` [PATCH net-next v2 0/3] net: ipv6/addrconf: ensure that temporary addresses' preferred lifetimes are long enough Alex Henrie 2024-02-14 6:26 ` [PATCH net-next v2 1/3] net: ipv6/addrconf: ensure that regen_advance is at least 2 seconds Alex Henrie 2024-02-14 6:26 ` [PATCH net-next v2 2/3] net: ipv6/addrconf: introduce a regen_min_advance sysctl Alex Henrie @ 2024-02-14 6:26 ` Alex Henrie 2024-02-14 15:40 ` David Ahern 2024-02-15 14:50 ` [PATCH net-next v2 0/3] net: ipv6/addrconf: ensure that temporary addresses' preferred lifetimes are long enough patchwork-bot+netdevbpf 3 siblings, 1 reply; 16+ messages in thread From: Alex Henrie @ 2024-02-14 6:26 UTC (permalink / raw) To: netdev, dan, bagasdotme, davem, dsahern, edumazet, kuba, pabeni, jikos Cc: Alex Henrie If the preferred lifetime was less than the minimum required lifetime, 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 5 seconds or less, after a few minutes with the preferred lifetime set to 6 seconds, and not at all with the preferred lifetime set to 7 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. The preferred lifetime is a preference, not a hard requirement. The kernel does not strictly forbid new connections on a deprecated address, nor does it guarantee that the address will be disposed of the instant its total valid lifetime expires. So rather than disable IPv6 privacy extensions altogether if the minimum required lifetime swells above the preferred lifetime, it is more in keeping with the user's intent to increase the temporary address's lifetime to the minimum necessary for the current network conditions. With these fixes, setting the preferred lifetime to 5 or 6 seconds "just works" because the extra fraction of a second is practically unnoticeable. It's even possible to reduce the time before deprecation to 1 or 2 seconds by setting /proc/sys/net/ipv6/conf/*/regen_min_advance and /proc/sys/net/ipv6/conf/*/dad_transmits to 0. I realize that that is a pretty niche use case, but I know at least one person who would gladly sacrifice performance and convenience to be sure that they are getting the maximum possible level of privacy. Link: https://serverfault.com/a/1031168/310447 Signed-off-by: Alex Henrie <alexhenrie24@gmail.com> --- Documentation/networking/ip-sysctl.rst | 2 +- net/ipv6/addrconf.c | 43 ++++++++++++++++++++------ 2 files changed, 35 insertions(+), 10 deletions(-) diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst index 407d917d1a36..bd50df6a5a42 100644 --- a/Documentation/networking/ip-sysctl.rst +++ b/Documentation/networking/ip-sysctl.rst @@ -2511,7 +2511,7 @@ temp_valid_lft - INTEGER temp_prefered_lft - INTEGER Preferred lifetime (in seconds) for temporary addresses. If temp_prefered_lft is less than the minimum required lifetime (typically - 5-7 seconds), temporary addresses will not be created. If + 5-7 seconds), the preferred lifetime is the minimum required. If temp_prefered_lft is greater than temp_valid_lft, the preferred lifetime is temp_valid_lft. diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c index 9af56b73d08c..f31ab973864a 100644 --- a/net/ipv6/addrconf.c +++ b/net/ipv6/addrconf.c @@ -1354,6 +1354,7 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, bool block) unsigned long tmp_tstamp, age; unsigned long regen_advance; unsigned long now = jiffies; + u32 if_public_preferred_lft; s32 cnf_temp_preferred_lft; struct inet6_ifaddr *ift; struct ifa6_config cfg; @@ -1409,11 +1410,13 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, bool block) } } + if_public_preferred_lft = ifp->prefered_lft; + memset(&cfg, 0, sizeof(cfg)); cfg.valid_lft = min_t(__u32, ifp->valid_lft, idev->cnf.temp_valid_lft + age); cfg.preferred_lft = cnf_temp_preferred_lft + age - idev->desync_factor; - cfg.preferred_lft = min_t(__u32, ifp->prefered_lft, cfg.preferred_lft); + cfg.preferred_lft = min_t(__u32, if_public_preferred_lft, cfg.preferred_lft); cfg.preferred_lft = min_t(__u32, cfg.valid_lft, cfg.preferred_lft); cfg.plen = ifp->prefix_len; @@ -1422,19 +1425,41 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, bool block) write_unlock_bh(&idev->lock); - /* A temporary address is created only if this calculated Preferred - * Lifetime is greater than REGEN_ADVANCE time units. In particular, - * an implementation must not create a temporary address with a zero - * Preferred Lifetime. + /* From RFC 4941: + * + * A temporary address is created only if this calculated Preferred + * Lifetime is greater than REGEN_ADVANCE time units. In + * particular, an implementation must not create a temporary address + * with a zero Preferred Lifetime. + * + * ... + * + * When creating a temporary address, the lifetime values MUST be + * derived from the corresponding prefix as follows: + * + * ... + * + * * Its Preferred Lifetime is the lower of the Preferred Lifetime + * of the public address or TEMP_PREFERRED_LIFETIME - + * DESYNC_FACTOR. + * + * To comply with the RFC's requirements, clamp the preferred lifetime + * to a minimum of regen_advance, unless that would exceed valid_lft or + * ifp->prefered_lft. + * * Use age calculation as in addrconf_verify to avoid unnecessary * temporary addresses being generated. */ age = (now - tmp_tstamp + ADDRCONF_TIMER_FUZZ_MINUS) / HZ; if (cfg.preferred_lft <= regen_advance + age) { - in6_ifa_put(ifp); - in6_dev_put(idev); - ret = -1; - goto out; + cfg.preferred_lft = regen_advance + age + 1; + if (cfg.preferred_lft > cfg.valid_lft || + cfg.preferred_lft > if_public_preferred_lft) { + in6_ifa_put(ifp); + in6_dev_put(idev); + ret = -1; + goto out; + } } cfg.ifa_flags = IFA_F_TEMPORARY; -- 2.43.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH net-next v2 3/3] net: ipv6/addrconf: clamp preferred_lft to the minimum required 2024-02-14 6:26 ` [PATCH net-next v2 3/3] net: ipv6/addrconf: clamp preferred_lft to the minimum required Alex Henrie @ 2024-02-14 15:40 ` David Ahern 0 siblings, 0 replies; 16+ messages in thread From: David Ahern @ 2024-02-14 15:40 UTC (permalink / raw) To: Alex Henrie, netdev, dan, bagasdotme, davem, edumazet, kuba, pabeni, jikos On 2/13/24 11:26 PM, Alex Henrie wrote: > If the preferred lifetime was less than the minimum required lifetime, > 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 5 seconds or less, after a few minutes with > the preferred lifetime set to 6 seconds, and not at all with the > preferred lifetime set to 7 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. > > The preferred lifetime is a preference, not a hard requirement. The > kernel does not strictly forbid new connections on a deprecated address, > nor does it guarantee that the address will be disposed of the instant > its total valid lifetime expires. So rather than disable IPv6 privacy > extensions altogether if the minimum required lifetime swells above the > preferred lifetime, it is more in keeping with the user's intent to > increase the temporary address's lifetime to the minimum necessary for > the current network conditions. > > With these fixes, setting the preferred lifetime to 5 or 6 seconds "just > works" because the extra fraction of a second is practically > unnoticeable. It's even possible to reduce the time before deprecation > to 1 or 2 seconds by setting /proc/sys/net/ipv6/conf/*/regen_min_advance > and /proc/sys/net/ipv6/conf/*/dad_transmits to 0. I realize that that is > a pretty niche use case, but I know at least one person who would gladly > sacrifice performance and convenience to be sure that they are getting > the maximum possible level of privacy. > > Link: https://serverfault.com/a/1031168/310447 > Signed-off-by: Alex Henrie <alexhenrie24@gmail.com> > --- > Documentation/networking/ip-sysctl.rst | 2 +- > net/ipv6/addrconf.c | 43 ++++++++++++++++++++------ > 2 files changed, 35 insertions(+), 10 deletions(-) > Reviewed-by: David Ahern <dsahern@kernel.org> ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next v2 0/3] net: ipv6/addrconf: ensure that temporary addresses' preferred lifetimes are long enough 2024-02-14 6:26 ` [PATCH net-next v2 0/3] net: ipv6/addrconf: ensure that temporary addresses' preferred lifetimes are long enough Alex Henrie ` (2 preceding siblings ...) 2024-02-14 6:26 ` [PATCH net-next v2 3/3] net: ipv6/addrconf: clamp preferred_lft to the minimum required Alex Henrie @ 2024-02-15 14:50 ` patchwork-bot+netdevbpf 3 siblings, 0 replies; 16+ messages in thread From: patchwork-bot+netdevbpf @ 2024-02-15 14:50 UTC (permalink / raw) To: Alex Henrie Cc: netdev, dan, bagasdotme, davem, dsahern, edumazet, kuba, pabeni, jikos Hello: This series was applied to netdev/net-next.git (main) by Paolo Abeni <pabeni@redhat.com>: On Tue, 13 Feb 2024 23:26:29 -0700 you wrote: > v2 corrects and updates the documentation for these features. > > Changes from v1: > - Update the typical minimum lifetime stated in the documentation, and > make it a range to emphasize the variability > - Fix spelling of "determine" in the documentation > - Mention RFC 8981's requirements in the documentation > - Arrange variables in "reverse Christmas tree" > - Update documentation of what happens if temp_prefered_lft is less > than the minimum required lifetime > > [...] Here is the summary with links: - [net-next,v2,1/3] net: ipv6/addrconf: ensure that regen_advance is at least 2 seconds https://git.kernel.org/netdev/net-next/c/2aa8f155b095 - [net-next,v2,2/3] net: ipv6/addrconf: introduce a regen_min_advance sysctl https://git.kernel.org/netdev/net-next/c/a5fcea2d2f79 - [net-next,v2,3/3] net: ipv6/addrconf: clamp preferred_lft to the minimum required https://git.kernel.org/netdev/net-next/c/f4bcbf360ac8 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2024-02-15 14:50 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-02-09 6:10 [PATCH net-next 1/3] net: ipv6/addrconf: ensure that regen_advance is at least 2 seconds Alex Henrie 2024-02-09 6:10 ` [PATCH net-next 2/3] net: ipv6/addrconf: introduce a regen_min_advance sysctl Alex Henrie 2024-02-13 15:36 ` David Ahern 2024-02-09 6:10 ` [PATCH net-next 3/3] net: ipv6/addrconf: clamp preferred_lft to the minimum required Alex Henrie 2024-02-13 10:13 ` Paolo Abeni 2024-02-13 15:40 ` David Ahern 2024-02-13 21:07 ` Dan Moulding 2024-02-13 15:35 ` [PATCH net-next 1/3] net: ipv6/addrconf: ensure that regen_advance is at least 2 seconds David Ahern 2024-02-14 6:26 ` [PATCH net-next v2 0/3] net: ipv6/addrconf: ensure that temporary addresses' preferred lifetimes are long enough Alex Henrie 2024-02-14 6:26 ` [PATCH net-next v2 1/3] net: ipv6/addrconf: ensure that regen_advance is at least 2 seconds Alex Henrie 2024-02-14 15:38 ` David Ahern 2024-02-14 6:26 ` [PATCH net-next v2 2/3] net: ipv6/addrconf: introduce a regen_min_advance sysctl Alex Henrie 2024-02-14 15:39 ` David Ahern 2024-02-14 6:26 ` [PATCH net-next v2 3/3] net: ipv6/addrconf: clamp preferred_lft to the minimum required Alex Henrie 2024-02-14 15:40 ` David Ahern 2024-02-15 14:50 ` [PATCH net-next v2 0/3] net: ipv6/addrconf: ensure that temporary addresses' preferred lifetimes are long enough patchwork-bot+netdevbpf
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).