* [PATCH] libbpf: avoid inline hint definition from 'linux/stddef.h' @ 2021-03-14 17:38 Pedro Tammela 2021-03-16 21:01 ` Daniel Borkmann 0 siblings, 1 reply; 4+ messages in thread From: Pedro Tammela @ 2021-03-14 17:38 UTC (permalink / raw) Cc: Pedro Tammela, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Nathan Chancellor, Nick Desaulniers, netdev, bpf, linux-kernel, clang-built-linux Linux headers might pull 'linux/stddef.h' which defines '__always_inline' as the following: #ifndef __always_inline #define __always_inline __inline__ #endif This becomes an issue if the program picks up the 'linux/stddef.h' definition as the macro now just hints inline to clang. This change now enforces the proper definition for BPF programs regardless of the include order. Signed-off-by: Pedro Tammela <pctammela@gmail.com> --- tools/lib/bpf/bpf_helpers.h | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h index ae6c975e0b87..5fa483c0b508 100644 --- a/tools/lib/bpf/bpf_helpers.h +++ b/tools/lib/bpf/bpf_helpers.h @@ -29,9 +29,12 @@ */ #define SEC(NAME) __attribute__((section(NAME), used)) -#ifndef __always_inline +/* + * Avoid 'linux/stddef.h' definition of '__always_inline'. + */ +#undef __always_inline #define __always_inline inline __attribute__((always_inline)) -#endif + #ifndef __noinline #define __noinline __attribute__((noinline)) #endif -- 2.25.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] libbpf: avoid inline hint definition from 'linux/stddef.h' 2021-03-14 17:38 [PATCH] libbpf: avoid inline hint definition from 'linux/stddef.h' Pedro Tammela @ 2021-03-16 21:01 ` Daniel Borkmann 2021-03-16 21:34 ` Andrii Nakryiko 0 siblings, 1 reply; 4+ messages in thread From: Daniel Borkmann @ 2021-03-16 21:01 UTC (permalink / raw) To: Pedro Tammela Cc: Alexei Starovoitov, Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Nathan Chancellor, Nick Desaulniers, netdev, bpf, linux-kernel, clang-built-linux On 3/14/21 6:38 PM, Pedro Tammela wrote: > Linux headers might pull 'linux/stddef.h' which defines > '__always_inline' as the following: > > #ifndef __always_inline > #define __always_inline __inline__ > #endif > > This becomes an issue if the program picks up the 'linux/stddef.h' > definition as the macro now just hints inline to clang. How did the program end up including linux/stddef.h ? Would be good to also have some more details on how we got here for the commit desc. > This change now enforces the proper definition for BPF programs > regardless of the include order. > > Signed-off-by: Pedro Tammela <pctammela@gmail.com> > --- > tools/lib/bpf/bpf_helpers.h | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h > index ae6c975e0b87..5fa483c0b508 100644 > --- a/tools/lib/bpf/bpf_helpers.h > +++ b/tools/lib/bpf/bpf_helpers.h > @@ -29,9 +29,12 @@ > */ > #define SEC(NAME) __attribute__((section(NAME), used)) > > -#ifndef __always_inline > +/* > + * Avoid 'linux/stddef.h' definition of '__always_inline'. > + */ I think the comment should have more details on 'why' we undef it as in few months looking at it again, the next question to dig into would be what was wrong with linux/stddef.h. Providing a better rationale would be nice for readers here. > +#undef __always_inline > #define __always_inline inline __attribute__((always_inline)) > -#endif > + > #ifndef __noinline > #define __noinline __attribute__((noinline)) > #endif > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] libbpf: avoid inline hint definition from 'linux/stddef.h' 2021-03-16 21:01 ` Daniel Borkmann @ 2021-03-16 21:34 ` Andrii Nakryiko 2021-03-16 21:57 ` Daniel Borkmann 0 siblings, 1 reply; 4+ messages in thread From: Andrii Nakryiko @ 2021-03-16 21:34 UTC (permalink / raw) To: Daniel Borkmann Cc: Pedro Tammela, Alexei Starovoitov, Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Nathan Chancellor, Nick Desaulniers, Networking, bpf, open list, clang-built-linux On Tue, Mar 16, 2021 at 2:01 PM Daniel Borkmann <daniel@iogearbox.net> wrote: > > On 3/14/21 6:38 PM, Pedro Tammela wrote: > > Linux headers might pull 'linux/stddef.h' which defines > > '__always_inline' as the following: > > > > #ifndef __always_inline > > #define __always_inline __inline__ > > #endif > > > > This becomes an issue if the program picks up the 'linux/stddef.h' > > definition as the macro now just hints inline to clang. > > How did the program end up including linux/stddef.h ? Would be good to > also have some more details on how we got here for the commit desc. It's an UAPI header, so why not? Is there anything special about stddef.h that makes it unsuitable to be included? > > > This change now enforces the proper definition for BPF programs > > regardless of the include order. > > > > Signed-off-by: Pedro Tammela <pctammela@gmail.com> > > --- > > tools/lib/bpf/bpf_helpers.h | 7 +++++-- > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h > > index ae6c975e0b87..5fa483c0b508 100644 > > --- a/tools/lib/bpf/bpf_helpers.h > > +++ b/tools/lib/bpf/bpf_helpers.h > > @@ -29,9 +29,12 @@ > > */ > > #define SEC(NAME) __attribute__((section(NAME), used)) > > > > -#ifndef __always_inline > > +/* > > + * Avoid 'linux/stddef.h' definition of '__always_inline'. > > + */ > > I think the comment should have more details on 'why' we undef it as in > few months looking at it again, the next question to dig into would be > what was wrong with linux/stddef.h. Providing a better rationale would > be nice for readers here. So for whatever reason commit bot didn't send notification, but I've already landed this yesterday. To me, with #undef + #define it's pretty clear that we "force-define" __always_inline exactly how we want it, but we can certainly add clarifying comment in the follow up, if you think it's needed. > > > +#undef __always_inline > > #define __always_inline inline __attribute__((always_inline)) > > -#endif > > + > > #ifndef __noinline > > #define __noinline __attribute__((noinline)) > > #endif > > > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] libbpf: avoid inline hint definition from 'linux/stddef.h' 2021-03-16 21:34 ` Andrii Nakryiko @ 2021-03-16 21:57 ` Daniel Borkmann 0 siblings, 0 replies; 4+ messages in thread From: Daniel Borkmann @ 2021-03-16 21:57 UTC (permalink / raw) To: Andrii Nakryiko Cc: Pedro Tammela, Alexei Starovoitov, Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Nathan Chancellor, Nick Desaulniers, Networking, bpf, open list, clang-built-linux On 3/16/21 10:34 PM, Andrii Nakryiko wrote: > On Tue, Mar 16, 2021 at 2:01 PM Daniel Borkmann <daniel@iogearbox.net> wrote: >> On 3/14/21 6:38 PM, Pedro Tammela wrote: >>> Linux headers might pull 'linux/stddef.h' which defines >>> '__always_inline' as the following: >>> >>> #ifndef __always_inline >>> #define __always_inline __inline__ >>> #endif >>> >>> This becomes an issue if the program picks up the 'linux/stddef.h' >>> definition as the macro now just hints inline to clang. >> >> How did the program end up including linux/stddef.h ? Would be good to >> also have some more details on how we got here for the commit desc. > > It's an UAPI header, so why not? Is there anything special about > stddef.h that makes it unsuitable to be included? Hm, fair enough, looks like linux/types.h already pulls it in, so no. We defined our own stddef.h longer time ago, so looks like we never ran into this issue. >>> This change now enforces the proper definition for BPF programs >>> regardless of the include order. >>> >>> Signed-off-by: Pedro Tammela <pctammela@gmail.com> >>> --- >>> tools/lib/bpf/bpf_helpers.h | 7 +++++-- >>> 1 file changed, 5 insertions(+), 2 deletions(-) >>> >>> diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h >>> index ae6c975e0b87..5fa483c0b508 100644 >>> --- a/tools/lib/bpf/bpf_helpers.h >>> +++ b/tools/lib/bpf/bpf_helpers.h >>> @@ -29,9 +29,12 @@ >>> */ >>> #define SEC(NAME) __attribute__((section(NAME), used)) >>> >>> -#ifndef __always_inline >>> +/* >>> + * Avoid 'linux/stddef.h' definition of '__always_inline'. >>> + */ >> >> I think the comment should have more details on 'why' we undef it as in >> few months looking at it again, the next question to dig into would be >> what was wrong with linux/stddef.h. Providing a better rationale would >> be nice for readers here. > > So for whatever reason commit bot didn't send notification, but I've > already landed this yesterday. To me, with #undef + #define it's > pretty clear that we "force-define" __always_inline exactly how we > want it, but we can certainly add clarifying comment in the follow up, > if you think it's needed. Up to you, but given you applied it it's probably not worth the trouble; missed it earlier given I didn't see the patchbot message in the thread initially. :/ >>> +#undef __always_inline >>> #define __always_inline inline __attribute__((always_inline)) >>> -#endif >>> + >>> #ifndef __noinline >>> #define __noinline __attribute__((noinline)) >>> #endif >>> >> ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-03-16 21:58 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-03-14 17:38 [PATCH] libbpf: avoid inline hint definition from 'linux/stddef.h' Pedro Tammela 2021-03-16 21:01 ` Daniel Borkmann 2021-03-16 21:34 ` Andrii Nakryiko 2021-03-16 21:57 ` Daniel Borkmann
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).