I am following up on https://lore.kernel.org/stable/20230925211034.905320-1-prohr@google.com/ with cherry-picks for 5.10.198. I have run our test suite with the changes applied and all relevant tests passed.
Thanks, -Patrick
Patrick Rohr (3): net: add sysctl accept_ra_min_rtr_lft net: change accept_ra_min_rtr_lft to affect all RA lifetimes net: release reference to inet6_dev pointer
Documentation/networking/ip-sysctl.rst | 8 ++++++++ include/linux/ipv6.h | 1 + include/uapi/linux/ipv6.h | 7 +++++++ net/ipv6/addrconf.c | 13 +++++++++++++ net/ipv6/ndisc.c | 13 +++++++++++-- 5 files changed, 40 insertions(+), 2 deletions(-)
commit 1671bcfd76fdc0b9e65153cf759153083755fe4c upstream.
This change adds a new sysctl accept_ra_min_rtr_lft to specify the minimum acceptable router lifetime in an RA. If the received RA router lifetime is less than the configured value (and not 0), the RA is ignored. This is useful for mobile devices, whose battery life can be impacted by networks that configure RAs with a short lifetime. On such networks, the device should never gain IPv6 provisioning and should attempt to drop RAs via hardware offload, if available.
Signed-off-by: Patrick Rohr prohr@google.com Cc: Maciej Żenczykowski maze@google.com Cc: Lorenzo Colitti lorenzo@google.com Signed-off-by: David S. Miller davem@davemloft.net --- Documentation/networking/ip-sysctl.rst | 8 ++++++++ include/linux/ipv6.h | 1 + include/uapi/linux/ipv6.h | 7 +++++++ net/ipv6/addrconf.c | 10 ++++++++++ net/ipv6/ndisc.c | 18 ++++++++++++++++-- 5 files changed, 42 insertions(+), 2 deletions(-)
diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst index 252212998378..f7db7652341d 100644 --- a/Documentation/networking/ip-sysctl.rst +++ b/Documentation/networking/ip-sysctl.rst @@ -1916,6 +1916,14 @@ accept_ra_min_hop_limit - INTEGER
Default: 1
+accept_ra_min_rtr_lft - INTEGER + Minimum acceptable router lifetime in Router Advertisement. + + RAs with a router lifetime less than this value shall be + ignored. RAs with a router lifetime of 0 are unaffected. + + Default: 0 + accept_ra_pinfo - BOOLEAN Learn Prefix Information in Router Advertisement.
diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h index 510f87656479..b91925a70296 100644 --- a/include/linux/ipv6.h +++ b/include/linux/ipv6.h @@ -32,6 +32,7 @@ struct ipv6_devconf { __s32 max_addresses; __s32 accept_ra_defrtr; __s32 accept_ra_min_hop_limit; + __s32 accept_ra_min_rtr_lft; __s32 accept_ra_pinfo; __s32 ignore_routes_with_linkdown; #ifdef CONFIG_IPV6_ROUTER_PREF diff --git a/include/uapi/linux/ipv6.h b/include/uapi/linux/ipv6.h index d44d0483fd73..2038eff9b63f 100644 --- a/include/uapi/linux/ipv6.h +++ b/include/uapi/linux/ipv6.h @@ -192,6 +192,13 @@ enum { DEVCONF_ACCEPT_RA_RT_INFO_MIN_PLEN, DEVCONF_NDISC_TCLASS, DEVCONF_RPL_SEG_ENABLED, + DEVCONF_RA_DEFRTR_METRIC, + DEVCONF_IOAM6_ENABLED, + DEVCONF_IOAM6_ID, + DEVCONF_IOAM6_ID_WIDE, + DEVCONF_NDISC_EVICT_NOCARRIER, + DEVCONF_ACCEPT_UNTRACKED_NA, + DEVCONF_ACCEPT_RA_MIN_RTR_LFT, DEVCONF_MAX };
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c index 0eafe26c05f7..7f122f27137f 100644 --- a/net/ipv6/addrconf.c +++ b/net/ipv6/addrconf.c @@ -207,6 +207,7 @@ static struct ipv6_devconf ipv6_devconf __read_mostly = { .accept_ra_defrtr = 1, .accept_ra_from_local = 0, .accept_ra_min_hop_limit= 1, + .accept_ra_min_rtr_lft = 0, .accept_ra_pinfo = 1, #ifdef CONFIG_IPV6_ROUTER_PREF .accept_ra_rtr_pref = 1, @@ -262,6 +263,7 @@ static struct ipv6_devconf ipv6_devconf_dflt __read_mostly = { .accept_ra_defrtr = 1, .accept_ra_from_local = 0, .accept_ra_min_hop_limit= 1, + .accept_ra_min_rtr_lft = 0, .accept_ra_pinfo = 1, #ifdef CONFIG_IPV6_ROUTER_PREF .accept_ra_rtr_pref = 1, @@ -5559,6 +5561,7 @@ static inline void ipv6_store_devconf(struct ipv6_devconf *cnf, array[DEVCONF_DISABLE_POLICY] = cnf->disable_policy; array[DEVCONF_NDISC_TCLASS] = cnf->ndisc_tclass; array[DEVCONF_RPL_SEG_ENABLED] = cnf->rpl_seg_enabled; + array[DEVCONF_ACCEPT_RA_MIN_RTR_LFT] = cnf->accept_ra_min_rtr_lft; }
static inline size_t inet6_ifla6_size(void) @@ -6716,6 +6719,13 @@ static const struct ctl_table addrconf_sysctl[] = { .mode = 0644, .proc_handler = proc_dointvec, }, + { + .procname = "accept_ra_min_rtr_lft", + .data = &ipv6_devconf.accept_ra_min_rtr_lft, + .maxlen = sizeof(int), + .mode = 0644, + .proc_handler = proc_dointvec, + }, { .procname = "accept_ra_pinfo", .data = &ipv6_devconf.accept_ra_pinfo, diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c index ac1e51087b1d..215ea5dbc5f0 100644 --- a/net/ipv6/ndisc.c +++ b/net/ipv6/ndisc.c @@ -1222,6 +1222,8 @@ static void ndisc_router_discovery(struct sk_buff *skb) return; }
+ lifetime = ntohs(ra_msg->icmph.icmp6_rt_lifetime); + if (!ipv6_accept_ra(in6_dev)) { ND_PRINTK(2, info, "RA: %s, did not accept ra for dev: %s\n", @@ -1229,6 +1231,13 @@ static void ndisc_router_discovery(struct sk_buff *skb) goto skip_linkparms; }
+ if (lifetime != 0 && lifetime < in6_dev->cnf.accept_ra_min_rtr_lft) { + ND_PRINTK(2, info, + "RA: router lifetime (%ds) is too short: %s\n", + lifetime, skb->dev->name); + goto skip_linkparms; + } + #ifdef CONFIG_IPV6_NDISC_NODETYPE /* skip link-specific parameters from interior routers */ if (skb->ndisc_nodetype == NDISC_NODETYPE_NODEFAULT) { @@ -1281,8 +1290,6 @@ static void ndisc_router_discovery(struct sk_buff *skb) goto skip_defrtr; }
- lifetime = ntohs(ra_msg->icmph.icmp6_rt_lifetime); - #ifdef CONFIG_IPV6_ROUTER_PREF pref = ra_msg->icmph.icmp6_router_pref; /* 10b is handled as if it were 00b (medium) */ @@ -1429,6 +1436,13 @@ static void ndisc_router_discovery(struct sk_buff *skb) goto out; }
+ if (lifetime != 0 && lifetime < in6_dev->cnf.accept_ra_min_rtr_lft) { + ND_PRINTK(2, info, + "RA: router lifetime (%ds) is too short: %s\n", + lifetime, skb->dev->name); + goto out; + } + #ifdef CONFIG_IPV6_ROUTE_INFO if (!in6_dev->cnf.accept_ra_from_local && ipv6_chk_addr(dev_net(in6_dev->dev), &ipv6_hdr(skb)->saddr,
commit 5027d54a9c30bc7ec808360378e2b4753f053f25 upstream.
accept_ra_min_rtr_lft only considered the lifetime of the default route and discarded entire RAs accordingly.
This change renames accept_ra_min_rtr_lft to accept_ra_min_lft, and applies the value to individual RA sections; in particular, router lifetime, PIO preferred lifetime, and RIO lifetime. If any of those lifetimes are lower than the configured value, the specific RA section is ignored.
In order for the sysctl to be useful to Android, it should really apply to all lifetimes in the RA, since that is what determines the minimum frequency at which RAs must be processed by the kernel. Android uses hardware offloads to drop RAs for a fraction of the minimum of all lifetimes present in the RA (some networks have very frequent RAs (5s) with high lifetimes (2h)). Despite this, we have encountered networks that set the router lifetime to 30s which results in very frequent CPU wakeups. Instead of disabling IPv6 (and dropping IPv6 ethertype in the WiFi firmware) entirely on such networks, it seems better to ignore the misconfigured routers while still processing RAs from other IPv6 routers on the same network (i.e. to support IoT applications).
The previous implementation dropped the entire RA based on router lifetime. This turned out to be hard to expand to the other lifetimes present in the RA in a consistent manner; dropping the entire RA based on RIO/PIO lifetimes would essentially require parsing the whole thing twice.
Fixes: 1671bcfd76fd ("net: add sysctl accept_ra_min_rtr_lft") Cc: Lorenzo Colitti lorenzo@google.com Signed-off-by: Patrick Rohr prohr@google.com Reviewed-by: Maciej Żenczykowski maze@google.com Reviewed-by: David Ahern dsahern@kernel.org Link: https://lore.kernel.org/r/20230726230701.919212-1-prohr@google.com Signed-off-by: Jakub Kicinski kuba@kernel.org --- Documentation/networking/ip-sysctl.rst | 8 ++++---- include/linux/ipv6.h | 2 +- include/uapi/linux/ipv6.h | 2 +- net/ipv6/addrconf.c | 13 ++++++++----- net/ipv6/ndisc.c | 27 +++++++++++--------------- 5 files changed, 25 insertions(+), 27 deletions(-)
diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst index f7db7652341d..e912a47765f3 100644 --- a/Documentation/networking/ip-sysctl.rst +++ b/Documentation/networking/ip-sysctl.rst @@ -1916,11 +1916,11 @@ accept_ra_min_hop_limit - INTEGER
Default: 1
-accept_ra_min_rtr_lft - INTEGER - Minimum acceptable router lifetime in Router Advertisement. +accept_ra_min_lft - INTEGER + Minimum acceptable lifetime value in Router Advertisement.
- RAs with a router lifetime less than this value shall be - ignored. RAs with a router lifetime of 0 are unaffected. + RA sections with a lifetime less than this value shall be + ignored. Zero lifetimes stay unaffected.
Default: 0
diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h index b91925a70296..d758c131ed5e 100644 --- a/include/linux/ipv6.h +++ b/include/linux/ipv6.h @@ -32,7 +32,7 @@ struct ipv6_devconf { __s32 max_addresses; __s32 accept_ra_defrtr; __s32 accept_ra_min_hop_limit; - __s32 accept_ra_min_rtr_lft; + __s32 accept_ra_min_lft; __s32 accept_ra_pinfo; __s32 ignore_routes_with_linkdown; #ifdef CONFIG_IPV6_ROUTER_PREF diff --git a/include/uapi/linux/ipv6.h b/include/uapi/linux/ipv6.h index 2038eff9b63f..4fa8511b1e35 100644 --- a/include/uapi/linux/ipv6.h +++ b/include/uapi/linux/ipv6.h @@ -198,7 +198,7 @@ enum { DEVCONF_IOAM6_ID_WIDE, DEVCONF_NDISC_EVICT_NOCARRIER, DEVCONF_ACCEPT_UNTRACKED_NA, - DEVCONF_ACCEPT_RA_MIN_RTR_LFT, + DEVCONF_ACCEPT_RA_MIN_LFT, DEVCONF_MAX };
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c index 7f122f27137f..2a389895d0a2 100644 --- a/net/ipv6/addrconf.c +++ b/net/ipv6/addrconf.c @@ -207,7 +207,7 @@ static struct ipv6_devconf ipv6_devconf __read_mostly = { .accept_ra_defrtr = 1, .accept_ra_from_local = 0, .accept_ra_min_hop_limit= 1, - .accept_ra_min_rtr_lft = 0, + .accept_ra_min_lft = 0, .accept_ra_pinfo = 1, #ifdef CONFIG_IPV6_ROUTER_PREF .accept_ra_rtr_pref = 1, @@ -263,7 +263,7 @@ static struct ipv6_devconf ipv6_devconf_dflt __read_mostly = { .accept_ra_defrtr = 1, .accept_ra_from_local = 0, .accept_ra_min_hop_limit= 1, - .accept_ra_min_rtr_lft = 0, + .accept_ra_min_lft = 0, .accept_ra_pinfo = 1, #ifdef CONFIG_IPV6_ROUTER_PREF .accept_ra_rtr_pref = 1, @@ -2726,6 +2726,9 @@ void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len, bool sllao) return; }
+ if (valid_lft != 0 && valid_lft < in6_dev->cnf.accept_ra_min_lft) + return; + /* * Two things going on here: * 1) Add routes for on-link prefixes @@ -5561,7 +5564,7 @@ static inline void ipv6_store_devconf(struct ipv6_devconf *cnf, array[DEVCONF_DISABLE_POLICY] = cnf->disable_policy; array[DEVCONF_NDISC_TCLASS] = cnf->ndisc_tclass; array[DEVCONF_RPL_SEG_ENABLED] = cnf->rpl_seg_enabled; - array[DEVCONF_ACCEPT_RA_MIN_RTR_LFT] = cnf->accept_ra_min_rtr_lft; + array[DEVCONF_ACCEPT_RA_MIN_LFT] = cnf->accept_ra_min_lft; }
static inline size_t inet6_ifla6_size(void) @@ -6720,8 +6723,8 @@ static const struct ctl_table addrconf_sysctl[] = { .proc_handler = proc_dointvec, }, { - .procname = "accept_ra_min_rtr_lft", - .data = &ipv6_devconf.accept_ra_min_rtr_lft, + .procname = "accept_ra_min_lft", + .data = &ipv6_devconf.accept_ra_min_lft, .maxlen = sizeof(int), .mode = 0644, .proc_handler = proc_dointvec, diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c index 215ea5dbc5f0..14251347c4a5 100644 --- a/net/ipv6/ndisc.c +++ b/net/ipv6/ndisc.c @@ -1222,8 +1222,6 @@ static void ndisc_router_discovery(struct sk_buff *skb) return; }
- lifetime = ntohs(ra_msg->icmph.icmp6_rt_lifetime); - if (!ipv6_accept_ra(in6_dev)) { ND_PRINTK(2, info, "RA: %s, did not accept ra for dev: %s\n", @@ -1231,13 +1229,6 @@ static void ndisc_router_discovery(struct sk_buff *skb) goto skip_linkparms; }
- if (lifetime != 0 && lifetime < in6_dev->cnf.accept_ra_min_rtr_lft) { - ND_PRINTK(2, info, - "RA: router lifetime (%ds) is too short: %s\n", - lifetime, skb->dev->name); - goto skip_linkparms; - } - #ifdef CONFIG_IPV6_NDISC_NODETYPE /* skip link-specific parameters from interior routers */ if (skb->ndisc_nodetype == NDISC_NODETYPE_NODEFAULT) { @@ -1278,6 +1269,14 @@ static void ndisc_router_discovery(struct sk_buff *skb) goto skip_defrtr; }
+ lifetime = ntohs(ra_msg->icmph.icmp6_rt_lifetime); + if (lifetime != 0 && lifetime < in6_dev->cnf.accept_ra_min_lft) { + ND_PRINTK(2, info, + "RA: router lifetime (%ds) is too short: %s\n", + lifetime, skb->dev->name); + goto skip_defrtr; + } + /* Do not accept RA with source-addr found on local machine unless * accept_ra_from_local is set to true. */ @@ -1436,13 +1435,6 @@ static void ndisc_router_discovery(struct sk_buff *skb) goto out; }
- if (lifetime != 0 && lifetime < in6_dev->cnf.accept_ra_min_rtr_lft) { - ND_PRINTK(2, info, - "RA: router lifetime (%ds) is too short: %s\n", - lifetime, skb->dev->name); - goto out; - } - #ifdef CONFIG_IPV6_ROUTE_INFO if (!in6_dev->cnf.accept_ra_from_local && ipv6_chk_addr(dev_net(in6_dev->dev), &ipv6_hdr(skb)->saddr, @@ -1467,6 +1459,9 @@ static void ndisc_router_discovery(struct sk_buff *skb) if (ri->prefix_len == 0 && !in6_dev->cnf.accept_ra_defrtr) continue; + if (ri->lifetime != 0 && + ntohl(ri->lifetime) < in6_dev->cnf.accept_ra_min_lft) + continue; if (ri->prefix_len < in6_dev->cnf.accept_ra_rt_info_min_plen) continue; if (ri->prefix_len > in6_dev->cnf.accept_ra_rt_info_max_plen)
commit 5cb249686e67dbef3ffe53887fa725eefc5a7144 upstream.
addrconf_prefix_rcv returned early without releasing the inet6_dev pointer when the PIO lifetime is less than accept_ra_min_lft.
Fixes: 5027d54a9c30 ("net: change accept_ra_min_rtr_lft to affect all RA lifetimes") Cc: Maciej Żenczykowski maze@google.com Cc: Lorenzo Colitti lorenzo@google.com Cc: David Ahern dsahern@kernel.org Cc: Simon Horman horms@kernel.org Reviewed-by: Simon Horman horms@kernel.org Reviewed-by: Maciej Żenczykowski maze@google.com Signed-off-by: Patrick Rohr prohr@google.com Reviewed-by: Leon Romanovsky leonro@nvidia.com Signed-off-by: David S. Miller davem@davemloft.net --- net/ipv6/addrconf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c index 2a389895d0a2..193e5f275733 100644 --- a/net/ipv6/addrconf.c +++ b/net/ipv6/addrconf.c @@ -2727,7 +2727,7 @@ void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len, bool sllao) }
if (valid_lft != 0 && valid_lft < in6_dev->cnf.accept_ra_min_lft) - return; + goto put;
/* * Two things going on here:
On Fri, Oct 13, 2023 at 02:44:11PM -0700, Patrick Rohr wrote:
I am following up on https://lore.kernel.org/stable/20230925211034.905320-1-prohr@google.com/ with cherry-picks for 5.10.198. I have run our test suite with the changes applied and all relevant tests passed.
All now queued up, thanks.
greg k-h
On Sun, Oct 15, 2023 at 10:46 AM Greg KH gregkh@linuxfoundation.org wrote:
On Fri, Oct 13, 2023 at 02:44:11PM -0700, Patrick Rohr wrote:
I am following up on https://lore.kernel.org/stable/20230925211034.905320-1-prohr@google.com/ with cherry-picks for 5.10.198. I have run our test suite with the changes applied and all relevant tests passed.
All now queued up, thanks.
greg k-h
Great! Thank you.
linux-stable-mirror@lists.linaro.org
-
Greg KH -
Maciej Żenczykowski -
Patrick Rohr