netdev - Re: [PATCH v2 net-next 3/3] tools: ynl: add support for encoding multi-attr

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2b3ec0f1-303d-4e0c-92de-5d0430470c33@gmail.com>
Date: Fri, 2 Feb 2024 12:38:11 +0100
From: Alessandro Marcolini <alessandromarcolini99@...il.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: davem@...emloft.net, edumazet@...gle.com, pabeni@...hat.com,
 donald.hunter@...il.com, sdf@...gle.com, chuck.lever@...cle.com,
 lorenzo@...nel.org, jacob.e.keller@...el.com, jiri@...nulli.us,
 netdev@...r.kernel.org
Subject: Re: [PATCH v2 net-next 3/3] tools: ynl: add support for encoding
 multi-attr

On 2/2/24 02:24, Jakub Kicinski wrote:
> I think you're trying to handle this at the wrong level. The main
> message can also contain multi-attr, so looping inside nests won't
> cut it.
>
> Early in the function check if attr.is_multi and isinstance(value,
> list), and if so do:
>
> 	attr_payload = b''
> 	for subvalue in value:
> 		attr_payload += self._add_attr(space, name, subvalue,
> 					       search_attrs) 
> 	return attr_payload
>
> IOW all you need to do is recursively call _add_attr() with the
> subvalues stripped. You don't have to descend into a nest.

I (wrongly) supposed that multi-attr attributes were always inside a nest (that's because I've only experimented with the tc spec). That's also because I (mistakenly, again) thought that the syntax for specifying a multi-attr would be:
"parent-attr":[{multi-attr:{values}}, {multi-attr: {values}}, ... ]
Instead of:
"optional-parent-attr": {"multi-attr": [{values in multi-attr}, ...]}

By reading the docs [1]:
"multi-attr (arrays)
Boolean property signifying that the attribute may be present multiple times. Allowing an attribute to repeat is the recommended way of implementing arrays (no extra nesting)."

I understood that the syntax should be the former (I was thinking of an array containing all the multi-attr attributes, and not only their values), albeit really verbose and not that readable.

I've now made the changes as you suggested and tested it, it works as expected!
I'll post a v3 soon, thanks for your review :)

[1] https://docs.kernel.org/userspace-api/netlink/specs.html#multi-attr-arrays