[PATCH bpf-next 0/3] xdpsock: allow mmap2 usage for 32bits

[PATCH bpf-next 0/3] xdpsock: allow mmap2 usage for 32bits

[Ivan Khoronzhuk]

This patchset contains several improvements for af_xdp socket umem
mappings for 32bit systems. Also, there is one more patch outside of
othis series that can be applied to another tree and related to mmap2
af_xdp umem offsets:
"mm: mmap: increase sockets maximum memory size pgoff for 32bits"
https://lkml.org/lkml/2019/8/12/549

Based on bpf-next/master

Ivan Khoronzhuk (3):
  libbpf: add asm/unistd.h to xsk to get __NR_mmap2
  xdp: xdp_umem: replace kmap on vmap for umem map
  samples: bpf: syscal_nrs: use mmap2 if defined

 net/xdp/xdp_umem.c         | 16 ++++++++++++----
 samples/bpf/syscall_nrs.c  |  5 +++++
 samples/bpf/tracex5_kern.c | 11 +++++++++++
 tools/lib/bpf/xsk.c        |  1 +
 4 files changed, 29 insertions(+), 4 deletions(-)

-- 
2.17.1

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

[Ivan Khoronzhuk]

That's needed to get __NR_mmap2 when mmap2 syscall is used.

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
 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>
 #include <arpa/inet.h>
 #include <asm/barrier.h>
 #include <linux/compiler.h>
-- 
2.17.1

[PATCH bpf-next 2/3] xdp: xdp_umem: replace kmap on vmap for umem map

[Ivan Khoronzhuk]

For 64-bit there is no reason to use vmap/vunmap, so use page_address
as it was initially. For 32 bits, in some apps, like in samples
xdpsock_user.c when number of pgs in use is quite big, the kmap
memory can be not enough, despite on this, kmap looks like is
deprecated in such cases as it can block and should be used rather
for dynamic mm.

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
 net/xdp/xdp_umem.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c
index a0607969f8c0..907c9019fe21 100644
--- a/net/xdp/xdp_umem.c
+++ b/net/xdp/xdp_umem.c
@@ -14,7 +14,7 @@
 #include <linux/netdevice.h>
 #include <linux/rtnetlink.h>
 #include <linux/idr.h>
-#include <linux/highmem.h>
+#include <linux/vmalloc.h>
 
 #include "xdp_umem.h"
 #include "xsk_queue.h"
@@ -167,10 +167,12 @@ void xdp_umem_clear_dev(struct xdp_umem *umem)
 
 static void xdp_umem_unmap_pages(struct xdp_umem *umem)
 {
+#if BITS_PER_LONG == 32
 	unsigned int i;
 
 	for (i = 0; i < umem->npgs; i++)
-		kunmap(umem->pgs[i]);
+		vunmap(umem->pages[i].addr);
+#endif
 }
 
 static void xdp_umem_unpin_pages(struct xdp_umem *umem)
@@ -378,8 +380,14 @@ static int xdp_umem_reg(struct xdp_umem *umem, struct xdp_umem_reg *mr)
 		goto out_account;
 	}
 
-	for (i = 0; i < umem->npgs; i++)
-		umem->pages[i].addr = kmap(umem->pgs[i]);
+	for (i = 0; i < umem->npgs; i++) {
+#if BITS_PER_LONG == 32
+		umem->pages[i].addr = vmap(&umem->pgs[i], 1, VM_MAP,
+					   PAGE_KERNEL);
+#else
+		umem->pages[i].addr = page_address(umem->pgs[i]);
+#endif
+	}
 
 	return 0;
 
-- 
2.17.1

[PATCH bpf-next 3/3] samples: bpf: syscal_nrs: use mmap2 if defined

[Ivan Khoronzhuk]

For arm32 xdp sockets mmap2 is preferred, so use it if it's defined.

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
 samples/bpf/syscall_nrs.c  |  5 +++++
 samples/bpf/tracex5_kern.c | 11 +++++++++++
 2 files changed, 16 insertions(+)

diff --git a/samples/bpf/syscall_nrs.c b/samples/bpf/syscall_nrs.c
index 516e255cbe8f..2dec94238350 100644
--- a/samples/bpf/syscall_nrs.c
+++ b/samples/bpf/syscall_nrs.c
@@ -9,5 +9,10 @@ void syscall_defines(void)
 	COMMENT("Linux system call numbers.");
 	SYSNR(__NR_write);
 	SYSNR(__NR_read);
+#ifdef __NR_mmap2
+	SYSNR(__NR_mmap2);
+#else
 	SYSNR(__NR_mmap);
+#endif
+
 }
diff --git a/samples/bpf/tracex5_kern.c b/samples/bpf/tracex5_kern.c
index f57f4e1ea1ec..300350ad299a 100644
--- a/samples/bpf/tracex5_kern.c
+++ b/samples/bpf/tracex5_kern.c
@@ -68,12 +68,23 @@ PROG(SYS__NR_read)(struct pt_regs *ctx)
 	return 0;
 }
 
+#ifdef __NR_mmap2
+PROG(SYS__NR_mmap2)(struct pt_regs *ctx)
+{
+	char fmt[] = "mmap2\n";
+
+	bpf_trace_printk(fmt, sizeof(fmt));
+	return 0;
+}
+#else
 PROG(SYS__NR_mmap)(struct pt_regs *ctx)
 {
 	char fmt[] = "mmap\n";
+
 	bpf_trace_printk(fmt, sizeof(fmt));
 	return 0;
 }
+#endif
 
 char _license[] SEC("license") = "GPL";
 u32 _version SEC("version") = LINUX_VERSION_CODE;
-- 
2.17.1

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

[Jonathan Lemon]

On 13 Aug 2019, at 3:23, Ivan Khoronzhuk wrote:

> That's needed to get __NR_mmap2 when mmap2 syscall is used.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>

Acked-by: Jonathan Lemon <jonathan.lemon@gmail.com>

Re: [PATCH bpf-next 3/3] samples: bpf: syscal_nrs: use mmap2 if defined

