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

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

[Lawrence Lee]

Neighbor Advertisements for existing NUD_FAILED entries are currently
ignored. This can leave a first-hop router with an unusable neighbor after
the host becomes reachable and announces itself, particularly when
forwarding is offloaded away from the kernel and traffic destined to the
FAILED neighbor is unable to trigger the kernel's normal neighbor
resolution process.
Allowing neighbor entries to be updated from NAs already received by the
kernel is preferable since it creates less work compared to periodically
probing FAILED neighbors (e.g. using NTF_EXT_MANAGED).

Extend accept_untracked_na to treat FAILED entries like absent entries.
Require a target link-layer address, IPv6 forwarding, and acceptance by the
sysctl, including its source-prefix check in mode 2. Recover the entry only
to STALE, including for solicited NAs, to maintain parity with handling of
non-existent neighbor cache entries.

Patch #1 implements the behavior and updates RFC 9131-related comments and
accept_untracked_na documentation.

Patch #2 extends ndisc_unsolicited_na_test.sh to cover FAILED-entry
recovery and each acceptance gate, including in-prefix and out-of-prefix
mode 2.

Thanks to Ido Schimmel for feedback on the original RFC.

---
v2:
- Full implementation of logic to update FAILED entries to STALE.
- Update the RFC 9131-related comments and sysctl documentation.
- Add selftests to verify state changes only when the accept_untracked_na
  setting is enabled.

v1 (RFC):
https://lore.kernel.org/r/20260813233344.445265-1-lfqlee314@gmail.com

Testing:
- x86_64 current-config and allyesconfig builds with W=1
- tools/testing/selftests/net/ndisc_unsolicited_na_test.sh: 14 passed
- Local NIPA checks: all substantive checks passed
- Sashiko local review completed

Lawrence Lee (2):
  ipv6: update NUD_FAILED neighbors from NA messages
  selftests: net: test untracked NA recovery of FAILED neighbors

 Documentation/networking/ip-sysctl.rst        |  28 ++--
 include/net/ndisc.h                           |  15 ++-
 net/6lowpan/ndisc.c                           |  15 ++-
 net/ipv6/ndisc.c                              |  94 ++++++++-----
 .../net/ndisc_unsolicited_na_test.sh          | 124 +++++++++++++++---
 5 files changed, 201 insertions(+), 75 deletions(-)


base-commit: 272a65db243bfa34b9277632830e0e06d7e3518e
-- 
2.43.0

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

[Lawrence Lee]

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

[PATCH net-next v2 2/2] selftests: net: test untracked NA recovery of FAILED neighbors

[Lawrence Lee]

Seed a FAILED neighbor before triggering an unsolicited NA. Verify that
the entry transitions to STALE only when drop_unsolicited_na is disabled,
accept_untracked_na mode 1 or in-prefix mode 2 is enabled, and IPv6
forwarding is enabled. Verify that disabling each gate or using an
out-of-prefix source with mode 2 keeps the entry in FAILED.

Mark the seed externally learned and disable carrier-based eviction so
the entry cannot be garbage-collected before the NA arrives. Require the
marker after processing to prove that a recovered entry was updated in
place instead of deleted and recreated.

Check both the command that seeds the entry and its resulting state.
Capture the verifier status before constructing the test description so
the array assignment cannot mask a failure.

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>
---
 .../net/ndisc_unsolicited_na_test.sh          | 124 +++++++++++++++---
 1 file changed, 105 insertions(+), 19 deletions(-)

diff --git a/tools/testing/selftests/net/ndisc_unsolicited_na_test.sh b/tools/testing/selftests/net/ndisc_unsolicited_na_test.sh
index 5db69dad0cfc..5f4f29f0ac4a 100755
--- a/tools/testing/selftests/net/ndisc_unsolicited_na_test.sh
+++ b/tools/testing/selftests/net/ndisc_unsolicited_na_test.sh
@@ -3,12 +3,18 @@
 
 # This test is for the accept_untracked_na feature to
 # enable RFC9131 behaviour. The following is the test-matrix.
-# drop   accept  fwding                   behaviour
-# ----   ------  ------  ----------------------------------------------
-#    1        X       X  Don't update NC
-#    0        0       X  Don't update NC
-#    0        1       0  Don't update NC
-#    0        1       1  Add a STALE NC entry
+# state   drop   accept  fwding                   behaviour
+# ------  ----   ------  ------  ----------------------------------------------
+# absent     1        X       X  Don't update NC
+# absent     0        0       X  Don't update NC
+# absent     0        1       0  Don't update NC
+# absent     0        1       1  Add a STALE NC entry
+# failed     1        X       X  Keep the NC entry in FAILED state
+# failed     0        0       X  Keep the NC entry in FAILED state
+# failed     0        1       0  Keep the NC entry in FAILED state
+# failed     0        1       1  Update the NC entry to STALE
+# failed     0        2       1  Update the NC entry to STALE if in-network
+# failed     0        2       1  Keep the NC entry FAILED if out-of-network
 
 source lib.sh
 ret=0
