lkml.org 
[lkml]   [2021]   [May]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [RFC v2] bpf.2: Use standard types and attributes
From
Date
Hi Greg, Daniel,

On 5/4/21 6:06 PM, Greg KH wrote:
> There's a very old post from Linus where he describes the difference
> between things like __u32 and uint32_t. They are not the same, they
> live in different namespaces, and worlds, and can not always be swapped
> out for each other on all arches.>
> Dig it up if you are curious, but for user/kernel apis you HAVE to use
> the __uNN and can not use uintNN_t variants, so don't try to mix/match
> them, it's good to just follow the kernel standard please.
I found these:

* [RFC] Splitting kernel headers and deprecating __KERNEL__
<https://lore.kernel.org/lkml/Pine.LNX.4.58.0412140734340.3279@ppc970.osdl.org/T/>

* coding style
<https://lore.kernel.org/lkml/alpine.LFD.0.98.0706160840290.14121@woody.linux-foundation.org/>

* [patch] Small input fixes for 2.5.29
<https://lore.kernel.org/lkml/Pine.LNX.4.33.0207301417190.2051-100000@penguin.transmeta.com/T/>

I already knew the first one, and now found the other two. If there's
any other thread that is relevant, I couldn't find it.

The thing is, in all of those threads, the only reasons to avoid
<stdint.h> types in the kernel (at least, the only explicitly mentioned
ones) are (a bit simplified, but this is the general idea of those threads):

* Possibly breaking something in such a big automated change.
* Namespace collision with userspace (the C standard allows defining
uint32_t for nefarious purposes as long as you don't include <stdint.h>.
POSIX prohibits that, though)
* Uglier

But

* The manual pages only document the variable size and signedness by
using either '__u32' or 'uint32_t'. We state that the variable is an
unsigned integer of exactly 32 bits; nothing more and nothing less. It
doesn't specify that those types are defined in <linux/bpf.h> (or
whatever header a specific manual page uses). In fact, in uint32_t(3)
we clearly state the headers that shall provide the type. In the end,
the kernel will receive a 32 bit number. I'm not exactly sure about
what is wrong with this. Is there any magic in the kernel/user
interface beyond what the standard and the compiler define that I ignore?

* At that time (~2004), the C99 and POSIX.1-2001 standards were quite
young, and it was likely to find code that defined uint32_t. Currently,
it is hard to find something that compiles without C99, and even if C99
allows you to define uint32_t as long as you don't include <stdint.h>,
it would be really stupid to do so. And POSIX, which completely
prohibits defining uint32_t, is also very present in Linux and other
UNIX systems. So we can probably guarantee that using <stdint.h> in the
kernel wouldn't break anything. But yet this isn't trying to do so.
This is only about the manual pages.

I haven't read it in any of those threads, but suspect that the static
analyzer used for the kernel might use extra information from the
different 'u32'/'__u32' type names to do some extra checks. Does it?

> and can not always be swapped out for each other on all arches.

Really? 'uint32_t' is defined as "an unsigned integer type of a fixed
width of exactly 32 bits". How is that different from '[__]u32'?
Aren't the kernel types guaranteed to be unsigned integers of exactly 32
bits? AFAICT, they are 100% binary compatible; and if not, it's
probably a kernel bug.

Yes there are archs that don't provide 64 bit integers (I ignore if any
of the archs supported by Linux does though), but if an arch doesn't
provide 'uint64_t', it will neither be possible to have '__u64'.

[
uintN_t
Include: <stdint.h>. Alternatively, <inttypes.h>.

uint8_t, uint16_t, uint32_t, uint64_t

An unsigned integer type of a fixed width of ex‐
actly N bits, N being the value specified in its
type name. According to the C language standard,
they shall be capable of storing values in the
range [0, UINTN_MAX], substituting N by the appro‐
priate number.

According to POSIX, uint8_t, uint16_t, and
uint32_t are required; uint64_t is only required
in implementations that provide integer types with
width 64; and all other types of this form are op‐
tional.

] -- uint32_t(3)


>
> So consider this my:
>
> Nacked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>
> as well.
Okay.

On 5/4/21 6:08 PM, Daniel Borkmann wrote:
>
> But what /problem/ is this really solving? Why bother to change this
/now/
> after so many years?! I think this is causing more confusion than solving
> anything, really. Moreover, what are you doing with all the
> __{le,be}{16,32,64}
> types in uapi? Anyway, NAK for bpf.2 specifically, and the idea
generally..
>

I'm trying to clarify the manual pages as much as possible, by using
standard conventions and similar structure all around the pages. Not
everyone understands kernel conventions. Basically, Zack said very much
what I had in mind with this patch.


Thanks for your reviews!

Regards,

Alex

--
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/

\
 
 \ /
  Last update: 2021-05-04 21:39    [W:0.383 / U:0.096 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site