Re: [RFC bpf-next 00/12] Use uapi kernel headers with vmlinux.h

[Eduard Zingerman <eddyz87@gmail.com>]

On Tue, 2022-10-25 at 16:46 -0700, Alexei Starovoitov wrote:
> On Wed, Oct 26, 2022 at 01:27:49AM +0300, Eduard Zingerman wrote:
> > 
> > Include the following system header:
> > - /usr/include/sys/socket.h (all via linux/if.h)
> > 
> > The sys/socket.h conflicts with vmlinux.h in:
> > - types: struct iovec, struct sockaddr, struct msghdr, ...
> > - constants: SOCK_STREAM, SOCK_DGRAM, ...
> > 
> > However, only two types are actually used:
> > - struct sockaddr
> > - struct sockaddr_storage (used only in linux/mptcp.h)
> > 
> > In 'vmlinux.h' this type originates from 'kernel/include/socket.h'
> > (non UAPI header), thus does not have a header guard.
> > 
> > The only workaround that I see is to:
> > - define a stub sys/socket.h as follows:
> > 
> >     #ifndef __BPF_SOCKADDR__
> >     #define __BPF_SOCKADDR__
> >     
> >     /* For __kernel_sa_family_t */
> >     #include <linux/socket.h>
> >     
> >     struct sockaddr {
> >         __kernel_sa_family_t sa_family;
> >         char sa_data[14];
> >     };
> >     
> >     #endif
> > 
> > - hardcode generation of __BPF_SOCKADDR__ bracket for
> >   'struct sockaddr' in vmlinux.h.
> 
> we don't need to hack sys/socket.h and can hardcode
> #ifdef _SYS_SOCKET_H as header guard for sockaddr instead, right?
> bits/socket.h has this:
> #ifndef _SYS_SOCKET_H
> # error "Never include <bits/socket.h> directly; use <sys/socket.h> instead."
> 
> So that ifdef is kinda stable.

The `if.h` only uses two types from `sys/socket.h`, namely:
- `struct sockaddr`
- `struct sockaddr_storage`

However `sys/socket.h` itself defines more types, here is a complete
list of types from `sys/socket.h` that conflict with `vmlinux.h`
(generated for my x86_64 laptop):

Type name       System header
iovec           /usr/include/bits/types/struct_iovec.h
loff_t          /usr/include/sys/types.h
dev_t           /usr/include/sys/types.h
nlink_t         /usr/include/sys/types.h
timer_t         /usr/include/bits/types/timer_t.h
int16_t         /usr/include/bits/stdint-intn.h
int32_t         /usr/include/bits/stdint-intn.h
int64_t         /usr/include/bits/stdint-intn.h
u_int64_t       /usr/include/sys/types.h
sigset_t        /usr/include/bits/types/sigset_t.h
fd_set          /usr/include/sys/select.h
blkcnt_t        /usr/include/sys/types.h
SOCK_STREAM     /usr/include/bits/socket_type.h
SOCK_DGRAM      /usr/include/bits/socket_type.h
SOCK_RAW        /usr/include/bits/socket_type.h
SOCK_RDM        /usr/include/bits/socket_type.h
SOCK_SEQPACKET  /usr/include/bits/socket_type.h
SOCK_DCCP       /usr/include/bits/socket_type.h
SOCK_PACKET     /usr/include/bits/socket_type.h
sockaddr        /usr/include/bits/socket.h
msghdr          /usr/include/bits/socket.h
cmsghdr         /usr/include/bits/socket.h
linger          /usr/include/bits/socket.h
SHUT_RD         /usr/include/sys/socket.h
SHUT_WR         /usr/include/sys/socket.h
SHUT_RDWR       /usr/include/sys/socket.h

It would be safe to wrap the corresponding types in the vmlinux.h with
_SYS_SOCKET_H / _SYS_TYPES guards if the definitions above match
between libc and kernel. To my surprise not all of them match. Here is
the list of genuine conflicts (for typedefs I skip the intermediate
definitions and print the last typedef in the chain):

Type: dev_t
typedef unsigned int __u32                                vmlinux.h
typedef unsigned long int __dev_t         /usr/include/bits/types.h

Type: nlink_t
typedef unsigned int __u32                                vmlinux.h
typedef unsigned long int __nlink_t       /usr/include/bits/types.h

Type: timer_t
typedef int __kernel_timer _t                             vmlinux.h
typedef void *__timer_t                   /usr/include/bits/types.h

Type: sigset_t
typedef struct                                            vmlinux.h
{
  long unsigned int sig[1];
} sigset_t

typedef struct                 /usr/include/bits/types/__sigset_t.h
{
  unsigned long int __val[1024 / (8 * (sizeof(unsigned long int)))];
} __sigset_t

Type: fd_set
typedef struct                                            vmlinux.h
{
  long unsigned int fds_bits[16];
} __kernel_fd_set

typedef struct                            /usr/include/sys/select.h
{
  __fd_mask __fds_bits[1024 / (8 * ((int) (sizeof(__fd_mask))))];
} fd_set

Type: sigset_t
typedef struct                                            vmlinux.h 
{
  long unsigned int sig[1];
} sigset_t

typedef struct                 /usr/include/bits/types/__sigset_t.h
{
  unsigned long int __val[1024 / (8 * (sizeof(unsigned long int)))];
} __sigset_t

Type: msghdr
struct msghdr                                             vmlinux.h
{
  void *msg_name;
  int msg_namelen;
  int msg_inq;
  struct iov_iter msg_iter;
  union 
  {
    void *msg_control;
    void *msg_control_user;
  };
  bool msg_control_is_user : 1;
  bool msg_get_inq : 1;
  unsigned int msg_flags;
  __kernel_size_t msg_controllen;
  struct kiocb *msg_iocb;
  struct ubuf_info *msg_ubuf;
  struct sock *, struct sk_buff *, struct iov_iter *, size_tint;
}

struct msghdr                            /usr/include/bits/socket.h
{
  void *msg_name;
  socklen_t msg_namelen;
  struct iovec *msg_iov;
  size_t msg_iovlen;
  void *msg_control;
  size_t msg_controllen;
  int msg_flags;
}

> 
> > Another possibility is to move the definition of 'struct sockaddr'
> > from 'kernel/include/socket.h' to 'kernel/include/uapi/linux/socket.h',
> > but I expect that this won't fly with the mainline as it might break
> > the programs that include both 'linux/socket.h' and 'sys/socket.h'.
> > 
> > Conflict with vmlinux.h
> > ----
> > 
> > Uapi header:
> > - linux/signal.h
> > 
> > Conflict with vmlinux.h in definition of 'struct sigaction'.
> > Defined in:
> > - vmlinux.h: kernel/include/linux/signal_types.h
> > - uapi:      kernel/arch/x86/include/asm/signal.h
> > 
> > Uapi headers:
> > - linux/tipc_sockets_diag.h
> > - linux/sock_diag.h
> > 
> > Conflict with vmlinux.h in definition of 'SOCK_DESTROY'.
> 
> Interesting one!
> I think we can hard code '#undef SOCK_DESTROY' in vmlinux.h
> 
> The goal is not to be able to mix arbitrary uapi header with
> vmlinux.h, but only those that could be useful out of bpf progs.
> Currently it's tcp.h and few other network related headers
> because they have #define-s in them that are useful inside bpf progs.
> As long as the solution covers this small subset we're fine.

Well, tcp.h works :) It would be great if someone could list the
interesting headers.

Thanks,
Eduard