netdev - Fwd: [PATCH net-next 1/5] 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>] [day] [month] [year] [list]
Date: Tue, 2 Jul 2024 21:51:13 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Network Development <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>
Subject: Fwd: [PATCH net-next 1/5] netlink: spec: add shaper YAML spec

Oops, I unintentionally dropped most recipients, re-adding them, sorry
for the duplicate.

On Tue, Jul 2, 2024 at 5:05 PM Jakub Kicinski <kuba@...nel.org> wrote:
> On Tue, 02 Jul 2024 16:21:38 +0200 Paolo Abeni wrote:
> > > I see, I had a look at patch 2 now.
> > > But that's really "Andrew's use-case" it doesn't cover deletion, right?
> > > Sorry that I don't have a perfect suggestion either but it seems like
> > > a half-measure. It's a partial support for transactions. If we want
> > > transactions we should group ops like nftables. Have normal ops (add,
> > > delete, modify) and control ops (start, commit) which clone the entire
> > > tree, then ops change it, and commit presents new tree to the device.
> >
> > Yes, it does not cover deletion _and_ update/add/move within the same
> > atomic operation.
> >
> > Still any configuration could be reached from default/initial state
> > with set(<possibly many shapers>). Additionally, given any arbitrary
> > configuration, the default/initial state could be restored with a
> > single delete(<possibly many handlers>).
>
> From:
>
> q0 -. RR \
> q1 /      > SP
> q2 -. RR /
> q3 /

Call this C1

> To:
>
> q0 ------\
> q1 -------> SP
> q2 -. RR /
> q3 /

Call this C2

> You have to both delete an RR node, and set SP params on Q0 and Q1.

default -> C1:

./tools/net/ynl/cli.py --spec Documentation/netlink/specs/shaper.yaml \
   --do set --json '{ "ifindex":2, "shapers": [ \
                         { "parent": { "scope": "netdev"}, "handle": {
"scope": "detached", "id": 1 }, "priority": 1 },
                         { "parent": { "scope": "netdev"}, "handle": {
"scope": "detached", "id": 2 }, "priority": 2 },
                         { "parent": { "scope": "detached", "id":1},
"handle": { "scope": "queue", "id": 1 }, "weight": 1 },
                         { "parent": { "scope": "detached", "id":1},
"handle": { "scope": "queue", "id": 2 }, "weight": 2 },
                         { "parent": { "scope": "detached" "id":2},
"handle": { "scope": "queue", "id": 3 }, "weight": 1 },
                         { "parent": { "scope": "detached" "id":2},
"handle": { "scope": "queue", "id": 4 }, "weight": 2 }]}
C1 -> C2:

./tools/net/ynl/cli.py --spec Documentation/netlink/specs/shaper.yaml \
   --do delete --json '{ "ifindex":2, "handles": [ \
                         { "scope": "queue", "id": 1 },
                         { "scope": "queue", "id": 2 },
                         { "scope": "queue", "id": 3 },
                         { "scope": "queue", "id": 4 },
                         {  "scope": "detached", "id": 1 },
                         {  "scope": "detached", "id": 2 }]}

./tools/net/ynl/cli.py --spec Documentation/netlink/specs/shaper.yaml \
   --do set --json '{ "ifindex":2, "shapers": [ \
                         { "parent": { "scope": "netdev"}, "handle": {
"scope": "detached", "id": 1 }, "priority": 1 },
                         { "parent": { "scope": "netdev"}, "handle": {
"scope": "queue", "id": 1 }, "priority": 2 },
                         { "parent": { "scope": "netdev"}, "handle": {
"scope": "queue", "id": 2 }, "priorirty": 3 },
                         { "parent": { "scope": "detached" "id":1},
"handle": { "scope": "queue", "id": 3 }, "weight": 1 },
                         { "parent": { "scope": "detached" "id":1},
"handle": { "scope": "queue", "id": 4 }, "weight": 2 },

The goal is to allow the system to reach the final configuration, even
with the assumption the H/W does not support any configuration except
the starting one and the final one.
But the infra requires that the system _must_ support at least a 3rd
configuration, the default one.

> > The above covers any possible limitation enforced by the H/W, not just
> > the DSA use-case.
> >
> > Do you have a strong feeling for atomic transactions from any arbitrary
> > state towards any other? If so, I’d like to understand why?
>
> I don't believe this is covers all cases.

Given any pair of configurations C1, C2 we can reach C2 via C1 ->
default, default -> C2. respecting any H/W constraint.

> > Dealing with transactions allowing arbitrary any state <> any state
> > atomic changes will involve some complex logic that seems better
> > assigned to user-space.
>
> Complex logic in which part of the code?

IIRC in a past iteration you pointed out that the complexity of
computing the delta between 2 arbitrary configurations is
significantly higher than going through the empty/default
configuration.

In any case I think that the larger complexity to implement a full
transactional model. nft had proven that to be very hard and bug
prone. I really would avoid that option, if possible.

Thanks,

Paolo