[Jonathan Lemon]

On 13 Aug 2019, at 3:23, Ivan Khoronzhuk wrote:

> For arm32 xdp sockets mmap2 is preferred, so use it if it's defined.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>

Doesn't this change the application API?
-- 
Jonathan


> ---
>  samples/bpf/syscall_nrs.c  |  5 +++++
>  samples/bpf/tracex5_kern.c | 11 +++++++++++
>  2 files changed, 16 insertions(+)
>
> diff --git a/samples/bpf/syscall_nrs.c b/samples/bpf/syscall_nrs.c
> index 516e255cbe8f..2dec94238350 100644
> --- a/samples/bpf/syscall_nrs.c
> +++ b/samples/bpf/syscall_nrs.c
> @@ -9,5 +9,10 @@ void syscall_defines(void)
>  	COMMENT("Linux system call numbers.");
>  	SYSNR(__NR_write);
>  	SYSNR(__NR_read);
> +#ifdef __NR_mmap2
> +	SYSNR(__NR_mmap2);
> +#else
>  	SYSNR(__NR_mmap);
> +#endif
> +
>  }
> diff --git a/samples/bpf/tracex5_kern.c b/samples/bpf/tracex5_kern.c
> index f57f4e1ea1ec..300350ad299a 100644
> --- a/samples/bpf/tracex5_kern.c
> +++ b/samples/bpf/tracex5_kern.c
> @@ -68,12 +68,23 @@ PROG(SYS__NR_read)(struct pt_regs *ctx)
>  	return 0;
>  }
>
> +#ifdef __NR_mmap2
> +PROG(SYS__NR_mmap2)(struct pt_regs *ctx)
> +{
> +	char fmt[] = "mmap2\n";
> +
> +	bpf_trace_printk(fmt, sizeof(fmt));
> +	return 0;
> +}
> +#else
>  PROG(SYS__NR_mmap)(struct pt_regs *ctx)
>  {
>  	char fmt[] = "mmap\n";
> +
>  	bpf_trace_printk(fmt, sizeof(fmt));
>  	return 0;
>  }
> +#endif
>
>  char _license[] SEC("license") = "GPL";
>  u32 _version SEC("version") = LINUX_VERSION_CODE;
> -- 
> 2.17.1

Re: [PATCH bpf-next 2/3] xdp: xdp_umem: replace kmap on vmap for umem map

[Jonathan Lemon]

On 13 Aug 2019, at 3:23, Ivan Khoronzhuk wrote:

> For 64-bit there is no reason to use vmap/vunmap, so use page_address
> as it was initially. For 32 bits, in some apps, like in samples
> xdpsock_user.c when number of pgs in use is quite big, the kmap
> memory can be not enough, despite on this, kmap looks like is
> deprecated in such cases as it can block and should be used rather
> for dynamic mm.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>

Seems a bit overkill - if not high memory, kmap() falls back
to just page_address(), unlike vmap().
-- 
Jonathan

> ---
>  net/xdp/xdp_umem.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c
> index a0607969f8c0..907c9019fe21 100644
> --- a/net/xdp/xdp_umem.c
> +++ b/net/xdp/xdp_umem.c
> @@ -14,7 +14,7 @@
>  #include <linux/netdevice.h>
>  #include <linux/rtnetlink.h>
>  #include <linux/idr.h>
> -#include <linux/highmem.h>
> +#include <linux/vmalloc.h>
>
>  #include "xdp_umem.h"
>  #include "xsk_queue.h"
> @@ -167,10 +167,12 @@ void xdp_umem_clear_dev(struct xdp_umem *umem)
>
>  static void xdp_umem_unmap_pages(struct xdp_umem *umem)
>  {
> +#if BITS_PER_LONG == 32
>  	unsigned int i;
>
>  	for (i = 0; i < umem->npgs; i++)
> -		kunmap(umem->pgs[i]);
> +		vunmap(umem->pages[i].addr);
> +#endif
>  }
>
>  static void xdp_umem_unpin_pages(struct xdp_umem *umem)
> @@ -378,8 +380,14 @@ static int xdp_umem_reg(struct xdp_umem *umem, 
> struct xdp_umem_reg *mr)
>  		goto out_account;
>  	}
>
> -	for (i = 0; i < umem->npgs; i++)
> -		umem->pages[i].addr = kmap(umem->pgs[i]);
> +	for (i = 0; i < umem->npgs; i++) {
> +#if BITS_PER_LONG == 32
> +		umem->pages[i].addr = vmap(&umem->pgs[i], 1, VM_MAP,
> +					   PAGE_KERNEL);
> +#else
> +		umem->pages[i].addr = page_address(umem->pgs[i]);
> +#endif
> +	}
>
>  	return 0;
>
> -- 
> 2.17.1

Re: [PATCH bpf-next 2/3] xdp: xdp_umem: replace kmap on vmap for umem map

[Ivan Khoronzhuk]

On Tue, Aug 13, 2019 at 10:42:18AM -0700, Jonathan Lemon wrote:
>
>
>On 13 Aug 2019, at 3:23, Ivan Khoronzhuk wrote:
>
>>For 64-bit there is no reason to use vmap/vunmap, so use page_address
>>as it was initially. For 32 bits, in some apps, like in samples
>>xdpsock_user.c when number of pgs in use is quite big, the kmap
>>memory can be not enough, despite on this, kmap looks like is
>>deprecated in such cases as it can block and should be used rather
>>for dynamic mm.
>>
>>Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>
>Seems a bit overkill - if not high memory, kmap() falls back
>to just page_address(), unlike vmap().

>-- Jonathan

So, as kmap has limitation... if I correctly understood, you propose
to avoid macros and do smth like kmap:

	void *addr;
	if (!PageHighMem(&umem->pgs[i]))
		addr =  page_address(page);
	else
		addr = vmap(&umem->pgs[i], 1, VM_MAP, PAGE_KERNEL);

	umem->pages[i].addr = addr;

