2022-02-27 16:30:38

by James Hilliard

[permalink] [raw]
Subject: [PATCH 1/1] libbpf: ensure F_DUPFD_CLOEXEC is defined

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 <[email protected]>
---
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
--
2.25.1


2022-02-28 18:07:22

by Daniel Borkmann

[permalink] [raw]
Subject: Re: [PATCH 1/1] libbpf: ensure F_DUPFD_CLOEXEC is defined

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 <[email protected]>

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

2022-03-04 20:48:26

by Andrii Nakryiko

[permalink] [raw]
Subject: Re: [PATCH 1/1] libbpf: ensure F_DUPFD_CLOEXEC is defined

On Mon, Feb 28, 2022 at 7:00 AM Daniel Borkmann <[email protected]> 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 <[email protected]>
>
> 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

2022-03-04 22:41:32

by Daniel Borkmann

[permalink] [raw]
Subject: Re: [PATCH 1/1] libbpf: ensure F_DUPFD_CLOEXEC is defined

On 3/4/22 8:01 PM, Andrii Nakryiko wrote:
> On Mon, Feb 28, 2022 at 7:00 AM Daniel Borkmann <[email protected]> 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 <[email protected]>
>>
>> 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

2022-03-05 10:22:52

by James Hilliard

[permalink] [raw]
Subject: Re: [PATCH 1/1] libbpf: ensure F_DUPFD_CLOEXEC is defined

On Fri, Mar 4, 2022 at 12:01 PM Andrii Nakryiko
<[email protected]> wrote:
>
> On Mon, Feb 28, 2022 at 7:00 AM Daniel Borkmann <[email protected]> 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 <[email protected]>
> >
> > 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

2022-03-05 19:58:56

by James Hilliard

[permalink] [raw]
Subject: Re: [PATCH 1/1] libbpf: ensure F_DUPFD_CLOEXEC is defined

On Mon, Feb 28, 2022 at 8:00 AM Daniel Borkmann <[email protected]> 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 <[email protected]>
>
> 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

2022-03-08 02:04:08

by Andrii Nakryiko

[permalink] [raw]
Subject: Re: [PATCH 1/1] libbpf: ensure F_DUPFD_CLOEXEC is defined

On Sat, Mar 5, 2022 at 1:54 AM James Hilliard <[email protected]> wrote:
>
> On Mon, Feb 28, 2022 at 8:00 AM Daniel Borkmann <[email protected]> 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 <[email protected]>
> >
> > 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