Message-ID: <PH0PR12MB5481978B796DC00AF681FAC0DC599@PH0PR12MB5481.namprd12.prod.outlook.com>
Date:   Wed, 19 Jan 2022 05:49:35 +0000
From:   Parav Pandit <parav@...dia.com>
To:     Jakub Kicinski <kuba@...nel.org>,
        Saeed Mahameed <saeedm@...dia.com>
CC:     Saeed Mahameed <saeed@...nel.org>,
        Sunil Sudhakar Rani <sunrani@...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: Wednesday, January 19, 2022 5:46 AM
> 
> On Tue, 18 Jan 2022 14:33:28 -0800 Saeed Mahameed wrote:
> > On 18 Jan 10:02, Jakub Kicinski wrote:
> > >On Fri, 14 Jan 2022 22:15:48 -0800 Saeed Mahameed wrote:
> > >> I think the term privilege is misused here, due to the global knob
> > >> proposed initially. Anyway the issue is exactly as I explained
> > >> above, SW steering requires FW pre-allocated resources and
> > >> initializations, for VFs it is disabled since there was no demand for it and
> FW wanted to save on resources.
> > >>
> > >> Now as SW steering is catching up with FW steering in terms of
> > >> functionality, people want it also on VFs to help with rule
> > >> insertion rate for use cases other than switchdev and TC, e.g TLS,
> > >> connection tracking, etc ..
> > >
> > >Sorry long weekend here, thanks for the explanation!
> > >
> > >Where do we stand? Are you okay with an explicit API for enabling /
> > >disabling VF features? If SMFS really is about conntrack and TLS
> > >maybe
> >
> > I am as skeptical as you are. But what other options do we have ? It's
> > a fact that "Smart" VFs have different use-cases and customization is
> > necessary to allow full scalability and better system resource
> > utilization.
> >
> > As you already said, PTP for instance makes total sense as a VF
> > feature knob
> 
> To be clear when I was talking about PTP initially I was thinking about real PTP
> clocks. "Modern" NICs sometimes do shenanigans in the FW to pretend they
> have more clocks that they really have.
> There is a difference between delegating the PHC to the VF and allowing the
> VF to use some SW pretend clock. I'm not sure which camp your PTP falls into.
> 
> > for the same reason I would say any standard stateful feature/offloads
> > (e.g Crypto) also deserve own knobs.
> >
> > If we agree on the need for a VF customization API, I would use one
> > API for all features. Having explicit enable/disable API for some then
> > implicit resources re-size API for other features is a bit confusing.
> >
> > e.g.
> >
> > # Enable ptp on specific vf
> > devlink port function <port idx> set feature PTP ON/OFF
> >
> > # disable TLS on specific vf
> > devlink resource set <DEV> TLS size 0
> >
> > And I am pretty sure resource API is not yet available for port
> > functions (e.g before VF instantiation, which is one of the main
> > points of this RFC, so some plumbing is necessary to expose resource API for
> port functions.
> >
> > TBH, I actually like your resources idea, i would like to explore that
> > more with Parav, see what we can do about it ..
> 
> Right, that'd be great, although I'd imagine if the resource is very flexible (e.g.
> memory) delegating N bytes to a function does not tell the device how to
> perform the "diet". Obviously that's pure speculation I don't know how things
> work on your SmartNIC :)
>
Right, we at least need to tell fw that only X bytes are allowed for sw_steering diet.
And _right_ amount of X bytes specific for sw_steering was not very clear.
Hence the on/off resource knob looked more doable and abtract.

I do agree you and Saeed that instead of port function param, port function resource is more suitable here even though its bool.
 
> > >it can be implied by the delegation of appropriate bits meaningful to
> > >netdev world?
> >
> > I don't get this point, netdev bits are known only after the VF has
> > been fully initialized.
> 
> I meant this as a simple starting point to enumerate the features.
> It was an off-cuff suggestion, really. Reusing some approximation of existing
> bits with clear code-driven semantics is simpler than defining and
> documenting new ones.
> 
> We can start a new enum.
> 
> I hope you didn't mean "PTP" to be a string carried all the way to the driver in
> your example command?
>
Yet to sync with Saeed, but I think it will be a enum + string during resource registration time.
For generic features, enum and string are defined by devlink core.
For smfs kind of rare knob, enum and string is supplied by driver.

Powered by blists - more mailing lists