netdev - Re: [RFC net-next 2/4] ynl: add the schema for the schemas

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: <c4a4744c0f0a86433beec5035f2150b8427eb3d5.camel@sipsolutions.net>
Date:   Tue, 16 Aug 2022 21:30:54 +0200
From:   Johannes Berg <johannes@...solutions.net>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, edumazet@...gle.com,
        pabeni@...hat.com, sdf@...gle.com, jacob.e.keller@...el.com,
        vadfed@...com, jiri@...nulli.us, dsahern@...nel.org,
        stephen@...workplumber.org, fw@...len.de, linux-doc@...r.kernel.org
Subject: Re: [RFC net-next 2/4] ynl: add the schema for the schemas

On Tue, 2022-08-16 at 08:53 -0700, Jakub Kicinski wrote:
> 
> My guess was that some of the wrapping was for ease of canceling here
> (cancel is used both on skip and on error). 
> 

Not sure I'd say that, but can't say I really remember why I did it this
way.

> What I think we should push
> for is multi-attr, so the same attribute happens multiple times.
> 
> [msg]
>  [ATTR1]
>  [ATTR2] // elem 1
>    [SubATTR1]
>    [SubATTR2]
>  [ATTR2] // elem 2
>    [SubATTR1]
>    [SubATTR2]
>  [ATTR2] // elem 3
>    [SubATTR1]
>    [SubATTR2]
>  [ATTR3]
>  [ATTR4]
> 
> Instead of wrapping into an array and then elements.

Hmm, ok, I guess that works.

> 
> As Michal pointed out a number of times - the wrapping ends up limiting 
> the size of the array to U16_MAX,

True.

> and I have a suspicion that most of
> wrapping is done because we tend to parse into a pointer array, which
> makes multi-attr a little tricky. But we shouldn't let one parsing
> technique in a relatively uncommon language like C dictate the format :)

:-)

To be fair, for cases where today we use nla_for_each_nested() we could
also invent an "nlmsg_for_each_attr_of_type()" macro:

#define nlmsg_for_each_attr_of_type(type, pos, nlh, hdrlen, rem) \
	nlmsg_for_each_attr(pos, nlh, hdrlen, rem)               \
		if (pos->nla_type == type)

and then that's basically all you need?

In the policy we'd declare it as a normal nested (not array), and I
think that's it because today if you give the same attribute type twice,
the last one wins in the normal parsing anyway (IIRC)...

> I'm leaning heavily towards defining a subset of the YAML spec as 
> "the way to do things in new family" which will allow only one form 
> of arrays.

Fair enough.

johannes