netdev - Re: [PATCH net-next V5 00/11] net/mlx5: ConnectX-8 SW Steering + Rate management on traffic classes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <14a4d5351081551d2e1266bf26c5b7384ed2b44f.camel@nvidia.com>
Date: Tue, 17 Dec 2024 15:14:58 +0000
From: Cosmin Ratiu <cratiu@...dia.com>
To: "kuba@...nel.org" <kuba@...nel.org>
CC: "andrew+netdev@...n.ch" <andrew+netdev@...n.ch>, "ttoukan.linux@...il.com"
	<ttoukan.linux@...il.com>, Tariq Toukan <tariqt@...dia.com>, Gal Pressman
	<gal@...dia.com>, "davem@...emloft.net" <davem@...emloft.net>,
	"jiri@...nulli.us" <jiri@...nulli.us>, Leon Romanovsky <leonro@...dia.com>,
	"edumazet@...gle.com" <edumazet@...gle.com>, "pabeni@...hat.com"
	<pabeni@...hat.com>, Saeed Mahameed <saeedm@...dia.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next V5 00/11] net/mlx5: ConnectX-8 SW Steering + Rate
 management on traffic classes

On Mon, 2024-12-16 at 21:45 -0800, Jakub Kicinski wrote:
> > 
> > Right now TCs are not modeled in net-shapers at all. That's why I'm
> > waiting for Paolo to comment, I have no idea what thoughts were put
> > into modeling traffic classes in net-shapers.
> 
> I'm not sure if Paolo is familiar with details of any device capable
> of
> implementing shaping based on TC. You are, I hope, as you're trying
> to add Linux uAPI for it.

I guess nobody has perfect expertise in both TC and net-shapers. But
between all of us, we'll find a way.

> > > 
> > The current patchset extends the well-established devlink rate API
> > with
> > the minimal set of fields required to model traffic classes and
> > offers
> > a full implementation of this new feature in a driver.
> 
> "I just need these few extra fields" is not conducive to good
> designs which can stand the test of time.

I had meant that as "the uAPI needs at least that set of fields to
express tc bandwidth proportions anyway, regardless of discussions
around net-shapers". I am not making any claims about the quality of
the design besides its minimalistic nature.

> > It was designed and started before net-shapers was merged. 
> > We did consider integrating with net-shapers, but because it wasn't
> > yet merged, it couldn't do TCs or multi-device grouping (nor intend
> > to do multi-device), it was rejected as the API of choice.
> 
> net-shapers took months of meetings and many, many revisions.
> Providing an abstraction for all scheduling hierarchies was
> an explicit goal. And someone from nVidia was actively reviewing.
> Now you say that in another part of nVidia a decision was made
> to ignore what the community was working on, sit out discussions 
> and then try to get your own code merged. Did I get that right?

You got it wrong. Nobody decided to "ignore what the community was
working on" or "get our own code merged".

Back in July, we reached out to Paolo explicitly asking about this use
case. Back then things were still in flux with net-shapers but the
direction was clear for it, to model shaping at netdev and below
levels. Paolo's recommendation was to "stick it with the devlink-rate
objects". Our mistake was not cc-ing the ML in that discussion.
What followed was not "ignoring what the community was working on", but
"focusing on solving the TC shaping at VF and group-of-VFs level" with
the more suitable API as opposed to investing time and effort into
influencing net-shapers towards handling use-cases it didn't intend to.

I'm also not sure who else from nVidia was actively reviewing. I asked
a question at some point, Jiri was also active (but mostly with his
maintainer hat, I guess).

> > Can the missing link between TC modeling in net-shapers can be
> > added
> > in another series once it is better understood and discussed with
> > Paolo?
> 
> Certainly Paolo's contributions would be appreciated. But I hope 
> you realize that he is a networking maintainer (like myself)
> and may not have the time to solve your problem for you.

I'm not waiting for him to solve the problem for us, but to offer his
opinion as someone who spent the most time thinking about net-shapers.
It would be bad form for me to start hacking away at that API without
at least hearing from the original author.

> I want to be clear that I don't expect you to migrate devlink rate
> to net-shaper style API at this point. But we do need at the very
> least a clear understanding of how the objects are mapped, and
> therefore how TC support would fit into net-shapers.
> Ideally, also, less disregard for community efforts.

Again, there was no "disregard for community efforts". There was maybe
a missed opportunity to influence the design of net-shapers towards
handling TCs from the start because we focused on devlink-rate instead.
That will now be corrected.

Separately from that, I was planning to add support for net-shapers ops
in the mlx5 QoS infra, similar with how it was done for the other
drivers that do hw QoS.

Cosmin.