netdev - Re: [PATCH net-next V3 14/15] devlink: Implement devlink param multi attribute nested data values

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] [day] [month] [year] [list]

Message-ID: <ggvcsmrox4iyozy664zz4gmp54s4s67gbgfldgkx4i4vdebfsn@tnn3ozx5yiol>
Date: Wed, 30 Apr 2025 08:24:59 +0200
From: Jiri Pirko <jiri@...nulli.us>
To: Jakub Kicinski <kuba@...nel.org>
Cc: Saeed Mahameed <saeed@...nel.org>, 
	"David S. Miller" <davem@...emloft.net>, Paolo Abeni <pabeni@...hat.com>, 
	Eric Dumazet <edumazet@...gle.com>, Saeed Mahameed <saeedm@...dia.com>, netdev@...r.kernel.org, 
	Tariq Toukan <tariqt@...dia.com>, Gal Pressman <gal@...dia.com>, 
	Leon Romanovsky <leonro@...dia.com>, Jiri Pirko <jiri@...dia.com>
Subject: Re: [PATCH net-next V3 14/15] devlink: Implement devlink param multi
 attribute nested data values

Tue, Apr 29, 2025 at 06:58:09PM +0200, kuba@...nel.org wrote:
>On Tue, 29 Apr 2025 13:34:57 +0200 Jiri Pirko wrote:
>> >I'd really rather not build any more complexity into this funny
>> >indirect attribute construct. Do you have many more arrays to expose?  
>> 
>> How else do you imagine to expose arrays in params?
>> Btw, why is it "funny"? I mean, if you would be designing it from
>> scratch, how would you do that (params with multiple types) differently?
>> From netlink perspective there's nothing wrong with it, is it?
>
>The attribute type (nla_type) should define the nested type. Having 
>the nested type carried as a value in another attribute makes writing
>generic parsers so much harder. I made a similar mistake in one the the
>ethtool commands.
>
>We should have basically have separate attr types for each of the value
>sizes:
>	DEVLINK_ATTR_PARAM_VALUE_DATA_U32
>	DEVLINK_ATTR_PARAM_VALUE_DATA_BOOL
>etc. They should be in a separate attr space, not the main devlink_attr
>one, but every type should have its own value_data attr type.

You say *should*, but where's the rule? I mean, the reader/parse is in
charge of determining the type of attr. How he does it is up to him, if
it is fixed policy or some dynamic means. Why not?

Looks like a matter of personal preference to me.