Thread (91 messages) 91 messages, 5 authors, 2024-08-29

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

From: Donald Hunter <donald.hunter@gmail.com>
Date: 2024-08-02 11:23:43

Paolo Abeni [off-list ref] writes:
On 7/31/24 23:13, Donald Hunter wrote:
quoted
Paolo Abeni [off-list ref] writes:
quoted
+        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.
Yeah, I was just thinking of using .rst markup like ``code`` or
`italics`, but the meaning of @ is pretty obvious when reading the spec.
If you stick with @ then we could always teach ynl-to-rst to render it
as ``code``.
[...]
quoted
quoted
+    -
+      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/ (local)
quoted
quoted
+        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?
My expectation for 'group' would be to group existing things, with a
reciprocal 'ungroup' operation. I think you intend 'group' to both be
able to group existing shapers/groups and create a group of shapers.

Am I right in saying that delete lets you delete something from a group
(with side-effect of deleting group if it becomes empty), or delete a
whole group?

It feels a lot like each of 'set', 'group' and 'delete' are doing
multiple things and the interaction between them all becomes challenging
to describe, or to handle all the corner cases. I think part of the
problem is the mixed terminology of input, output for groups, handle,
parent for shapers and using detached to differentiate from 'implicitly
attached to a resource'.

Perhaps the API would be better if you had:

- shaper-new
- shaper-delete
- shaper-get/dump
- shaper-set
- group-new
- group-delete
- group-get/dump
- group-set

If you went with Jakub's suggestion to give every shaper n x inputs and
an output, then you could recombine groups and shapers and just have 4
ops. And you could rename 'detached' to 'shaper' so that an attachment
is one of port, netdev, queue or shaper.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help