netdev - Re: [PATCH net-next] neighbour: add support for NUD

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <DAOUOYHC2G4F.2SOAYIUMZV3QY@gmail.com>
Date: Tue, 17 Jun 2025 15:42:42 +0200
From: "Nicolas Escande" <nico.escande@...il.com>
To: "Paolo Abeni" <pabeni@...hat.com>, <davem@...emloft.net>,
 <edumazet@...gle.com>, <kuba@...nel.org>
Cc: <netdev@...r.kernel.org>, <decot+git@...gle.com>
Subject: Re: [PATCH net-next] neighbour: add support for NUD_PERMANENT proxy
 entries

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@...il.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