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