@@ -20,7 +26,9 @@ HOST_INTF="veth-host"
 ROUTER_INTF="veth-router"
 
 ROUTER_ADDR="2000:20::1"
-HOST_ADDR="2000:20::2"
+HOST_ADDR_IN_NETWORK="2000:20::2"
+HOST_ADDR_OUT_OF_NETWORK="2000:21::2"
+HOST_ADDR="${HOST_ADDR_IN_NETWORK}"
 SUBNET_WIDTH=64
 ROUTER_ADDR_WITH_MASK="${ROUTER_ADDR}/${SUBNET_WIDTH}"
 HOST_ADDR_WITH_MASK="${HOST_ADDR}/${SUBNET_WIDTH}"
@@ -88,6 +96,8 @@ setup()
                 ${ROUTER_CONF}.drop_unsolicited_na=${drop_unsolicited_na}
 	${IP_ROUTER_EXEC} sysctl -qw \
                 ${ROUTER_CONF}.accept_untracked_na=${accept_untracked_na}
+	${IP_ROUTER_EXEC} sysctl -qw \
+                ${ROUTER_CONF}.ndisc_evict_nocarrier=0
 	${IP_ROUTER_EXEC} sysctl -qw ${ROUTER_CONF}.disable_ipv6=0
 	${IP_ROUTER} addr add ${ROUTER_ADDR_WITH_MASK} dev ${ROUTER_INTF}
 
@@ -140,24 +150,77 @@ verify_ndisc() {
 	local drop_unsolicited_na=$1
 	local accept_untracked_na=$2
 	local forwarding=$3
+	local initial_state=${4:-absent}
+	local same_subnet=${5:-1}
+	local neigh_show_output
+	local expected_state
+
+	if [ "${drop_unsolicited_na}" -eq 0 ] &&
+			[ "${forwarding}" -eq 1 ]; then
+		case "${accept_untracked_na}" in
+		1)
+			expected_state=STALE
+			;;
+		2)
+			[ "${same_subnet}" -eq 1 ] && expected_state=STALE
+			;;
+		esac
+	fi
+	if [ -z "${expected_state}" ] &&
+			[ "${initial_state}" = "failed" ]; then
+		expected_state=FAILED
+	fi
 
-	neigh_show_output=$(${IP_ROUTER} neigh show \
-                to ${HOST_ADDR} dev ${ROUTER_INTF} nud stale)
-	if [ ${drop_unsolicited_na} -eq 0 ] && \
-			[ ${accept_untracked_na} -eq 1 ] && \
-			[ ${forwarding} -eq 1 ]; then
-		# Neighbour entry expected to be present for 011 case
-		[[ ${neigh_show_output} ]]
+	if [ -n "${expected_state}" ]; then
+		neigh_show_output=$(${IP_ROUTER} neigh show \
+			to "${HOST_ADDR}" dev "${ROUTER_INTF}")
+		if [[ " ${neigh_show_output} " != \
+				*" ${expected_state} "* ]]; then
+			return 1
+		fi
+		if [ "${initial_state}" = "failed" ]; then
+			[[ "${neigh_show_output}" == *"extern_learn"* ]]
+		fi
 	else
