On 8/30/24 21:14, Jakub Kicinski wrote:
On Fri, 30 Aug 2024 17:43:08 +0200 Paolo Abeni wrote:
Please allow me to put a few high level questions together, to both
underline them as most critical, and keep the thread focused.
On 8/30/24 03:20, Jakub Kicinski wrote:
> This 'binding' has the same meaning as 'binding' in TCP ZC? :(
I hope we can agree that good naming is difficult. I thought we agreed
on such naming in the past week’s discussion. The term 'binding' is
already used in the networking stack in many places to identify
different things (i.e. device tree, socket, netfilter.. ). The name
prefix avoids any ambiguity and I think this a good name, but if you
have any better suggestions, this change should be trivial.
Ack. Maybe we can cut down the number of ambiguous nouns elsewhere:
maybe call net_shaper_info -> net_shaper ?
maybe net_shaper_data -> net_shaper_hierarchy ?
Is everybody fine with the above?
[about separate handle from shaper_info arguments]
> Wouldn't it be convenient to store the handle in the "info"
> object? AFAIU the handle is forever for an info, so no risk of it
> being out of sync…
Was that way a couple of iterations ago. Jiri explicitly asked for the
separation, I asked for confirmation and nobody objected.
Could you link to that? I must have not read it.
https://lore.kernel.org/netdev/[email protected]/
search for "I wonder if the handle should be part of this structure"
I must admit by wannabe reply on such point never left my outbox.
You can keep it wrapped in a struct *_handle, that's fine.
But it can live inside the shaper object.
That is basically the opposite of what Jiri asked. @Jiri would you be ok
reverting to such layout?
Which if the 2 options is acceptable from both of you?
[about queue limit and channel reconf]
> we probably want to trim the queue shapers on channel reconfig,
> then, too? :(
what about exposing to the drivers an helper alike:
net_shaper_notify_delete(binding, handle);
that tells the core the shaper at the given handle just went away in the
H/W? The driver will call it in the queue deletion helper, and such
helper could be later on used more generically, i.e. for vf/devlink port
deletion.
We can either prevent disabling queues which have shapers attached,
or auto-removing the shapers.
I think/fear that prevent disabling queues would lead to
weird/unexpected results and more difficult administration, I prefer the
callback option.
No preference on that. But put the
callback in the core, please, netif_set_real_num_rx_queues() ?
Why not?
It makes sense. I'll add a net_shaper_set_real_num_rx_queues() callback
there.
/P
