netdev - Re: [RFC] netlink: add variable-length / auto integers

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: <8d3fc502-f568-4fab-96e3-aead6bd29063@6wind.com>
Date: Thu, 12 Oct 2023 08:47:55 +0200
From: Nicolas Dichtel <nicolas.dichtel@...nd.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: Johannes Berg <johannes@...solutions.net>, netdev@...r.kernel.org,
 fw@...len.de, pablo@...filter.org, jiri@...nulli.us, mkubecek@...e.cz,
 aleksander.lobakin@...el.com, Thomas Haller <thaller@...hat.com>
Subject: Re: [RFC] netlink: add variable-length / auto integers



Le 11/10/2023 à 17:52, Jakub Kicinski a écrit :
> On Wed, 11 Oct 2023 16:03:26 +0200 Nicolas Dichtel wrote:
>>> On Tue, 2023-10-10 at 17:33 -0700, Jakub Kicinski wrote:  
>>>> We currently push everyone to use padding to align 64b values in netlink.
>>>> I'm not sure what the story behind this is. I found this:
>>>> https://lore.kernel.org/all/1461339084-3849-1-git-send-email-nicolas.dichtel@6wind.com/#t  
>> There was some attempts before:
>> https://lore.kernel.org/netdev/20121205.125453.1457654258131828976.davem@davemloft.net/
>> https://lore.kernel.org/netdev/1355500160.2626.9.camel@bwh-desktop.uk.solarflarecom.com/
>> https://lore.kernel.org/netdev/1461142655-5067-1-git-send-email-nicolas.dichtel@6wind.com/
>>
>>>> but it doesn't go into details WRT the motivation.
>>>> Even for arches which don't have good unaligned access - I'd think
>>>> that access aligned to 4B *is* pretty efficient, and that's all
>>>> we need. Plus kernel deals with unaligned input. Why can't user space?  
>>>
>>> Hmm. I have a vague recollection that it was related to just not doing
>>> it - the kernel will do get_unaligned() or similar, but userspace if it
>>> just accesses it might take a trap on some architectures?
>>>
>>> But I can't find any record of this in public discussions, so ...  
>> If I remember well, at this time, we had some (old) architectures that triggered
>> traps (in kernel) when a 64-bit field was accessed and unaligned. Maybe a mix
>> between 64-bit kernel / 32-bit userspace, I don't remember exactly. The goal was
>> to align u64 fields on 8 bytes.
> 
> Reading the discussions I think we can chalk the alignment up 
> to "old way of doing things". Discussion was about stats64, 
> if someone wants to access stats directly in the message then yes, 
> they care a lot about alignment.
> 
> Today we try to steer people towards attr-per-field, rather than
> dumping structs. Instead of doing:
> 
> 	struct stats *stats = nla_data(attr);
> 	print("A: %llu", stats->a);
> 
> We will do:
> 
> 	print("A: %llu", nla_get_u64(attrs[NLA_BLA_STAT_A]));
> 
> Assuming nla_get_u64() is unalign-ready the problem doesn't exist.
> 
> If user space goes thru a standard parsing library like YNL
> the application never even sees the raw netlink message,
> and deals with deserialized structs.
> 
> 
> Does the above sounds like a fair summary? If so I'll use it in 
> the commit message?
I think it is, it's ok for me.


Thank you,
Nicolas