and while unmap

	if (!PageHighMem(&umem->pgs[i]))
		vunmap(umem->pages[i].addr);

I can try it, and add this in v2 if no objection.

>
>>---
>> net/xdp/xdp_umem.c | 16 ++++++++++++----
>> 1 file changed, 12 insertions(+), 4 deletions(-)
>>
>>diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c
>>index a0607969f8c0..907c9019fe21 100644
>>--- a/net/xdp/xdp_umem.c
>>+++ b/net/xdp/xdp_umem.c
>>@@ -14,7 +14,7 @@
>> #include <linux/netdevice.h>
>> #include <linux/rtnetlink.h>
>> #include <linux/idr.h>
>>-#include <linux/highmem.h>
>>+#include <linux/vmalloc.h>
>>
>> #include "xdp_umem.h"
>> #include "xsk_queue.h"
>>@@ -167,10 +167,12 @@ void xdp_umem_clear_dev(struct xdp_umem *umem)
>>
>> static void xdp_umem_unmap_pages(struct xdp_umem *umem)
>> {
>>+#if BITS_PER_LONG == 32
>> 	unsigned int i;
>>
>> 	for (i = 0; i < umem->npgs; i++)
>>-		kunmap(umem->pgs[i]);
>>+		vunmap(umem->pages[i].addr);
>>+#endif
>> }
>>
>> static void xdp_umem_unpin_pages(struct xdp_umem *umem)
>>@@ -378,8 +380,14 @@ static int xdp_umem_reg(struct xdp_umem *umem, 
>>struct xdp_umem_reg *mr)
>> 		goto out_account;
>> 	}
>>
>>-	for (i = 0; i < umem->npgs; i++)
>>-		umem->pages[i].addr = kmap(umem->pgs[i]);
>>+	for (i = 0; i < umem->npgs; i++) {
>>+#if BITS_PER_LONG == 32
>>+		umem->pages[i].addr = vmap(&umem->pgs[i], 1, VM_MAP,
>>+					   PAGE_KERNEL);
>>+#else
>>+		umem->pages[i].addr = page_address(umem->pgs[i]);
>>+#endif
>>+	}
>>
>> 	return 0;
>>
>>-- 
>>2.17.1

-- 
Regards,
Ivan Khoronzhuk

Re: [PATCH bpf-next 2/3] xdp: xdp_umem: replace kmap on vmap for umem map

[Jonathan Lemon]

On 13 Aug 2019, at 11:30, Ivan Khoronzhuk wrote:

> On Tue, Aug 13, 2019 at 10:42:18AM -0700, Jonathan Lemon wrote:
>>
>>
>> On 13 Aug 2019, at 3:23, Ivan Khoronzhuk wrote:
>>
>>> For 64-bit there is no reason to use vmap/vunmap, so use 
>>> page_address
>>> as it was initially. For 32 bits, in some apps, like in samples
>>> xdpsock_user.c when number of pgs in use is quite big, the kmap
>>> memory can be not enough, despite on this, kmap looks like is
>>> deprecated in such cases as it can block and should be used rather
>>> for dynamic mm.
>>>
>>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>>
>> Seems a bit overkill - if not high memory, kmap() falls back
>> to just page_address(), unlike vmap().
>
>> -- Jonathan
>
> So, as kmap has limitation... if I correctly understood, you propose
> to avoid macros and do smth like kmap:
>
> 	void *addr;
> 	if (!PageHighMem(&umem->pgs[i]))
> 		addr =  page_address(page);
> 	else
> 		addr = vmap(&umem->pgs[i], 1, VM_MAP, PAGE_KERNEL);
>
> 	umem->pages[i].addr = addr;
>
> and while unmap
>
> 	if (!PageHighMem(&umem->pgs[i]))
> 		vunmap(umem->pages[i].addr);
>
> I can try it, and add this in v2 if no objection.

Seems like a reasonable compromise to me.
-- 
Jonathan


>
>>
>>> ---
>>> net/xdp/xdp_umem.c | 16 ++++++++++++----
>>> 1 file changed, 12 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c
>>> index a0607969f8c0..907c9019fe21 100644
>>> --- a/net/xdp/xdp_umem.c
>>> +++ b/net/xdp/xdp_umem.c
>>> @@ -14,7 +14,7 @@
>>> #include <linux/netdevice.h>
>>> #include <linux/rtnetlink.h>
>>> #include <linux/idr.h>
>>> -#include <linux/highmem.h>
>>> +#include <linux/vmalloc.h>
>>>
>>> #include "xdp_umem.h"
>>> #include "xsk_queue.h"
>>> @@ -167,10 +167,12 @@ void xdp_umem_clear_dev(struct xdp_umem *umem)
>>>
>>> static void xdp_umem_unmap_pages(struct xdp_umem *umem)
>>> {
>>> +#if BITS_PER_LONG == 32
>>> 	unsigned int i;
>>>
>>> 	for (i = 0; i < umem->npgs; i++)
>>> -		kunmap(umem->pgs[i]);
>>> +		vunmap(umem->pages[i].addr);
>>> +#endif
>>> }
>>>
>>> static void xdp_umem_unpin_pages(struct xdp_umem *umem)
>>> @@ -378,8 +380,14 @@ static int xdp_umem_reg(struct xdp_umem *umem, 
>>> struct xdp_umem_reg *mr)
>>> 		goto out_account;
>>> 	}
>>>
>>> -	for (i = 0; i < umem->npgs; i++)
>>> -		umem->pages[i].addr = kmap(umem->pgs[i]);
>>> +	for (i = 0; i < umem->npgs; i++) {
>>> +#if BITS_PER_LONG == 32
>>> +		umem->pages[i].addr = vmap(&umem->pgs[i], 1, VM_MAP,
>>> +					   PAGE_KERNEL);
>>> +#else
>>> +		umem->pages[i].addr = page_address(umem->pgs[i]);
>>> +#endif
>>> +	}
>>>
>>> 	return 0;
>>>
>>> -- 
>>> 2.17.1
>
> -- 
> Regards,
> Ivan Khoronzhuk

