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: <x6dxuv4fakclafbubk22akzlrsu43ds24jgj6t4uqtx7tpmim3@zxnggsxzh6sc>
Date: Fri, 2 May 2025 13:16:06 +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

Wed, Apr 30, 2025 at 08:24:59AM +0200, jiri@...nulli.us wrote:
>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.

Anyway, we can't do this for params as it would break the UAPI :/
So we are stuck with the current approach.