Message-ID: <20220118223328.tq5kopdrit5frvap@sx1>
Date:   Tue, 18 Jan 2022 14:33:28 -0800
From:   Saeed Mahameed <saeedm@...dia.com>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     Saeed Mahameed <saeed@...nel.org>, Parav Pandit <parav@...dia.com>,
        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 18 Jan 10:02, Jakub Kicinski wrote:
>On Fri, 14 Jan 2022 22:15:48 -0800 Saeed Mahameed wrote:
>> On 14 Jan 18:34, Jakub Kicinski wrote:
>> >> HV is composing the device before giving it to the VM.
>> >> VM can always disable certain feature if it doesn't want to use by ethtool or other means.
>> >> But here we are discussing about offering/not offering the feature to the VF from HV.
>> >> HV can choose to not offer certain features based on some instruction received from orchestration.
>> >
>> >I'm still missing why go thru orchestration and HV rather than making
>> >the driver load more clever to avoid wasting time on initializing
>> >unnecessary caps.
>>
>> unfortunately for "smartnics" of this era, many of these initilizations
>> and resources are only manged by FW and the details are hidden away from
>> drivers, we need the knobs to tell the FW, hey we don't need all of these
>> features for this particular vf, save the resources for something else.
>> After all VF users need only a small portion of all the features we offer
>> to them, but again unfortunately the FW pre-allocates precious HW
>> resources to allow such features per VFs.
>>
>> I know in this case smartnic === dumb FW, and sometimes there is no way
>> around it, this is the hw arch we have currently, not everything is a
>> nice generic flexible resources, not when it has to be wrapped with FW
>> "__awesome__" logic ;), and for proper virtualization we need this FW.
>>
>> But i totally agree with your point, when we can limit with resources, we
>> should limit with resources, otherwise we need a knob to communicate to FW
>> what is the user intention for this VF.
>>
>> >> Privilege.
>> >> VFs have SMFS today, but by default it is disabled. The proposed knob will enable it.
>> >
>> >Could you rephrase? What does it mean that VFs have SMFS but it's
>> >disabled? Again - privilege means security, I'd think that it can't have
>> >security implications if you're freely admitting that it's exposed.
>>
>> 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, 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 .. 

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

And sometimes users want TLS without the optimization of SMFS, so as a vendor
driver maintainer i would prefer having control knobs per feature, instead of
maintaining some weird driver feature discovery and brokerage logic..

Powered by blists - more mailing lists