From: Simon Horman <horms@kernel.org>
To: Paolo Abeni <pabeni@redhat.com>
Cc: Andrew Lunn <andrew@lunn.ch>,
netdev@vger.kernel.org, Jakub Kicinski <kuba@kernel.org>,
Jiri Pirko <jiri@resnulli.us>,
Madhu Chittim <madhu.chittim@intel.com>,
Sridhar Samudrala <sridhar.samudrala@intel.com>,
John Fastabend <john.fastabend@gmail.com>,
Sunil Kovvuri Goutham <sgoutham@marvell.com>,
Jamal Hadi Salim <jhs@mojatatu.com>
Subject: Re: [RFC PATCH] net: introduce HW Rate Limiting Driver API
Date: Wed, 15 May 2024 10:51:47 +0100 [thread overview]
Message-ID: <20240515095147.GB154012@kernel.org> (raw)
In-Reply-To: <db51b7ccff835dd5a96293fb84d527be081de062.camel@redhat.com>
On Fri, May 10, 2024 at 01:05:41PM +0200, Paolo Abeni wrote:
> On Thu, 2024-05-09 at 18:17 +0200, Andrew Lunn wrote:
> > > > Now the question is, how do i get between these two states? It is not
> > > > possible to mix WRR and strict priority. Any kAPI which only modifies
> > > > one queue at once will go straight into an invalid state, and the
> > > > driver will need to return -EOPNOTSUPP. So it seems like there needs
> > > > to be an atomic set N queue configuration at once, so i can cleanly go
> > > > from strict priority across 8 queues to WRR across 8 queues. Is that
> > > > foreseen?
> > >
> > > You could delete all the WRR shapers and then create/add SP based ones.
> >
> > But that does not match the hardware. I cannot delete the hardware. It
> > will either do strict priority or WRR. If i delete the software
> > representation of the shaper, the hardware shaper will keep on doing
> > what it was doing. So i don't see this as a good model. I think the
> > driver will create shapers to represent the hardware, and you are not
> > allowed to delete them or add more of them, because that is what the
> > hardware is. All you can do is configure the shapers that exist.
> >
> > > The 'create' op is just an abstraction to tell the NIC to switch from
> > > the default configuration to the specified one.
> >
> > Well, the hardware default is i think WRR for the queues, and line
> > rate. That will be what the software representation of the shapers
> > will be set to when the driver probes and creates the shapers
> > representors.
>
> If I read correctly, allowing each NIC to expose it's own different
> starting configuration still will not solve the problem for this H/W to
> switch from WRR to SP (and vice versa).
>
> AFAICS, what would be needed there is an atomic set of operations:
> 'set_many' (and e.v. 'delete_many', 'create_many') that will allow
> changing all the shapers at once.
>
> With such operations, that H/W could still fit the expected 'no-op'
> default, as WRR on the queue shapers is what we expect. I agree with
> Jakub, handling the complexity of arbitrary starting configuration
> would pose a lot of trouble to the user/admin.
>
> If all the above stands together, I think we have a few options (in
> random order):
>
> - add both set of operations: the ones operating on a single shaper and
> the ones operating on multiple shapers
> - use only the multiple shapers ops.
>
> And the latter looks IMHO the simple/better. At that point I would
> probably drop the 'add' op and would rename 'delete' as
> 'reset':
>
> int (*set)(struct net_device *dev, int how_many, const u32 *handles,
> const struct net_shaper_info *shapers,
> struct netlink_ext_ack *extack);
> int (*reset)(struct net_device *dev, int how_many, const u32 *handles,
> struct netlink_ext_ack *extack);
> int (*move)(struct net_device *dev, int how_many, const u32 *handles,
> const u32 *new_parent_handles,
> struct netlink_ext_ack *extack);
>
> An NIC with 'static' shapers can implement a dummy move always
> returning EOPNOTSUPP and eventually filling a detailed extack.
>
> NIC without any constraints on mixing and matching different kind of
> shapers could implement the above as a loop over whatever they will do
> for the corresponding 'single shaper op'
>
> NIC with constrains alike the one you pointed out could validate the
> final state before atomically applying the specified operation.
>
> After a successful 'reset' operation, the kernel could drop any data
> it retains/caches for the relevant shapers - the current idea is to
> keep a copy of all successfully configured shaper_info in a xarray,
> using the 'handle' as the index.
>
> Side note: the move() operation could look like a complex artifact, but
> it's the simplest instrument I could think of to support scenarios
> where the user creates/configures/sets a queue group and 'move' some
> queue under the newly created group
>
> WDYT?
Hi Andrew,
Picking up this discussion, I'm wondering if Paolo's proposal
addresses your concerns.
next prev parent reply other threads:[~2024-05-15 9:51 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-08 20:20 [RFC PATCH] net: introduce HW Rate Limiting Driver API Paolo Abeni
2024-05-08 21:47 ` Andrew Lunn
2024-05-09 14:19 ` Paolo Abeni
2024-05-09 15:00 ` Andrew Lunn
2024-05-09 15:43 ` Paolo Abeni
2024-05-09 16:17 ` Andrew Lunn
2024-05-10 11:05 ` Paolo Abeni
2024-05-15 9:51 ` Simon Horman [this message]
2024-05-15 14:19 ` Andrew Lunn
2024-05-15 14:56 ` Simon Horman
2024-05-28 17:28 ` Jakub Kicinski
2024-05-09 15:09 ` Andrew Lunn
2024-05-10 7:10 ` Naveen Mamindlapalli
2024-05-10 7:58 ` Paolo Abeni
2024-05-15 14:41 ` Andrew Lunn
2024-05-15 14:50 ` Simon Horman
2024-05-28 17:18 ` Jakub Kicinski
2024-05-31 9:22 ` Paolo Abeni
2024-05-31 16:00 ` Jakub Kicinski
2024-06-03 11:11 ` Paolo Abeni
2024-06-03 23:29 ` Jakub Kicinski
2024-06-05 15:04 ` Cosmin Ratiu
2024-06-05 15:52 ` Paolo Abeni
2024-06-05 19:40 ` 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=20240515095147.GB154012@kernel.org \
--to=horms@kernel.org \
--cc=andrew@lunn.ch \
--cc=jhs@mojatatu.com \
--cc=jiri@resnulli.us \
--cc=john.fastabend@gmail.com \
--cc=kuba@kernel.org \
--cc=madhu.chittim@intel.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=sgoutham@marvell.com \
--cc=sridhar.samudrala@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 read-only IMAP folder(s) and NNTP newsgroup(s).