Re: [Intel-wired-lan] [PATCH iwl-next v4 0/5] iavf: Add devlink and devlink rate support'

From: Jiri Pirko <jiri@resnulli.us>
To: "Zhang, Xuejun" <xuejun.zhang@intel.com>
Cc: "Samudrala, Sridhar" <sridhar.samudrala@intel.com>,
	netdev@vger.kernel.org, maxtram95@gmail.com,
	qi.z.zhang@intel.com, "Chittim, Madhu" <madhu.chittim@intel.com>,
	intel-wired-lan@lists.osuosl.org,
	Jakub Kicinski <kuba@kernel.org>,
	anthony.l.nguyen@intel.com, pabeni@redhat.com,
	Wenjun Wu <wenjun1.wu@intel.com>
Subject: Re: [Intel-wired-lan] [PATCH iwl-next v4 0/5] iavf: Add devlink and devlink rate support'
Date: Fri, 17 Nov 2023 12:21:08 +0100	[thread overview]
Message-ID: <ZVdMpLz1LPfMyM8S@nanopsycho> (raw)
In-Reply-To: <bdb0137a-b735-41d9-9fea-38b238db0305@intel.com>

Fri, Nov 17, 2023 at 06:52:49AM CET, xuejun.zhang@intel.com wrote:
>Hello Jiri & Jakub,
>
>Thanks for looking into our last patch with devlink API. Really appreciate
>your candid review.
>
>Following your suggestion, we have looked into 3 tc offload options to
>support queue rate limiting
>
>#1 mq + matchall + police

This looks most suitable. Why it would not work?

>
>#2 mq + tbf
>
>#3 htb
>
>all 3 tc offload options require some level of tc extensions to support VF tx
>queue rate limiting (tx_maxrate & tx_minrate)
>
>htb offload requires minimal tc changes or no change with similar change done
>@ driver (we can share patch for review).
>
>After discussing with Maxim Mikityanskiy( https://lore.kernel.org/netdev/54a7dd27-a612-46f1-80dd-b43e28f8e4ce@intel.com/
>), looks like sysfs interface with tx_minrate extension could be the option

I don't undestand how any sysfs know is related to any of the tree tc
solutions above.

>we can take.
>
>Look forward your opinion & guidance. Thanks for your time!
>
>Regards,
>
>Jun
>
>On 8/28/2023 3:46 PM, Zhang, Xuejun wrote:
>> 
>> On 8/24/2023 12:04 AM, Jiri Pirko wrote:
>> > Wed, Aug 23, 2023 at 09:13:34PM CEST, xuejun.zhang@intel.com wrote:
>> > > On 8/22/2023 8:34 AM, Jiri Pirko wrote:
>> > > > Tue, Aug 22, 2023 at 05:12:55PM CEST,kuba@kernel.org  wrote:
>> > > > > On Tue, 22 Aug 2023 08:12:28 +0200 Jiri Pirko wrote:
>> > > > > > NACK! Port function is there to configure the VF/SF from the eswitch
>> > > > > > side. Yet you use it for the configureation of the
>> > > > > > actual VF, which is
>> > > > > > clear misuse. Please don't
>> > > > > Stating where they are supposed to configure the rate
>> > > > > would be helpful.
>> > > > TC?
>> > > Our implementation is an extension to this commit 42c2eb6b1f43
>> > > ice: Implement
>> > > devlink-rate API).
>> > > 
>> > > We are setting the Tx max & share rates of individual queues in a
>> > > VF using
>> > > the devlink rate API.
>> > > 
>> > > Here we are using DEVLINK_PORT_FLAVOUR_VIRTUAL as the attribute
>> > > for the port
>> > > to distinguish it from being eswitch.
>> > I understand, that is a wrong object. So again, you should use
>> > "function" subobject of devlink port to configure "the other side of the
>> > wire", that means the function related to a eswitch port. Here, you are
>> > doing it for the VF directly, which is wrong. If you need some rate
>> > limiting to be configured on an actual VF, use what you use for any
>> > other nic. Offload TC.
>> Thanks for detailed explanation and suggestions. Sorry for late reply as
>> it took a bit longer to understand options.
>> 
>> As sysfs has similar rate configuration on per queue basis with
>> tx_maxrate, is it a viable option for our use case (i.e allow user to
>> configure tx rate for each allocated queue in a VF).
>> 
>> Pls aslo see If adding tx_minrate to sysfs tx queue entry is feasible on
>> the current framework.
>> _______________________________________________
>> Intel-wired-lan mailing list
>> Intel-wired-lan@osuosl.org
>> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan