| Message ID | 20220227142551.2349805-1-james.hilliard1@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | [1/1] libbpf: ensure F_DUPFD_CLOEXEC is defined | expand |
| Context | Check | Description |
|---|---|---|
| bpf/vmtest-bpf-next-PR | success | PR summary |
| bpf/vmtest-bpf-next | success | VM_Test |
| netdev/tree_selection | success | Not a local patch |
Hi James, On 2/27/22 3:25 PM, James Hilliard wrote: > This definition seems to be missing from some older toolchains. > > Note that the fcntl.h in libbpf_internal.h is not a kernel header > but rather a toolchain libc header. > > Fixes: > libbpf_internal.h:521:18: error: 'F_DUPFD_CLOEXEC' undeclared (first use in this function); did you mean 'FD_CLOEXEC'? > fd = fcntl(fd, F_DUPFD_CLOEXEC, 3); > ^~~~~~~~~~~~~~~ > FD_CLOEXEC > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> Do you have some more info on your env (e.g. libc)? Looks like F_DUPFD_CLOEXEC was added back in 2.6.24 kernel. When did libc add it? Should we instead just add an include for <linux/fcntl.h> to libbpf_internal.h (given it defines F_DUPFD_CLOEXEC as well)? > --- > tools/lib/bpf/libbpf_internal.h | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h > index 4fda8bdf0a0d..d2a86b5a457a 100644 > --- a/tools/lib/bpf/libbpf_internal.h > +++ b/tools/lib/bpf/libbpf_internal.h > @@ -31,6 +31,10 @@ > #define EM_BPF 247 > #endif > > +#ifndef F_DUPFD_CLOEXEC > +#define F_DUPFD_CLOEXEC 1030 > +#endif > + > #ifndef R_BPF_64_64 > #define R_BPF_64_64 1 > #endif > Thanks, Daniel
On Mon, Feb 28, 2022 at 7:00 AM Daniel Borkmann <daniel@iogearbox.net> wrote: > > Hi James, > > On 2/27/22 3:25 PM, James Hilliard wrote: > > This definition seems to be missing from some older toolchains. > > > > Note that the fcntl.h in libbpf_internal.h is not a kernel header > > but rather a toolchain libc header. > > > > Fixes: > > libbpf_internal.h:521:18: error: 'F_DUPFD_CLOEXEC' undeclared (first use in this function); did you mean 'FD_CLOEXEC'? > > fd = fcntl(fd, F_DUPFD_CLOEXEC, 3); > > ^~~~~~~~~~~~~~~ > > FD_CLOEXEC > > > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> > > Do you have some more info on your env (e.g. libc)? Looks like F_DUPFD_CLOEXEC > was added back in 2.6.24 kernel. When did libc add it? It seems like it's guarded by __USE_XOPEN2K8 in glibc (from a quick glance at glibc code). But it's been there since 2010 or so, at the very least. > > Should we instead just add an include for <linux/fcntl.h> to libbpf_internal.h > (given it defines F_DUPFD_CLOEXEC as well)? yep, this is UAPI header so we can use it easily (we'll need to sync it into Github repo, but that's not a problem) > > > --- > > tools/lib/bpf/libbpf_internal.h | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h > > index 4fda8bdf0a0d..d2a86b5a457a 100644 > > --- a/tools/lib/bpf/libbpf_internal.h > > +++ b/tools/lib/bpf/libbpf_internal.h > > @@ -31,6 +31,10 @@ > > #define EM_BPF 247 > > #endif > > > > +#ifndef F_DUPFD_CLOEXEC > > +#define F_DUPFD_CLOEXEC 1030 > > +#endif > > + > > #ifndef R_BPF_64_64 > > #define R_BPF_64_64 1 > > #endif > > > > Thanks, > Daniel
On 3/4/22 8:01 PM, Andrii Nakryiko wrote: > On Mon, Feb 28, 2022 at 7:00 AM Daniel Borkmann <daniel@iogearbox.net> wrote: >> On 2/27/22 3:25 PM, James Hilliard wrote: >>> This definition seems to be missing from some older toolchains. >>> >>> Note that the fcntl.h in libbpf_internal.h is not a kernel header >>> but rather a toolchain libc header. >>> >>> Fixes: >>> libbpf_internal.h:521:18: error: 'F_DUPFD_CLOEXEC' undeclared (first use in this function); did you mean 'FD_CLOEXEC'? >>> fd = fcntl(fd, F_DUPFD_CLOEXEC, 3); >>> ^~~~~~~~~~~~~~~ >>> FD_CLOEXEC >>> >>> Signed-off-by: James Hilliard <james.hilliard1@gmail.com> >> >> Do you have some more info on your env (e.g. libc)? Looks like F_DUPFD_CLOEXEC >> was added back in 2.6.24 kernel. When did libc add it? > > It seems like it's guarded by __USE_XOPEN2K8 in glibc (from a quick > glance at glibc code). But it's been there since 2010 or so, at the > very least. > >> Should we instead just add an include for <linux/fcntl.h> to libbpf_internal.h >> (given it defines F_DUPFD_CLOEXEC as well)? > > yep, this is UAPI header so we can use it easily (we'll need to sync > it into Github repo, but that's not a problem) Sgtm, James, could you respin with using the include? Thanks, Daniel
On Mon, Feb 28, 2022 at 8:00 AM Daniel Borkmann <daniel@iogearbox.net> wrote: > > Hi James, > > On 2/27/22 3:25 PM, James Hilliard wrote: > > This definition seems to be missing from some older toolchains. > > > > Note that the fcntl.h in libbpf_internal.h is not a kernel header > > but rather a toolchain libc header. > > > > Fixes: > > libbpf_internal.h:521:18: error: 'F_DUPFD_CLOEXEC' undeclared (first use in this function); did you mean 'FD_CLOEXEC'? > > fd = fcntl(fd, F_DUPFD_CLOEXEC, 3); > > ^~~~~~~~~~~~~~~ > > FD_CLOEXEC > > > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> > > Do you have some more info on your env (e.g. libc)? Looks like F_DUPFD_CLOEXEC > was added back in 2.6.24 kernel. When did libc add it? > > Should we instead just add an include for <linux/fcntl.h> to libbpf_internal.h > (given it defines F_DUPFD_CLOEXEC as well)? That seems to cause a conflict: error: redefinition of ‘struct flock’ > > > --- > > tools/lib/bpf/libbpf_internal.h | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h > > index 4fda8bdf0a0d..d2a86b5a457a 100644 > > --- a/tools/lib/bpf/libbpf_internal.h > > +++ b/tools/lib/bpf/libbpf_internal.h > > @@ -31,6 +31,10 @@ > > #define EM_BPF 247 > > #endif > > > > +#ifndef F_DUPFD_CLOEXEC > > +#define F_DUPFD_CLOEXEC 1030 > > +#endif > > + > > #ifndef R_BPF_64_64 > > #define R_BPF_64_64 1 > > #endif > > > > Thanks, > Daniel
On Fri, Mar 4, 2022 at 12:01 PM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Mon, Feb 28, 2022 at 7:00 AM Daniel Borkmann <daniel@iogearbox.net> wrote: > > > > Hi James, > > > > On 2/27/22 3:25 PM, James Hilliard wrote: > > > This definition seems to be missing from some older toolchains. > > > > > > Note that the fcntl.h in libbpf_internal.h is not a kernel header > > > but rather a toolchain libc header. > > > > > > Fixes: > > > libbpf_internal.h:521:18: error: 'F_DUPFD_CLOEXEC' undeclared (first use in this function); did you mean 'FD_CLOEXEC'? > > > fd = fcntl(fd, F_DUPFD_CLOEXEC, 3); > > > ^~~~~~~~~~~~~~~ > > > FD_CLOEXEC > > > > > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> > > > > Do you have some more info on your env (e.g. libc)? Looks like F_DUPFD_CLOEXEC > > was added back in 2.6.24 kernel. When did libc add it? > > It seems like it's guarded by __USE_XOPEN2K8 in glibc (from a quick > glance at glibc code). But it's been there since 2010 or so, at the > very least. The toolchain that hit this issue appears to be uclibc based which seems to have had some bugs with the F_DUPFD_CLOEXEC definition. > > > > > Should we instead just add an include for <linux/fcntl.h> to libbpf_internal.h > > (given it defines F_DUPFD_CLOEXEC as well)? > > yep, this is UAPI header so we can use it easily (we'll need to sync > it into Github repo, but that's not a problem) > > > > > > > --- > > > tools/lib/bpf/libbpf_internal.h | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h > > > index 4fda8bdf0a0d..d2a86b5a457a 100644 > > > --- a/tools/lib/bpf/libbpf_internal.h > > > +++ b/tools/lib/bpf/libbpf_internal.h > > > @@ -31,6 +31,10 @@ > > > #define EM_BPF 247 > > > #endif > > > > > > +#ifndef F_DUPFD_CLOEXEC > > > +#define F_DUPFD_CLOEXEC 1030 > > > +#endif > > > + > > > #ifndef R_BPF_64_64 > > > #define R_BPF_64_64 1 > > > #endif > > > > > > > Thanks, > > Daniel
On Sat, Mar 5, 2022 at 1:54 AM James Hilliard <james.hilliard1@gmail.com> wrote: > > On Mon, Feb 28, 2022 at 8:00 AM Daniel Borkmann <daniel@iogearbox.net> wrote: > > > > Hi James, > > > > On 2/27/22 3:25 PM, James Hilliard wrote: > > > This definition seems to be missing from some older toolchains. > > > > > > Note that the fcntl.h in libbpf_internal.h is not a kernel header > > > but rather a toolchain libc header. > > > > > > Fixes: > > > libbpf_internal.h:521:18: error: 'F_DUPFD_CLOEXEC' undeclared (first use in this function); did you mean 'FD_CLOEXEC'? > > > fd = fcntl(fd, F_DUPFD_CLOEXEC, 3); > > > ^~~~~~~~~~~~~~~ > > > FD_CLOEXEC > > > > > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> > > > > Do you have some more info on your env (e.g. libc)? Looks like F_DUPFD_CLOEXEC > > was added back in 2.6.24 kernel. When did libc add it? > > > > Should we instead just add an include for <linux/fcntl.h> to libbpf_internal.h > > (given it defines F_DUPFD_CLOEXEC as well)? > > That seems to cause a conflict: error: redefinition of ‘struct flock’ > > > > > > > --- > > > tools/lib/bpf/libbpf_internal.h | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h > > > index 4fda8bdf0a0d..d2a86b5a457a 100644 > > > --- a/tools/lib/bpf/libbpf_internal.h > > > +++ b/tools/lib/bpf/libbpf_internal.h > > > @@ -31,6 +31,10 @@ > > > #define EM_BPF 247 > > > #endif > > > > > > +#ifndef F_DUPFD_CLOEXEC > > > +#define F_DUPFD_CLOEXEC 1030 > > > +#endif Let's just do this then (assuming the value of F_DUPFD_CLOEXEC is architecture-independent) > > > + > > > #ifndef R_BPF_64_64 > > > #define R_BPF_64_64 1 > > > #endif > > > > > > > Thanks, > > Daniel
diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h index 4fda8bdf0a0d..d2a86b5a457a 100644 --- a/tools/lib/bpf/libbpf_internal.h +++ b/tools/lib/bpf/libbpf_internal.h @@ -31,6 +31,10 @@ #define EM_BPF 247 #endif +#ifndef F_DUPFD_CLOEXEC +#define F_DUPFD_CLOEXEC 1030 +#endif + #ifndef R_BPF_64_64 #define R_BPF_64_64 1 #endif
This definition seems to be missing from some older toolchains. Note that the fcntl.h in libbpf_internal.h is not a kernel header but rather a toolchain libc header. Fixes: libbpf_internal.h:521:18: error: 'F_DUPFD_CLOEXEC' undeclared (first use in this function); did you mean 'FD_CLOEXEC'? fd = fcntl(fd, F_DUPFD_CLOEXEC, 3); ^~~~~~~~~~~~~~~ FD_CLOEXEC Signed-off-by: James Hilliard <james.hilliard1@gmail.com> --- tools/lib/bpf/libbpf_internal.h | 4 ++++ 1 file changed, 4 insertions(+)