bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Typical way to handle missing macros in vmlinux.h
@ 2021-04-28 19:13 Grant Seltzer Richman
  2021-04-28 21:15 ` Andrii Nakryiko
  0 siblings, 1 reply; 10+ messages in thread
From: Grant Seltzer Richman @ 2021-04-28 19:13 UTC (permalink / raw)
  To: bpf

Hi all,

I'm working on enabling CO:RE in a project I work on, tracee, and am
running into the dilemma of missing macros that we previously were
able to import from their various header files. I understand that
macros don't make their way into BTF and therefore the generated
vmlinux.h won't have them. However I can't import the various header
files because of multiple-definition issues.

Do people typically redefine each of these macros for their project?
If so is there anything I should be careful of, such as architectural
differences. Does anyone have creative ideas, even if not developed
fully yet that I can possibly contribute to libbpf?

Thanks so much,
Grant Seltzer

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Typical way to handle missing macros in vmlinux.h
  2021-04-28 19:13 Typical way to handle missing macros in vmlinux.h Grant Seltzer Richman
@ 2021-04-28 21:15 ` Andrii Nakryiko
  2021-05-03 18:32   ` Grant Seltzer Richman
  0 siblings, 1 reply; 10+ messages in thread
From: Andrii Nakryiko @ 2021-04-28 21:15 UTC (permalink / raw)
  To: Grant Seltzer Richman; +Cc: bpf

On Wed, Apr 28, 2021 at 1:53 PM Grant Seltzer Richman
<grantseltzer@gmail.com> wrote:
>
> Hi all,
>
> I'm working on enabling CO:RE in a project I work on, tracee, and am
> running into the dilemma of missing macros that we previously were
> able to import from their various header files. I understand that
> macros don't make their way into BTF and therefore the generated
> vmlinux.h won't have them. However I can't import the various header
> files because of multiple-definition issues.

Sadly, copy/pasting has been the only way so far.

>
> Do people typically redefine each of these macros for their project?
> If so is there anything I should be careful of, such as architectural
> differences. Does anyone have creative ideas, even if not developed
> fully yet that I can possibly contribute to libbpf?

We've discussed adding Clang built-in to detect if a specific type is
already defined and doing something like this in vmlinux.h:

#if !__builtin_is_type_defined(struct task_struct)
struct task_struct {
     ...
}
#endif

And just do that for every struct, union, typedef. That would allow
vmlinux.h to co-exist (somewhat) with other types.

Another alternative is to not use vmlinux.h and use just linux
headers, but mark necessary types with
__attribute__((preserve_access_index)) to make them CO-RE relocatable.
You can add that to existing types with the same pragma that vmlinux.h
uses.

>
> Thanks so much,
> Grant Seltzer

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Typical way to handle missing macros in vmlinux.h
  2021-04-28 21:15 ` Andrii Nakryiko
@ 2021-05-03 18:32   ` Grant Seltzer Richman
  2021-05-03 18:43     ` Andrii Nakryiko
  0 siblings, 1 reply; 10+ messages in thread
From: Grant Seltzer Richman @ 2021-05-03 18:32 UTC (permalink / raw)
  To: Andrii Nakryiko; +Cc: bpf

On Wed, Apr 28, 2021 at 5:15 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Wed, Apr 28, 2021 at 1:53 PM Grant Seltzer Richman
> <grantseltzer@gmail.com> wrote:
> >
> > Hi all,
> >
> > I'm working on enabling CO:RE in a project I work on, tracee, and am
> > running into the dilemma of missing macros that we previously were
> > able to import from their various header files. I understand that
> > macros don't make their way into BTF and therefore the generated
> > vmlinux.h won't have them. However I can't import the various header
> > files because of multiple-definition issues.
>
> Sadly, copy/pasting has been the only way so far.
>
> >
> > Do people typically redefine each of these macros for their project?
> > If so is there anything I should be careful of, such as architectural
> > differences. Does anyone have creative ideas, even if not developed
> > fully yet that I can possibly contribute to libbpf?
>
> We've discussed adding Clang built-in to detect if a specific type is
> already defined and doing something like this in vmlinux.h:
>
> #if !__builtin_is_type_defined(struct task_struct)
> struct task_struct {
>      ...
> }
> #endif
>
> And just do that for every struct, union, typedef. That would allow
> vmlinux.h to co-exist (somewhat) with other types.
>
> Another alternative is to not use vmlinux.h and use just linux
> headers, but mark necessary types with
> __attribute__((preserve_access_index)) to make them CO-RE relocatable.
> You can add that to existing types with the same pragma that vmlinux.h
> uses.

I'm attempting to try doing the above. I'm just replacing
bpf_probe_read with bpf_core_read and not importing vmlinux.h, just
all the kernel headers I need.

When you say "Add that to existing types with the same pragma that
vmlinux.h uses", Should I be able to add the following to my bpf
source file before importing my headers?

ifndef BPF_NO_PRESERVE_ACCESS_INDEX
#pragma clang attribute push (__attribute__((preserve_access_index)),
apply_to = record)
#endif

and then pop the attribute at the bottom of the file, or after the
header includes.

I've tried this and get a whole bunch of 'unknown attribute' warnings,
leading me to believe that I either have something installed
incorrectly or don't understand how to use clang attributes. Do I need
to edit the types in the actual header files?

