Re: [RFC] HW TX Rate Limiting Driver API

[Jakub Kicinski <kuba@kernel.org>]

On Fri, 19 Apr 2024 13:53:53 +0200 Paolo Abeni wrote:
> > They don't have to be nodes. They can appear as parent or child of 
> > a real node, but they don't themselves carry any configuration.
> > 
> > IOW you can represent them as a special encoding of the ID field,
> > rather than a real node.  
> 
> I'm sorry for the latency, I got distracted elsewhere. 
> 
> It's not clear the benefit of introducing this 'attach points' concept.
> 
> With the current proposal, configuring a queue shaper would be:
> 
> info.bw_min = ...
> dev->shaper_ops->set(dev, SHAPER_LOOKUP_BY_QUEUE, queue_id, &info, &ack);
> 
> and restoring the default could be either:
> 
> info.bw_min = 0;
> dev->shaper_ops->set(dev, SHAPER_LOOKUP_BY_QUEUE, queue_id, &info, &ack);

And presumably also bw_max = 0 also means "delete" or will it be bw_max
= ~0 ?

> or:
> 
> dev->shaper_ops->delete(dev, SHAPER_LOOKUP_BY_QUEUE, queue_id, &info, &ack);

Which confusingly will not actually delete the node, subsequent get()
will still return it.

> With the 'attach points' I guess it will be something alike the
> following (am not defining a different node type here just to keep the
> example short):
> 
> # configure a queue shaper
> struct shaper_info attach_info;
> dev->shaper_ops->get(dev, SHAPER_LOOKUP_BY_QUEUE, queue_id, &attach_info, &ack);
> info.parent_id = attach_info.id;
> info.bw_min = ...
> new_node_id = dev->shaper_ops->add(dev, &info, &ack);
>
> # restore defaults:
> dev->shaper_ops->delete(dev, SHAPER_LOOKUP_BY_TREE_ID, new_node_id, &info, &ack);
> 
> likely some additional operation would be needed to traverse/fetch
> directly the actual shaper present at the attach points???

Whether type + ID (here SHAPER_LOOKUP_BY_QUEUE, queue_id) identifies
the node sufficiently to avoid the get is orthogonal. Your ->set
example assumes you don't have to do a get first to find exact
(synthetic) node ID. The same can be true for an ->add, if you prefer.
 
> I think the operations will be simpler without the 'attach points', am
> I missing something? 
> 
> A clear conventions/definition about the semantic of deleting shapers
> at specific locations (e.g. restoring the default behaviour) should
> suffice, together with the current schema.

I guess. I do find it odd that we have objects in multiple places of 
the hierarchy when there is no configuration intended. Especially that
the HW may actually not support such configuration (say there is always
a DRR before the egress, now we insert a shaping stage there).