netdev - Re: [PATCH v3 02/12] netlink: spec: add shaper YAML spec

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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <74a14ded-298f-4ccc-aa15-54070d3a35b7@redhat.com>
Date: Mon, 5 Aug 2024 17:11:09 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Jiri Pirko <jiri@...nulli.us>, Jakub Kicinski <kuba@...nel.org>
Cc: netdev@...r.kernel.org, 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>
Subject: Re: [PATCH v3 02/12] netlink: spec: add shaper YAML spec

Hi all,

(same remark of my previous email). My replies this week will be 
delayed, please allow for some extra latency.

On 8/2/24 12:49, Jiri Pirko wrote:
> Thu, Aug 01, 2024 at 05:12:01PM CEST, pabeni@...hat.com wrote:
>> On 8/1/24 15:10, Jiri Pirko wrote:
>>> Tue, Jul 30, 2024 at 10:39:45PM CEST, pabeni@...hat.com wrote:
>>>> +    type: enum
>>>> +    name: scope
>>>> +    doc: the different scopes where a shaper can be attached
>>>> +    render-max: true
>>>> +    entries:
>>>> +      - name: unspec
>>>> +        doc: The scope is not specified
>>>> +      -
>>>> +        name: port
>>>> +        doc: The root for the whole H/W
>>>
>>> What is this "port"?
>>
>> ~ a wire plug.
> 
> What's "wire plug"? What of existing kernel objects this relates to? Is
> it a devlink port?


I'm sorry, my hasty translation of my native language was really 
inaccurate. Let me re-phrase from scratch: that is actually the root of 
the whole scheduling tree (yes, it's a tree) for a given network device.

One source of confusion is that in a previous iteration we intended to 
allow configuring even objects 'above' the network device level, but 
such feature has been dropped.

We could probably drop this scope entirely.

>>>> +      -
>>>> +        name: netdev
>>>> +        doc: The main shaper for the given network device.
>>>> +      -
>>>> +        name: queue
>>>> +        doc: The shaper is attached to the given device queue.
>>>> +      -
>>>> +        name: detached
>>>> +        doc: |
>>>> +             The shaper is not attached to any user-visible network
>>>> +             device component and allows nesting and grouping of
>>>> +             queues or others detached shapers.
>>>
>>> What is the purpose of the "detached" thing?
>>
>> I fear I can't escape reusing most of the wording above. 'detached' nodes
>> goal is to create groups of other shapers. i.e. queue groups,
>> allowing multiple levels nesting, i.e. to implement this kind of hierarchy:
>>
>> q1 ----- \
>> q2 - \SP / RR ------
>> q3 - /    	    \
>> 	q4 - \ SP -> (netdev)
>> 	q5 - /	    /
>>                    /
>> 	q6 - \ RR
>> 	q7 - /
>>
>> where q1..q7 are queue-level shapers and all the SP/RR are 'detached' one.
>> The conf. does not necessary make any functional sense, just to describe the
>> things.
> 
> Can you "attach" the "detached" ones? They are "detached" from what?

I see such name is very confusing. An alternative one could be 'group', 
but IIRC it was explicitly discarded while discussing a previous iteration.

The 'detached' name comes from the fact the such shapers are not a 
direct representation of some well-known kernel object (queues, devices),

>>>> +    -
>>>> +      name: group
>>>> +      doc: |
>>>> +        Group the specified input shapers under the specified
>>>> +        output shaper, eventually creating the latter, if needed.
>>>> +        Input shapers scope must be either @queue or @detached.
>>>> +        Output shaper scope must be either @detached or @netdev.
>>>> +        When using an output @detached scope shaper, if the
>>>> +        @handle @id is not specified, a new shaper of such scope
>>>> +        is created and, otherwise the specified output shaper
>>>> +        must be already existing.
>>>
>>> I'm lost. Could this designt be described in details in the doc I asked
>>> in the cover letter? :/ Please.
>>
>> I'm unsure if the context information here and in the previous replies helped
>> somehow.
>>
>> The group operation creates and configure a scheduling group, i.e. this
>>
>> q1 ----- \
>> q2 - \SP / RR ------
>> q3 - /    	    \
>> 	q4 - \ SP -> (netdev)
>> 	q5 - /	    /
>>                    /
>> 	q6 - \ RR
>> 	q7 - /
>>
>> can be create with:
>>
>> group(inputs:[q6, q7], output:[detached,parent:netdev])
>> group(inputs:[q4, q5], output:[detached,parent:netdev])
>> group(inputs:[q1], output:[detached,parent:netdev])
>> group(inputs:[q2,q3], output:[detached,parent:<the detached shaper create
>> above>])
> 
> So by "inputs" and "output" you are basically building a tree. In
> devlink rate, we have leaf and node, which is in sync with standard tree
> terminology.
> 
> If what you are building is tree, why don't you use the same
> terminology? If you are building tree, you just need to have the link to
> upper noded (output in your terminology). Why you have "inputs"? Isn't
> that redundant?

The idea behind the inputs/outputs naming is to represent the data flow 
towards the wire.
I'm fine with the parent/children naming, but IIRC Jakub was not happy 
with it. Is there any intermediate ground that could satisfy both of you?

Thanks,

Paolo