Re: [PATCH bpf-next 1/3] libbpf: add asm/unistd.h to xsk to get __NR_mmap2

From: Yonghong Song
Date: Wed Aug 14 2019 - 11:52:00 EST




On 8/14/19 2:24 AM, Ivan Khoronzhuk wrote:
> On Tue, Aug 13, 2019 at 04:38:13PM -0700, Andrii Nakryiko wrote:
>
> Hi, Andrii
>
>> On Tue, Aug 13, 2019 at 3:24 AM Ivan Khoronzhuk
>> <ivan.khoronzhuk@xxxxxxxxxx> wrote:
>>>
>>> That's needed to get __NR_mmap2 when mmap2 syscall is used.
>>>
>>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@xxxxxxxxxx>
>>> ---
>>> Âtools/lib/bpf/xsk.c | 1 +
>>> Â1 file changed, 1 insertion(+)
>>>
>>> diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
>>> index 5007b5d4fd2c..f2fc40f9804c 100644
>>> --- a/tools/lib/bpf/xsk.c
>>> +++ b/tools/lib/bpf/xsk.c
>>> @@ -12,6 +12,7 @@
>>> Â#include <stdlib.h>
>>> Â#include <string.h>
>>> Â#include <unistd.h>
>>> +#include <asm/unistd.h>
>>
>> asm/unistd.h is not present in Github libbpf projection. Is there any
>
> Look on includes from
> tools/lib/bpf/libpf.c
> tools/lib/bpf/bpf.c
>
> That's how it's done... Copping headers to arch/arm will not
> solve this, it includes both of them anyway, and anyway it needs
> asm/unistd.h inclusion here, only because xsk.c needs __NR_*
>
>
>> way to avoid including this header? Generally, libbpf can't easily use
>> all of kernel headers, we need to re-implemented all the extra used
>> stuff for Github version of libbpf, so we try to minimize usage of new
>> headers that are not just plain uapi headers from include/uapi.
>
> Yes I know, it's far away from real number of changes needed.
> I faced enough about this already and kernel headers, especially
> for arm32 it's a bit decency problem. But this patch it's part of
> normal one. I have couple issues despite this normally fixed mmap2
> that is the same even if uapi includes are coppied to tools/arch/arm.
>
> In continuation of kernel headers inclusion and arm build:
>
> For instance, what about this rough "kernel headers" hack:
> https://github.com/ikhorn/af_xdp_stuff/commit/aa645ccca4d844f404ec3c2b27402d4d7848d1b5

The ".syntax unified" is mentioned a couple of times
in bcc mailing list as well. llvm bpf backend might
be able to solve it. I have not looked at the details though.

>
> or this one related for arm32 only:
> https://github.com/ikhorn/af_xdp_stuff/commit/2c6c6d538605aac39600dcb3c9b66de11c70b963

This may not work if bpf program tries to handle kernel headers.
bpf program may get wrong layout.

Anyway, the above two comments are irrelevant to this patch set
and if needed should be discussed separately.

>
>
> I have more...
>
>>
>>> Â#include <arpa/inet.h>
>>> Â#include <asm/barrier.h>
>>> Â#include <linux/compiler.h>
>>> --
>>> 2.17.1
>>>
>