Re: [PATCH net-next 5/8] net: fib_rules: Enable port mask usage

[Kory Maincent <kory.maincent@bootlin.com>]

On Tue, 18 Feb 2025 20:15:49 +0200
Ido Schimmel <idosch@nvidia.com> wrote:

> On Tue, Feb 18, 2025 at 06:15:23PM +0100, Kory Maincent wrote:
> > On Mon, 17 Feb 2025 15:41:06 +0200
> > Ido Schimmel <idosch@nvidia.com> wrote:
> >   
> > > Allow user space to configure FIB rules that match on the source and
> > > destination ports with a mask, now that support has been added to the
> > > FIB rule core and the IPv4 and IPv6 address families.
> > > 
> > > Reviewed-by: Petr Machata <petrm@nvidia.com>
> > > Signed-off-by: Ido Schimmel <idosch@nvidia.com>
> > > ---
> > >  net/core/fib_rules.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c
> > > index ba6beaa63f44..5ddd34cbe7f6 100644
> > > --- a/net/core/fib_rules.c
> > > +++ b/net/core/fib_rules.c
> > > @@ -843,8 +843,8 @@ static const struct nla_policy
> > > fib_rule_policy[FRA_MAX + 1] = { [FRA_DSCP]	=
> > > NLA_POLICY_MAX(NLA_U8, INET_DSCP_MASK >> 2), [FRA_FLOWLABEL] = { .type =
> > > NLA_BE32 }, [FRA_FLOWLABEL_MASK] = { .type = NLA_BE32 },
> > > -	[FRA_SPORT_MASK] = { .type = NLA_REJECT },
> > > -	[FRA_DPORT_MASK] = { .type = NLA_REJECT },
> > > +	[FRA_SPORT_MASK] = { .type = NLA_U16 },
> > > +	[FRA_DPORT_MASK] = { .type = NLA_U16 },
> > >  };  
> > 
> > I don't get the purpose of this patch and patch 1.
> > Couldn't you have patch 3 and 4 first, then patch 2 that adds the netlink
> > and UAPI support?  
> 
> Current order is:
> 
> 1. Add attributes as REJECT.
> 2. Add support in core.
> 3. Add support in IPv4.
> 4. Add support in IPv6.
> 5. Expose feature to user space.
> 
> Looks straight forward and easy to review to me and that's the order I
> prefer.

Ok, it is surprising to me. If there is an issue in patch 2,3 or 4. git
bisect will locate patch 5 and it won't be easy to find the real patch that
cause the issue. Having this type of patch series in the git history will harder
the issue debugging.
I was not am not a net maintainer so I won't complain more and will let them
decide.

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com