netdev - Re: [RFC] HW TX Rate Limiting Driver API

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20240422110654.2f843133@kernel.org>
Date: Mon, 22 Apr 2024 11:06:54 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Paolo Abeni <pabeni@...hat.com>
Cc: Simon Horman <horms@...nel.org>, netdev@...r.kernel.org, Jiri Pirko
 <jiri@...nulli.us>, Madhu Chittim <madhu.chittim@...el.com>, Sridhar
 Samudrala <sridhar.samudrala@...el.com>
Subject: Re: [RFC] HW TX Rate Limiting Driver API

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