Re: [PATCH net-next] net: ipv4: replace the right route in case prefsrc is used

[Antoine Tenart <atenart@kernel.org>]

Quoting Julian Anastasov (2023-11-17 17:56:57)
> On Fri, 17 Nov 2023, Antoine Tenart wrote:
> > 
> > Note: a selftest wasn't added as `ip route` use NLM_F_EXCL which
> > prevents us from constructing the above routes. But this is a valid
> 
> ip route append/prepend are standard way to create alternative routes,
> if you want to encode a selftest.

Thanks for the pointer.

> > +                     if (cfg->fc_prefsrc &&
> > +                         cfg->fc_prefsrc == fa->fa_info->fib_prefsrc)
> 
>         You may prefer to restrict it for the change operation by
> adding && (cfg->fc_nlflags & NLM_F_REPLACE) check, otherwise if
> we change the prepend position (fa_first) route with such prefsrc
> can not be installed as first one:

Good point, I'll fix in v2 (if any).

>         Even if we consider just the change operation, this patch
> will change the expectation that we replace the first alternative
> route. But I don't know if one uses alternative routes that
> differ in prefsrc.

For example NetworkManager does, but I don't think NLM_F_REPLACE is
used. One could do it manually, but I don't think that is used in
practice either.

>         Note that link routes (nhc_scope RT_SCOPE_HOST) or
> routes with prefixlen!=0 (fib_select_path) are not considered
> as alternatives by the kernel. So, even if we can create such
> routes, they are probably not used. So, deleting link routes
> by prefsrc is good as we remove routes with deleted prefsrc
> but for routing we are using just the first link route.

Right. Replacing routes by prefsrc could help configuring those routes
so they have the right config when the first link route is deleted, but
that's theoretical too. Reasoning here was since we can delete by
prefsrc, we might want to allow to replace too. As you said, there is a
good reason to allow this for route deletion which might not extend to
the change operation.

Thanks!
Antoine