Re: [PATCH bpf-next 3/3] samples: bpf: syscal_nrs: use mmap2 if defined

[Ivan Khoronzhuk]

On Tue, Aug 13, 2019 at 10:41:54AM -0700, Jonathan Lemon wrote:
>
>
>On 13 Aug 2019, at 3:23, Ivan Khoronzhuk wrote:
>
>> For arm32 xdp sockets mmap2 is preferred, so use it if it's defined.
>>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>
>Doesn't this change the application API?
>-- 
>Jonathan

From what I know there is no reason to use both, so if __NR_mmap2 is defined
but not __NR_mmap. Despite the fact that it can be defined internally, say
#define __NR_mmap (__NR_SYSCALL_BASE + 90)
and be used anyway, at least arm use 2 definition one is for old abi and one is
for new and names as their numbers are different:

#define __NR_mmap (__NR_SYSCALL_BASE + 90)
#define __NR_mmap2 (__NR_SYSCALL_BASE + 192)

, so they are not interchangeable and if eabi is used then only __NR_mmap2 is
defined if oeabi then __NR_mmap only... But mmap() use only one and can hide
this from user.

In this patch, seems like here is direct access, so I have no declaration for
__NR_mmap and it breaks build. So here several solutions, I can block __NR_mmap
at all or replace it on __NR_mmap2...or define it by hand (for what then?).
I decided to replace on real one.

>
>
>> ---
>>  samples/bpf/syscall_nrs.c  |  5 +++++
>>  samples/bpf/tracex5_kern.c | 11 +++++++++++
>>  2 files changed, 16 insertions(+)
>>
>> diff --git a/samples/bpf/syscall_nrs.c b/samples/bpf/syscall_nrs.c
>> index 516e255cbe8f..2dec94238350 100644
>> --- a/samples/bpf/syscall_nrs.c
>> +++ b/samples/bpf/syscall_nrs.c
>> @@ -9,5 +9,10 @@ void syscall_defines(void)
>>  	COMMENT("Linux system call numbers.");
>>  	SYSNR(__NR_write);
>>  	SYSNR(__NR_read);
>> +#ifdef __NR_mmap2
>> +	SYSNR(__NR_mmap2);
>> +#else
>>  	SYSNR(__NR_mmap);
>> +#endif
>> +
>>  }
>> diff --git a/samples/bpf/tracex5_kern.c b/samples/bpf/tracex5_kern.c
>> index f57f4e1ea1ec..300350ad299a 100644
>> --- a/samples/bpf/tracex5_kern.c
>> +++ b/samples/bpf/tracex5_kern.c
>> @@ -68,12 +68,23 @@ PROG(SYS__NR_read)(struct pt_regs *ctx)
>>  	return 0;
>>  }
>>
>> +#ifdef __NR_mmap2
>> +PROG(SYS__NR_mmap2)(struct pt_regs *ctx)
>> +{
>> +	char fmt[] = "mmap2\n";
>> +
>> +	bpf_trace_printk(fmt, sizeof(fmt));
>> +	return 0;
>> +}
>> +#else
>>  PROG(SYS__NR_mmap)(struct pt_regs *ctx)
>>  {
>>  	char fmt[] = "mmap\n";
>> +
>>  	bpf_trace_printk(fmt, sizeof(fmt));
>>  	return 0;
>>  }
>> +#endif
>>
>>  char _license[] SEC("license") = "GPL";
>>  u32 _version SEC("version") = LINUX_VERSION_CODE;
>> --
>> 2.17.1

-- 
Regards,
Ivan Khoronzhuk

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

[Andrii Nakryiko]

On Tue, Aug 13, 2019 at 3:24 AM Ivan Khoronzhuk
<ivan.khoronzhuk@linaro.org> wrote:
>
> That's needed to get __NR_mmap2 when mmap2 syscall is used.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> ---
>  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
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.

>  #include <arpa/inet.h>
>  #include <asm/barrier.h>
>  #include <linux/compiler.h>
> --
> 2.17.1
>

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

[Yonghong Song]

On 8/13/19 3:23 AM, Ivan Khoronzhuk wrote:
> That's needed to get __NR_mmap2 when mmap2 syscall is used.

It seems I did not have this issue on x64 machine e.g., Fedora 29.
My glibc version is 2.28. gcc 8.2.1.

What is your particular system glibc version?
So needing kernel asm/unistd.h is because of older glibc on your
system, or something else? Could you clarify?

> 
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> ---
>   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>
>   #include <arpa/inet.h>
>   #include <asm/barrier.h>
>   #include <linux/compiler.h>
> 

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

[Ivan Khoronzhuk]

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@linaro.org> wrote:
>>
>> That's needed to get __NR_mmap2 when mmap2 syscall is used.
>>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> ---
>>  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

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

I have more...

>
>>  #include <arpa/inet.h>
>>  #include <asm/barrier.h>
>>  #include <linux/compiler.h>
>> --
>> 2.17.1
>>

-- 
Regards,
Ivan Khoronzhuk

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

[Ivan Khoronzhuk]

On Wed, Aug 14, 2019 at 12:32:41AM +0000, Yonghong Song wrote:

Hi, Yonghong Song

>
>
>On 8/13/19 3:23 AM, Ivan Khoronzhuk wrote:
>> That's needed to get __NR_mmap2 when mmap2 syscall is used.
>
>It seems I did not have this issue on x64 machine e.g., Fedora 29.
>My glibc version is 2.28. gcc 8.2.1.

On 64 there is no the issue.

>
>What is your particular system glibc version?
>So needing kernel asm/unistd.h is because of older glibc on your
>system, or something else? Could you clarify?

It doesn't fix build issues, only runtime one on 32bits.

If no such inclusion -> no __NR_mmap2 definition - just mmap() is used ->
no problems on x64.

Is the inclusion -> no NR_mmap2 or is NR_mmap2 -> no problems on x64


-- 
Regards,
Ivan Khoronzhuk

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

[Ivan Khoronzhuk]

