Message-ID: <1a364cc3-3b22-447d-bfa8-376de41d1f64@redhat.com>
Date: Tue, 27 Aug 2024 09:54:52 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: netdev@...r.kernel.org, Jiri Pirko <jiri@...nulli.us>,
 Madhu Chittim <madhu.chittim@...el.com>,
 Sridhar Samudrala <sridhar.samudrala@...el.com>,
 Simon Horman <horms@...nel.org>, John Fastabend <john.fastabend@...il.com>,
 Sunil Kovvuri Goutham <sgoutham@...vell.com>,
 Jamal Hadi Salim <jhs@...atatu.com>, Donald Hunter <donald.hunter@...il.com>
Subject: Re: [PATCH v4 net-next 00/12] net: introduce TX H/W shaping API

On 8/27/24 04:14, Jakub Kicinski wrote:
> On Fri, 23 Aug 2024 09:51:24 +0200 Paolo Abeni wrote:
>> On 8/23/24 02:43, Jakub Kicinski wrote:
>>> On Tue, 20 Aug 2024 17:12:21 +0200 Paolo Abeni wrote:
>>>> * Delegation
>>>>
>>>> A containers wants to limit the aggregate B/W bandwidth of 2 of the 3
>>>> queues it owns - the starting configuration is the one from the
>>>> previous point:
>>>>
>>>> SPEC=Documentation/netlink/specs/net_shaper.yaml
>>>> ./tools/net/ynl/cli.py --spec $SPEC \
>>>> 	--do group --json '{"ifindex":'$IFINDEX',
>>>> 			"leaves": [
>>>> 			  {"handle": {"scope": "queue", "id":'$QID1' },
>>>> 			   "weight": '$W1'},
>>>> 			  {"handle": {"scope": "queue", "id":'$QID2' },
>>>> 			   "weight": '$W2'}],
>>>> 			"root": { "handle": {"scope": "node"},
>>>> 				  "parent": {"scope": "node", "id": 0},
>>>
>>> In the delegation use case I was hoping "parent" would be automatic.
>>
>> Currently the parent is automatic/implicit when creating a node directly
>> nested to the the netdev shaper.
>>
>> I now see we can use as default parent the current leaves' parent, when
>> that is the same for all the to-be-grouped leaves.
>>
>> Actually, if we restrict the group operation to operate only on set of
>> leaves respecting the above, I *guess* we will not lose generality and
>> we could simplify a bit the spec. WDYT?
> 
> I remember having a use case in mind where specifying parent would be
> very useful. I think it may have been related to atomic changes.
> I'm not sure if what I describe below is exactly that case...
> 
> Imagine:
> 
> Qx -{hierarchy}---\
>                     \{hierarchy}-- netdev
> Q0-------P0\ SP----/
> Q1--\ RR-P1/
> Q2--/
> 
> Let's say we own queues 0,1,2 and want to remove the SP layer.
> It's convenient to do:
> 
> 	$node = get($SP-node)
> 	group(leaves: [Q0, Q1, Q2], parent=$node.parent)
> 
> And have the kernel "garbage collect" the old RR node and the old SP
> node (since they will now have no children). We want to avoid the
> situations where user space has to do complex transitions thru
> states which device may not support (make sure Q1, Q2 have right prios,
> delete old RR, now we have SP w/ 3 inputs, delete the SP, create a new
> group).

FTR, while updating the group() implementation to infer the root's 
parent handle in most cases, I stumbled upon a similar scenario.

> For the case above we could technically identify the correct parent by
> skipping the nodes which will be garbage collected later. 

I think that implementation would be quite non trivial/error prone, and 
I think making the new root's parent explicit would be more clear from 
user-space perspective.

What I have now in my local tree is a group() implementation the 
inherits the newly created root's parent handle from the leaves, if all 
of them have the same parent prior to the group() invocation. Otherwise 
it requires the user to specify the root's parent handle. In any case, 
the user-specified root's parent handle value overrides the 
'inherited'/guessed one.

It will cover the above and will not require an explicit parent in most 
case. Would that be good enough?

Thanks,

Paolo

Powered by blists - more mailing lists