netdev - RE: [PATCH net-next 1/2] devlink: Add support to set port function as trusted

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]

Date:   Thu, 13 Jan 2022 03:37:47 +0000
From:   Parav Pandit <parav@...dia.com>
To:     Jakub Kicinski <kuba@...nel.org>
CC:     Sunil Sudhakar Rani <sunrani@...dia.com>,
        Saeed Mahameed <saeedm@...dia.com>,
        Jiri Pirko <jiri@...dia.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        Bodong Wang <bodong@...dia.com>
Subject: RE: [PATCH net-next 1/2] devlink: Add support to set port function as
 trusted



> From: Jakub Kicinski <kuba@...nel.org>
> Sent: Thursday, January 13, 2022 6:06 AM
> 
> On Wed, 12 Jan 2022 04:40:01 +0000 Parav Pandit wrote:
> > > > It's the capability that is turned on/off.
> > > > A device is composed based on what is needed. ipsec offload is not
> always needed.
> > > > Its counter intuitive to expose some low level hardware resource to
> disable ipsec indirectly.
> > > > So it is better to do as capability/param rather than some resource.
> > > > It is capability is more than just resource.
> > >
> > > Wouldn't there be some limitation on the number of SAs or max
> > > throughput or such to limit on VF hogging the entire crypto path?
> >
> > The fairness among VFs is present via the QoS knobs. Hence it doesn't hogg
> the entire crypto path.
> 
> Why do you want to disable it, then?
Each enabled feature consumes 
(a) driver level memory resource such as querying ip sec capabilities and more later,
(b) time in querying those capabilities, 
(c) device level initialization in supporting this capability

So for light weight devices which doesn't need it we want to keep it disabled.

> 
> > > I was expecting such a knob, and then turning it to 0 would
> > > effectively remove the capability (FW can completely hide it or driver
> ignore it).
> > >
> > > > May be. But it is in use at [1] for a long time now.
> > > >
> > > > [1] docker run -p 9090:9090 prom/prometheus
> > >
> > > How is it "in use" if we haven't merged the patch to enable it? :)
> > > What does it monitor? PHYs port does not include east-west traffic,
> > > exposing just the PHYs stats seems like a half measure.
> >
> > Containerized monitors are in use by running in monitor in same net ns of
> the PF having full access to the PF.
> > The monitor is interested in physical port counters related to link transitions,
> link errors, buffer overruns etc.
> 
> I don't think we should support this use case. VFs and PFs are not the same
> thing.
> 
> > > > Not sure I follow you.
> > > > Netdev of a mlx5 function talks to the driver internal steering
> > > > API in addition to other drivers operating this mlx5 function.
> > >
> > > But there is no such thing as "steering API" in netdev. We can
> > > expose the functionality we do have, if say PTP requires some
> > > steering then enabling PTP implies the required steering is enabled.
> "steering API"
> > > as an entity is meaningless to a netdev user.
> > It is the internal mlx5 implementation of how to do steering, triggered by
> netdev ndo's and other devices callback.
> > There are multiple options on how steering is done.
> > Such as sw_steering or dev managed steering.
> > There is already a control knob to choose sw vs dev steering as devlink
> param on the PF at [1].
> > This [1] device specific param is only limited to PF. For VFs, HV need to
> enable/disable this capability on selected VF.
> > API wise nothing drastic is getting added here, it's only on different object.
> (instead of device, it is port function).
> >
> > [1]
> > https://www.kernel.org/doc/html/v5.8/networking/device_drivers/mellano
> > x/mlx5.html#devlink-parameters
> 
> Ah, that thing. IIRC this was added for TC offloads, VFs don't own the eswitch
> so what rules are they inserting to require "high insertion rate"? My suspicion
> is that since it's not TC it'd be mostly for the "DR" feature you have hence my
> comment on it not being netdev.
No it is limited to tc offloads.
A VF netdev inserts flow steering rss rules on nic rx table.
This also uses the same smfs/dmfs when a VF is capable to do so.