netdev - Re: [PATCH net-next 1/2] neighbor: Add NTF_EXT_VALIDATED flag for externally validated entries

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: <cc84011a-0170-42c6-8e85-d789551beab6@iogearbox.net>
Date: Tue, 17 Jun 2025 16:21:18 +0200
From: Daniel Borkmann <daniel@...earbox.net>
To: Ido Schimmel <idosch@...dia.com>
Cc: netdev@...r.kernel.org, davem@...emloft.net, kuba@...nel.org,
 pabeni@...hat.com, edumazet@...gle.com, horms@...nel.org,
 donald.hunter@...il.com, petrm@...dia.com, razor@...ckwall.org
Subject: Re: [PATCH net-next 1/2] neighbor: Add NTF_EXT_VALIDATED flag for
 externally validated entries

Hi Ido,

On 6/17/25 4:09 PM, Ido Schimmel wrote:
> On Fri, Jun 13, 2025 at 10:23:26AM +0200, Daniel Borkmann wrote:
>> On 6/11/25 4:15 PM, Ido Schimmel wrote:
>>> In the above scheme, when the control plane (e.g., FRR) advertises a
>>> neighbor entry with a proxy indication, it expects the corresponding
>>> entry in the data plane (i.e., the kernel) to remain valid and not be
>>> removed due to garbage collection. The control plane also expects the
>>> kernel to notify it if the entry was learned locally (i.e., became
>>> "reachable") so that it will remove the proxy indication from the EVPN
>>> MAC/IP advertisement route. That is why these entries cannot be
>>> programmed with dummy states such as "permanent" or "noarp".
>>
>> Meaning, in contrast to "permanent" the initial user-provided lladdr
>> can still be updated by the kernel if it learned that there was a
>> migration, right?
> 
> Yes. In addition, user space will be notified when the kernel locally
> learns the entry. FRR installs such entries as "stale" and a
> notification will be emitted when they transition to "reachable".

Thanks for clarifying, perhaps makes sense to mention this bit in the
commit message also in v2.

>>> Instead, add a new neighbor flag ("extern_valid") which indicates that
>>> the entry was learned and determined to be valid externally and should
>>> not be removed or invalidated by the kernel. The kernel can probe the
>>> entry and notify user space when it becomes "reachable". However, if the
>>> kernel does not receive a confirmation, have it return the entry to the
>>> "stale" state instead of the "failed" state.
>>>
>>> In other words, an entry marked with the "extern_valid" flag behaves
>>> like any other dynamically learned entry other than the fact that the
>>> kernel cannot remove or invalidate it.
>>
>> How is the expected neigh_flush_dev() behavior? I presume in that case if
>> the neigh entry is in use and was NUD_STALE then we go into NUD_NONE state
>> right? (Asking as NUD_PERMANENT skips all that and whether that should be
>> similar or not for NTF_EXT_VALIDATED?)
> 
> Currently, unlike "permanent" entries, such entries will be flushed when
> the interface loses its carrier. Given the description of "[...] behaves
> like any other dynamically learned entry other than the fact that the
> kernel cannot remove or invalidate it" I think it makes sense to not
> flush such entries when the carrier goes down.

Yeah agree given the user intent that would make sense imho otherwise
you'd need some additional watcher to then reinstall if that happens.

> Like "permanent" entries, such entries will be flushed when the
> interface is put administratively down or when its MAC changes, both of
> which are user initiated actions.
> 
> IOW, I will squash the following diff and add test cases.

lgtm!

>   static void neigh_flush_dev(struct neigh_table *tbl, struct net_device *dev,
> -                           bool skip_perm)
> +                           bool skip_perm_ext_valid)
>   {
>          struct hlist_head *dev_head;
>          struct hlist_node *tmp;
> @@ -378,7 +388,9 @@ static void neigh_flush_dev(struct neigh_table *tbl, struct net_device *dev,
>          dev_head = neigh_get_dev_table(dev, tbl->family);
>   
>          hlist_for_each_entry_safe(n, tmp, dev_head, dev_list) {
> -               if (skip_perm && n->nud_state & NUD_PERMANENT)
> +               if (skip_perm_ext_valid &&
> +                   (n->nud_state & NUD_PERMANENT ||
> +                    n->flags & NTF_EXT_VALIDATED))
>                          continue;
>   
>                  hlist_del_rcu(&n->hash);
> @@ -419,10 +431,10 @@ void neigh_changeaddr(struct neigh_table *tbl, struct net_device *dev)
>   EXPORT_SYMBOL(neigh_changeaddr);
>   
>   static int __neigh_ifdown(struct neigh_table *tbl, struct net_device *dev,
> -                         bool skip_perm)
> +                         bool skip_perm_ext_valid)
>   {
>          write_lock_bh(&tbl->lock);
> -       neigh_flush_dev(tbl, dev, skip_perm);
> +       neigh_flush_dev(tbl, dev, skip_perm_ext_valid);
>          pneigh_ifdown_and_unlock(tbl, dev);
>          pneigh_queue_purge(&tbl->proxy_queue, dev ? dev_net(dev) : NULL,
>                             tbl->family);

Thanks,
Daniel