[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <07bae4f7-4450-4ec5-a2fe-37b563f6105d@redhat.com>
Date: Thu, 1 Aug 2024 16:31:04 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Donald Hunter <donald.hunter@...il.com>
Cc: netdev@...r.kernel.org, Jakub Kicinski <kuba@...nel.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>
Subject: Re: [PATCH v3 02/12] netlink: spec: add shaper YAML spec
On 7/31/24 23:13, Donald Hunter wrote:
> Paolo Abeni <pabeni@...hat.com> writes:
>
>> diff --git a/Documentation/netlink/specs/shaper.yaml b/Documentation/netlink/specs/shaper.yaml
>> new file mode 100644
>> index 000000000000..7327f5596fdb
>> --- /dev/null
>> +++ b/Documentation/netlink/specs/shaper.yaml
>
> It's probably more user-friendly to use the same filename as the spec
> name, so net-shaper.yaml
No big objection on my side, but if we enforce 'Name:' to be $(basename
$file .yaml), the 'Name' field becomes redundant.
[...]
>> + render-max: true
>> + entries:
>> + - name: unspec
>> + doc: The scope is not specified
>
> What are the semantics of 'unspec' ? When can it be used?
I guess at this point it can be dropped. It was introduced in a previous
incarnation to represent the port parent - the port does not have a
parent, being the root of the hierarchy.
>> + -
>> + name: port
>> + doc: The root for the whole H/W
>> + -
>> + name: netdev
>> + doc: The main shaper for the given network device.
>
> What are the semantic differences between netdev and port?
netdev == Linux network device
port == wire plug
>> + -
>> + 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.
>
> I assume that shapers are always owned by the netdev regardless of
> attach status?
If you mean that it's up to the netdev clean them up on (netdev)
removal, yes.
>> +>> + -
>> + name: inputs
>> + type: nest
>> + multi-attr: true
>> + nested-attributes: ns-info
>> + doc: |
>> + Describes a set of inputs shapers for a @group operation
>
> The @group renders exactly as-is in the generated htmldocs. There may be
> a more .rst friendly markup you can use that will render better.
Uhm... AFAICS the problem is the target (e.g. 'group') is outside the
htmldoc section itself, I can't find any existing markup to serve this
purpose well. What about sticking to quotes '' everywhere?
FTR, I used @ following the kdoc style.
[...]
>> + -
>> + 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.
>
> It says above that you cannot create a detached shaper, so how do you
> create one to use as an input shaper here? Is this group op more like a
> multi-create op?
The group operation has the main goal of configuring a single WRR or SP
scheduling group atomically. It can creates the needed shapers as
needed, see below.
The need for such operation sparks from some H/W constraints:
https://lore.kernel.org/netdev/9dd818dc-1fef-4633-b388-6ce7272f9cb4@lunn.ch/
>> + 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.
>> + The operation is atomic, on failures the extack is set
>> + accordingly and no change is applied to the device
>> + shaping configuration, otherwise the output shaper
>> + handle is provided as reply.
>> + attribute-set: net-shaper
>> + flags: [ admin-perm ]
>
> Does there need to be a reciprocal 'ungroup' operation? Without it,
> create / group / delete seems like they will have ambiguous semantics.
I guess we need a better description. Can you please tell where/how the
current one is ambiguous?
Thanks,
Paolo
Powered by blists - more mailing lists