Message-ID: <20220120004039.qriwo4vrvizz7qry@sx1>
Date:   Wed, 19 Jan 2022 16:40:39 -0800
From:   Saeed Mahameed <saeedm@...dia.com>
To:     Parav Pandit <parav@...dia.com>
Cc:     Jakub Kicinski <kuba@...nel.org>,
        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

On 19 Jan 05:49, Parav Pandit wrote:
>
>> 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.
>>

delegating.

>> > 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.
>

I believe flexibility can be achieved with some FW message? Parav can you
investigate ? To be clear here the knob must be specific to sw_steering
exposed as memory resource.

>> > >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.
>>

doable, although can be confusing. 

>> 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?
>>

No :), well defined enums, similar to devlink params. but yes we need a
clear cut of what is vendor specific and what's not.

>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