Thank you very very much for the help!
- Grant
>
> >
> > Thanks so much,
> > Grant Seltzer

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Typical way to handle missing macros in vmlinux.h
  2021-05-03 18:32   ` Grant Seltzer Richman
@ 2021-05-03 18:43     ` Andrii Nakryiko
  2021-05-03 20:20       ` Grant Seltzer Richman
  0 siblings, 1 reply; 10+ messages in thread
From: Andrii Nakryiko @ 2021-05-03 18:43 UTC (permalink / raw)
  To: Grant Seltzer Richman; +Cc: bpf

On Mon, May 3, 2021 at 11:32 AM Grant Seltzer Richman
<grantseltzer@gmail.com> wrote:
>
> On Wed, Apr 28, 2021 at 5:15 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Wed, Apr 28, 2021 at 1:53 PM Grant Seltzer Richman
> > <grantseltzer@gmail.com> wrote:
> > >
> > > Hi all,
> > >
> > > I'm working on enabling CO:RE in a project I work on, tracee, and am
> > > running into the dilemma of missing macros that we previously were
> > > able to import from their various header files. I understand that
> > > macros don't make their way into BTF and therefore the generated
> > > vmlinux.h won't have them. However I can't import the various header
> > > files because of multiple-definition issues.
> >
> > Sadly, copy/pasting has been the only way so far.
> >
> > >
> > > Do people typically redefine each of these macros for their project?
> > > If so is there anything I should be careful of, such as architectural
> > > differences. Does anyone have creative ideas, even if not developed
> > > fully yet that I can possibly contribute to libbpf?
> >
> > We've discussed adding Clang built-in to detect if a specific type is
> > already defined and doing something like this in vmlinux.h:
> >
> > #if !__builtin_is_type_defined(struct task_struct)
> > struct task_struct {
> >      ...
> > }
> > #endif
> >
> > And just do that for every struct, union, typedef. That would allow
> > vmlinux.h to co-exist (somewhat) with other types.
> >
> > Another alternative is to not use vmlinux.h and use just linux
> > headers, but mark necessary types with
> > __attribute__((preserve_access_index)) to make them CO-RE relocatable.
> > You can add that to existing types with the same pragma that vmlinux.h
> > uses.
>
> I'm attempting to try doing the above. I'm just replacing
> bpf_probe_read with bpf_core_read and not importing vmlinux.h, just
> all the kernel headers I need.

Yes, that will work, bpf_core_read() uses preserve_access_index
built-in to achieve the same effect.

>
> When you say "Add that to existing types with the same pragma that
> vmlinux.h uses", Should I be able to add the following to my bpf
> source file before importing my headers?
>
> ifndef BPF_NO_PRESERVE_ACCESS_INDEX
> #pragma clang attribute push (__attribute__((preserve_access_index)),
> apply_to = record)
> #endif
>
> and then pop the attribute at the bottom of the file, or after the
> header includes.

Yeah, that's the idea and that's what vmlinux.h does for all its
structs. It doesn't add __attribute__((preserve_access_index)) after
each struct/union. So I wonder why you are getting those unknown
attribute errors. Can you paste an example?

Also check that you use Clang that supports preserve_access_index, of course.

>
> I've tried this and get a whole bunch of 'unknown attribute' warnings,
> leading me to believe that I either have something installed
> incorrectly or don't understand how to use clang attributes. Do I need
> to edit the types in the actual header files?

No, the whole idea is to not touch original headers.

>
> Thank you very very much for the help!
> - Grant
> >
> > >
> > > Thanks so much,
> > > Grant Seltzer

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Typical way to handle missing macros in vmlinux.h
  2021-05-03 18:43     ` Andrii Nakryiko
@ 2021-05-03 20:20       ` Grant Seltzer Richman
  2021-05-03 21:21         ` Andrii Nakryiko
  0 siblings, 1 reply; 10+ messages in thread
From: Grant Seltzer Richman @ 2021-05-03 20:20 UTC (permalink / raw)
  To: Andrii Nakryiko; +Cc: bpf

On Mon, May 3, 2021 at 2:43 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Mon, May 3, 2021 at 11:32 AM Grant Seltzer Richman
> <grantseltzer@gmail.com> wrote:
> >
> > On Wed, Apr 28, 2021 at 5:15 PM Andrii Nakryiko
> > <andrii.nakryiko@gmail.com> wrote:
> > >
> > > On Wed, Apr 28, 2021 at 1:53 PM Grant Seltzer Richman
> > > <grantseltzer@gmail.com> wrote:
> > > >
> > > > Hi all,
> > > >
> > > > I'm working on enabling CO:RE in a project I work on, tracee, and am
> > > > running into the dilemma of missing macros that we previously were
> > > > able to import from their various header files. I understand that
> > > > macros don't make their way into BTF and therefore the generated
> > > > vmlinux.h won't have them. However I can't import the various header
> > > > files because of multiple-definition issues.
> > >
> > > Sadly, copy/pasting has been the only way so far.
> > >
> > > >
> > > > Do people typically redefine each of these macros for their project?
> > > > If so is there anything I should be careful of, such as architectural
> > > > differences. Does anyone have creative ideas, even if not developed
> > > > fully yet that I can possibly contribute to libbpf?
> > >
> > > We've discussed adding Clang built-in to detect if a specific type is
> > > already defined and doing something like this in vmlinux.h:
> > >
> > > #if !__builtin_is_type_defined(struct task_struct)
> > > struct task_struct {
> > >      ...
> > > }
> > > #endif
> > >
> > > And just do that for every struct, union, typedef. That would allow
> > > vmlinux.h to co-exist (somewhat) with other types.
> > >
> > > Another alternative is to not use vmlinux.h and use just linux
> > > headers, but mark necessary types with
> > > __attribute__((preserve_access_index)) to make them CO-RE relocatable.
> > > You can add that to existing types with the same pragma that vmlinux.h
> > > uses.
> >
> > I'm attempting to try doing the above. I'm just replacing
> > bpf_probe_read with bpf_core_read and not importing vmlinux.h, just
> > all the kernel headers I need.
>
> Yes, that will work, bpf_core_read() uses preserve_access_index
> built-in to achieve the same effect.
>
> >
> > When you say "Add that to existing types with the same pragma that
> > vmlinux.h uses", Should I be able to add the following to my bpf
> > source file before importing my headers?
> >
> > ifndef BPF_NO_PRESERVE_ACCESS_INDEX
> > #pragma clang attribute push (__attribute__((preserve_access_index)),
> > apply_to = record)
> > #endif
> >
> > and then pop the attribute at the bottom of the file, or after the
> > header includes.
>
> Yeah, that's the idea and that's what vmlinux.h does for all its
> structs. It doesn't add __attribute__((preserve_access_index)) after
> each struct/union. So I wonder why you are getting those unknown
> attribute errors. Can you paste an example?

Here's a couple examples of the warnings:

```
tracee/tracee.bpf.c:5:46: warning: unknown attribute
'preserve_access_index' ignored [-Wunknown-attributes]
#pragma clang attribute push (__attribute__((preserve_access_index)),
apply_to = record)
                                             ^
/lib/modules/5.10.21-200.fc33.x86_64/source/include/linux/ipv6.h:185:1:
note: when applied to this declaration
struct ipv6_fl_socklist;
^
tracee/tracee.bpf.c:5:46: warning: unknown attribute
'preserve_access_index' ignored [-Wunknown-attributes]
#pragma clang attribute push (__attribute__((preserve_access_index)),
apply_to = record)
                                             ^
/lib/modules/5.10.21-200.fc33.x86_64/source/include/linux/ipv6.h:187:1:
note: when applied to this declaration
struct inet6_cork {
```

