netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Jiri Pirko <jiri@resnulli.us>
Cc: Jiri Pirko <jiri@nvidia.com>, 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>,
	Michal Wilczynski <michal.wilczynski@intel.com>
Subject: Re: [PATCH net-next 0/5] devlink rate police limiter
Date: Fri, 8 Jul 2022 11:05:35 -0700	[thread overview]
Message-ID: <20220708110535.63a2b8e9@kernel.org> (raw)
In-Reply-To: <YsfcUlF9KjFEGGVW@nanopsycho>

Adding Michal

On Fri, 8 Jul 2022 09:27:14 +0200 Jiri Pirko wrote:
> >> 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.  
> 
> Hmm, I believe it really does. We have objects which we configure. There
> is a function object, which has some configuration (including this).
> Making user to configure function object via another object (eswitch
> port netdevice on the other side of the wire), is quite confusing and I
> feel it is wrong. The only reason is to somehow fit TC interface for
> which we don't have an anchor for port function.
> 
> What about another configuration? would it be ok to use eswitch port
> netdev to configure port function too, if there is an interface for it?
> I believe not, that is why we introduced port function.

I resisted the port function aberration as long as I could. It's 
a limitation of your design as far as I'm concerned.

Switches use TC to configure egress queuing, that's our Linux model.
Representor is the switch side, TC qdisc on it maps to the egress
of the switch.

I don't understand where the disconnect between us is, you know that's
what mlxsw does..

> >> 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.  
> 
> The legacy ndo is legacy. Drivers that implement switchdev mode do
> not implement those, and should not.

That's irrelevant - what I'm saying is that in practice drivers have to
implement _all_ of these interfaces today. Just because they are not
needed in eswitch mode doesn't mean the sales department won't find a
customer who's happy with the non-switchdev mode and doesn't want to
move.

  reply	other threads:[~2022-07-08 18:05 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
2022-07-08  7:27           ` Jiri Pirko
2022-07-08 18:05             ` Jakub Kicinski [this message]
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=20220708110535.63a2b8e9@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=jiri@resnulli.us \
    --cc=michal.wilczynski@intel.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).