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

[Saeed Mahameed <saeed@kernel.org>]

On 14 Jan 18:34, Jakub Kicinski wrote:
>On Fri, 14 Jan 2022 04:52:24 +0000 Parav Pandit wrote:
>> > > Each enabled feature consumes
>> > > (a) driver level memory resource such as querying ip sec capabilities and more later,
>> > > (b) time in querying those capabilities,
>> >
>> > These are on the VM's side, it's not hypervisors responsibility to help the client
>> > by stripping features.
>> >
>> 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.

>
>> > > (c) device level initialization in supporting this capability
>> > >
>> > > So for light weight devices which doesn't need it we want to keep it disabled.
>> >
>> > You need to explain this better. We are pretty far from "trust"
>> > settings, which are about privilege and not breaking isolation.
>>
>> We split the abstract trust to more granular settings, some related to privilege and some to capabilities.
>>
>> > "device level initialization" tells me nothing.
>> >
>> Above one belongs to capabilities bucket. Sw_steering belongs to trust bucket.
>>
>> > > 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.
>> >
>> > Given the above are you concerned about privilege or also just resources use
>> > here? Do VFs have SMFS today?
>> 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 ..