[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <m2v80jnpkd.fsf@gmail.com>
Date: Fri, 02 Aug 2024 12:15:30 +0100
From: Donald Hunter <donald.hunter@...il.com>
To: Paolo Abeni <pabeni@...hat.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
Paolo Abeni <pabeni@...hat.com> writes:
> On 7/31/24 23:13, Donald Hunter wrote:
>> Paolo Abeni <pabeni@...hat.com> writes:
>>
>>> + 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``.
>
> [...]
>>> + -
>>> + 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?
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.
Powered by blists - more mailing lists