after these warnings are emitted (it seems as if there's one for every
data type, though I can't confirm), I get errors that look like this:

```
tracee/tracee.bpf.c:445:22: error: nested
builtin_preserve_access_index() not supported
    return READ_KERN(READ_KERN(task->thread_pid)->numbers[level].nr);
                     ^
tracee/tracee.bpf.c:206:27: note: expanded from macro 'READ_KERN'
                          bpf_core_read(&_val, sizeof(_val), &ptr); \
```
I believe this is just a result of the warnings above, but if you're
curious it's what i'm doing here:
https://github.com/aquasecurity/tracee/blob/core-experiment/tracee-ebpf/tracee/tracee.bpf.c#L204-L208

>
> Also check that you use Clang that supports preserve_access_index, of course.

I'm using clang 11.0 on Fedora 33. All dependencies appear properly
installed (libelf, zlib, dwarves [provides pahole], llvm, llc,
llvm-devel,...)

>
> >
> > I've tried this and get a whole bunch of 'unknown attribute' warnings,
> > leading me to believe that I either have something installed
> > incorrectly or don't understand how to use clang attributes. Do I need
> > to edit the types in the actual header files?
>
> No, the whole idea is to not touch original headers.

Got it - that's good to know.

>
> >
> > Thank you very very much for the help!
> > - Grant
> > >
> > > >
> > > > Thanks so much,
> > > > Grant Seltzer

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Typical way to handle missing macros in vmlinux.h
  2021-05-03 20:20       ` Grant Seltzer Richman
@ 2021-05-03 21:21         ` Andrii Nakryiko
  2021-05-04 15:31           ` Grant Seltzer Richman
  0 siblings, 1 reply; 10+ messages in thread
From: Andrii Nakryiko @ 2021-05-03 21:21 UTC (permalink / raw)
  To: Grant Seltzer Richman; +Cc: bpf

On Mon, May 3, 2021 at 1:20 PM Grant Seltzer Richman
<grantseltzer@gmail.com> wrote:
>
> On Mon, May 3, 2021 at 2:43 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Mon, May 3, 2021 at 11:32 AM Grant Seltzer Richman
> > <grantseltzer@gmail.com> wrote:
> > >
> > > On Wed, Apr 28, 2021 at 5:15 PM Andrii Nakryiko
> > > <andrii.nakryiko@gmail.com> wrote:
> > > >
> > > > On Wed, Apr 28, 2021 at 1:53 PM Grant Seltzer Richman
> > > > <grantseltzer@gmail.com> wrote:
> > > > >
> > > > > Hi all,
> > > > >
> > > > > I'm working on enabling CO:RE in a project I work on, tracee, and am
> > > > > running into the dilemma of missing macros that we previously were
> > > > > able to import from their various header files. I understand that
> > > > > macros don't make their way into BTF and therefore the generated
> > > > > vmlinux.h won't have them. However I can't import the various header
> > > > > files because of multiple-definition issues.
> > > >
> > > > Sadly, copy/pasting has been the only way so far.
> > > >
> > > > >
> > > > > Do people typically redefine each of these macros for their project?
> > > > > If so is there anything I should be careful of, such as architectural
> > > > > differences. Does anyone have creative ideas, even if not developed
> > > > > fully yet that I can possibly contribute to libbpf?
> > > >
> > > > We've discussed adding Clang built-in to detect if a specific type is
> > > > already defined and doing something like this in vmlinux.h:
> > > >
> > > > #if !__builtin_is_type_defined(struct task_struct)
> > > > struct task_struct {
> > > >      ...
> > > > }
> > > > #endif
> > > >
> > > > And just do that for every struct, union, typedef. That would allow
> > > > vmlinux.h to co-exist (somewhat) with other types.
> > > >
> > > > Another alternative is to not use vmlinux.h and use just linux
> > > > headers, but mark necessary types with
> > > > __attribute__((preserve_access_index)) to make them CO-RE relocatable.
> > > > You can add that to existing types with the same pragma that vmlinux.h
> > > > uses.
> > >
> > > I'm attempting to try doing the above. I'm just replacing
> > > bpf_probe_read with bpf_core_read and not importing vmlinux.h, just
> > > all the kernel headers I need.
> >
> > Yes, that will work, bpf_core_read() uses preserve_access_index
> > built-in to achieve the same effect.
> >
> > >
> > > When you say "Add that to existing types with the same pragma that
> > > vmlinux.h uses", Should I be able to add the following to my bpf
> > > source file before importing my headers?
> > >
> > > ifndef BPF_NO_PRESERVE_ACCESS_INDEX
> > > #pragma clang attribute push (__attribute__((preserve_access_index)),
> > > apply_to = record)
> > > #endif
> > >
> > > and then pop the attribute at the bottom of the file, or after the
> > > header includes.
> >
> > Yeah, that's the idea and that's what vmlinux.h does for all its
> > structs. It doesn't add __attribute__((preserve_access_index)) after
> > each struct/union. So I wonder why you are getting those unknown
> > attribute errors. Can you paste an example?
>
> Here's a couple examples of the warnings:
>
> ```
> tracee/tracee.bpf.c:5:46: warning: unknown attribute
> 'preserve_access_index' ignored [-Wunknown-attributes]
> #pragma clang attribute push (__attribute__((preserve_access_index)),
> apply_to = record)
>                                              ^
> /lib/modules/5.10.21-200.fc33.x86_64/source/include/linux/ipv6.h:185:1:
> note: when applied to this declaration
> struct ipv6_fl_socklist;
> ^
> tracee/tracee.bpf.c:5:46: warning: unknown attribute
> 'preserve_access_index' ignored [-Wunknown-attributes]
> #pragma clang attribute push (__attribute__((preserve_access_index)),
> apply_to = record)
>                                              ^
> /lib/modules/5.10.21-200.fc33.x86_64/source/include/linux/ipv6.h:187:1:
> note: when applied to this declaration
> struct inet6_cork {
> ```
>
> after these warnings are emitted (it seems as if there's one for every
> data type, though I can't confirm), I get errors that look like this:
>
> ```
> tracee/tracee.bpf.c:445:22: error: nested
> builtin_preserve_access_index() not supported
>     return READ_KERN(READ_KERN(task->thread_pid)->numbers[level].nr);
>                      ^
> tracee/tracee.bpf.c:206:27: note: expanded from macro 'READ_KERN'
>                           bpf_core_read(&_val, sizeof(_val), &ptr); \
> ```
> I believe this is just a result of the warnings above, but if you're
> curious it's what i'm doing here:
> https://github.com/aquasecurity/tracee/blob/core-experiment/tracee-ebpf/tracee/tracee.bpf.c#L204-L208
>

Looking at your Makefile, you are not using `clang -target bpf` to
compile BPF object files, which is probably what causes you trouble.
preserve_access_index is a BPF target-only attribute. There is no need
to do the legacy clang -emit-llvm | llc, especially when you are using
CO-RE.

> >
> > Also check that you use Clang that supports preserve_access_index, of course.
>
> I'm using clang 11.0 on Fedora 33. All dependencies appear properly
> installed (libelf, zlib, dwarves [provides pahole], llvm, llc,
> llvm-devel,...)
>
> >
> > >
> > > I've tried this and get a whole bunch of 'unknown attribute' warnings,
> > > leading me to believe that I either have something installed
> > > incorrectly or don't understand how to use clang attributes. Do I need
> > > to edit the types in the actual header files?
> >
> > No, the whole idea is to not touch original headers.
>
> Got it - that's good to know.
>
> >
> > >
> > > Thank you very very much for the help!
> > > - Grant
> > > >
> > > > >
> > > > > Thanks so much,
> > > > > Grant Seltzer

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Typical way to handle missing macros in vmlinux.h
  2021-05-03 21:21         ` Andrii Nakryiko
@ 2021-05-04 15:31           ` Grant Seltzer Richman
  2021-05-04 16:24             ` Yonghong Song
  2021-05-04 22:31             ` Andrii Nakryiko
  0 siblings, 2 replies; 10+ messages in thread
From: Grant Seltzer Richman @ 2021-05-04 15:31 UTC (permalink / raw)
  To: Andrii Nakryiko; +Cc: bpf

On Mon, May 3, 2021 at 5:22 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Mon, May 3, 2021 at 1:20 PM Grant Seltzer Richman
> <grantseltzer@gmail.com> wrote:
> >
> > On Mon, May 3, 2021 at 2:43 PM Andrii Nakryiko
> > <andrii.nakryiko@gmail.com> wrote:
> > >
> > > On Mon, May 3, 2021 at 11:32 AM Grant Seltzer Richman
> > > <grantseltzer@gmail.com> wrote:
> > > >
> > > > On Wed, Apr 28, 2021 at 5:15 PM Andrii Nakryiko
> > > > <andrii.nakryiko@gmail.com> wrote:
> > > > >
> > > > > On Wed, Apr 28, 2021 at 1:53 PM Grant Seltzer Richman
> > > > > <grantseltzer@gmail.com> wrote:
> > > > > >
> > > > > > Hi all,
> > > > > >
> > > > > > I'm working on enabling CO:RE in a project I work on, tracee, and am
> > > > > > running into the dilemma of missing macros that we previously were
> > > > > > able to import from their various header files. I understand that
> > > > > > macros don't make their way into BTF and therefore the generated
> > > > > > vmlinux.h won't have them. However I can't import the various header
> > > > > > files because of multiple-definition issues.
> > > > >
> > > > > Sadly, copy/pasting has been the only way so far.
> > > > >
> > > > > >
> > > > > > Do people typically redefine each of these macros for their project?
> > > > > > If so is there anything I should be careful of, such as architectural
> > > > > > differences. Does anyone have creative ideas, even if not developed
> > > > > > fully yet that I can possibly contribute to libbpf?
> > > > >
> > > > > We've discussed adding Clang built-in to detect if a specific type is
> > > > > already defined and doing something like this in vmlinux.h:
> > > > >
> > > > > #if !__builtin_is_type_defined(struct task_struct)
> > > > > struct task_struct {
> > > > >      ...
> > > > > }
> > > > > #endif
> > > > >
> > > > > And just do that for every struct, union, typedef. That would allow
> > > > > vmlinux.h to co-exist (somewhat) with other types.
> > > > >
> > > > > Another alternative is to not use vmlinux.h and use just linux
> > > > > headers, but mark necessary types with
> > > > > __attribute__((preserve_access_index)) to make them CO-RE relocatable.
> > > > > You can add that to existing types with the same pragma that vmlinux.h
> > > > > uses.
> > > >
> > > > I'm attempting to try doing the above. I'm just replacing
> > > > bpf_probe_read with bpf_core_read and not importing vmlinux.h, just
> > > > all the kernel headers I need.
> > >
> > > Yes, that will work, bpf_core_read() uses preserve_access_index
> > > built-in to achieve the same effect.
> > >
> > > >
> > > > When you say "Add that to existing types with the same pragma that
> > > > vmlinux.h uses", Should I be able to add the following to my bpf
> > > > source file before importing my headers?
> > > >
> > > > ifndef BPF_NO_PRESERVE_ACCESS_INDEX
> > > > #pragma clang attribute push (__attribute__((preserve_access_index)),
> > > > apply_to = record)
> > > > #endif
> > > >
> > > > and then pop the attribute at the bottom of the file, or after the
> > > > header includes.
> > >
> > > Yeah, that's the idea and that's what vmlinux.h does for all its
> > > structs. It doesn't add __attribute__((preserve_access_index)) after
> > > each struct/union. So I wonder why you are getting those unknown
> > > attribute errors. Can you paste an example?
> >
> > Here's a couple examples of the warnings:
> >
> > ```
> > tracee/tracee.bpf.c:5:46: warning: unknown attribute
> > 'preserve_access_index' ignored [-Wunknown-attributes]
> > #pragma clang attribute push (__attribute__((preserve_access_index)),
> > apply_to = record)
> >                                              ^
> > /lib/modules/5.10.21-200.fc33.x86_64/source/include/linux/ipv6.h:185:1:
> > note: when applied to this declaration
> > struct ipv6_fl_socklist;
> > ^
> > tracee/tracee.bpf.c:5:46: warning: unknown attribute
> > 'preserve_access_index' ignored [-Wunknown-attributes]
> > #pragma clang attribute push (__attribute__((preserve_access_index)),
> > apply_to = record)
> >                                              ^
> > /lib/modules/5.10.21-200.fc33.x86_64/source/include/linux/ipv6.h:187:1:
> > note: when applied to this declaration
> > struct inet6_cork {
> > ```
> >
> > after these warnings are emitted (it seems as if there's one for every
> > data type, though I can't confirm), I get errors that look like this:
> >
> > ```
> > tracee/tracee.bpf.c:445:22: error: nested
> > builtin_preserve_access_index() not supported
> >     return READ_KERN(READ_KERN(task->thread_pid)->numbers[level].nr);
> >                      ^
> > tracee/tracee.bpf.c:206:27: note: expanded from macro 'READ_KERN'
> >                           bpf_core_read(&_val, sizeof(_val), &ptr); \
> > ```
> > I believe this is just a result of the warnings above, but if you're
> > curious it's what i'm doing here:
> > https://github.com/aquasecurity/tracee/blob/core-experiment/tracee-ebpf/tracee/tracee.bpf.c#L204-L208
> >
>
> Looking at your Makefile, you are not using `clang -target bpf` to
> compile BPF object files, which is probably what causes you trouble.
> preserve_access_index is a BPF target-only attribute. There is no need
> to do the legacy clang -emit-llvm | llc, especially when you are using
> CO-RE.

Got it. Funny enough, it turns out this is just a continuation of a
conversation you had with my coworker Yaniv last year:
https://lore.kernel.org/bpf/CAEf4BzbshRMCX1T1ooAtYGYuUGefbbo2=ProkMg5iOtUKh3YtQ@mail.gmail.com/

But to summarize our continued challenge: Adding the
`preserve_access_index` attribute, compiling with `-target bpf`, and
using the same kernel headers we used (not vmlinux.h) causes issues
because of architecture specific asm errors (likely stemming from
headers we include). Unless there's a way to get around those we're
going to need to include "vmlinux.h", change our Makefile to `-target
bpf`, and redefine macros and/or functions that vmlinux.h does not
provide.

I think this is a pretty significant usability challenge. The idea you
mentioned of having a built-in to detect if a type is defined would be
a huge step forward. Has any progress been made towards this?

Another thought is having vmlinux.h include function definitions,
aren't they included in DWARF/BTF?

Thanks for your help, as always, Andrii!

>
> > >
> > > Also check that you use Clang that supports preserve_access_index, of course.
> >
> > I'm using clang 11.0 on Fedora 33. All dependencies appear properly
> > installed (libelf, zlib, dwarves [provides pahole], llvm, llc,
> > llvm-devel,...)
> >
> > >
> > > >
> > > > I've tried this and get a whole bunch of 'unknown attribute' warnings,
> > > > leading me to believe that I either have something installed
> > > > incorrectly or don't understand how to use clang attributes. Do I need
> > > > to edit the types in the actual header files?
> > >
> > > No, the whole idea is to not touch original headers.
> >
> > Got it - that's good to know.
> >
> > >
> > > >
> > > > Thank you very very much for the help!
> > > > - Grant
> > > > >
> > > > > >
> > > > > > Thanks so much,
> > > > > > Grant Seltzer

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Typical way to handle missing macros in vmlinux.h
  2021-05-04 15:31           ` Grant Seltzer Richman
@ 2021-05-04 16:24             ` Yonghong Song
  2021-05-04 16:57               ` Grant Seltzer Richman
  2021-05-04 22:31             ` Andrii Nakryiko
  1 sibling, 1 reply; 10+ messages in thread
From: Yonghong Song @ 2021-05-04 16:24 UTC (permalink / raw)
  To: Grant Seltzer Richman, Andrii Nakryiko; +Cc: bpf



On 5/4/21 8:31 AM, Grant Seltzer Richman wrote:
> On Mon, May 3, 2021 at 5:22 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
>>
>> On Mon, May 3, 2021 at 1:20 PM Grant Seltzer Richman
>> <grantseltzer@gmail.com> wrote:
>>>
>>> On Mon, May 3, 2021 at 2:43 PM Andrii Nakryiko
>>> <andrii.nakryiko@gmail.com> wrote:
>>>>
>>>> On Mon, May 3, 2021 at 11:32 AM Grant Seltzer Richman
>>>> <grantseltzer@gmail.com> wrote:
>>>>>
>>>>> On Wed, Apr 28, 2021 at 5:15 PM Andrii Nakryiko
>>>>> <andrii.nakryiko@gmail.com> wrote:
>>>>>>
>>>>>> On Wed, Apr 28, 2021 at 1:53 PM Grant Seltzer Richman
>>>>>> <grantseltzer@gmail.com> wrote:
>>>>>>>
>>>>>>> Hi all,
>>>>>>>
>>>>>>> I'm working on enabling CO:RE in a project I work on, tracee, and am
>>>>>>> running into the dilemma of missing macros that we previously were
>>>>>>> able to import from their various header files. I understand that
>>>>>>> macros don't make their way into BTF and therefore the generated
>>>>>>> vmlinux.h won't have them. However I can't import the various header
>>>>>>> files because of multiple-definition issues.
>>>>>>
>>>>>> Sadly, copy/pasting has been the only way so far.
>>>>>>
>>>>>>>
>>>>>>> Do people typically redefine each of these macros for their project?
>>>>>>> If so is there anything I should be careful of, such as architectural
>>>>>>> differences. Does anyone have creative ideas, even if not developed
>>>>>>> fully yet that I can possibly contribute to libbpf?
>>>>>>
>>>>>> We've discussed adding Clang built-in to detect if a specific type is
>>>>>> already defined and doing something like this in vmlinux.h:
>>>>>>
>>>>>> #if !__builtin_is_type_defined(struct task_struct)
>>>>>> struct task_struct {
>>>>>>       ...
>>>>>> }
>>>>>> #endif
>>>>>>
>>>>>> And just do that for every struct, union, typedef. That would allow
>>>>>> vmlinux.h to co-exist (somewhat) with other types.
>>>>>>
>>>>>> Another alternative is to not use vmlinux.h and use just linux
>>>>>> headers, but mark necessary types with
>>>>>> __attribute__((preserve_access_index)) to make them CO-RE relocatable.
>>>>>> You can add that to existing types with the same pragma that vmlinux.h
>>>>>> uses.
>>>>>
>>>>> I'm attempting to try doing the above. I'm just replacing
>>>>> bpf_probe_read with bpf_core_read and not importing vmlinux.h, just
>>>>> all the kernel headers I need.
>>>>
>>>> Yes, that will work, bpf_core_read() uses preserve_access_index
>>>> built-in to achieve the same effect.
>>>>
>>>>>
>>>>> When you say "Add that to existing types with the same pragma that
>>>>> vmlinux.h uses", Should I be able to add the following to my bpf
>>>>> source file before importing my headers?
>>>>>
>>>>> ifndef BPF_NO_PRESERVE_ACCESS_INDEX
>>>>> #pragma clang attribute push (__attribute__((preserve_access_index)),
>>>>> apply_to = record)
>>>>> #endif
>>>>>
>>>>> and then pop the attribute at the bottom of the file, or after the
>>>>> header includes.
>>>>
>>>> Yeah, that's the idea and that's what vmlinux.h does for all its
>>>> structs. It doesn't add __attribute__((preserve_access_index)) after
>>>> each struct/union. So I wonder why you are getting those unknown
>>>> attribute errors. Can you paste an example?
>>>
>>> Here's a couple examples of the warnings:
>>>
>>> ```
>>> tracee/tracee.bpf.c:5:46: warning: unknown attribute
>>> 'preserve_access_index' ignored [-Wunknown-attributes]
>>> #pragma clang attribute push (__attribute__((preserve_access_index)),
>>> apply_to = record)
>>>                                               ^
>>> /lib/modules/5.10.21-200.fc33.x86_64/source/include/linux/ipv6.h:185:1:
>>> note: when applied to this declaration
>>> struct ipv6_fl_socklist;
>>> ^
>>> tracee/tracee.bpf.c:5:46: warning: unknown attribute
>>> 'preserve_access_index' ignored [-Wunknown-attributes]
>>> #pragma clang attribute push (__attribute__((preserve_access_index)),
>>> apply_to = record)
>>>                                               ^
>>> /lib/modules/5.10.21-200.fc33.x86_64/source/include/linux/ipv6.h:187:1:
>>> note: when applied to this declaration
>>> struct inet6_cork {
>>> ```
>>>
>>> after these warnings are emitted (it seems as if there's one for every
>>> data type, though I can't confirm), I get errors that look like this:
>>>
>>> ```
>>> tracee/tracee.bpf.c:445:22: error: nested
>>> builtin_preserve_access_index() not supported
>>>      return READ_KERN(READ_KERN(task->thread_pid)->numbers[level].nr);
>>>                       ^
>>> tracee/tracee.bpf.c:206:27: note: expanded from macro 'READ_KERN'
>>>                            bpf_core_read(&_val, sizeof(_val), &ptr); \
>>> ```
>>> I believe this is just a result of the warnings above, but if you're
>>> curious it's what i'm doing here:
>>> https://github.com/aquasecurity/tracee/blob/core-experiment/tracee-ebpf/tracee/tracee.bpf.c#L204-L208
>>>
>>
>> Looking at your Makefile, you are not using `clang -target bpf` to
>> compile BPF object files, which is probably what causes you trouble.
>> preserve_access_index is a BPF target-only attribute. There is no need
>> to do the legacy clang -emit-llvm | llc, especially when you are using
>> CO-RE.
> 
> Got it. Funny enough, it turns out this is just a continuation of a
> conversation you had with my coworker Yaniv last year:
> https://lore.kernel.org/bpf/CAEf4BzbshRMCX1T1ooAtYGYuUGefbbo2=ProkMg5iOtUKh3YtQ@mail.gmail.com/
> 
> But to summarize our continued challenge: Adding the
> `preserve_access_index` attribute, compiling with `-target bpf`, and
> using the same kernel headers we used (not vmlinux.h) causes issues
> because of architecture specific asm errors (likely stemming from
> headers we include). Unless there's a way to get around those we're
> going to need to include "vmlinux.h", change our Makefile to `-target
> bpf`, and redefine macros and/or functions that vmlinux.h does not
> provide.
> 
> I think this is a pretty significant usability challenge. The idea you
> mentioned of having a built-in to detect if a type is defined would be
> a huge step forward. Has any progress been made towards this?

I briefly looked at this probably one and half years ago.
It will involve tweak clang frontend cpp side. Now I haven't
done any concrete work yet. But will look at it in the future.

> 
> Another thought is having vmlinux.h include function definitions,
> aren't they included in DWARF/BTF?
> 
> Thanks for your help, as always, Andrii!
> 
>>
>>>>
>>>> Also check that you use Clang that supports preserve_access_index, of course.
>>>
>>> I'm using clang 11.0 on Fedora 33. All dependencies appear properly
>>> installed (libelf, zlib, dwarves [provides pahole], llvm, llc,
>>> llvm-devel,...)
>>>
>>>>
>>>>>
>>>>> I've tried this and get a whole bunch of 'unknown attribute' warnings,
>>>>> leading me to believe that I either have something installed
>>>>> incorrectly or don't understand how to use clang attributes. Do I need
>>>>> to edit the types in the actual header files?
>>>>
>>>> No, the whole idea is to not touch original headers.
>>>
>>> Got it - that's good to know.
>>>
>>>>
>>>>>
>>>>> Thank you very very much for the help!
>>>>> - Grant
>>>>>>
>>>>>>>
>>>>>>> Thanks so much,
>>>>>>> Grant Seltzer

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Typical way to handle missing macros in vmlinux.h
  2021-05-04 16:24             ` Yonghong Song
@ 2021-05-04 16:57               ` Grant Seltzer Richman
  0 siblings, 0 replies; 10+ messages in thread
From: Grant Seltzer Richman @ 2021-05-04 16:57 UTC (permalink / raw)
  To: Yonghong Song; +Cc: Andrii Nakryiko, bpf

On Tue, May 4, 2021 at 12:25 PM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 5/4/21 8:31 AM, Grant Seltzer Richman wrote:
> > On Mon, May 3, 2021 at 5:22 PM Andrii Nakryiko
> > <andrii.nakryiko@gmail.com> wrote:
> >>
> >> On Mon, May 3, 2021 at 1:20 PM Grant Seltzer Richman
> >> <grantseltzer@gmail.com> wrote:
> >>>
> >>> On Mon, May 3, 2021 at 2:43 PM Andrii Nakryiko
> >>> <andrii.nakryiko@gmail.com> wrote:
> >>>>
> >>>> On Mon, May 3, 2021 at 11:32 AM Grant Seltzer Richman
> >>>> <grantseltzer@gmail.com> wrote:
> >>>>>
> >>>>> On Wed, Apr 28, 2021 at 5:15 PM Andrii Nakryiko
> >>>>> <andrii.nakryiko@gmail.com> wrote:
> >>>>>>
> >>>>>> On Wed, Apr 28, 2021 at 1:53 PM Grant Seltzer Richman
> >>>>>> <grantseltzer@gmail.com> wrote:
> >>>>>>>
> >>>>>>> Hi all,
> >>>>>>>
> >>>>>>> I'm working on enabling CO:RE in a project I work on, tracee, and am
> >>>>>>> running into the dilemma of missing macros that we previously were
> >>>>>>> able to import from their various header files. I understand that
> >>>>>>> macros don't make their way into BTF and therefore the generated
> >>>>>>> vmlinux.h won't have them. However I can't import the various header
> >>>>>>> files because of multiple-definition issues.
> >>>>>>
> >>>>>> Sadly, copy/pasting has been the only way so far.
> >>>>>>
> >>>>>>>
> >>>>>>> Do people typically redefine each of these macros for their project?
> >>>>>>> If so is there anything I should be careful of, such as architectural
> >>>>>>> differences. Does anyone have creative ideas, even if not developed
> >>>>>>> fully yet that I can possibly contribute to libbpf?
> >>>>>>
> >>>>>> We've discussed adding Clang built-in to detect if a specific type is
> >>>>>> already defined and doing something like this in vmlinux.h:
> >>>>>>
> >>>>>> #if !__builtin_is_type_defined(struct task_struct)
> >>>>>> struct task_struct {
> >>>>>>       ...
> >>>>>> }
> >>>>>> #endif
> >>>>>>
> >>>>>> And just do that for every struct, union, typedef. That would allow
> >>>>>> vmlinux.h to co-exist (somewhat) with other types.
> >>>>>>
> >>>>>> Another alternative is to not use vmlinux.h and use just linux
> >>>>>> headers, but mark necessary types with
> >>>>>> __attribute__((preserve_access_index)) to make them CO-RE relocatable.
> >>>>>> You can add that to existing types with the same pragma that vmlinux.h
> >>>>>> uses.
> >>>>>
> >>>>> I'm attempting to try doing the above. I'm just replacing
> >>>>> bpf_probe_read with bpf_core_read and not importing vmlinux.h, just
> >>>>> all the kernel headers I need.
> >>>>
> >>>> Yes, that will work, bpf_core_read() uses preserve_access_index
> >>>> built-in to achieve the same effect.
> >>>>
> >>>>>
> >>>>> When you say "Add that to existing types with the same pragma that
> >>>>> vmlinux.h uses", Should I be able to add the following to my bpf
> >>>>> source file before importing my headers?
> >>>>>
> >>>>> ifndef BPF_NO_PRESERVE_ACCESS_INDEX
> >>>>> #pragma clang attribute push (__attribute__((preserve_access_index)),
> >>>>> apply_to = record)
> >>>>> #endif
> >>>>>
> >>>>> and then pop the attribute at the bottom of the file, or after the
> >>>>> header includes.
> >>>>
> >>>> Yeah, that's the idea and that's what vmlinux.h does for all its
> >>>> structs. It doesn't add __attribute__((preserve_access_index)) after
> >>>> each struct/union. So I wonder why you are getting those unknown
> >>>> attribute errors. Can you paste an example?
> >>>
> >>> Here's a couple examples of the warnings:
> >>>
> >>> ```
> >>> tracee/tracee.bpf.c:5:46: warning: unknown attribute
> >>> 'preserve_access_index' ignored [-Wunknown-attributes]
> >>> #pragma clang attribute push (__attribute__((preserve_access_index)),
> >>> apply_to = record)
> >>>                                               ^
> >>> /lib/modules/5.10.21-200.fc33.x86_64/source/include/linux/ipv6.h:185:1:
> >>> note: when applied to this declaration
> >>> struct ipv6_fl_socklist;
> >>> ^
> >>> tracee/tracee.bpf.c:5:46: warning: unknown attribute
> >>> 'preserve_access_index' ignored [-Wunknown-attributes]
> >>> #pragma clang attribute push (__attribute__((preserve_access_index)),
> >>> apply_to = record)
> >>>                                               ^
> >>> /lib/modules/5.10.21-200.fc33.x86_64/source/include/linux/ipv6.h:187:1:
> >>> note: when applied to this declaration
> >>> struct inet6_cork {
> >>> ```
> >>>
> >>> after these warnings are emitted (it seems as if there's one for every
> >>> data type, though I can't confirm), I get errors that look like this:
> >>>
> >>> ```
> >>> tracee/tracee.bpf.c:445:22: error: nested
> >>> builtin_preserve_access_index() not supported
> >>>      return READ_KERN(READ_KERN(task->thread_pid)->numbers[level].nr);
> >>>                       ^
> >>> tracee/tracee.bpf.c:206:27: note: expanded from macro 'READ_KERN'
> >>>                            bpf_core_read(&_val, sizeof(_val), &ptr); \
> >>> ```
> >>> I believe this is just a result of the warnings above, but if you're
> >>> curious it's what i'm doing here:
> >>> https://github.com/aquasecurity/tracee/blob/core-experiment/tracee-ebpf/tracee/tracee.bpf.c#L204-L208
> >>>
> >>
> >> Looking at your Makefile, you are not using `clang -target bpf` to
> >> compile BPF object files, which is probably what causes you trouble.
> >> preserve_access_index is a BPF target-only attribute. There is no need
> >> to do the legacy clang -emit-llvm | llc, especially when you are using
> >> CO-RE.
> >
> > Got it. Funny enough, it turns out this is just a continuation of a
> > conversation you had with my coworker Yaniv last year:
> > https://lore.kernel.org/bpf/CAEf4BzbshRMCX1T1ooAtYGYuUGefbbo2=ProkMg5iOtUKh3YtQ@mail.gmail.com/
> >
> > But to summarize our continued challenge: Adding the
> > `preserve_access_index` attribute, compiling with `-target bpf`, and
> > using the same kernel headers we used (not vmlinux.h) causes issues
> > because of architecture specific asm errors (likely stemming from
> > headers we include). Unless there's a way to get around those we're
> > going to need to include "vmlinux.h", change our Makefile to `-target
> > bpf`, and redefine macros and/or functions that vmlinux.h does not
> > provide.
> >
> > I think this is a pretty significant usability challenge. The idea you
> > mentioned of having a built-in to detect if a type is defined would be
> > a huge step forward. Has any progress been made towards this?
>
> I briefly looked at this probably one and half years ago.
> It will involve tweak clang frontend cpp side. Now I haven't
> done any concrete work yet. But will look at it in the future.

Got it - thanks for the info. Also any advice you or anyone else has
to avoid having to redefine functions would be very much appreciated!

>
> >
> > Another thought is having vmlinux.h include function definitions,
> > aren't they included in DWARF/BTF?
> >
> > Thanks for your help, as always, Andrii!
> >
> >>
> >>>>
> >>>> Also check that you use Clang that supports preserve_access_index, of course.
> >>>
> >>> I'm using clang 11.0 on Fedora 33. All dependencies appear properly
> >>> installed (libelf, zlib, dwarves [provides pahole], llvm, llc,
> >>> llvm-devel,...)
> >>>
> >>>>
> >>>>>
> >>>>> I've tried this and get a whole bunch of 'unknown attribute' warnings,
> >>>>> leading me to believe that I either have something installed
> >>>>> incorrectly or don't understand how to use clang attributes. Do I need
> >>>>> to edit the types in the actual header files?
> >>>>
> >>>> No, the whole idea is to not touch original headers.
> >>>
> >>> Got it - that's good to know.
> >>>
> >>>>
> >>>>>
> >>>>> Thank you very very much for the help!
> >>>>> - Grant
> >>>>>>
> >>>>>>>
> >>>>>>> Thanks so much,
> >>>>>>> Grant Seltzer

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Typical way to handle missing macros in vmlinux.h
  2021-05-04 15:31           ` Grant Seltzer Richman
  2021-05-04 16:24             ` Yonghong Song
@ 2021-05-04 22:31             ` Andrii Nakryiko
  1 sibling, 0 replies; 10+ messages in thread
From: Andrii Nakryiko @ 2021-05-04 22:31 UTC (permalink / raw)
  To: Grant Seltzer Richman; +Cc: bpf

On Tue, May 4, 2021 at 8:32 AM Grant Seltzer Richman
<grantseltzer@gmail.com> wrote:
>
> On Mon, May 3, 2021 at 5:22 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Mon, May 3, 2021 at 1:20 PM Grant Seltzer Richman
> > <grantseltzer@gmail.com> wrote:
> > >
> > > On Mon, May 3, 2021 at 2:43 PM Andrii Nakryiko
> > > <andrii.nakryiko@gmail.com> wrote:
> > > >
> > > > On Mon, May 3, 2021 at 11:32 AM Grant Seltzer Richman
> > > > <grantseltzer@gmail.com> wrote:
> > > > >
> > > > > On Wed, Apr 28, 2021 at 5:15 PM Andrii Nakryiko
> > > > > <andrii.nakryiko@gmail.com> wrote:
> > > > > >
> > > > > > On Wed, Apr 28, 2021 at 1:53 PM Grant Seltzer Richman
> > > > > > <grantseltzer@gmail.com> wrote:
> > > > > > >
> > > > > > > Hi all,
> > > > > > >
> > > > > > > I'm working on enabling CO:RE in a project I work on, tracee, and am
> > > > > > > running into the dilemma of missing macros that we previously were
> > > > > > > able to import from their various header files. I understand that
> > > > > > > macros don't make their way into BTF and therefore the generated
> > > > > > > vmlinux.h won't have them. However I can't import the various header
> > > > > > > files because of multiple-definition issues.
> > > > > >
> > > > > > Sadly, copy/pasting has been the only way so far.
> > > > > >
> > > > > > >
> > > > > > > Do people typically redefine each of these macros for their project?
> > > > > > > If so is there anything I should be careful of, such as architectural
> > > > > > > differences. Does anyone have creative ideas, even if not developed
> > > > > > > fully yet that I can possibly contribute to libbpf?
> > > > > >
> > > > > > We've discussed adding Clang built-in to detect if a specific type is
> > > > > > already defined and doing something like this in vmlinux.h:
> > > > > >
> > > > > > #if !__builtin_is_type_defined(struct task_struct)
> > > > > > struct task_struct {
> > > > > >      ...
> > > > > > }
> > > > > > #endif
> > > > > >
> > > > > > And just do that for every struct, union, typedef. That would allow
> > > > > > vmlinux.h to co-exist (somewhat) with other types.
> > > > > >
> > > > > > Another alternative is to not use vmlinux.h and use just linux
> > > > > > headers, but mark necessary types with
> > > > > > __attribute__((preserve_access_index)) to make them CO-RE relocatable.
> > > > > > You can add that to existing types with the same pragma that vmlinux.h
> > > > > > uses.
> > > > >
> > > > > I'm attempting to try doing the above. I'm just replacing
> > > > > bpf_probe_read with bpf_core_read and not importing vmlinux.h, just
> > > > > all the kernel headers I need.
> > > >
> > > > Yes, that will work, bpf_core_read() uses preserve_access_index
> > > > built-in to achieve the same effect.
> > > >
> > > > >
> > > > > When you say "Add that to existing types with the same pragma that
> > > > > vmlinux.h uses", Should I be able to add the following to my bpf
> > > > > source file before importing my headers?
> > > > >
> > > > > ifndef BPF_NO_PRESERVE_ACCESS_INDEX
> > > > > #pragma clang attribute push (__attribute__((preserve_access_index)),
> > > > > apply_to = record)
> > > > > #endif
> > > > >
> > > > > and then pop the attribute at the bottom of the file, or after the
> > > > > header includes.
> > > >
> > > > Yeah, that's the idea and that's what vmlinux.h does for all its
> > > > structs. It doesn't add __attribute__((preserve_access_index)) after
> > > > each struct/union. So I wonder why you are getting those unknown
> > > > attribute errors. Can you paste an example?
> > >
> > > Here's a couple examples of the warnings:
> > >
> > > ```
> > > tracee/tracee.bpf.c:5:46: warning: unknown attribute
> > > 'preserve_access_index' ignored [-Wunknown-attributes]
> > > #pragma clang attribute push (__attribute__((preserve_access_index)),
> > > apply_to = record)
> > >                                              ^
> > > /lib/modules/5.10.21-200.fc33.x86_64/source/include/linux/ipv6.h:185:1:
> > > note: when applied to this declaration
> > > struct ipv6_fl_socklist;
> > > ^
> > > tracee/tracee.bpf.c:5:46: warning: unknown attribute
> > > 'preserve_access_index' ignored [-Wunknown-attributes]
> > > #pragma clang attribute push (__attribute__((preserve_access_index)),
> > > apply_to = record)
> > >                                              ^
> > > /lib/modules/5.10.21-200.fc33.x86_64/source/include/linux/ipv6.h:187:1:
> > > note: when applied to this declaration
> > > struct inet6_cork {
> > > ```
> > >
> > > after these warnings are emitted (it seems as if there's one for every
> > > data type, though I can't confirm), I get errors that look like this:
> > >
> > > ```
> > > tracee/tracee.bpf.c:445:22: error: nested
> > > builtin_preserve_access_index() not supported
> > >     return READ_KERN(READ_KERN(task->thread_pid)->numbers[level].nr);
> > >                      ^
> > > tracee/tracee.bpf.c:206:27: note: expanded from macro 'READ_KERN'
> > >                           bpf_core_read(&_val, sizeof(_val), &ptr); \
> > > ```
> > > I believe this is just a result of the warnings above, but if you're
> > > curious it's what i'm doing here:
> > > https://github.com/aquasecurity/tracee/blob/core-experiment/tracee-ebpf/tracee/tracee.bpf.c#L204-L208
> > >
> >
> > Looking at your Makefile, you are not using `clang -target bpf` to
> > compile BPF object files, which is probably what causes you trouble.
> > preserve_access_index is a BPF target-only attribute. There is no need
> > to do the legacy clang -emit-llvm | llc, especially when you are using
> > CO-RE.
>
> Got it. Funny enough, it turns out this is just a continuation of a
> conversation you had with my coworker Yaniv last year:
> https://lore.kernel.org/bpf/CAEf4BzbshRMCX1T1ooAtYGYuUGefbbo2=ProkMg5iOtUKh3YtQ@mail.gmail.com/
>
> But to summarize our continued challenge: Adding the
> `preserve_access_index` attribute, compiling with `-target bpf`, and
> using the same kernel headers we used (not vmlinux.h) causes issues
> because of architecture specific asm errors (likely stemming from
> headers we include). Unless there's a way to get around those we're
> going to need to include "vmlinux.h", change our Makefile to `-target
> bpf`, and redefine macros and/or functions that vmlinux.h does not
> provide.
>
> I think this is a pretty significant usability challenge. The idea you
> mentioned of having a built-in to detect if a type is defined would be
> a huge step forward. Has any progress been made towards this?
>
> Another thought is having vmlinux.h include function definitions,
> aren't they included in DWARF/BTF?

DWARF might have some #defines recorded if one enables some extra
DWARF subsets (which I don't think are enabled by default). BTF
definitely doesn't have anything like that.

>
> Thanks for your help, as always, Andrii!
>
> >
> > > >
> > > > Also check that you use Clang that supports preserve_access_index, of course.
> > >
> > > I'm using clang 11.0 on Fedora 33. All dependencies appear properly
> > > installed (libelf, zlib, dwarves [provides pahole], llvm, llc,
> > > llvm-devel,...)
> > >
> > > >
> > > > >
> > > > > I've tried this and get a whole bunch of 'unknown attribute' warnings,
> > > > > leading me to believe that I either have something installed
> > > > > incorrectly or don't understand how to use clang attributes. Do I need
> > > > > to edit the types in the actual header files?
> > > >
> > > > No, the whole idea is to not touch original headers.
> > >
> > > Got it - that's good to know.
> > >
> > > >
> > > > >
> > > > > Thank you very very much for the help!
> > > > > - Grant
> > > > > >
> > > > > > >
> > > > > > > Thanks so much,
> > > > > > > Grant Seltzer

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-05-04 22:32 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-28 19:13 Typical way to handle missing macros in vmlinux.h Grant Seltzer Richman
2021-04-28 21:15 ` Andrii Nakryiko
2021-05-03 18:32   ` Grant Seltzer Richman
2021-05-03 18:43     ` Andrii Nakryiko
2021-05-03 20:20       ` Grant Seltzer Richman
2021-05-03 21:21         ` Andrii Nakryiko
2021-05-04 15:31           ` Grant Seltzer Richman
2021-05-04 16:24             ` Yonghong Song
2021-05-04 16:57               ` Grant Seltzer Richman
2021-05-04 22:31             ` Andrii Nakryiko

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).