-		# Neighbour entry expected to be absent for all other cases
+		neigh_show_output=$(${IP_ROUTER} neigh show \
+			to "${HOST_ADDR}" dev "${ROUTER_INTF}")
 		[[ -z ${neigh_show_output} ]]
 	fi
 }
 
 test_unsolicited_na_common()
 {
+	local same_subnet=${5:-1}
+	local neigh_show_output
+
+	if [ "${same_subnet}" -eq 1 ]; then
+		HOST_ADDR="${HOST_ADDR_IN_NETWORK}"
+	else
+		HOST_ADDR="${HOST_ADDR_OUT_OF_NETWORK}"
+	fi
+	HOST_ADDR_WITH_MASK="${HOST_ADDR}/${SUBNET_WIDTH}"
+
 	# Setup the test bed, but keep links down
-	setup $1 $2 $3
+	setup "$1" "$2" "$3"
+
+	if [ "${4:-absent}" = "failed" ]; then
+		if ! ${IP_ROUTER} neigh replace "${HOST_ADDR}" \
+				dev "${ROUTER_INTF}" \
+				nud failed extern_learn; then
+			echo "Unable to create NUD_FAILED neighbor entry"
+			return 1
+		fi
+		neigh_show_output=$(${IP_ROUTER} neigh show \
+			to "${HOST_ADDR}" dev "${ROUTER_INTF}")
+		if [[ " ${neigh_show_output} " != *" FAILED "* ]]; then
+			echo "Unable to verify NUD_FAILED neighbor entry"
+			return 1
+		fi
+		if [[ "${neigh_show_output}" != *"extern_learn"* ]]; then
+			echo "Neighbor entry is not externally learned"
+			return 1
+		fi
+	fi
 
 	# Bring the link up, wait for the NA,
 	# and add a delay to ensure neighbour processing is done.
@@ -165,22 +228,35 @@ test_unsolicited_na_common()
 	start_tcpdump
 
 	# Verify the neighbour table
-	verify_ndisc $1 $2 $3
+	verify_ndisc "$1" "$2" "$3" "$4" "${same_subnet}"
 
 }
 
 test_unsolicited_na_combination() {
-	test_unsolicited_na_common $1 $2 $3
+	local initial_state=${4:-absent}
+	local same_subnet=${5:-1}
+	local rc
+
+	test_unsolicited_na_common "$1" "$2" "$3" "${initial_state}" \
+		"${same_subnet}"
+	rc=$?
 	test_msg=("test_unsolicited_na: "
 		"drop_unsolicited_na=$1 "
 		"accept_untracked_na=$2 "
 		"forwarding=$3")
-	log_test $? 0 "${test_msg[*]}"
+	if [ "${initial_state}" = "failed" ]; then
+		test_msg+=("initial_state=failed")
+	fi
+	if [ "$2" -eq 2 ]; then
+		test_msg+=("same_subnet=${same_subnet}")
+	fi
+	log_test "${rc}" 0 "${test_msg[*]}"
 	cleanup
 }
 
 test_unsolicited_na_combinations() {
 	# Args: drop_unsolicited_na accept_untracked_na forwarding
+	#       [initial_state] [same_subnet]
 
 	# Expect entry
 	test_unsolicited_na_combination 0 1 1
@@ -193,6 +269,16 @@ test_unsolicited_na_combinations() {
 	test_unsolicited_na_combination 1 0 1
 	test_unsolicited_na_combination 1 1 0
 	test_unsolicited_na_combination 1 1 1
+
+	# Expect FAILED entry to become STALE
+	test_unsolicited_na_combination 0 1 1 failed
+	test_unsolicited_na_combination 0 2 1 failed 1
+
+	# Expect FAILED entry to remain FAILED
+	test_unsolicited_na_combination 0 0 1 failed
+	test_unsolicited_na_combination 0 1 0 failed
+	test_unsolicited_na_combination 1 1 1 failed
+	test_unsolicited_na_combination 0 2 1 failed 0
 }
 
 ###############################################################################
-- 
2.43.0

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

[Randy Dunlap]

On 9/14/26 10:01 PM, Lawrence Lee wrote:
> Testing:
> - x86_64 current-config and allyesconfig builds with W=1

What does current-config mean?

> - tools/testing/selftests/net/ndisc_unsolicited_na_test.sh: 14 passed
> - Local NIPA checks: all substantive checks passed
> - Sashiko local review completed

thanks.
-- 
~Randy

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

[Lawrence Lee]

On Tue Sep 15, 2026 at 5:26 AM UTC, Randy Dunlap wrote:
>
>
> On 9/14/26 10:01 PM, Lawrence Lee wrote:
> > Testing:
> > - x86_64 current-config and allyesconfig builds with W=1
>
> What does current-config mean?
>
Accidentally forgot to reply-all when responding to Randy before, 
copying my original reply here:

That's referring to my local .config build options. It's essentially 
just the defconfig target but with VETH and Overlay FS enabled. Sorry 
for the confusion.

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

[Ido Schimmel]

On Tue, Sep 15, 2026 at 05:01:31AM +0000, Lawrence Lee wrote:
> 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.

RFC 4861 didn't introduce NDP (RFC 1970 did), so please omit this bit.
But if you're going to mention RFC 4861, then cite 7.3.3 which says that
"If address resolution fails, the entry SHOULD be deleted". The fact
that Linux keeps it as FAILED is an implementation detail and treating
it as untracked is correct from RFC perspective.

> 
> 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(-)

The RFC was:

1 file changed, 11 insertions(+), 2 deletions(-)

I'm not sure how this ballooned to this size...

> 
> 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

The doc update looks OK.

> 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);
>  }

All the changes in this file can be dropped. See below.

>  
>  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,

I'm not sure why you added these lowpan changes to the patch. They are
not described in the commit message. Given that lowpan_ndisc_update()
already has a TODO comment about only handling overrides, I suggest to
ignore it. If needed, it can be modified in the future by someone who
can explain the use case and test the change.

> 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);
>  }

This hunk can be dropped.

>  
>  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
> +     */

Redundant given the comments in the caller and the sysctl documentation.
Simply modify the existing comments below to mention FAILED case.

>  	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;

Try to maintain reverse xmas tree:

https://docs.kernel.org/next/process/maintainer-netdev.html#local-variable-ordering-reverse-xmas-tree-rcs

>  	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.
>  	 */

The comment looks OK.

