Transition a FAILED neighbor entry to STALE upon receipt of an NA message on routers when accept_untracked_na is enabled. This extends the RFC 9131 accept_untracked_na behavior so that FAILED entries are treated the same as non-existent entries. In the context of RFC 4861 which introduced NDP, both non-existent and FAILED entries are considered untracked since they do not have a valid neighbor cache entry.
Trying to resolve FAILED neighbors via periodic probing (e.g. using NTF_EXT_MANAGED) is more work compared to this approach which uses information in NAs that the kernel may already be receiving. Note that because this behavior in IPv6 is dependent on the accept_untracked_na sysctl setting, this approach is more conservative than IPv4 which transitions FAILED neighbors to STALE by default upon receiving GARPs. Link: https://lore.kernel.org/r/[email protected] Assisted-by: LLM Sashiko sparse Signed-off-by: Lawrence Lee <[email protected]> --- Documentation/networking/ip-sysctl.rst | 28 ++++---- include/net/ndisc.h | 15 ++-- net/6lowpan/ndisc.c | 15 ++-- net/ipv6/ndisc.c | 94 +++++++++++++++++--------- 4 files changed, 96 insertions(+), 56 deletions(-) diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst index 208f46967ee5..4cc57a6be99b 100644 --- a/Documentation/networking/ip-sysctl.rst +++ b/Documentation/networking/ip-sysctl.rst @@ -3223,18 +3223,19 @@ drop_unsolicited_na - BOOLEAN Default: 0 (disabled). accept_untracked_na - INTEGER - Define behavior for accepting neighbor advertisements from devices that - are absent in the neighbor cache: + Define behavior for accepting neighbor advertisements for IPv6 addresses + that are absent from the neighbor cache or whose entries are in FAILED + state: - - 0 - (default) Do not accept unsolicited and untracked neighbor - advertisements. + - 0 - (default) Do not create new neighbor cache entries or update + FAILED entries from neighbor advertisements. - - 1 - Add a new neighbor cache entry in STALE state for routers on - receiving a neighbor advertisement (either solicited or unsolicited) - with target link-layer address option specified if no neighbor entry - is already present for the advertised IPv6 address. Without this knob, - NAs received for untracked addresses (absent in neighbor cache) are - silently ignored. + - 1 - For routers, add a new neighbor cache entry or update an existing + FAILED entry to STALE upon receiving a neighbor advertisement (either + solicited or unsolicited) with the target link-layer address option + specified. Without this knob, NAs received for untracked addresses + (absent from the neighbor cache or in FAILED state) are silently + ignored. This is as per router-side behavior documented in RFC9131. @@ -3249,9 +3250,10 @@ accept_untracked_na - INTEGER used in conjunction with the ndisc_notify setting on the host to satisfy this prerequisite. - - 2 - Extend option (1) to add a new neighbor cache entry only if the - source IP address is in the same subnet as an address configured on - the interface that received the neighbor advertisement. + - 2 - Extend option (1) to add a new neighbor cache entry or update a + FAILED entry only if the source IP address is in the same subnet as + an address configured on the interface that received the neighbor + advertisement. enhanced_dad - BOOLEAN Include a nonce option in the IPv6 neighbor solicitation messages used for diff --git a/include/net/ndisc.h b/include/net/ndisc.h index 96e3bb6e83af..7fb3f10eca6c 100644 --- a/include/net/ndisc.h +++ b/include/net/ndisc.h @@ -154,11 +154,13 @@ void __ndisc_fill_addr_option(struct sk_buff *skb, int type, const void *data, * option parser will take care about that option. * * void (*update)(const struct net_device *dev, struct neighbour *n, - * u32 flags, u8 icmp6_type, + * u32 flags, bool failed_recovery, u8 icmp6_type, * const struct ndisc_options *ndopts): * This function is called when IPv6 ndisc updates the neighbour cache * entry. Additional options which can be updated may be previously * parsed by parse_opts callback and accessible over ndopts parameter. + * failed_recovery indicates that ndisc accepted the packet to recover + * an entry observed in NUD_FAILED. * * int (*opt_addr_space)(const struct net_device *dev, u8 icmp6_type, * struct neighbour *neigh, u8 *ha_buf, @@ -197,7 +199,7 @@ struct ndisc_ops { struct nd_opt_hdr *nd_opt, struct ndisc_options *ndopts); void (*update)(const struct net_device *dev, struct neighbour *n, - u32 flags, u8 icmp6_type, + u32 flags, bool failed_recovery, u8 icmp6_type, const struct ndisc_options *ndopts); int (*opt_addr_space)(const struct net_device *dev, u8 icmp6_type, struct neighbour *neigh, u8 *ha_buf, @@ -227,12 +229,13 @@ static inline int ndisc_ops_parse_options(const struct net_device *dev, } static inline void ndisc_ops_update(const struct net_device *dev, - struct neighbour *n, u32 flags, - u8 icmp6_type, - const struct ndisc_options *ndopts) + struct neighbour *n, u32 flags, + bool failed_recovery, u8 icmp6_type, + const struct ndisc_options *ndopts) { if (dev->ndisc_ops && dev->ndisc_ops->update) - dev->ndisc_ops->update(dev, n, flags, icmp6_type, ndopts); + dev->ndisc_ops->update(dev, n, flags, failed_recovery, + icmp6_type, ndopts); } static inline int ndisc_ops_opt_addr_space(const struct net_device *dev, diff --git a/net/6lowpan/ndisc.c b/net/6lowpan/ndisc.c index 868d28583c0a..8fedfef93740 100644 --- a/net/6lowpan/ndisc.c +++ b/net/6lowpan/ndisc.c @@ -47,7 +47,8 @@ static int lowpan_ndisc_parse_options(const struct net_device *dev, } } -static void lowpan_ndisc_802154_update(struct neighbour *n, u32 flags, +static void lowpan_ndisc_802154_update(struct neighbour *n, + bool failed_recovery, u8 icmp6_type, const struct ndisc_options *ndopts) { @@ -87,20 +88,24 @@ static void lowpan_ndisc_802154_update(struct neighbour *n, u32 flags, ieee802154_be16_to_le16(&neigh->short_addr, lladdr_short); if (!lowpan_802154_is_valid_src_short_addr(neigh->short_addr)) neigh->short_addr = cpu_to_le16(IEEE802154_ADDR_SHORT_UNSPEC); + } else if (failed_recovery) { + neigh->short_addr = cpu_to_le16(IEEE802154_ADDR_SHORT_UNSPEC); } write_unlock_bh(&n->lock); } static void lowpan_ndisc_update(const struct net_device *dev, - struct neighbour *n, u32 flags, u8 icmp6_type, + struct neighbour *n, u32 flags, + bool failed_recovery, u8 icmp6_type, const struct ndisc_options *ndopts) { if (!lowpan_is_ll(dev, LOWPAN_LLTYPE_IEEE802154)) return; - /* react on overrides only. TODO check if this is really right. */ - if (flags & NEIGH_UPDATE_F_OVERRIDE) - lowpan_ndisc_802154_update(n, flags, icmp6_type, ndopts); + /* React to overrides or accepted FAILED-entry recovery. */ + if ((flags & NEIGH_UPDATE_F_OVERRIDE) || failed_recovery) + lowpan_ndisc_802154_update(n, failed_recovery, icmp6_type, + ndopts); } static int lowpan_ndisc_opt_addr_space(const struct net_device *dev, diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c index 90cd5d852569..84d70c09205a 100644 --- a/net/ipv6/ndisc.c +++ b/net/ipv6/ndisc.c @@ -778,13 +778,23 @@ static int pndisc_is_router(const void *pkey, return ret; } +static void __ndisc_update(const struct net_device *dev, + struct neighbour *neigh, const u8 *lladdr, u8 new, + u32 flags, bool failed_recovery, u8 icmp6_type, + struct ndisc_options *ndopts) +{ + neigh_update(neigh, lladdr, new, flags, 0); + /* report ndisc ops about neighbour update */ + ndisc_ops_update(dev, neigh, flags, failed_recovery, icmp6_type, + ndopts); +} + void ndisc_update(const struct net_device *dev, struct neighbour *neigh, const u8 *lladdr, u8 new, u32 flags, u8 icmp6_type, struct ndisc_options *ndopts) { - neigh_update(neigh, lladdr, new, flags, 0); - /* report ndisc ops about neighbour update */ - ndisc_ops_update(dev, neigh, flags, icmp6_type, ndopts); + __ndisc_update(dev, neigh, lladdr, new, flags, false, icmp6_type, + ndopts); } static enum skb_drop_reason ndisc_recv_ns(struct sk_buff *skb) @@ -972,14 +982,18 @@ static enum skb_drop_reason ndisc_recv_ns(struct sk_buff *skb) static int accept_untracked_na(struct inet6_dev *idev, struct in6_addr *saddr) { + /* For any given neighbor IP address, consider it an untracked neighbor if + * it is absent from the neighbor cache or if it has a NUD_FAILED entry in + * the neighbor cache + */ switch (READ_ONCE(idev->cnf.accept_untracked_na)) { - case 0: /* Don't accept untracked na (absent in neighbor cache) */ + case 0: /* Reject NAs for untracked neighbours */ return 0; - case 1: /* Create new entries from na if currently untracked */ + case 1: /* Accept NAs for untracked neighbours */ return 1; - case 2: /* Create new entries from untracked na only if saddr is in the + case 2: /* Accept NAs for untracked neighbours only if saddr is in the * same subnet as an address configured on the interface that - * received the na + * received the NA */ return !!ipv6_chk_prefix(saddr, idev->dev); default: @@ -1001,6 +1015,9 @@ static enum skb_drop_reason ndisc_recv_na(struct sk_buff *skb) struct neigh_table *tbl; struct neighbour *neigh; struct inet6_dev *idev; + bool neigh_failed = false; + bool neigh_untracked = false; + bool accept_untracked = false; u8 *lladdr = NULL; SKB_DR(reason); u8 new_state; @@ -1067,32 +1084,41 @@ static enum skb_drop_reason ndisc_recv_na(struct sk_buff *skb) neigh = neigh_lookup(tbl, &msg->target, dev); /* RFC 9131 updates original Neighbour Discovery RFC 4861. - * NAs with Target LL Address option without a corresponding - * entry in the neighbour cache can now create a STALE neighbour - * cache entry on routers. + * NAs with Target LL Address option can now create a STALE neighbor + * cache entry on routers if the NA does not have a corresponding entry + * in the neighbour cache or has a corresponding FAILED entry. * - * entry accept fwding solicited behaviour - * ------- ------ ------ --------- ---------------------- - * present X X 0 Set state to STALE - * present X X 1 Set state to REACHABLE - * absent 0 X X Do nothing - * absent 1 0 X Do nothing - * absent 1 1 X Add a new STALE entry + * entry accept fwding solicited behaviour + * ----------- ------ ------ --------- ---------------------- + * non-FAILED X X 0 Set state to STALE + * non-FAILED X X 1 Set state to REACHABLE + * FAILED 0 X X Do nothing + * FAILED 1 0 X Do nothing + * FAILED 1 1 X Set state to STALE + * absent 0 X X Do nothing + * absent 1 0 X Do nothing + * absent 1 1 X Add a new STALE entry * * Note that we don't do a (daddr == all-routers-mcast) check. */ new_state = msg->icmph.icmp6_solicited ? NUD_REACHABLE : NUD_STALE; - if (!neigh && lladdr && idev && READ_ONCE(idev->cnf.forwarding)) { - if (accept_untracked_na(idev, saddr)) { - neigh = neigh_create(tbl, &msg->target, dev); - new_state = NUD_STALE; - } - } + neigh_failed = neigh && + (READ_ONCE(neigh->nud_state) & NUD_FAILED); + neigh_untracked = !neigh || neigh_failed; + if (neigh_untracked) { + accept_untracked = lladdr && idev && + READ_ONCE(idev->cnf.forwarding) && + accept_untracked_na(idev, saddr); + new_state = NUD_STALE; + } + if (!neigh && accept_untracked) + neigh = neigh_create(tbl, &msg->target, dev); if (neigh && !IS_ERR(neigh)) { + u32 update_flags; u8 old_flags = neigh->flags; - if (READ_ONCE(neigh->nud_state) & NUD_FAILED) + if (neigh_untracked && !accept_untracked) goto out; /* @@ -1108,19 +1134,23 @@ static enum skb_drop_reason ndisc_recv_na(struct sk_buff *skb) goto out; } - ndisc_update(dev, neigh, lladdr, - new_state, - NEIGH_UPDATE_F_WEAK_OVERRIDE| - (msg->icmph.icmp6_override ? NEIGH_UPDATE_F_OVERRIDE : 0)| - NEIGH_UPDATE_F_OVERRIDE_ISROUTER| - (msg->icmph.icmp6_router ? NEIGH_UPDATE_F_ISROUTER : 0), - NDISC_NEIGHBOUR_ADVERTISEMENT, &ndopts); + update_flags = NEIGH_UPDATE_F_WEAK_OVERRIDE | + (msg->icmph.icmp6_override ? + NEIGH_UPDATE_F_OVERRIDE : 0) | + NEIGH_UPDATE_F_OVERRIDE_ISROUTER | + (msg->icmph.icmp6_router ? + NEIGH_UPDATE_F_ISROUTER : 0); + + __ndisc_update(dev, neigh, lladdr, + new_state, update_flags, neigh_failed, + NDISC_NEIGHBOUR_ADVERTISEMENT, &ndopts); if ((old_flags & ~neigh->flags) & NTF_ROUTER) { /* * Change: router to host */ - rt6_clean_tohost(dev_net(dev), saddr); + rt6_clean_tohost(net, + neigh_failed ? &msg->target : saddr); } reason = SKB_CONSUMED; out: -- 2.43.0
