Re: [PATCH] net: ipmr_base: Check iif when returning a (*, G) MFC

[Nicolas Dichtel <nicolas.dichtel@6wind.com>]

Le 02/11/2023 à 12:48, Yang Sun a écrit :
>> Is this a regression (doesn't seem that way)? If not, the change should
>> be targeted at net-next which is closed right now:
> 
>> https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html
> 
> I see.
> 
>>> - if (c->mfc_un.res.ttls[vifi] < 255)
>>> + if (c->mfc_parent == vifi && c->mfc_un.res.ttls[vifi] < 255)
> 
>> What happens if the route doesn't have an iif (-1)? It won't match
>> anymore?
> 
> Looks like the mfc_parent can't be -1? There is the check:
>     if (mfc->mf6cc_parent >= MAXMIFS)
>         return -ENFILE;
> before setting the parent:
>     c->_c.mfc_parent = mfc->mf6cc_parent;
> 
> I wrote this patch thinking (*, G) MFCs could be per iif, similar to the
> (S, G) MFCs, like we can add the following MFCs to forward packets from
> any address with group destination ff05::aa from if1 to if2, and forward
> packets from any address with group destination ff05::aa from if2 to
> both if1 and if3.
> 
> (::, ff05::aa)      Iif: if1 Oifs: if1 if2  State: resolved
> (::, ff05::aa)      Iif: if2 Oifs: if1 if2 if3  State: resolved
> 
> But reading Nicolas's initial commit message again, it seems to me that
> (*, G) has to be used together with (*, *) and there should be only one
> (*, G) entry per group address and include all relevant interfaces in
> the oifs? Like the following:
> 
> (::, ::)         Iif: if1 Oifs: if1 if2 if3   State: resolved
> (::, ff05::aa)   Iif: if1 Oifs: if1 if2 if3   State: resolved
> 
> Is this how the (*, *|G) MFCs are intended to be used? which means packets
> to ff05::aa are forwarded from any one of the interfaces to all the other
> interfaces? If this is the intended way it works then my patch would break
> things and should be rejected.
Yes, this was the intend. Only one (*, G) entry was expected (per G).

> 
> Is there a way to achieve the use case I described above? Like having
> different oifs for different iif?
Instead of being too strict, maybe you could try to return the 'best' entry.

#1 (::, ff05::aa)      Iif: if1 Oifs: if1 if2  State: resolved
#2 (::, ff05::aa)      Iif: if2 Oifs: if1 if2 if3  State: resolved

If a packet comes from if2, returns #2, but if a packet comes from if3, returns
the first matching entry, ie #1 here.


Regards,
Nicolas