netdev - Re: [PATCH net-next 2/3] net: Add SRIOV VGT+ support

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: <20191030175909.7ab24da6@cakuba.netronome.com>
Date:   Wed, 30 Oct 2019 17:59:09 -0700
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Saeed Mahameed <saeedm@...lanox.com>
Cc:     Ariel Levkovich <lariel@...lanox.com>,
        "dsahern@...il.com" <dsahern@...il.com>,
        "sbrivio@...hat.com" <sbrivio@...hat.com>,
        "nikolay@...ulusnetworks.com" <nikolay@...ulusnetworks.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "gerlitz.or@...il.com" <gerlitz.or@...il.com>,
        "sd@...asysnail.net" <sd@...asysnail.net>,
        Jiri Pirko <jiri@...lanox.com>
Subject: Re: [PATCH net-next 2/3] net: Add SRIOV VGT+ support

On Wed, 30 Oct 2019 22:50:06 +0000, Saeed Mahameed wrote:
> On Wed, 2019-10-30 at 14:34 -0700, Jakub Kicinski wrote:
> > On Wed, 30 Oct 2019 20:44:44 +0000, Ariel Levkovich wrote:  
> > > Signed-off-by: Ariel Levkovich <lariel@...lanox.com>
> > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > > index 3207e0b..da79976 100644
> > > --- a/include/linux/netdevice.h
> > > +++ b/include/linux/netdevice.h
> > > @@ -1067,6 +1067,10 @@ struct netdev_name_node {
> > >   *      Hash Key. This is needed since on some devices VF share
> > > this information
> > >   *      with PF and querying it may introduce a theoretical
> > > security risk.
> > >   * int (*ndo_set_vf_rss_query_en)(struct net_device *dev, int vf,
> > > bool setting);
> > > + * int (*ndo_add_vf_vlan_trunk_range)(struct net_device *dev, int
> > > vf,
> > > + *				      u16 start_vid, u16 end_vid,
> > > __be16 proto);
> > > + * int (*ndo_del_vf_vlan_trunk_range)(struct net_device *dev, int
> > > vf,
> > > + *				      u16 start_vid, u16 end_vid,
> > > __be16 proto);
> > >   * int (*ndo_get_vf_port)(struct net_device *dev, int vf, struct
> > > sk_buff *skb);
> > >   * int (*ndo_setup_tc)(struct net_device *dev, enum tc_setup_type
> > > type,
> > >   *		       void *type_data);
> > > @@ -1332,6 +1336,14 @@ struct net_device_ops {
> > >  	int			(*ndo_set_vf_rss_query_en)(
> > >  						   struct net_device
> > > *dev,
> > >  						   int vf, bool
> > > setting);
> > > +	int			(*ndo_add_vf_vlan_trunk_range)(
> > > +						   struct net_device
> > > *dev,
> > > +						   int vf, u16
> > > start_vid,
> > > +						   u16 end_vid, __be16
> > > proto);
> > > +	int			(*ndo_del_vf_vlan_trunk_range)(
> > > +						   struct net_device
> > > *dev,
> > > +						   int vf, u16
> > > start_vid,
> > > +						   u16 end_vid, __be16
> > > proto);
> > >  	int			(*ndo_setup_tc)(struct net_device *dev,
> > >  						enum tc_setup_type
> > > type,
> > >  						void *type_data);  
> > 
> > Is this official Mellanox patch submission or do you guys need time
> > to
> > decide between each other if you like legacy VF ndos or not? ;-)  
> 
> It is official :), as much as we want to move away from legacy mode, we
> do still have two major customers that are not quite ready yet to move
> to switchdev mode. the silver-lining here is that they are welling to
> move to upstream kernel (advanced distros), but we need this feature in
> legacy mode.

So they are willing to update the kernel, just not willing to move the
orchestration to the new way of doing things? Sounds familiar :(

> The ability to configure per VF ACL tables vlan filters is a must.
> 
> I tried to think of an API where we can expose the whole VF ACL tables
> to users and let them configure it the way they want with TC flower
> maybe (sort of hybrid legacy-switchdev mode that can act only on VF ACL
> tables but not on the FDB). The problem with this is that it can easily
> conflict with VST/trust mode and other settings that can be done via
> legacy VF ndos... so i guess the complexity of such API is not worthy
> and a simple vlan list filter API is more natural for legacy sriov ?!

The "we don't want any more legacy VF ndos" policy which I think we
wanted to follow is much easier to stick to than "we don't want any
more legacy VF ndos, unless..".

There's nothing here that can't be done in switchdev mode (perhaps
bridge offload would actually be more suitable than just flower),
and the uAPI extension is not an insignificant one.

I don't think we should be growing both legacy and switchdev APIs, at
some point we got to pick one. The switchdev extension to set hwaddr
for which patches were posted recently had been implemented through
legacy API a while ago (by Chelsio IIRC) so it's not that we're looking
towards switchdev where legacy API is impossible to extend. It's purely
a policy decision to pick one and deprecate the other.

Only if we freeze the legacy API completely will the orchestration
layers have an incentive to support switchdev. And we can save the few
hundred lines of code per feature in every driver..