netdev - Re: [patch net-next 02/10] tools: ynl-gen: introduce support for bitfield32 attribute type

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: <ZSe8SGY3QeaJsYfg@nanopsycho>
Date: Thu, 12 Oct 2023 11:28:40 +0200
From: Jiri Pirko <jiri@...nulli.us>
To: Jakub Kicinski <kuba@...nel.org>
Cc: netdev@...r.kernel.org, pabeni@...hat.com, davem@...emloft.net,
	edumazet@...gle.com, jacob.e.keller@...el.com,
	johannes@...solutions.net
Subject: Re: [patch net-next 02/10] tools: ynl-gen: introduce support for
 bitfield32 attribute type

Wed, Oct 11, 2023 at 08:25:37PM CEST, kuba@...nel.org wrote:
>On Wed, 11 Oct 2023 19:04:42 +0200 Jiri Pirko wrote:
>> >> Why? Should be usable for all, same as other types, no?  
>> >
>> >array-nest already isn't. I don't see much value in bitfiled32
>> >and listing it means every future codegen for genetlink will
>> >have to support it to be compatible. It's easier to add stuff
>> >than to remove it, so let's not.  
>> 
>> Interesting. You want to somehow mark bitfield32 obsolete? But why is
>> it? I mean, what is the reason to discourage use of bitfield32?
>
>It's a tradeoff between simplicity of base types and usefulness.
>bitfield32 is not bad in any way, but:
>
> - it's 32b, new features/caps like to start with 64b

That's fun. Back when Jamal (I think it was him) was pushing bitfield32,
I argued that it would be better to make it flexible bitfield so it it
future proof. IIRC DavidM said that it should be enough and that you can
use extra attr in case the current one overflows.

Sigh :/

> - it doesn't support "by name" operations so ethtool didn't use it

It follows the original Netlink rule: "all uapi should be well defined in
enums/defines".

> - it can be trivially re-implemented with 2 attrs

Yeah, it's basically a wrapper to avoid unnecessary boilerplate and
re-implementations. But I think that is a good thing. Or do you say it
is not desirable to rather re-implement this with 2 attrs instead of
using bitfield32 directly? 

>
>all in all there aren't very many new uses. So I think we should
>put it in legacy for now. Maybe somehow mark it as being there due
>to limited applicability rather than being "bad"?

I think it is odd, but if you insists, sure. Your the boss.