[PATCH net-next] neighbour: add support for NUD_PERMANENT proxy entries

[PATCH net-next] neighbour: add support for NUD_PERMANENT proxy entries

[Nicolas Escande]

As discussesd in [0] proxy entries (which are more configuration than
runtime data) should stay when the link goes does down (carrier wise).
This is what happens for regular neighbour entries added manually.

So lets fix this by:
  - storing in the proxy entries the mdn_state (only NUD_PERMANENT for now)
  - not removing NUD_PERMANENT proxy entries on carrier down by adding a
    skip_perm arg to pneigh_ifdown_and_unlock() (same as how it's done in
    neigh_flush_dev() for regular non-proxy entries)

Link: https://lore.kernel.org/netdev/c584ef7e-6897-01f3-5b80-12b53f7b4bf4@kernel.org/ [0]
Signed-off-by: Nicolas Escande <nico.escande@gmail.com>
---
 include/net/neighbour.h |  1 +
 net/core/neighbour.c    | 13 ++++++++++---
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/include/net/neighbour.h b/include/net/neighbour.h
index 9a832cab5b1d..d1e05b39cbb1 100644
--- a/include/net/neighbour.h
+++ b/include/net/neighbour.h
@@ -182,6 +182,7 @@ struct pneigh_entry {
 	netdevice_tracker	dev_tracker;
 	u32			flags;
 	u8			protocol;
+	u8			state;
 	u32			key[];
 };
 
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 49dce9a82295..419f2f984d64 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -54,7 +54,8 @@ static void __neigh_notify(struct neighbour *n, int type, int flags,
 			   u32 pid);
 static void neigh_update_notify(struct neighbour *neigh, u32 nlmsg_pid);
 static int pneigh_ifdown_and_unlock(struct neigh_table *tbl,
-				    struct net_device *dev);
+				    struct net_device *dev,
+				    bool skip_perm);
 
 #ifdef CONFIG_PROC_FS
 static const struct seq_operations neigh_stat_seq_ops;