>  	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;
>  
>  		/*

This can be simplified to:

diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index 90cd5d852569..7d114bdb263e 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -1082,19 +1082,21 @@ static enum skb_drop_reason ndisc_recv_na(struct sk_buff *skb)
 	 * 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;
+	if (!neigh || (READ_ONCE(neigh->nud_state) & NUD_FAILED)) {
+		if (!lladdr || !idev || !READ_ONCE(idev->cnf.forwarding) ||
+		    !accept_untracked_na(idev, saddr)) {
+			if (neigh)
+				neigh_release(neigh);
+			return reason;
 		}
+		if (!neigh)
+			neigh = neigh_create(tbl, &msg->target, dev);
+		new_state = NUD_STALE;
 	}
 
 	if (neigh && !IS_ERR(neigh)) {
 		u8 old_flags = neigh->flags;
 
-		if (READ_ONCE(neigh->nud_state) & NUD_FAILED)
-			goto out;
-
 		/*
 		 * Don't update the neighbor cache entry on a proxy NA from
 		 * ourselves because either the proxied node is off link or it

> @@ -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);

Please drop this hunk. It's only needed because of the unnecessary
lowpan change.

>  
>  		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);
>  		}

What is the reason for this change? It's also not explained in the
commit message and I suspect it's not needed.

>  		reason = SKB_CONSUMED;
>  out:
> -- 
> 2.43.0

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

[Lawrence Lee]

On Wed Sep 16, 2026 at 12:02 PM UTC, Ido Schimmel wrote:
> On Tue, Sep 15, 2026 at 05:01:31AM +0000, Lawrence Lee wrote:
> > 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.
>
> RFC 4861 didn't introduce NDP (RFC 1970 did), so please omit this bit.
> But if you're going to mention RFC 4861, then cite 7.3.3 which says that
> "If address resolution fails, the entry SHOULD be deleted". The fact
> that Linux keeps it as FAILED is an implementation detail and treating
> it as untracked is correct from RFC perspective.
>

That's my mistake, I'll update this to mention that 4861 is just the 
most recent NDP standard and will cite 7.3.3.

> > 
> > 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(-)
>
> The RFC was:
>
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> I'm not sure how this ballooned to this size...
>

Some of the additional size can be attributed to doc/comment changes, 
but a lot of it comes from changes I implemented to address issues found 
by the local Sashiko review I ran. It was definitely an oversight on my 
part to not mention these in either the commit message or comments, 
sorry about that.

> > 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);
> >  }
>
> All the changes in this file can be dropped. See below.
>

This is tied to the lowpan changes below.

> >  
> >  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,
>
> I'm not sure why you added these lowpan changes to the patch. They are
> not described in the commit message. Given that lowpan_ndisc_update()
> already has a TODO comment about only handling overrides, I suggest to
> ignore it. If needed, it can be modified in the future by someone who
> can explain the use case and test the change.
>

I added the lowpan changes after my local Sashiko review run identified 
an issue where a FAILED neighbor can retain an outdated private short 
address if it's moved to STALE by a non-override NA. Happy to drop all 
lowpan-related changes or update comments/commit message to reflect the 
changes, please let me know your preference.

> > 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);
> >  }
>
> This hunk can be dropped.
>

Tied to the lowpan changes above.

> >  
> >  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
> > +     */
>
> Redundant given the comments in the caller and the sysctl documentation.
> Simply modify the existing comments below to mention FAILED case.
>

Will update.

> >  	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;
>
> Try to maintain reverse xmas tree:
>
> https://docs.kernel.org/next/process/maintainer-netdev.html#local-variable-ordering-reverse-xmas-tree-rcs
>

Will fix.

> >  	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;
> >  
> >  		/*
>
> This can be simplified to:
>
> diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
> index 90cd5d852569..7d114bdb263e 100644
> --- a/net/ipv6/ndisc.c
> +++ b/net/ipv6/ndisc.c
> @@ -1082,19 +1082,21 @@ static enum skb_drop_reason ndisc_recv_na(struct sk_buff *skb)
>  	 * 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;
> +	if (!neigh || (READ_ONCE(neigh->nud_state) & NUD_FAILED)) {
> +		if (!lladdr || !idev || !READ_ONCE(idev->cnf.forwarding) ||
> +		    !accept_untracked_na(idev, saddr)) {
> +			if (neigh)
> +				neigh_release(neigh);
> +			return reason;
>  		}
> +		if (!neigh)
> +			neigh = neigh_create(tbl, &msg->target, dev);
> +		new_state = NUD_STALE;
>  	}
>  
>  	if (neigh && !IS_ERR(neigh)) {
>  		u8 old_flags = neigh->flags;
>  
> -		if (READ_ONCE(neigh->nud_state) & NUD_FAILED)
> -			goto out;
> -
>  		/*
>  		 * Don't update the neighbor cache entry on a proxy NA from
>  		 * ourselves because either the proxied node is off link or it
>

Was originally unsure if I should modify the existing logical structure.  
Thanks for the suggestion, will implement this. Is it appropriate to 
credit you with a commit tag?

> >  
> >  		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);
> >  		}
>
> What is the reason for this change? It's also not explained in the
> commit message and I suspect it's not needed.

This was added in response to another Sashiko local review finding.  
Let's say we have some FAILED neighbor T with NTF_ROUTER set. If we get 
an NA from source address S with target address T and with the Router 
bit clear, existing kernel code will cleanup routes with gateway S, but 
IMO we should clean routes with gateway T instead since that is the 
neighbor which was updated by the NA. I can either update 
comments/commit message to reflect this or remove the change entirely, 
please let me know your preference.

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

[Ido Schimmel]

On Wed, Sep 16, 2026 at 05:43:11PM +0000, Lawrence Lee wrote:
> On Wed Sep 16, 2026 at 12:02 PM UTC, Ido Schimmel wrote:
> > I'm not sure why you added these lowpan changes to the patch. They are
> > not described in the commit message. Given that lowpan_ndisc_update()
> > already has a TODO comment about only handling overrides, I suggest to
> > ignore it. If needed, it can be modified in the future by someone who
> > can explain the use case and test the change.
> >
> 
> I added the lowpan changes after my local Sashiko review run identified 
> an issue where a FAILED neighbor can retain an outdated private short 
> address if it's moved to STALE by a non-override NA. Happy to drop all 
> lowpan-related changes or update comments/commit message to reflect the 
> changes, please let me know your preference.

And if the entry is INCOMPLETE and we get a non-override NA?
lowpan_ndisc_update() will ignore it. It's a pre-existing issue and
there is a TODO comment about this:

"/* react on overrides only. TODO check if this is really right. */"

