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

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

quoted

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

[ ... ]

quoted

@@ -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/ (local)