[PATCH] ipv6: check per-interface proxy_ndp in forwarding and NA paths

Adriano Cordova posted 1 patch 6 days, 7 hours ago
net/ipv6/ip6_output.c | 4 ++--
net/ipv6/ndisc.c      | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
[PATCH] ipv6: check per-interface proxy_ndp in forwarding and NA paths
Posted by Adriano Cordova 6 days, 7 hours ago
proxy_ndp can be enabled per interface, with net.ipv6.conf.all.proxy_ndp
as a global default. ndisc_recv_ns() checks both, but ip6_forward() and
ndisc_recv_na() check only the global value. With the per-interface proxy
ndp set and the global one left at 0, the router answers proxy NS but
does not pass NDP messages to the proxied target.

Check both values at these two sites, as ndisc_recv_ns() already does.

Fixes: fbea49e1e240 ("[IPV6] NDISC: Add proxy_ndp sysctl.")
Signed-off-by: Adriano Cordova <adrianox@gmail.com>
---
 net/ipv6/ip6_output.c | 4 ++--
 net/ipv6/ndisc.c      | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 550965058991..738fa46a892a 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -581,8 +581,8 @@ int ip6_forward(struct sk_buff *skb)
 		return -ETIMEDOUT;
 	}
 
-	/* XXX: idev->cnf.proxy_ndp? */
-	if (READ_ONCE(net->ipv6.devconf_all->proxy_ndp) &&
+	if ((READ_ONCE(net->ipv6.devconf_all->proxy_ndp) ||
+	     (idev && READ_ONCE(idev->cnf.proxy_ndp))) &&
 	    pneigh_lookup(&nd_tbl, net, &hdr->daddr, skb->dev)) {
 		int proxied = ip6_forward_proxy_check(skb);
 
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index 75515fd99383..f787082069b7 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -1097,9 +1097,9 @@ static enum skb_drop_reason ndisc_recv_na(struct sk_buff *skb)
 		 */
 		if (lladdr && !memcmp(lladdr, dev->dev_addr, dev->addr_len) &&
 		    READ_ONCE(net->ipv6.devconf_all->forwarding) &&
-		    READ_ONCE(net->ipv6.devconf_all->proxy_ndp) &&
+		    (READ_ONCE(net->ipv6.devconf_all->proxy_ndp) ||
+		     (idev && READ_ONCE(idev->cnf.proxy_ndp))) &&
 		    pneigh_lookup(&nd_tbl, net, &msg->target, dev)) {
-			/* XXX: idev->cnf.proxy_ndp */
 			goto out;
 		}
 
-- 
2.51.0
Re: [PATCH] ipv6: check per-interface proxy_ndp in forwarding and NA paths
Posted by netdev-bot+sashiko@kernel.org 2 days, 7 hours ago
Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

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

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] Incomplete alignment in ndisc_recv_na(): the patch makes the
  proxy_ndp half of the proxy-NA suppression test per-interface aware…
- [Medium] In ip6_forward() the newly added per-interface test reads
  `idev->cnf.proxy_ndp`, where `idev` is derived from `IP6CB(skb)->iif`…

--

ipv6: check per-interface proxy_ndp in forwarding and NA paths

This makes ip6_forward() and ndisc_recv_na() consider the per-interface
proxy_ndp setting in addition to the global net.ipv6.conf.all.proxy_ndp,
matching what ndisc_recv_ns() already does.

> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index 550965058991..738fa46a892a 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -581,8 +581,8 @@ int ip6_forward(struct sk_buff *skb)
>  		return -ETIMEDOUT;
>  	}
>  
> -	/* XXX: idev->cnf.proxy_ndp? */
> -	if (READ_ONCE(net->ipv6.devconf_all->proxy_ndp) &&
> +	if ((READ_ONCE(net->ipv6.devconf_all->proxy_ndp) ||
> +	     (idev && READ_ONCE(idev->cnf.proxy_ndp))) &&
>  	    pneigh_lookup(&nd_tbl, net, &hdr->daddr, skb->dev)) {

[Severity: Medium]
Is the device used for the new per-interface read the same one used by the
lookup on the next line?  In ip6_forward(), idev comes from IP6CB(skb)->iif:

net/ipv6/ip6_output.c:ip6_forward() {
	idev = __in6_dev_get_safely(dev_get_by_index_rcu(net, IP6CB(skb)->iif));
	...
}

while pneigh_lookup() is called with skb->dev.

IP6CB(skb)->iif is recorded in ip6_rcv_core() before the l3mdev receive
handler runs:

net/ipv6/ip6_input.c:ip6_rcv_core() {
	IP6CB(skb)->iif = skb_valid_dst(skb) ?
				ip6_dst_idev(skb_dst(skb))->dev->ifindex :
				dev->ifindex;
}

and vrf_ip6_rcv() then replaces skb->dev without rewriting IP6CB->iif:

drivers/net/vrf.c:vrf_ip6_rcv() {
	if (!is_ndisc) {
		struct net_device *orig_dev = skb->dev;

		dev_dstats_rx_add(vrf_dev, skb->len);
		skb->dev = vrf_dev;
		skb->skb_iif = vrf_dev->ifindex;
	...
}

The device-preserving NDISC branch is chosen by ipv6_ndisc_frame(), which
only matches when iph->nexthdr == NEXTHDR_ICMP, with the in-file comment
noting "This really fails for multiple next protocols (e.g., NEXTHDR_HOP).
But it is a start."

So for non-NDP traffic to a proxied address, or an NS/NA behind a
hop-by-hop or destination-option header, idev is the slave while skb->dev
is the VRF master.  pneigh_lookup() only matches the requested device or a
wildcard entry:

net/core/neighbour.c:pneigh_lookup() {
		if (!memcmp(n->key, pkey, key_len) &&
		    net_eq(pneigh_net(n), net) &&
		    (n->dev == dev || !n->dev))
	...
}

Can this end up consulting proxy_ndp on one interface while looking up
proxy entries on another?  In that case the master's own proxy_ndp is
ignored (the packet is forwarded with the hop limit decremented instead of
going to ip6_input(), and the link-local dst_link_failure() path in
ip6_forward_proxy_check() is skipped), and conversely a slave's flag can
enable the proxy check against a wildcard entry on the master.

Both ndisc.c call sites use skb->dev for the per-device read and for the
lookup, and the removed "/* XXX: idev->cnf.proxy_ndp? */" comment sat
directly above the skb->dev based lookup.  Would reading
__in6_dev_get(skb->dev) here (or doing the lookup on the iif device) be
more consistent?

> diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
> index 75515fd99383..f787082069b7 100644
> --- a/net/ipv6/ndisc.c
> +++ b/net/ipv6/ndisc.c
> @@ -1097,9 +1097,9 @@ static enum skb_drop_reason ndisc_recv_na(struct sk_buff *skb)
>  		 */
>  		if (lladdr && !memcmp(lladdr, dev->dev_addr, dev->addr_len) &&
>  		    READ_ONCE(net->ipv6.devconf_all->forwarding) &&
> -		    READ_ONCE(net->ipv6.devconf_all->proxy_ndp) &&
> +		    (READ_ONCE(net->ipv6.devconf_all->proxy_ndp) ||
> +		     (idev && READ_ONCE(idev->cnf.proxy_ndp))) &&
>  		    pneigh_lookup(&nd_tbl, net, &msg->target, dev)) {
> -			/* XXX: idev->cnf.proxy_ndp */
>  			goto out;
>  		}

[Severity: Medium]
Should the forwarding test here be made per-interface too?  The condition
still short-circuits on devconf_all->forwarding, so with global forwarding
at 0 the newly added per-interface proxy_ndp term is never evaluated.

The site the changelog names as the reference uses the per-device value:

net/ipv6/ndisc.c:ndisc_recv_ns() {
		if (ipv6_chk_acast_addr(net, dev, &msg->target) ||
		    (READ_ONCE(idev->cnf.forwarding) &&
		     (READ_ONCE(net->ipv6.devconf_all->proxy_ndp) ||
		      READ_ONCE(idev->cnf.proxy_ndp)) &&
	...
}

and ndisc_recv_na() itself uses the per-device value a little earlier for
the RFC 9131 decision:

	if (!neigh && lladdr && idev && READ_ONCE(idev->cnf.forwarding)) {

ip6_forward() also admits forwarding on the per-device flag alone:

net/ipv6/ip6_output.c:ip6_forward() {
	if (!READ_ONCE(net->ipv6.devconf_all->forwarding) &&
	    (!idev || !READ_ONCE(idev->cnf.force_forwarding)))
		goto error;
}

with Documentation/networking/ip-sysctl.rst describing force_forwarding as
"Enable forwarding on this interface only -- regardless of the setting on
``conf/all/forwarding``".

For conf/all/forwarding=0, conf/<dev>/forwarding=1,
conf/<dev>/force_forwarding=1, conf/all/proxy_ndp=0 and
conf/<dev>/proxy_ndp=1 with a proxy neigh entry, ndisc_recv_ns() answers
the proxy NS and ip6_forward() now takes the proxy path, but this test
falls through to ndisc_update() for a proxy NA carrying the router's own
MAC, which is what the "Don't update the neighbor cache entry on a proxy NA
from ourselves" comment is guarding against.

For an entry that is not NUD_VALID, __neigh_update() installs the supplied
lladdr without the override restriction:

net/core/neighbour.c:__neigh_update() {
	if (old & NUD_VALID) {
		if (lladdr != neigh->ha && !(flags & NEIGH_UPDATE_F_OVERRIDE)) {
	...
}

so can the proxied target's entry end up holding the router's own MAC in
that configuration?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260918151405.716075-1-adrianox%40gmail.com
Re: [PATCH] ipv6: check per-interface proxy_ndp in forwarding and NA paths
Posted by Ido Schimmel 4 days, 16 hours ago
On Fri, Sep 18, 2026 at 12:14:05PM -0300, Adriano Cordova wrote:
> proxy_ndp can be enabled per interface, with net.ipv6.conf.all.proxy_ndp
> as a global default. ndisc_recv_ns() checks both, but ip6_forward() and
> ndisc_recv_na() check only the global value. With the per-interface proxy
> ndp set and the global one left at 0, the router answers proxy NS but
> does not pass NDP messages to the proxied target.
> 
> Check both values at these two sites, as ndisc_recv_ns() already does.
> 
> Fixes: fbea49e1e240 ("[IPV6] NDISC: Add proxy_ndp sysctl.")
> Signed-off-by: Adriano Cordova <adrianox@gmail.com>

Do you have an actual use case for this? Changing a 20 years old user
visible behavior for the sole reason of removing a TODO comment is not
good practice.
Re: [PATCH] ipv6: check per-interface proxy_ndp in forwarding and NA paths
Posted by Adriano Córdova 4 days, 9 hours ago
It does not hit any of the current selftests, but it would be possible to write
one that would currently not proxy NDPs, I think the following config in the
current code:

net.ipv6.conf.all.forwarding=1
net.ipv6.conf.eth0.proxy_ndp=1     # all.proxy_ndp == 0
ip -6 neigh add proxy 2001:db8::100 dev eth0

makes the initial multicast NS work but the subsequent unicast NS do not.


El dom, 20 sept 2026 a las 3:34, Ido Schimmel (<idosch@nvidia.com>) escribió:
>
> On Fri, Sep 18, 2026 at 12:14:05PM -0300, Adriano Cordova wrote:
> > proxy_ndp can be enabled per interface, with net.ipv6.conf.all.proxy_ndp
> > as a global default. ndisc_recv_ns() checks both, but ip6_forward() and
> > ndisc_recv_na() check only the global value. With the per-interface proxy
> > ndp set and the global one left at 0, the router answers proxy NS but
> > does not pass NDP messages to the proxied target.
> >
> > Check both values at these two sites, as ndisc_recv_ns() already does.
> >
> > Fixes: fbea49e1e240 ("[IPV6] NDISC: Add proxy_ndp sysctl.")
> > Signed-off-by: Adriano Cordova <adrianox@gmail.com>
>
> Do you have an actual use case for this? Changing a 20 years old user
> visible behavior for the sole reason of removing a TODO comment is not
> good practice.
Re: [PATCH] ipv6: check per-interface proxy_ndp in forwarding and NA paths
Posted by Ido Schimmel 4 days, 6 hours ago
On Sun, Sep 20, 2026 at 10:46:24AM -0300, Adriano Córdova wrote:
> It does not hit any of the current selftests, but it would be possible to write
> one that would currently not proxy NDPs, I think the following config in the
> current code:
> 
> net.ipv6.conf.all.forwarding=1
> net.ipv6.conf.eth0.proxy_ndp=1     # all.proxy_ndp == 0
> ip -6 neigh add proxy 2001:db8::100 dev eth0
> 
> makes the initial multicast NS work but the subsequent unicast NS do not.

From the above I understand that you don't have a use case for this
patch and that you didn't even test it ("I think"). As such, I'm not
going to spend any more time on this submission. Also, please don't top
post.

Thanks
Re: [PATCH] ipv6: check per-interface proxy_ndp in forwarding and NA paths
Posted by Hangbin Liu 4 days, 20 hours ago
On Fri, Sep 18, 2026 at 12:14:05PM -0300, Adriano Cordova wrote:
> proxy_ndp can be enabled per interface, with net.ipv6.conf.all.proxy_ndp
> as a global default. ndisc_recv_ns() checks both, but ip6_forward() and
> ndisc_recv_na() check only the global value. With the per-interface proxy
> ndp set and the global one left at 0, the router answers proxy NS but
> does not pass NDP messages to the proxied target.
> 
> Check both values at these two sites, as ndisc_recv_ns() already does.
> 
> Fixes: fbea49e1e240 ("[IPV6] NDISC: Add proxy_ndp sysctl.")
> Signed-off-by: Adriano Cordova <adrianox@gmail.com>
> ---
>  net/ipv6/ip6_output.c | 4 ++--
>  net/ipv6/ndisc.c      | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index 550965058991..738fa46a892a 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -581,8 +581,8 @@ int ip6_forward(struct sk_buff *skb)
>  		return -ETIMEDOUT;
>  	}
>  
> -	/* XXX: idev->cnf.proxy_ndp? */
> -	if (READ_ONCE(net->ipv6.devconf_all->proxy_ndp) &&
> +	if ((READ_ONCE(net->ipv6.devconf_all->proxy_ndp) ||
> +	     (idev && READ_ONCE(idev->cnf.proxy_ndp))) &&
>  	    pneigh_lookup(&nd_tbl, net, &hdr->daddr, skb->dev)) {
>  		int proxied = ip6_forward_proxy_check(skb);
>  
> diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
> index 75515fd99383..f787082069b7 100644
> --- a/net/ipv6/ndisc.c
> +++ b/net/ipv6/ndisc.c
> @@ -1097,9 +1097,9 @@ static enum skb_drop_reason ndisc_recv_na(struct sk_buff *skb)
>  		 */
>  		if (lladdr && !memcmp(lladdr, dev->dev_addr, dev->addr_len) &&
>  		    READ_ONCE(net->ipv6.devconf_all->forwarding) &&
> -		    READ_ONCE(net->ipv6.devconf_all->proxy_ndp) &&
> +		    (READ_ONCE(net->ipv6.devconf_all->proxy_ndp) ||
> +		     (idev && READ_ONCE(idev->cnf.proxy_ndp))) &&
>  		    pneigh_lookup(&nd_tbl, net, &msg->target, dev)) {
> -			/* XXX: idev->cnf.proxy_ndp */
>  			goto out;
>  		}
>  
> -- 
> 2.51.0
> 

LGTM

Reviewed-by: Hangbin Liu <liuhangbin@kylinos.cn>