Re: [PATCH v3] Many pages: Document fixed-width types with ISO C naming

[Alexei Starovoitov via Gcc-patches]

On Wed, Aug 24, 2022 at 12:04 PM Alejandro Colomar
 wrote:
>
> diff --git a/man2/bpf.2 b/man2/bpf.2
> index d05b73ec2..84d1b62e5 100644
> --- a/man2/bpf.2
> +++ b/man2/bpf.2
> @@ -169,34 +169,34 @@ commands:
>  .EX
>  union bpf_attr {
>  struct {/* Used by BPF_MAP_CREATE */
> -__u32 map_type;
> -__u32 key_size;/* size of key in bytes */
> -__u32 value_size;  /* size of value in bytes */
> -__u32 max_entries; /* maximum number of entries
> +uint32_t  map_type;
> +uint32_t  key_size;/* size of key in bytes */
> +uint32_t  value_size;  /* size of value in bytes */
> +uint32_t  max_entries; /* maximum number of entries
>in a map */
>  };
>
>  struct {/* Used by BPF_MAP_*_ELEM and BPF_MAP_GET_NEXT_KEY
> commands */
> -__u32 map_fd;
> +uint32_t  map_fd;
>  __aligned_u64 key;
>  union {
>  __aligned_u64 value;
>  __aligned_u64 next_key;
>  };
> -__u64 flags;
> +uint64_t  flags;
>  };
>
>  struct {/* Used by BPF_PROG_LOAD */
> -__u32 prog_type;
> -__u32 insn_cnt;
> +uint32_t  prog_type;
> +uint32_t  insn_cnt;

For the N-th time:
Nacked-by: Alexei Starovoitov 

Please stop sending this patch.

Re: [RFC] bpf.2: Use standard types and attributes

[Alexei Starovoitov via Gcc-patches]

On Sat, Apr 24, 2021 at 10:56 AM Alejandro Colomar (man-pages)
 wrote:
>
> Hello Alexei,
>
> On 4/24/21 1:20 AM, Alexei Starovoitov wrote:
> > Nack.
> > The man page should describe the kernel api the way it is in .h file.
>
> Why?

Because man page must describe the linux uapi headers the way they
are installed in the system and not invent alternative implementations.
The users will include those .h with __u32 and will see them in their code.
Man page saying something else is a dangerous lie.

> using uint32_t in every situation where __u32 is expected.  They're both
> typedefs for the same basic type.

That's irrelevant. Languages like golang have their own bpf.h equivalent
that matches /usr/include/linux/bpf.h.

> I can understand why Linux will keep using u32 types (and their __ user
> space variants), but that doesn't mean user space programs need to use
> the same type.

No one says that the users must use __u32. See golang example.
But if the users do #include  they will get them and man page
must describe that.

> If we have a standard syntax for fixed-width integral types (and for
> anything, actually), the manual pages should probably follow it,
> whenever possible.

Absolutely not. linux man page must describe linux.

Re: [RFC v2] bpf.2: Use standard types and attributes

[Alexei Starovoitov via Gcc-patches]

On Tue, May 4, 2021 at 4:05 AM Alejandro Colomar  wrote:
>
> Some manual pages are already using C99 syntax for integral
> types 'uint32_t', but some aren't.  There are some using kernel
> syntax '__u32'.  Fix those.
>
> Some pages also document attributes, using GNU syntax
> '__attribute__((xxx))'.  Update those to use the shorter and more
> portable C11 keywords such as 'alignas()' when possible, and C2x
> syntax '[[gnu::xxx]]' elsewhere, which hasn't been standardized
> yet, but is already implemented in GCC, and available through
> either --std=c2x or any of the --std=gnu... options.
>
> The standard isn't very clear on how to use alignas() or
> [[]]-style attributes, so the following link is useful in the case
> of 'alignas()' and '[[gnu::aligned()]]':
> 
>
> Signed-off-by: Alejandro Colomar 
> Cc: LKML 
> Cc: glibc 
> Cc: GCC 
> Cc: Alexei Starovoitov 
> Cc: bpf 
> Cc: David Laight 
> Cc: Zack Weinberg 
> Cc: Joseph Myers 
> ---
>  man2/bpf.2 | 49 -
>  1 file changed, 24 insertions(+), 25 deletions(-)
>
> diff --git a/man2/bpf.2 b/man2/bpf.2
> index 6e1ffa198..04b8fbcef 100644
> --- a/man2/bpf.2
> +++ b/man2/bpf.2
> @@ -186,41 +186,40 @@ commands:
>  .PP
>  .in +4n
>  .EX
> -union bpf_attr {
> +union [[gnu::aligned(8)]] bpf_attr {
>  struct {/* Used by BPF_MAP_CREATE */
> -__u32 map_type;
> -__u32 key_size;/* size of key in bytes */
> -__u32 value_size;  /* size of value in bytes */
> -__u32 max_entries; /* maximum number of entries
> -  in a map */
> +uint32_tmap_type;
> +uint32_tkey_size;/* size of key in bytes */
> +uint32_tvalue_size;  /* size of value in bytes */
> +uint32_tmax_entries; /* maximum number of entries
> +in a map */

For the same reasons as explained earlier:
Nacked-by: Alexei Starovoitov 

Re: [RFC v2] bpf.2: Use standard types and attributes

[Alexei Starovoitov via Gcc-patches]

On Tue, May 4, 2021 at 1:33 PM Zack Weinberg  wrote:
> the information that should
> appear in the manpages is the information that is most relevant to
> user space programmers.

The bpf programs are compiled for the kernel and run in the kernel.
Hence bpf man pages must reflect the kernel.
Also there is BTF where type names are part of the verification process.
if a user decides to rename a type it will be rejected by the kernel verifier.

Re: [RFC] bpf.2: Use standard types and attributes

[Alexei Starovoitov via Gcc-patches]

On Fri, Apr 23, 2021 at 4:15 PM Alejandro Colomar
 wrote:
>
> Some manual pages are already using C99 syntax for integral
> types 'uint32_t', but some aren't.  There are some using kernel
> syntax '__u32'.  Fix those.
>
> Some pages also document attributes, using GNU syntax
> '__attribute__((xxx))'.  Update those to use the shorter and more
> portable C2x syntax, which hasn't been standardized yet, but is
> already implemented in GCC, and available through either --std=c2x
> or any of the --std=gnu... options.
>
> Signed-off-by: Alejandro Colomar 
> ---
>  man2/bpf.2 | 47 +++
>  1 file changed, 23 insertions(+), 24 deletions(-)
>
> diff --git a/man2/bpf.2 b/man2/bpf.2
> index 6e1ffa198..204f01bfc 100644
> --- a/man2/bpf.2
> +++ b/man2/bpf.2
> @@ -188,39 +188,38 @@ commands:
>  .EX
>  union bpf_attr {
>  struct {/* Used by BPF_MAP_CREATE */
> -__u32 map_type;
> -__u32 key_size;/* size of key in bytes */
> -__u32 value_size;  /* size of value in bytes */
> -__u32 max_entries; /* maximum number of entries
> -  in a map */
> +uint32_tmap_type;
> +uint32_tkey_size;/* size of key in bytes */
> +uint32_tvalue_size;  /* size of value in bytes */
> +uint32_tmax_entries; /* maximum number of entries
> +in a map */

Nack.
The man page should describe the kernel api the way it is in .h file.