[PATCH net-next v2 1/2] ipv6: update NUD_FAILED neighbors from NA messages

[Lawrence Lee <lfqlee314@gmail.com>]

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/20260813233344.445265-1-lfqlee314@gmail.com
Assisted-by: LLM Sashiko sparse
Signed-off-by: Lawrence Lee <lfqlee314@gmail.com>
---
 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