netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Jiri Pirko <jiri@nvidia.com>
Cc: Dima Chumak <dchumak@nvidia.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Paolo Abeni <pabeni@redhat.com>,
	netdev@vger.kernel.org, Simon Horman <horms@verge.net.au>
Subject: Re: [PATCH net-next 0/5] devlink rate police limiter
Date: Thu, 7 Jul 2022 13:16:49 -0700	[thread overview]
Message-ID: <20220707131649.7302a997@kernel.org> (raw)
In-Reply-To: <YsbBbBt+DNvBIU2E@nanopsycho>

On Thu, 7 Jul 2022 13:20:12 +0200 Jiri Pirko wrote:
> Wait. Lets draw the basic picture of "the wire":
> 
> --------------------------+                +--------------------------
> eswitch representor netdev|=====thewire====|function (vf/sf/whatever
> --------------------------+                +-------------------------
> 
> Now the rate setting Dima is talking about, it is the configuration of
> the "function" side. Setting the rate is limitting the "function" TX/RX
> Note that this function could be of any type - netdev, rdma, vdpa, nvme.

The patches add policing, are you saying we're gonna drop RDMA or NVMe
I/O?

> Configuring the TX/RX rate (including groupping) applies to all of
> these.

I don't understand why the "side of the wire" matters when the patches
target both Rx and Tx. Surely that covers both directions.

> Putting the configuration on the eswitch representor does not fit:
> 1) it is configuring the other side of the wire, the configuration
>    should be of the eswitch port. Configuring the other side is
>    confusing and misleading. For the purpose of configuring the
>    "function" side, we introduced "port function" object in devlink.
> 2) it is confuguring netdev/ethernet however the confuguration applies
>    to all queues of the function.

If you think it's technically superior to put it in devlink that's fine.
I'll repeat myself - what I'm asking for is convergence so that drivers
don't have  to implement 3 different ways of configuring this. We have
devlink rate for from-VF direction shaping, tc police for bi-dir
policing and obviously legacy NDOs. None of them translate between each
other so drivers and user space have to juggle interfaces.

  reply	other threads:[~2022-07-07 20:17 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-20 15:26 [PATCH net-next 0/5] devlink rate police limiter Dima Chumak
2022-06-20 15:26 ` [PATCH net-next 1/5] devlink: Introduce limit_type attr for rate objects Dima Chumak
2022-06-20 15:26 ` [PATCH net-next 2/5] devlink: Introduce police rate limit type Dima Chumak
2022-06-20 15:26 ` [PATCH net-next 3/5] netdevsim: Support devlink rate limit_type police Dima Chumak
2022-06-20 15:26 ` [PATCH net-next 4/5] selftest: netdevsim: Add devlink rate police sub-test Dima Chumak
2022-06-20 15:26 ` [PATCH net-next 5/5] Documentation: devlink rate objects limit_type Dima Chumak
2022-06-20 15:35 ` [PATCH iproute2-next 1/5] uapi: devlink.h DEVLINK_ATTR_RATE_LIMIT_TYPE Dima Chumak
2022-06-20 15:35 ` [PATCH iproute2-next 2/5] devlink: Add port rate limit_type support Dima Chumak
2022-06-20 15:35 ` [PATCH iproute2-next 3/5] utils: Add get_size64() Dima Chumak
2022-06-20 15:35 ` [PATCH iproute2-next 4/5] uapi: devlink.h DEVLINK_RATE_LIMIT_TYPE_POLICE Dima Chumak
2022-06-20 15:35 ` [PATCH iproute2-next 5/5] devlink: Introduce port rate limit_type police Dima Chumak
2022-06-20 20:04 ` [PATCH net-next 0/5] devlink rate police limiter Jakub Kicinski
2022-06-30 15:27   ` Dima Chumak
2022-06-30 18:13     ` Jakub Kicinski
2022-07-07 11:20       ` Jiri Pirko
2022-07-07 20:16         ` Jakub Kicinski [this message]
2022-07-08  7:27           ` Jiri Pirko
2022-07-08 18:05             ` Jakub Kicinski
2022-07-09  5:14               ` Jiri Pirko
2022-07-11 17:29                 ` Jakub Kicinski
2022-07-12  6:03                   ` Jiri Pirko
2022-07-13  0:13                     ` Jakub Kicinski
2022-07-13  5:04                       ` Jiri Pirko
2022-07-13 17:52                         ` Jakub Kicinski
2022-07-14  4:55                           ` Jiri Pirko
2022-07-14 16:07                             ` Jakub Kicinski

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=20220707131649.7302a997@kernel.org \
    --to=kuba@kernel.org \
    --cc=davem@davemloft.net \
    --cc=dchumak@nvidia.com \
    --cc=edumazet@google.com \
    --cc=horms@verge.net.au \
    --cc=jiri@nvidia.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.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).