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

Message-ID: <20250429095809.1cbabba4@kernel.org>
Date: Tue, 29 Apr 2025 09:58:09 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Jiri Pirko <jiri@...nulli.us>
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

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.