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:

quoted

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.

quoted

quoted

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.

quoted

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).

quoted

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.