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

On Thu, 16 Nov 2023 21:52:49 -0800 Zhang, Xuejun wrote:
> 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
> 
> #2 mq + tbf

You can extend mqprio, too, if you wanted.

> #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 we can take.
> 
> Look forward your opinion & guidance. Thanks for your time!

My least favorite thing to do is to configure the same piece of silicon
with 4 different SW interfaces. It's okay if we have 4 different uAPIs
(user level APIs) but the driver should not be exposed to all these
options.

I'm saying 4 but really I can think of 6 ways of setting maxrate :(

IMHO we need to be a bit more realistic about the notion of "offloading
the SW thing" for qdiscs specifically. Normally we offload SW constructs
to have a fallback and have a clear definition of functionality.
I bet most data-centers will use BPF+FQ these days, so the "fallback"
argument does not apply. And the "clear definition" when it comes to
basic rate limiting is.. moot.

Besides we already have mqprio, sysfs maxrate, sriov ndo, devlink rate,
none of which have SW fallback.

So since you asked for my opinion - my opinion is that step 1 is to
create a common representation of what we already have and feed it
to the drivers via a single interface. I could just be taking sysfs
maxrate and feeding it to the driver via the devlink rate interface.
If we have the right internals I give 0 cares about what uAPI you pick.

  parent reply	other threads:[~2023-11-18 16:48 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20230727021021.961119-1-wenjun1.wu@intel.com>
2023-08-08  1:57 ` [PATCH iwl-next v2 0/5] iavf: Add devlink and devlink rate support Wenjun Wu
2023-08-08  1:57   ` [PATCH iwl-next v2 1/5] virtchnl: support queue rate limit and quanta size configuration Wenjun Wu
2023-08-08  1:57   ` [PATCH iwl-next v2 2/5] ice: Support VF " Wenjun Wu
2023-08-16 16:54     ` Brett Creeley
2023-08-08  1:57   ` [PATCH iwl-next v2 3/5] iavf: Add devlink and devlink port support Wenjun Wu
2023-08-16 17:11     ` Brett Creeley
2023-08-08  1:57   ` [PATCH iwl-next v2 4/5] iavf: Add devlink port function rate API support Wenjun Wu
2023-08-08 20:49     ` Simon Horman
2023-08-09 18:43       ` Zhang, Xuejun
2023-08-16 17:27     ` Brett Creeley
2023-08-08  1:57   ` [PATCH iwl-next v2 5/5] iavf: Add VIRTCHNL Opcodes Support for Queue bw Setting Wenjun Wu
2023-08-08 20:54     ` Simon Horman
2023-08-09 18:44       ` Zhang, Xuejun
2023-08-16 17:32     ` Brett Creeley
2023-08-16  3:33 ` [PATCH iwl-next v3 0/5] iavf: Add devlink and devlink rate support Wenjun Wu
2023-08-16  3:33   ` [PATCH iwl-next v3 1/5] virtchnl: support queue rate limit and quanta size configuration Wenjun Wu
2023-08-16  3:33   ` [PATCH iwl-next v3 2/5] ice: Support VF " Wenjun Wu
2023-08-16  3:33   ` [PATCH iwl-next v3 3/5] iavf: Add devlink and devlink port support Wenjun Wu
2023-08-16  3:33   ` [PATCH iwl-next v3 4/5] iavf: Add devlink port function rate API support Wenjun Wu
2023-08-16  3:33   ` [PATCH iwl-next v3 5/5] iavf: Add VIRTCHNL Opcodes Support for Queue bw Setting Wenjun Wu
2023-08-16  9:14     ` Simon Horman
2023-08-22  3:39 ` [PATCH iwl-next v4 0/5] iavf: Add devlink and devlink rate support Wenjun Wu
2023-08-22  3:39   ` [PATCH iwl-next v4 1/5] virtchnl: support queue rate limit and quanta size configuration Wenjun Wu
2023-08-22  3:40   ` [PATCH iwl-next v4 2/5] ice: Support VF " Wenjun Wu
2023-08-22  3:40   ` [PATCH iwl-next v4 3/5] iavf: Add devlink and devlink port support Wenjun Wu
2023-08-22  3:40   ` [PATCH iwl-next v4 4/5] iavf: Add devlink port function rate API support Wenjun Wu
2023-08-22  3:40   ` [PATCH iwl-next v4 5/5] iavf: Add VIRTCHNL Opcodes Support for Queue bw Setting Wenjun Wu
2023-08-22  6:12   ` [PATCH iwl-next v4 0/5] iavf: Add devlink and devlink rate support Jiri Pirko
2023-08-22 15:12     ` Jakub Kicinski
2023-08-22 15:34       ` [PATCH iwl-next v4 0/5] iavf: Add devlink and devlink rate support' Jiri Pirko
2023-08-23 21:39         ` Zhang, Xuejun
     [not found]         ` <0893327b-1c84-7c25-d10c-1cc93595825a@intel.com>
2023-08-24  7:04           ` Jiri Pirko
2023-08-28 22:46             ` Zhang, Xuejun
2023-11-17  5:52               ` [Intel-wired-lan] " Zhang, Xuejun
2023-11-17 11:21                 ` Jiri Pirko
2023-11-21  9:04                   ` Paolo Abeni
2023-11-18 16:48                 ` Jakub Kicinski [this message]
2023-11-22 22:19                   ` Zhang, Xuejun
2023-11-23  3:22                     ` Jakub Kicinski
2023-11-28  0:15                       ` Zhang, Xuejun
2023-11-28  1:43                         ` Jakub Kicinski
2023-12-14 20:29                           ` Paolo Abeni
2023-12-15  1:46                             ` Jakub Kicinski
2023-12-15 11:06                               ` Paolo Abeni
2023-12-15 11:47                                 ` Paolo Abeni
2023-12-15 12:30                                 ` Jiri Pirko
2023-12-15 22:41                                 ` Jakub Kicinski
2023-12-18 20:12                                   ` Paolo Abeni
2023-12-18 21:33                                     ` Jakub Kicinski
2023-12-15 12:22                               ` Jiri Pirko
2023-10-18  9:05             ` Paolo Abeni

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231118084843.70c344d9@kernel.org \
    --to=kuba@kernel.org \
    --cc=anthony.l.nguyen@intel.com \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=jiri@resnulli.us \
    --cc=madhu.chittim@intel.com \
    --cc=maxtram95@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=qi.z.zhang@intel.com \
    --cc=sridhar.samudrala@intel.com \
    --cc=wenjun1.wu@intel.com \
    --cc=xuejun.zhang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).