@@ -423,7 +424,7 @@ static int __neigh_ifdown(struct neigh_table *tbl, struct net_device *dev,
 {
 	write_lock_bh(&tbl->lock);
 	neigh_flush_dev(tbl, dev, skip_perm);
-	pneigh_ifdown_and_unlock(tbl, dev);
+	pneigh_ifdown_and_unlock(tbl, dev, skip_perm);
 	pneigh_queue_purge(&tbl->proxy_queue, dev ? dev_net(dev) : NULL,
 			   tbl->family);
 	if (skb_queue_empty_lockless(&tbl->proxy_queue))
@@ -803,7 +804,8 @@ int pneigh_delete(struct neigh_table *tbl, struct net *net, const void *pkey,
 }
 
 static int pneigh_ifdown_and_unlock(struct neigh_table *tbl,
-				    struct net_device *dev)
+				    struct net_device *dev,
+				    bool skip_perm)
 {
 	struct pneigh_entry *n, **np, *freelist = NULL;
 	u32 h;
@@ -811,12 +813,15 @@ static int pneigh_ifdown_and_unlock(struct neigh_table *tbl,
 	for (h = 0; h <= PNEIGH_HASHMASK; h++) {
 		np = &tbl->phash_buckets[h];
 		while ((n = *np) != NULL) {
+			if (skip_perm && n->state & NUD_PERMANENT)
+				goto skip;
 			if (!dev || n->dev == dev) {
 				*np = n->next;
 				n->next = freelist;
 				freelist = n;
 				continue;
 			}
+skip:
 			np = &n->next;
 		}
 	}
@@ -1972,6 +1977,7 @@ static int neigh_add(struct sk_buff *skb, struct nlmsghdr *nlh,
 	if (tb[NDA_PROTOCOL])
 		protocol = nla_get_u8(tb[NDA_PROTOCOL]);
 	if (ndm_flags & NTF_PROXY) {
+		u8 state = ndm->ndm_state & NUD_PERMANENT;
 		struct pneigh_entry *pn;
 
 		if (ndm_flags & NTF_MANAGED) {
@@ -1983,6 +1989,7 @@ static int neigh_add(struct sk_buff *skb, struct nlmsghdr *nlh,
 		pn = pneigh_lookup(tbl, net, dst, dev, 1);
 		if (pn) {
 			pn->flags = ndm_flags;
+			pn->state = state;
 			if (protocol)
 				pn->protocol = protocol;
 			err = 0;
-- 
2.49.0

Re: [PATCH net-next] neighbour: add support for NUD_PERMANENT proxy entries

[Paolo Abeni]

On 6/13/25 3:46 PM, Nicolas Escande wrote:
> As discussesd in [0] proxy entries (which are more configuration than
> runtime data) should stay when the link goes does down (carrier wise).
> This is what happens for regular neighbour entries added manually.
> 
> So lets fix this by:
>   - storing in the proxy entries the mdn_state (only NUD_PERMANENT for now)
>   - not removing NUD_PERMANENT proxy entries on carrier down by adding a
>     skip_perm arg to pneigh_ifdown_and_unlock() (same as how it's done in
>     neigh_flush_dev() for regular non-proxy entries)
> 
> Link: https://lore.kernel.org/netdev/c584ef7e-6897-01f3-5b80-12b53f7b4bf4@kernel.org/ [0]
> Signed-off-by: Nicolas Escande <nico.escande@gmail.com>
> ---
>  include/net/neighbour.h |  1 +
>  net/core/neighbour.c    | 13 ++++++++++---
>  2 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/include/net/neighbour.h b/include/net/neighbour.h
> index 9a832cab5b1d..d1e05b39cbb1 100644
> --- a/include/net/neighbour.h
> +++ b/include/net/neighbour.h
> @@ -182,6 +182,7 @@ struct pneigh_entry {
>  	netdevice_tracker	dev_tracker;
>  	u32			flags;
>  	u8			protocol;
> +	u8			state;

I think it's better to be consistent: either store the full state (u16,
without masking) or a `permanent` boolean alike: !!(ndm->ndm_state &
NUD_PERMANENT).

The current choice could confuse who is going to touch this code in the
future.

Thanks,

Paolo

Re: [PATCH net-next] neighbour: add support for NUD_PERMANENT proxy entries

[Nicolas Escande]

On Tue Jun 17, 2025 at 3:25 PM CEST, Paolo Abeni wrote:
> On 6/13/25 3:46 PM, Nicolas Escande wrote:
>> As discussesd in [0] proxy entries (which are more configuration than
>> runtime data) should stay when the link goes does down (carrier wise).
>> This is what happens for regular neighbour entries added manually.
>> 
>> So lets fix this by:
>>   - storing in the proxy entries the mdn_state (only NUD_PERMANENT for now)
>>   - not removing NUD_PERMANENT proxy entries on carrier down by adding a
>>     skip_perm arg to pneigh_ifdown_and_unlock() (same as how it's done in
>>     neigh_flush_dev() for regular non-proxy entries)
>> 
>> Link: https://lore.kernel.org/netdev/c584ef7e-6897-01f3-5b80-12b53f7b4bf4@kernel.org/ [0]
>> Signed-off-by: Nicolas Escande <nico.escande@gmail.com>
>> ---
>>  include/net/neighbour.h |  1 +
>>  net/core/neighbour.c    | 13 ++++++++++---
>>  2 files changed, 11 insertions(+), 3 deletions(-)
>> 
>> diff --git a/include/net/neighbour.h b/include/net/neighbour.h
>> index 9a832cab5b1d..d1e05b39cbb1 100644
>> --- a/include/net/neighbour.h
>> +++ b/include/net/neighbour.h
>> @@ -182,6 +182,7 @@ struct pneigh_entry {
>>  	netdevice_tracker	dev_tracker;
>>  	u32			flags;
>>  	u8			protocol;
>> +	u8			state;
>
> I think it's better to be consistent: either store the full state (u16,
> without masking) or a `permanent` boolean alike: !!(ndm->ndm_state &
> NUD_PERMANENT).

Sure I'll spin a v2 with a 'permanent' boolean then.

>
> The current choice could confuse who is going to touch this code in the
> future.
>
> Thanks,
>
> Paolo

Thanks for the review,

Nico