You can mention that it's a pre-existing issue under the '---'. That
way, hopefully, Sashiko will not flag it and it won't clutter the git
log.

[...]

> Was originally unsure if I should modify the existing logical structure.  
> Thanks for the suggestion, will implement this. Is it appropriate to 
> credit you with a commit tag?

No. I will add my R-b tag when appropriate.

[...]

> > What is the reason for this change? It's also not explained in the
> > commit message and I suspect it's not needed.
> 
> This was added in response to another Sashiko local review finding.  
> Let's say we have some FAILED neighbor T with NTF_ROUTER set. If we get 
> an NA from source address S with target address T and with the Router 
> bit clear, existing kernel code will cleanup routes with gateway S, but 
> IMO we should clean routes with gateway T instead since that is the 
> neighbor which was updated by the NA. I can either update 
> comments/commit message to reflect this or remove the change entirely, 
> please let me know your preference.

And if the neighbour was in a different state than FAILED? It seems like
another pre-existing issue. I guess that in practice, most of the time,
the source address of the NA is also the target address in the payload.
You can mention this under the '---'.

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

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 2

- [Medium] net/ipv6/ndisc.c:ndisc_recv_na() now passes two different
  gateway keys to rt6_clean_tohost() for the very same "router -> host"…
- [Low] Reported for completeness only; assessed as resolved within this
  series.
- [Low] The comment block added at net/ipv6/ndisc.c:985-988, at the top
  of accept_untracked_na(), is indented with four spaces instead of…

--

ipv6: update NUD_FAILED neighbors from NA messages

This teaches ndisc_recv_na() to treat a neighbour entry in NUD_FAILED the
same way as a missing entry, so an NA with a target link-layer address
option moves it to STALE on routers when accept_untracked_na is enabled.
The ndisc_ops update callback gains a failed_recovery argument and the
sysctl documentation is reworded accordingly.

Link: https://lore.kernel.org/r/20260813233344.445265-1-lfqlee314@gmail.com

A few questions below.

> diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
> index 90cd5d8525698..84d70c09205a2 100644
> --- a/net/ipv6/ndisc.c
> +++ b/net/ipv6/ndisc.c

[ ... ]

> @@ -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
> +     */
        ^^^^

[Severity: Low]
This isn't a bug, but this comment block in accept_untracked_na() is
indented with spaces while the switch statement right below it uses tabs,
so checkpatch reports "ERROR: code indent should use tabs where possible".

The comment also describes what the caller considers "untracked", based on
the neighbour cache, but accept_untracked_na() only reads the sysctl value
and never sees the neighbour entry. Would it fit better next to

	neigh_untracked = !neigh || neigh_failed;

in ndisc_recv_na(), where the RFC 9131 decision table already documents the
same concept?

>  	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;

[ ... ]

> @@ -1067,32 +1084,41 @@ static enum skb_drop_reason ndisc_recv_na(struct sk_buff *skb)

[ ... ]

>  	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);

[Severity: Low]
Is the new FAILED handling covered by an in-tree test at this point in the
series? As of this commit, ndisc_unsolicited_na_test.sh and
arp_ndisc_untracked_subnets.sh only seed absent entries, so the whole
FAILED to STALE half of the knob, including the mode 2 same-subnet
qualification, would still pass if it were broken.

For the record, this looks resolved later in the same series: the follow-up
patch "selftests: net: test untracked NA recovery of FAILED neighbors"
(7be3b5fcfd3f9adb7489aff8013c6d4beacbf20c) seeds entries with

	ip neigh replace ... nud failed extern_learn

and adds the failed combinations covering every row of the new table,
including the in-prefix and out-of-prefix mode 2 cases. Mentioning it here
only for completeness.

>  
>  	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)

[ ... ]

>  		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);
>  		}

[Severity: Medium]
Why does the gateway address handed to rt6_clean_tohost() now depend on the
previous NUD state of the entry? Before this patch the same "router to
host" event always flushed routes keyed by saddr.

fib6_clean_tohost() in net/ipv6/route.c matches the argument against the
gateway of RA-installed routes and against cached exceptions:

	if (((rt->fib6_flags & RTF_RA_ROUTER) == RTF_RA_ROUTER) &&
	    nh->fib_nh_gw_family && ipv6_addr_equal(gateway, &nh->fib_nh_gw6))
		return -1;

