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

["Nicolas Escande" <nico.escande@gmail.com>]

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