Re: [PATCH v2 2/2] bonding: reuse neigh_setup from slave neigh_parms

[Kuniyuki Iwashima]

On Fri, Jul 3, 2026 at 2:47 AM Paritosh Potukuchi
<paritoshpotukuchi@xxxxxxxxx> wrote:
>
> Hi Kuniyuki,
>
> >This introduces O(n) list traversal while it can be done
> >with fixed costs (3 dereferences + 1 call).
>
> >Since neigh_table is global (arp_tbl or nd_tbl), the O(n)
> >list traversal could take longer and rather de-optimise.
>
> Yes, that is true. One reason why I chose to do that is
> because ndo_neigh_setup is a function primarily meant
> to setup the neigh_parms structure, when neigh_parms does
> not exist.
> On the other hand, lookup_neigh_parms is meant
> to search for a near-complete neigh_parms structure,
> that is already associated with a netdev.
> Even if we want to use ndo_neigh_setup, since it takes
> less time, I would suggest using it as a fallback to
> not finding an already existing parms, setup.
>
> Moreover, time complexity might not be an issue in this
> path since, this is rarely used aggresively.

The real user is qeth_l3_main.c only and it's not compiled
on 99% host.

We usually bail out at if (!slave_ops->ndo_neigh_setup),
which is called after your neigh_parms_lookup_dev(), and
there is no need to do O(n) traversal.

With 2K netns, it could incur unnecessary 4K traversal (lo
+ another dev) for each neigh creation, which is done under
RTNL or from interrupt context.

So, it will be a problem.

>
>
> One issue with ndo_neigh_setup in bond-like devices is
> that, to get the underlying netdevs neigh_setup function,
> it expects us to pass a dummy neigh_parms structure that
> has been zeroed out. This seems to be fragile as suggested
> in a TODO in bond_neigh_init().
> Generally its main goal is to fill the parms.neigh_setup
> field.
>
> Can we populate a few more fields in the zeroed-
> out parms structure, before passing to the driver in
> ndo_neigh_setup? That seems to be a much safer approach.

It does not help.  Just populating fields does not change the
loop detection logic.