so it expects the address of the node that stopped being a router, which is
unrelated to whether the cache entry happened to be in NUD_FAILED.

When saddr differs from msg->target, and that is allowed since RFC 4861 4.4
only requires the NA source to be an address of the sending interface (a
router advertising a global target from its link-local source, or the proxy
NA case handled a few lines above via pneigh_lookup()), the FAILED recovery
path flushes routes keyed by msg->target while every other path flushes
routes keyed by saddr. Can both be right for the identical event?

If saddr was the wrong key all along, would it make sense to fix that
separately for all cases, with a Fixes: tag, rather than changing it only
for the FAILED path? The changelog describes only the FAILED to STALE
transition and does not mention this change of key.

>  		reason = SKB_CONSUMED;
>  out:

Thanks for looking at these.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1789448374.git.lfqlee314%40gmail.com

Re: [PATCH net-next v2 2/2] selftests: net: test untracked NA recovery of FAILED neighbors

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 2

- [Medium] The new FAILED-seed cases assert an in-place neighbour state
  transition but the script has no synchronization guaranteeing that the…
- [Low] The two new `accept_untracked_na=2` cases
  (`test_unsolicited_na_combination 0 2 1 failed 1` at line 275 and `...…
- [Low] The new FAILED-neighbor recovery cases verify only the resulting
  NUD state and the `extern_learn` marker, not that the link-layer…

--

selftests: net: test untracked NA recovery of FAILED neighbors

This extends ndisc_unsolicited_na_test.sh with a seeded NUD_FAILED
neighbour entry and six new matrix rows covering accept_untracked_na modes
1 and 2 plus the drop_unsolicited_na and forwarding gates. The seed is
marked extern_learn and ndisc_evict_nocarrier is disabled so the entry
survives until the advertisement arrives.

Link: https://lore.kernel.org/r/20260813233344.445265-1-lfqlee314@gmail.com

A few questions below about the synchronization and the coverage of the new
rows.

> diff --git a/tools/testing/selftests/net/ndisc_unsolicited_na_test.sh b/tools/testing/selftests/net/ndisc_unsolicited_na_test.sh
> index 5db69dad0cfc0..5f4f29f0ac4ab 100755
> --- a/tools/testing/selftests/net/ndisc_unsolicited_na_test.sh
> +++ b/tools/testing/selftests/net/ndisc_unsolicited_na_test.sh

[ ... ]

> @@ -20,7 +26,9 @@ HOST_INTF="veth-host"
>  ROUTER_INTF="veth-router"
>  
>  ROUTER_ADDR="2000:20::1"
> -HOST_ADDR="2000:20::2"
> +HOST_ADDR_IN_NETWORK="2000:20::2"
> +HOST_ADDR_OUT_OF_NETWORK="2000:21::2"
> +HOST_ADDR="${HOST_ADDR_IN_NETWORK}"
>  SUBNET_WIDTH=64
>  ROUTER_ADDR_WITH_MASK="${ROUTER_ADDR}/${SUBNET_WIDTH}"
>  HOST_ADDR_WITH_MASK="${HOST_ADDR}/${SUBNET_WIDTH}"

[ ... ]

> @@ -140,24 +150,77 @@ verify_ndisc() {

[ ... ]

> +	if [ -n "${expected_state}" ]; then
> +		neigh_show_output=$(${IP_ROUTER} neigh show \
> +			to "${HOST_ADDR}" dev "${ROUTER_INTF}")
> +		if [[ " ${neigh_show_output} " != \
> +				*" ${expected_state} "* ]]; then
> +			return 1
> +		fi
> +		if [ "${initial_state}" = "failed" ]; then
> +			[[ "${neigh_show_output}" == *"extern_learn"* ]]
> +		fi

[Severity: Low]
Should verify_ndisc() also check the link-layer address that the
advertisement carried?

The seed in test_unsolicited_na_common() deliberately creates the entry
without one:

	${IP_ROUTER} neigh replace "${HOST_ADDR}" dev "${ROUTER_INTF}" \
		nud failed extern_learn

while the recovery path in net/ipv6/ndisc.c:ndisc_recv_na() pulls the target
link-layer address out of the options and passes it down:

	lladdr = ndisc_opt_addr_data(ndopts.nd_opts_tgt_lladdr, dev);
	...
	__ndisc_update(dev, neigh, lladdr,
		       new_state, update_flags, neigh_failed,
		       NDISC_NEIGHBOUR_ADVERTISEMENT, &ndopts);

Since the only assertions here are the state substring and extern_learn,
would a change that moves FAILED -> STALE but installs no lladdr, or an
incorrect one, still report OK?  Comparing the resulting lladdr against the
host veth MAC would close that gap.

>  	else
> -		# Neighbour entry expected to be absent for all other cases
> +		neigh_show_output=$(${IP_ROUTER} neigh show \
> +			to "${HOST_ADDR}" dev "${ROUTER_INTF}")
>  		[[ -z ${neigh_show_output} ]]
>  	fi
>  }
>  
>  test_unsolicited_na_common()
>  {
> +	local same_subnet=${5:-1}
> +	local neigh_show_output
> +
> +	if [ "${same_subnet}" -eq 1 ]; then
> +		HOST_ADDR="${HOST_ADDR_IN_NETWORK}"
> +	else
> +		HOST_ADDR="${HOST_ADDR_OUT_OF_NETWORK}"
> +	fi
> +	HOST_ADDR_WITH_MASK="${HOST_ADDR}/${SUBNET_WIDTH}"

[Severity: Low]
Can this switch actually isolate the mode 2 in-prefix gate?

Moving HOST_ADDR from 2000:20::2 to 2000:21::2 changes the source address of
the advertisement, its target address, and the key of the seeded neighbour
entry all at once, while the router keeps 2000:20::1/64.

The advertisement under test comes from addrconf_dad_completed(), and
ndisc_send_na() uses the same address for both fields when it is local:

net/ipv6/ndisc.c:ndisc_send_na() {
	ifp = ipv6_get_ifaddr(dev_net(dev), solicited_addr, dev, 1);
	if (ifp) {
		src_addr = solicited_addr;
	...
		.target = *solicited_addr,
}

so saddr == target for every packet this test generates.  The kernel gate is
source based:

net/ipv6/ndisc.c:accept_untracked_na() {
	case 2:
		return !!ipv6_chk_prefix(saddr, idev->dev);
}

and Documentation/networking/ip-sysctl.rst describes mode 2 in terms of "the
source IP address".  Would a change that tested msg->target instead of saddr
leave both new mode 2 rows green?  Keeping HOST_ADDR fixed and instead
varying the prefix configured on the router side would exercise the gate
without also moving the target and the entry key.

>  
>  	# Setup the test bed, but keep links down
> -	setup $1 $2 $3
> +	setup "$1" "$2" "$3"
> +
> +	if [ "${4:-absent}" = "failed" ]; then
> +		if ! ${IP_ROUTER} neigh replace "${HOST_ADDR}" \
> +				dev "${ROUTER_INTF}" \
> +				nud failed extern_learn; then
> +			echo "Unable to create NUD_FAILED neighbor entry"
> +			return 1
> +		fi

[ ... ]

> @@ -165,22 +228,35 @@ test_unsolicited_na_common()
>  	# Bring the link up, wait for the NA,
>  	# and add a delay to ensure neighbour processing is done.
>  	link_up
>  	start_tcpdump
>  
>  	# Verify the neighbour table
> -	verify_ndisc $1 $2 $3
> +	verify_ndisc "$1" "$2" "$3" "$4" "${same_subnet}"

[Severity: Medium]
The comment says "add a delay to ensure neighbour processing is done", but
link_up, start_tcpdump and verify_ndisc run back to back with no sleep and no
retry.  Where is the delay?

start_tcpdump() blocks on:

	${IP_ROUTER_EXEC} timeout 15s \
		tcpdump --immediate-mode -tpni ${ROUTER_INTF} -c 1 \
		"icmp6 && icmp6[0] == 136 && src ${HOST_ADDR}"

but the AF_PACKET tap that satisfies -c 1 is fed from ptype_all delivery in
__netif_receive_skb_core(), which runs ahead of the state change the test
looks at:

ipv6_rcv() -> icmpv6_rcv() -> ndisc_recv_na() -> __ndisc_update() ->
neigh_update()

Does tcpdump exiting really order against that update, given verify_ndisc()
takes a single un-retried snapshot of ip neigh show?

There is a second effect for the new rows.  In the four expect-FAILED cases:

	test_unsolicited_na_combination 0 0 1 failed
	test_unsolicited_na_combination 0 1 0 failed
	test_unsolicited_na_combination 1 1 1 failed
	test_unsolicited_na_combination 0 2 1 failed 0

the expected value FAILED is exactly the state the entry was seeded with, so
"still FAILED" and "the advertisement was never processed" look identical.
Would a bounded poll for the expected state, or an explicit settle, make
these rows able to distinguish the two?

Related question on ordering of the capture: tcpdump is started after
link_up, so can the single unsolicited advertisement emitted at DAD
completion be missed if the fork and exec are slow?  In that case timeout
exits 124 while set -e is in effect inside start_tcpdump, which aborts the
whole script rather than failing one row.  Would arming the capture before
link_up avoid that?

>  
>  }

[ ... ]

> @@ -193,6 +269,16 @@ test_unsolicited_na_combinations() {
>  	test_unsolicited_na_combination 1 0 1
>  	test_unsolicited_na_combination 1 1 0
>  	test_unsolicited_na_combination 1 1 1
> +
> +	# Expect FAILED entry to become STALE
> +	test_unsolicited_na_combination 0 1 1 failed
> +	test_unsolicited_na_combination 0 2 1 failed 1
> +
> +	# Expect FAILED entry to remain FAILED
> +	test_unsolicited_na_combination 0 0 1 failed
> +	test_unsolicited_na_combination 0 1 0 failed
> +	test_unsolicited_na_combination 1 1 1 failed
> +	test_unsolicited_na_combination 0 2 1 failed 0
>  }

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1789448374.git.lfqlee314%40gmail.com

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

[Lawrence Lee]

On Thu Sep 17, 2026 at 2:37 PM UTC, Ido Schimmel wrote:
> On Wed, Sep 16, 2026 at 05:43:11PM +0000, Lawrence Lee wrote:
> > I added the lowpan changes after my local Sashiko review run 
> > identified an issue where a FAILED neighbor can retain an outdated 
> > private short address if it's moved to STALE by a non-override NA. 
> > Happy to drop all lowpan-related changes or update comments/commit 
> > message to reflect the changes, please let me know your preference.
>
> And if the entry is INCOMPLETE and we get a non-override NA?
> lowpan_ndisc_update() will ignore it. It's a pre-existing issue and
> there is a TODO comment about this:
>
> "/* react on overrides only. TODO check if this is really right. */"
>
> You can mention that it's a pre-existing issue under the '---'. That
> way, hopefully, Sashiko will not flag it and it won't clutter the git
> log.
>
> > This was added in response to another Sashiko local review finding.  
> > Let's say we have some FAILED neighbor T with NTF_ROUTER set. If we 
> > get an NA from source address S with target address T and with the 
> > Router bit clear, existing kernel code will cleanup routes with 
> > gateway S, but IMO we should clean routes with gateway T instead 
> > since that is the neighbor which was updated by the NA. I can either 
> > update comments/commit message to reflect this or remove the change 
> > entirely, please let me know your preference.
>
> And if the neighbour was in a different state than FAILED? It seems like
> another pre-existing issue. I guess that in practice, most of the time,
> the source address of the NA is also the target address in the payload.
> You can mention this under the '---'.

I will remove both of these changes, add comments below the '---' as 
suggested, and resubmit.

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

[Lawrence Lee]

On Thu Sep 17, 2026 at 5:02 PM UTC,  wrote:

> > diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
> > index 90cd5d8525698..84d70c09205a2 100644
> > --- a/net/ipv6/ndisc.c
> > +++ b/net/ipv6/ndisc.c
>
> [ ... ]
>
> > @@ -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
> > +     */
>         ^^^^
>
> [Severity: Low]
> This isn't a bug, but this comment block in accept_untracked_na() is
> indented with spaces while the switch statement right below it uses tabs,
> so checkpatch reports "ERROR: code indent should use tabs where possible".
>
> The comment also describes what the caller considers "untracked", based on
> the neighbour cache, but accept_untracked_na() only reads the sysctl value
> and never sees the neighbour entry. Would it fit better next to
>
> 	neigh_untracked = !neigh || neigh_failed;
>
> in ndisc_recv_na(), where the RFC 9131 decision table already documents the
> same concept?
This comment will be removed as per maintainer feedback.


> [Severity: Low]
> Is the new FAILED handling covered by an in-tree test at this point in the
> series? As of this commit, ndisc_unsolicited_na_test.sh and
> arp_ndisc_untracked_subnets.sh only seed absent entries, so the whole
> FAILED to STALE half of the knob, including the mode 2 same-subnet
> qualification, would still pass if it were broken.
>
> For the record, this looks resolved later in the same series: the follow-up
> patch "selftests: net: test untracked NA recovery of FAILED neighbors"
> (7be3b5fcfd3f9adb7489aff8013c6d4beacbf20c) seeds entries with
>
> 	ip neigh replace ... nud failed extern_learn
>
> and adds the failed combinations covering every row of the new table,
> including the in-prefix and out-of-prefix mode 2 cases. Mentioning it here
> only for completeness.
>
The tests are added in the next patch of this series

> [Severity: Medium]
> Why does the gateway address handed to rt6_clean_tohost() now depend on the
> previous NUD state of the entry? Before this patch the same "router to
> host" event always flushed routes keyed by saddr.
>
> fib6_clean_tohost() in net/ipv6/route.c matches the argument against the
> gateway of RA-installed routes and against cached exceptions:
>
> 	if (((rt->fib6_flags & RTF_RA_ROUTER) == RTF_RA_ROUTER) &&
> 	    nh->fib_nh_gw_family && ipv6_addr_equal(gateway, &nh->fib_nh_gw6))
> 		return -1;
>
> so it expects the address of the node that stopped being a router, which is
> unrelated to whether the cache entry happened to be in NUD_FAILED.
>
> When saddr differs from msg->target, and that is allowed since RFC 4861 4.4
> only requires the NA source to be an address of the sending interface (a
> router advertising a global target from its link-local source, or the proxy
> NA case handled a few lines above via pneigh_lookup()), the FAILED recovery
> path flushes routes keyed by msg->target while every other path flushes
> routes keyed by saddr. Can both be right for the identical event?
>
> If saddr was the wrong key all along, would it make sense to fix that
> separately for all cases, with a Fixes: tag, rather than changing it only
> for the FAILED path? The changelog describes only the FAILED to STALE
> transition and does not mention this change of key.
This change will be removed in the next version of the series as 
discussed in another thread: 
https://lore.kernel.org/all/DLHV5GF21AY4.3GAK241ZI2UR8@gmail.com/