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

[Ido Schimmel <idosch@nvidia.com>]

On Fri, Jun 13, 2025 at 10:23:26AM +0200, Daniel Borkmann wrote:
> Hi Ido,

Hi Daniel,

> 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".

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

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.

 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);