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@xxxxxxxxxx>
>
> 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