netdev - Re: [RFC PATCH net-next 05/22] nexthop: Add nexthop notification data structures

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] [day] [month] [year] [list]

Message-ID: <20200911145020.GB3160975@shredder>
Date:   Fri, 11 Sep 2020 17:50:20 +0300
From:   Ido Schimmel <idosch@...sch.org>
To:     David Ahern <dsahern@...il.com>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, kuba@...nel.org,
        roopa@...dia.com, mlxsw@...dia.com,
        Ido Schimmel <idosch@...dia.com>
Subject: Re: [RFC PATCH net-next 05/22] nexthop: Add nexthop notification
 data structures

On Tue, Sep 08, 2020 at 08:43:10AM -0600, David Ahern wrote:
> On 9/8/20 3:10 AM, Ido Schimmel wrote:
> > From: Ido Schimmel <idosch@...dia.com>
> > 
> > Add data structures that will be used for nexthop replace and delete
> > notifications in the previously introduced nexthop notification chain.
> > 
> > New data structures are added instead of passing the existing nexthop
> > code structures directly for several reasons.
> > 
> > First, the existing structures encode a lot of bookkeeping information
> > which is irrelevant for listeners of the notification chain.
> > 
> > Second, the existing structures can be changed without worrying about
> > introducing regressions in listeners since they are not accessed
> > directly by them.
> > 
> > Third, listeners of the notification chain do not need to each parse the
> > relatively complex nexthop code structures. They are passing the
> > required information in a simplified way.
> 
> agreed. My preference is for only nexthop.{c,h} to understand and parse
> the nexthop structs.
> 
> 
> > 
> > Signed-off-by: Ido Schimmel <idosch@...dia.com>
> > ---
> >  include/net/nexthop.h | 35 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 35 insertions(+)
> > 
> > diff --git a/include/net/nexthop.h b/include/net/nexthop.h
> > index 2e44efe5709b..0bde1aa867c0 100644
> > --- a/include/net/nexthop.h
> > +++ b/include/net/nexthop.h
> > @@ -109,6 +109,41 @@ enum nexthop_event_type {
> >  	NEXTHOP_EVENT_DEL
> >  };
> >  
> > +struct nh_notifier_single_info {
> > +	struct net_device *dev;
> > +	u8 gw_family;
> > +	union {
> > +		__be32 ipv4;
> > +		struct in6_addr ipv6;
> > +	};
> > +	u8 is_reject:1,
> > +	   is_fdb:1,
> > +	   is_encap:1;
> 
> use has_encap since it refers to a configuration of a nexthop versus a
> nexthop type.

Will change.

> 
> I take it this is a placeholder until lwt offload is supported?

Yes, I will mention this in the commit message. I didn't bother parsing
all the encap configuration into the struct since no listener is going
to look at it. Only added this bit so that listeners could reject
nexthops that perform encapsulation.

> 
> besides the naming nit,
> 
> Reviewed-by: David Ahern <dsahern@...il.com>

Thanks for the prompt review, David!