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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250218202241.3b0cf52c@kmaincent-XPS-13-7390>
Date: Tue, 18 Feb 2025 20:22:41 +0100
From: Kory Maincent <kory.maincent@...tlin.com>
To: Ido Schimmel <idosch@...dia.com>
Cc: netdev@...r.kernel.org, davem@...emloft.net, kuba@...nel.org,
 pabeni@...hat.com, edumazet@...gle.com, horms@...nel.org,
 donald.hunter@...il.com, dsahern@...nel.org, petrm@...dia.com,
 gnault@...hat.com
Subject: Re: [PATCH net-next 5/8] net: fib_rules: Enable port mask usage

On Tue, 18 Feb 2025 20:15:49 +0200
Ido Schimmel <idosch@...dia.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@...dia.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@...dia.com>
> > > Signed-off-by: Ido Schimmel <idosch@...dia.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