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

[Johannes Berg <johannes@sipsolutions.net>]

On Wed, 2023-10-11 at 08:52 -0700, Jakub Kicinski wrote:

> > > > 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]));

Well, yes, although the "struct stats" part _still_ even exists in the
kernel, we never fixed that with the nla_put_u64_64bit() stuff, that was
only for something that does

	print("A: %" PRIu64, *(uint64_t *)nla_data(attrs[NLA_BLA_STAT_A]));

> Assuming nla_get_u64() is unalign-ready the problem doesn't exist.

Depends on the library, but at least for libnl that's true since ever.
Same for libmnl and libnl-tiny. So I guess it only ever hit hand-coded
implementations.

For the record, I'm pretty sure (and was at the time) that for wifi
(nl80211) this was never an issue, but ...

> Does the above sounds like a fair summary? If so I'll use it in 
> the commit message?

As I said above, not sure about the whole struct thing - that's still
kind of broken and was never addressed by this, but otherwise yes.

johannes