Date:   Tue, 18 Jan 2022 16:16:29 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Saeed Mahameed <saeedm@...dia.com>
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 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 :)

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

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

Shifting the "weird feature discovery" onto the user really does not
solve the problem. Enable SMFS is not a meaningful knob, the devops
engineer setting up the infra will have to guess. If we want something
like SMFS to be directly controlled it should be a clearly vendor
specific knob, IMO.

Powered by blists - more mailing lists