On Wed, Aug 14, 2019 at 12:24:05PM +0300, 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@linaro.org> wrote:
>>>
>>>That's needed to get __NR_mmap2 when mmap2 syscall is used.
>>>
>>>Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>>>---
>>> 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_*
>
>

There is one more radical solution for this I can send, but I'm not sure how it
can impact on other syscals/arches...

Looks like:


diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
index 9312066a1ae3..8b2f8ff7ce44 100644
--- a/tools/lib/bpf/Makefile
+++ b/tools/lib/bpf/Makefile
@@ -113,6 +113,7 @@ override CFLAGS += -Werror -Wall
 override CFLAGS += -fPIC
 override CFLAGS += $(INCLUDES)
 override CFLAGS += -fvisibility=hidden
+override CFLAGS += -D_FILE_OFFSET_BITS=64
 
 ifeq ($(VERBOSE),1)
   Q =
diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
index f2fc40f9804c..ff2d03b8380d 100644
--- a/tools/lib/bpf/xsk.c
+++ b/tools/lib/bpf/xsk.c
@@ -75,23 +75,6 @@ struct xsk_nl_info {
 	int fd;
 };
 
-/* For 32-bit systems, we need to use mmap2 as the offsets are 64-bit.
- * Unfortunately, it is not part of glibc.
- */
-static inline void *xsk_mmap(void *addr, size_t length, int prot, int flags,
-			     int fd, __u64 offset)
-{
-#ifdef __NR_mmap2
-	unsigned int page_shift = __builtin_ffs(getpagesize()) - 1;
-	long ret = syscall(__NR_mmap2, addr, length, prot, flags, fd,
-			   (off_t)(offset >> page_shift));
-
-	return (void *)ret;
-#else
-	return mmap(addr, length, prot, flags, fd, offset);
-#endif
-}
-
 int xsk_umem__fd(const struct xsk_umem *umem)
 {
 	return umem ? umem->fd : -EINVAL;
@@ -211,10 +194,9 @@ int xsk_umem__create(struct xsk_umem **umem_ptr, void *umem_area, __u64 size,
 		goto out_socket;
 	}
 
-	map = xsk_mmap(NULL, off.fr.desc +
-		       umem->config.fill_size * sizeof(__u64),
-		       PROT_READ | PROT_WRITE, MAP_SHARED | MAP_POPULATE,
-		       umem->fd, XDP_UMEM_PGOFF_FILL_RING);
+	map = mmap(NULL, off.fr.desc + umem->config.fill_size * sizeof(__u64),
+		   PROT_READ | PROT_WRITE, MAP_SHARED | MAP_POPULATE, umem->fd,
+		   XDP_UMEM_PGOFF_FILL_RING);
 	if (map == MAP_FAILED) {
 		err = -errno;
 		goto out_socket;
@@ -228,10 +210,9 @@ int xsk_umem__create(struct xsk_umem **umem_ptr, void *umem_area, __u64 size,
 	fill->ring = map + off.fr.desc;
 	fill->cached_cons = umem->config.fill_size;
 
-	map = xsk_mmap(NULL,
-		       off.cr.desc + umem->config.comp_size * sizeof(__u64),
-		       PROT_READ | PROT_WRITE, MAP_SHARED | MAP_POPULATE,
-		       umem->fd, XDP_UMEM_PGOFF_COMPLETION_RING);
+	map = mmap(NULL, off.cr.desc + umem->config.comp_size * sizeof(__u64),
+		   PROT_READ | PROT_WRITE, MAP_SHARED | MAP_POPULATE, umem->fd,
+		   XDP_UMEM_PGOFF_COMPLETION_RING);
 	if (map == MAP_FAILED) {
 		err = -errno;
 		goto out_mmap;
@@ -552,11 +533,10 @@ int xsk_socket__create(struct xsk_socket **xsk_ptr, const char *ifname,
 	}
 
 	if (rx) {
-		rx_map = xsk_mmap(NULL, off.rx.desc +
-				  xsk->config.rx_size * sizeof(struct xdp_desc),
-				  PROT_READ | PROT_WRITE,
-				  MAP_SHARED | MAP_POPULATE,
-				  xsk->fd, XDP_PGOFF_RX_RING);
+		rx_map = mmap(NULL, off.rx.desc +
+			      xsk->config.rx_size * sizeof(struct xdp_desc),
+			      PROT_READ | PROT_WRITE, MAP_SHARED | MAP_POPULATE,
+			      xsk->fd, XDP_PGOFF_RX_RING);
 		if (rx_map == MAP_FAILED) {
 			err = -errno;
 			goto out_socket;
@@ -571,11 +551,10 @@ int xsk_socket__create(struct xsk_socket **xsk_ptr, const char *ifname,
 	xsk->rx = rx;
 
 	if (tx) {
-		tx_map = xsk_mmap(NULL, off.tx.desc +
-				  xsk->config.tx_size * sizeof(struct xdp_desc),
-				  PROT_READ | PROT_WRITE,
-				  MAP_SHARED | MAP_POPULATE,
-				  xsk->fd, XDP_PGOFF_TX_RING);
+		tx_map = mmap(NULL, off.tx.desc +
+			      xsk->config.tx_size * sizeof(struct xdp_desc),
+			      PROT_READ | PROT_WRITE, MAP_SHARED | MAP_POPULATE,
+			      xsk->fd, XDP_PGOFF_TX_RING);
 		if (tx_map == MAP_FAILED) {
 			err = -errno;
 			goto out_mmap_rx;


If maintainers are ready to accept this I can send.
What do you say?

-- 
Regards,
Ivan Khoronzhuk

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

[Björn Töpel]

On Wed, 14 Aug 2019 at 13:57, Ivan Khoronzhuk
<ivan.khoronzhuk@linaro.org> wrote:
>
> On Wed, Aug 14, 2019 at 12:24:05PM +0300, 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@linaro.org> wrote:
> >>>
> >>>That's needed to get __NR_mmap2 when mmap2 syscall is used.
> >>>
> >>>Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> >>>---
> >>> 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_*
> >
> >
>
> There is one more radical solution for this I can send, but I'm not sure how it
> can impact on other syscals/arches...
>
> Looks like:
>
>
> diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
> index 9312066a1ae3..8b2f8ff7ce44 100644
> --- a/tools/lib/bpf/Makefile
> +++ b/tools/lib/bpf/Makefile
> @@ -113,6 +113,7 @@ override CFLAGS += -Werror -Wall
>  override CFLAGS += -fPIC
>  override CFLAGS += $(INCLUDES)
>  override CFLAGS += -fvisibility=hidden
> +override CFLAGS += -D_FILE_OFFSET_BITS=64
>

Hmm, isn't this glibc-ism? Does is it work for, say, musl or bionic?

If this is portable, and works on 32-, and 64-bit archs, I'm happy
with the patch. :-)


Björn

>  ifeq ($(VERBOSE),1)
>    Q =
> diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
> index f2fc40f9804c..ff2d03b8380d 100644
> --- a/tools/lib/bpf/xsk.c
> +++ b/tools/lib/bpf/xsk.c
> @@ -75,23 +75,6 @@ struct xsk_nl_info {
>         int fd;
>  };
>
> -/* For 32-bit systems, we need to use mmap2 as the offsets are 64-bit.
> - * Unfortunately, it is not part of glibc.
> - */
> -static inline void *xsk_mmap(void *addr, size_t length, int prot, int flags,
> -                            int fd, __u64 offset)
> -{
> -#ifdef __NR_mmap2
> -       unsigned int page_shift = __builtin_ffs(getpagesize()) - 1;
> -       long ret = syscall(__NR_mmap2, addr, length, prot, flags, fd,
> -                          (off_t)(offset >> page_shift));
> -
> -       return (void *)ret;
> -#else
> -       return mmap(addr, length, prot, flags, fd, offset);
> -#endif
> -}
> -
>  int xsk_umem__fd(const struct xsk_umem *umem)
>  {
>         return umem ? umem->fd : -EINVAL;
> @@ -211,10 +194,9 @@ int xsk_umem__create(struct xsk_umem **umem_ptr, void *umem_area, __u64 size,
>                 goto out_socket;
>         }
>
> -       map = xsk_mmap(NULL, off.fr.desc +
> -                      umem->config.fill_size * sizeof(__u64),
> -                      PROT_READ | PROT_WRITE, MAP_SHARED | MAP_POPULATE,
> -                      umem->fd, XDP_UMEM_PGOFF_FILL_RING);
> +       map = mmap(NULL, off.fr.desc + umem->config.fill_size * sizeof(__u64),
> +                  PROT_READ | PROT_WRITE, MAP_SHARED | MAP_POPULATE, umem->fd,
> +                  XDP_UMEM_PGOFF_FILL_RING);
>         if (map == MAP_FAILED) {
>                 err = -errno;
>                 goto out_socket;
> @@ -228,10 +210,9 @@ int xsk_umem__create(struct xsk_umem **umem_ptr, void *umem_area, __u64 size,
>         fill->ring = map + off.fr.desc;
>         fill->cached_cons = umem->config.fill_size;
>
> -       map = xsk_mmap(NULL,
> -                      off.cr.desc + umem->config.comp_size * sizeof(__u64),
> -                      PROT_READ | PROT_WRITE, MAP_SHARED | MAP_POPULATE,
> -                      umem->fd, XDP_UMEM_PGOFF_COMPLETION_RING);
> +       map = mmap(NULL, off.cr.desc + umem->config.comp_size * sizeof(__u64),
> +                  PROT_READ | PROT_WRITE, MAP_SHARED | MAP_POPULATE, umem->fd,
> +                  XDP_UMEM_PGOFF_COMPLETION_RING);
>         if (map == MAP_FAILED) {
>                 err = -errno;
>                 goto out_mmap;
> @@ -552,11 +533,10 @@ int xsk_socket__create(struct xsk_socket **xsk_ptr, const char *ifname,
>         }
>
>         if (rx) {
> -               rx_map = xsk_mmap(NULL, off.rx.desc +
> -                                 xsk->config.rx_size * sizeof(struct xdp_desc),
> -                                 PROT_READ | PROT_WRITE,
> -                                 MAP_SHARED | MAP_POPULATE,
> -                                 xsk->fd, XDP_PGOFF_RX_RING);
> +               rx_map = mmap(NULL, off.rx.desc +
> +                             xsk->config.rx_size * sizeof(struct xdp_desc),
> +                             PROT_READ | PROT_WRITE, MAP_SHARED | MAP_POPULATE,
> +                             xsk->fd, XDP_PGOFF_RX_RING);
>                 if (rx_map == MAP_FAILED) {
>                         err = -errno;
>                         goto out_socket;
> @@ -571,11 +551,10 @@ int xsk_socket__create(struct xsk_socket **xsk_ptr, const char *ifname,
>         xsk->rx = rx;
>
>         if (tx) {
> -               tx_map = xsk_mmap(NULL, off.tx.desc +
> -                                 xsk->config.tx_size * sizeof(struct xdp_desc),
> -                                 PROT_READ | PROT_WRITE,
> -                                 MAP_SHARED | MAP_POPULATE,
> -                                 xsk->fd, XDP_PGOFF_TX_RING);
> +               tx_map = mmap(NULL, off.tx.desc +
> +                             xsk->config.tx_size * sizeof(struct xdp_desc),
> +                             PROT_READ | PROT_WRITE, MAP_SHARED | MAP_POPULATE,
> +                             xsk->fd, XDP_PGOFF_TX_RING);
>                 if (tx_map == MAP_FAILED) {
>                         err = -errno;
>                         goto out_mmap_rx;
>
>
> If maintainers are ready to accept this I can send.
> What do you say?
>
> --
> Regards,
> Ivan Khoronzhuk

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

[Yonghong Song]

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@linaro.org> wrote:
>>>
>>> That's needed to get __NR_mmap2 when mmap2 syscall is used.
>>>
>>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>>> ---
>>>  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
>>>
> 

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

[Yonghong Song]

On 8/14/19 6:32 AM, Björn Töpel wrote:
> On Wed, 14 Aug 2019 at 13:57, Ivan Khoronzhuk
> <ivan.khoronzhuk@linaro.org> wrote:
>>
>> On Wed, Aug 14, 2019 at 12:24:05PM +0300, 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@linaro.org> wrote:
>>>>>
>>>>> That's needed to get __NR_mmap2 when mmap2 syscall is used.
>>>>>
>>>>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>>>>> ---
>>>>> 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_*
>>>
>>>
>>
>> There is one more radical solution for this I can send, but I'm not sure how it
>> can impact on other syscals/arches...
>>
>> Looks like:
>>
>>
>> diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
>> index 9312066a1ae3..8b2f8ff7ce44 100644
>> --- a/tools/lib/bpf/Makefile
>> +++ b/tools/lib/bpf/Makefile
>> @@ -113,6 +113,7 @@ override CFLAGS += -Werror -Wall
>>   override CFLAGS += -fPIC
>>   override CFLAGS += $(INCLUDES)
>>   override CFLAGS += -fvisibility=hidden
>> +override CFLAGS += -D_FILE_OFFSET_BITS=64
>>
> 
> Hmm, isn't this glibc-ism? Does is it work for, say, musl or bionic?
> 
> If this is portable, and works on 32-, and 64-bit archs, I'm happy
> with the patch. :-)

Second here. Looks defining -D_FILE_OFFSET_BITS=64 is a well known
fix for 32bit system to deal with files > 2GB.
I remembered I used it in distant past. The below link
also explains the case.
https://digital-domain.net/largefiles.html

Testing on musl is necessary as Arnaldo's perf test suite
indeed tested it. Probably bionic too, not really familiar with that.

> 
> 
> Björn
> 
>>   ifeq ($(VERBOSE),1)
>>     Q =
>> diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
>> index f2fc40f9804c..ff2d03b8380d 100644
>> --- a/tools/lib/bpf/xsk.c
>> +++ b/tools/lib/bpf/xsk.c
>> @@ -75,23 +75,6 @@ struct xsk_nl_info {
>>          int fd;
>>   };
>>
>> -/* For 32-bit systems, we need to use mmap2 as the offsets are 64-bit.
>> - * Unfortunately, it is not part of glibc.
>> - */
>> -static inline void *xsk_mmap(void *addr, size_t length, int prot, int flags,
>> -                            int fd, __u64 offset)
>> -{
>> -#ifdef __NR_mmap2
>> -       unsigned int page_shift = __builtin_ffs(getpagesize()) - 1;
>> -       long ret = syscall(__NR_mmap2, addr, length, prot, flags, fd,
>> -                          (off_t)(offset >> page_shift));
>> -
>> -       return (void *)ret;
>> -#else
>> -       return mmap(addr, length, prot, flags, fd, offset);
>> -#endif
>> -}
>> -
>>   int xsk_umem__fd(const struct xsk_umem *umem)
>>   {
>>          return umem ? umem->fd : -EINVAL;
>> @@ -211,10 +194,9 @@ int xsk_umem__create(struct xsk_umem **umem_ptr, void *umem_area, __u64 size,
>>                  goto out_socket;
>>          }
>>
>> -       map = xsk_mmap(NULL, off.fr.desc +
>> -                      umem->config.fill_size * sizeof(__u64),
>> -                      PROT_READ | PROT_WRITE, MAP_SHARED | MAP_POPULATE,
>> -                      umem->fd, XDP_UMEM_PGOFF_FILL_RING);
>> +       map = mmap(NULL, off.fr.desc + umem->config.fill_size * sizeof(__u64),
>> +                  PROT_READ | PROT_WRITE, MAP_SHARED | MAP_POPULATE, umem->fd,
>> +                  XDP_UMEM_PGOFF_FILL_RING);
>>          if (map == MAP_FAILED) {
>>                  err = -errno;
>>                  goto out_socket;
>> @@ -228,10 +210,9 @@ int xsk_umem__create(struct xsk_umem **umem_ptr, void *umem_area, __u64 size,
>>          fill->ring = map + off.fr.desc;
>>          fill->cached_cons = umem->config.fill_size;
>>
>> -       map = xsk_mmap(NULL,
>> -                      off.cr.desc + umem->config.comp_size * sizeof(__u64),
>> -                      PROT_READ | PROT_WRITE, MAP_SHARED | MAP_POPULATE,
>> -                      umem->fd, XDP_UMEM_PGOFF_COMPLETION_RING);
>> +       map = mmap(NULL, off.cr.desc + umem->config.comp_size * sizeof(__u64),
>> +                  PROT_READ | PROT_WRITE, MAP_SHARED | MAP_POPULATE, umem->fd,
>> +                  XDP_UMEM_PGOFF_COMPLETION_RING);
>>          if (map == MAP_FAILED) {
>>                  err = -errno;
>>                  goto out_mmap;
>> @@ -552,11 +533,10 @@ int xsk_socket__create(struct xsk_socket **xsk_ptr, const char *ifname,
>>          }
>>
>>          if (rx) {
>> -               rx_map = xsk_mmap(NULL, off.rx.desc +
>> -                                 xsk->config.rx_size * sizeof(struct xdp_desc),
>> -                                 PROT_READ | PROT_WRITE,
>> -                                 MAP_SHARED | MAP_POPULATE,
>> -                                 xsk->fd, XDP_PGOFF_RX_RING);
>> +               rx_map = mmap(NULL, off.rx.desc +
>> +                             xsk->config.rx_size * sizeof(struct xdp_desc),
>> +                             PROT_READ | PROT_WRITE, MAP_SHARED | MAP_POPULATE,
>> +                             xsk->fd, XDP_PGOFF_RX_RING);
>>                  if (rx_map == MAP_FAILED) {
>>                          err = -errno;
>>                          goto out_socket;
>> @@ -571,11 +551,10 @@ int xsk_socket__create(struct xsk_socket **xsk_ptr, const char *ifname,
>>          xsk->rx = rx;
>>
>>          if (tx) {
>> -               tx_map = xsk_mmap(NULL, off.tx.desc +
>> -                                 xsk->config.tx_size * sizeof(struct xdp_desc),
>> -                                 PROT_READ | PROT_WRITE,
>> -                                 MAP_SHARED | MAP_POPULATE,
>> -                                 xsk->fd, XDP_PGOFF_TX_RING);
>> +               tx_map = mmap(NULL, off.tx.desc +
>> +                             xsk->config.tx_size * sizeof(struct xdp_desc),
>> +                             PROT_READ | PROT_WRITE, MAP_SHARED | MAP_POPULATE,
>> +                             xsk->fd, XDP_PGOFF_TX_RING);
>>                  if (tx_map == MAP_FAILED) {
>>                          err = -errno;
>>                          goto out_mmap_rx;
>>
>>
>> If maintainers are ready to accept this I can send.
>> What do you say?
>>
>> --
>> Regards,
>> Ivan Khoronzhuk

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

[Ivan Khoronzhuk]

On Wed, Aug 14, 2019 at 03:32:24PM +0200, Björn Töpel wrote:
>On Wed, 14 Aug 2019 at 13:57, Ivan Khoronzhuk
><ivan.khoronzhuk@linaro.org> wrote:
>>
>> On Wed, Aug 14, 2019 at 12:24:05PM +0300, 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@linaro.org> wrote:
>> >>>
>> >>>That's needed to get __NR_mmap2 when mmap2 syscall is used.
>> >>>
>> >>>Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> >>>---
>> >>> 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_*
>> >
>> >
>>
>> There is one more radical solution for this I can send, but I'm not sure how it
>> can impact on other syscals/arches...
>>
>> Looks like:
>>
>>
>> diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
>> index 9312066a1ae3..8b2f8ff7ce44 100644
>> --- a/tools/lib/bpf/Makefile
>> +++ b/tools/lib/bpf/Makefile
>> @@ -113,6 +113,7 @@ override CFLAGS += -Werror -Wall
>>  override CFLAGS += -fPIC
>>  override CFLAGS += $(INCLUDES)
>>  override CFLAGS += -fvisibility=hidden
>> +override CFLAGS += -D_FILE_OFFSET_BITS=64
>>
>
>Hmm, isn't this glibc-ism? Does is it work for, say, musl or bionic?
>
>If this is portable, and works on 32-, and 64-bit archs, I'm happy
>with the patch. :-)

https://users.suse.com/~aj/linux_lfs.html

BIONIС
======
https://android.googlesource.com/platform/bionic
So, LFS is in bionic since Fri Feb 6 22:28:49 2015
68dc20d41193831a94df04b994ff2f601dd38d10
Author: Elliott Hughes <enh@google.com>
Implement _FILE_OFFSET_BITS (mostly)


MUSL
====
I took here: git@github.com:kraj/musl.git
With musl situation is a little different, seems like, it provides
64bit off_t by default


#if defined(_LARGEFILE64_SOURCE) || defined(_GNU_SOURCE)
#define lseek64 lseek
#define pread64 pread
#define pwrite64 pwrite
#define truncate64 truncate
#define ftruncate64 ftruncate
#define lockf64 lockf
#define off64_t off_t
#endif

and

/* If _GNU_SOURCE was defined by the user, turn on all the other features.  */
#ifdef _GNU_SOURCE
# undef  _ISOC95_SOURCE
# define _ISOC95_SOURCE	1
# undef  _ISOC99_SOURCE
# define _ISOC99_SOURCE	1
# undef  _ISOC11_SOURCE
# define _ISOC11_SOURCE	1
# undef  _POSIX_SOURCE
# define _POSIX_SOURCE	1
# undef  _POSIX_C_SOURCE
# define _POSIX_C_SOURCE	200809L
# undef  _XOPEN_SOURCE
# define _XOPEN_SOURCE	700
# undef  _XOPEN_SOURCE_EXTENDED
# define _XOPEN_SOURCE_EXTENDED	1
# undef	 _LARGEFILE64_SOURCE
# define _LARGEFILE64_SOURCE	1
# undef  _DEFAULT_SOURCE
# define _DEFAULT_SOURCE	1
# undef  _ATFILE_SOURCE
# define _ATFILE_SOURCE	1
#endif

So shouldn't be issuse.

64 ARCHES
=========
Should also work, if grep on _FILE_OFFSET_BITS tool:

./lib/api/Makefile:CFLAGS += -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64
./lib/subcmd/Makefile:CFLAGS += -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE

So, it's used already and no problems.
But here one moment, _LARGEFILE64_SOURCE is also defined, probably for MUSL 
(despite it's defined anyway) just to be sure.

So, in Makefile, for sure, will be:

override CFLAGS += -D_FILE_OFFSET_BITS=64
verride CFLAGS += -D_LARGEFILE64_SOURCE

-- 
Regards,
Ivan Khoronzhuk

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

[Andrii Nakryiko]

On Wed, Aug 14, 2019 at 2:24 AM Ivan Khoronzhuk
<ivan.khoronzhuk@linaro.org> 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@linaro.org> wrote:
> >>
> >> That's needed to get __NR_mmap2 when mmap2 syscall is used.
> >>
> >> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> >> ---
> >>  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
>

Yeah, sorry for the noise. I missed that this is system header that's
expected to be present, not internal kernel header, parts of which we
need to re-implement for Github projection. Never mind my concerns.


> 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
>
> or this one related for arm32 only:
> https://github.com/ikhorn/af_xdp_stuff/commit/2c6c6d538605aac39600dcb3c9b66de11c70b963
>
> I have more...
>
> >
> >>  #include <arpa/inet.h>
> >>  #include <asm/barrier.h>
> >>  #include <linux/compiler.h>
> >> --
> >> 2.17.1
> >>
>
> --
> Regards,
> Ivan Khoronzhuk