From: Yonghong Song <yhs@fb.com>
To: Grant Seltzer Richman <grantseltzer@gmail.com>,
Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: bpf <bpf@vger.kernel.org>
Subject: Re: Typical way to handle missing macros in vmlinux.h
Date: Tue, 4 May 2021 09:24:54 -0700 [thread overview]
Message-ID: <294a5f06-19dd-d649-a000-c40f1fdbd299@fb.com> (raw)
In-Reply-To: <CAO658oX7_b18Q4OxZ_PxAPhBjQPXv4+dQsQzH1-TWKhozikWiA@mail.gmail.com>
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
next prev parent reply other threads:[~2021-05-04 16:25 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2021-05-04 16:57 ` Grant Seltzer Richman
2021-05-04 22:31 ` Andrii Nakryiko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=294a5f06-19dd-d649-a000-c40f1fdbd299@fb.com \
--to=yhs@fb.com \
--cc=andrii.nakryiko@gmail.com \
--cc=bpf@vger.kernel.org \
--cc=grantseltzer@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).