[PATCH bpf-next 00/11] bpf: random unpopular userspace fixes (32 bit et al.)

[PATCH bpf-next 00/11] bpf: random unpopular userspace fixes (32 bit et al.)

[Alexander Lobakin]

This mostly issues the cross build (1) errors for 32 bit (2)
MIPS (3) with minimal configuration (4) on Musl (5). The majority
of them aren't yesterday's, so it is a "who does need it outside
of x86_64 or ARM64?" moment again.
Trivial stuff in general, not counting the bpf_cookie build fix.

Alexander Lobakin (11):
  bpf, perf: fix bpftool compilation with !CONFIG_PERF_EVENTS
  bpf: always emit struct bpf_perf_link BTF
  tools, bpf: fix bpftool build with !CONFIG_BPF_EVENTS
  samples: bpf: add 'asm/mach-generic' include path for every MIPS
  samples: bpf: use host bpftool to generate vmlinux.h, not target
  tools, bpf: fix fcntl.h include in bpftool
  samples: bpf: fix uin64_t format literals
  samples: bpf: fix shifting unsigned long by 32 positions
  samples: bpf: fix include order for non-Glibc environments
  samples: bpf: fix -Wsequence-point
  samples: bpf: xdpsock: fix -Wmaybe-uninitialized

 include/linux/perf_event.h              |  2 ++
 kernel/bpf/syscall.c                    |  4 +++-
 samples/bpf/Makefile                    |  7 ++++---
 samples/bpf/cookie_uid_helper_example.c | 12 ++++++------
 samples/bpf/lathist_kern.c              |  2 +-
 samples/bpf/lwt_len_hist_kern.c         |  2 +-
 samples/bpf/lwt_len_hist_user.c         |  4 ++--
 samples/bpf/task_fd_query_user.c        |  2 +-
 samples/bpf/test_lru_dist.c             |  3 ++-
 samples/bpf/tracex2_kern.c              |  2 +-
 samples/bpf/xdpsock_user.c              |  5 +++--
 tools/bpf/bpftool/tracelog.c            |  2 +-
 12 files changed, 27 insertions(+), 20 deletions(-)

--
2.35.2

[PATCH bpf-next 01/11] bpf, perf: fix bpftool compilation with !CONFIG_PERF_EVENTS

[Alexander Lobakin]

When CONFIG_PERF_EVENTS is not set, struct perf_event remains empty.
However, the structure is being used by bpftool indirectly via BTF.
This leads to:

skeleton/pid_iter.bpf.c:49:30: error: no member named 'bpf_cookie' in 'struct perf_event'
        return BPF_CORE_READ(event, bpf_cookie);
               ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~

...

skeleton/pid_iter.bpf.c:49:9: error: returning 'void' from a function with incompatible result type '__u64' (aka 'unsigned long long')
        return BPF_CORE_READ(event, bpf_cookie);
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Tools and samples can't use any CONFIG_ definitions, so the fields
used there should always be present.
Move CONFIG_BPF_SYSCALL block out of the CONFIG_PERF_EVENTS block
to make it available unconditionally.

Fixes: cbdaf71f7e65 ("bpftool: Add bpf_cookie to link output")
Signed-off-by: Alexander Lobakin <alobakin@pm.me>
---
 include/linux/perf_event.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index af97dd427501..b1d5715b8b34 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -762,12 +762,14 @@ struct perf_event {
 	u64				(*clock)(void);
 	perf_overflow_handler_t		overflow_handler;
 	void				*overflow_handler_context;
+#endif /* CONFIG_PERF_EVENTS */
 #ifdef CONFIG_BPF_SYSCALL
 	perf_overflow_handler_t		orig_overflow_handler;
 	struct bpf_prog			*prog;
 	u64				bpf_cookie;
 #endif

+#ifdef CONFIG_PERF_EVENTS
 #ifdef CONFIG_EVENT_TRACING
 	struct trace_event_call		*tp_event;
 	struct event_filter		*filter;
--
2.35.2

Re: [PATCH bpf-next 01/11] bpf, perf: fix bpftool compilation with !CONFIG_PERF_EVENTS

[Song Liu]

On Thu, Apr 14, 2022 at 3:45 PM Alexander Lobakin <alobakin@pm.me> wrote:
>
> When CONFIG_PERF_EVENTS is not set, struct perf_event remains empty.
> However, the structure is being used by bpftool indirectly via BTF.
> This leads to:
>
> skeleton/pid_iter.bpf.c:49:30: error: no member named 'bpf_cookie' in 'struct perf_event'
>         return BPF_CORE_READ(event, bpf_cookie);
>                ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
>
> ...
>
> skeleton/pid_iter.bpf.c:49:9: error: returning 'void' from a function with incompatible result type '__u64' (aka 'unsigned long long')
>         return BPF_CORE_READ(event, bpf_cookie);
>                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Tools and samples can't use any CONFIG_ definitions, so the fields
> used there should always be present.
> Move CONFIG_BPF_SYSCALL block out of the CONFIG_PERF_EVENTS block
> to make it available unconditionally.
>
> Fixes: cbdaf71f7e65 ("bpftool: Add bpf_cookie to link output")
> Signed-off-by: Alexander Lobakin <alobakin@pm.me>

While I can't think of a real failure with this approach, it does feel
weird to me. Can we fix this with bpf_core_field_exists()?

Thanks,
Song


> ---
>  include/linux/perf_event.h | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index af97dd427501..b1d5715b8b34 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -762,12 +762,14 @@ struct perf_event {
>         u64                             (*clock)(void);
>         perf_overflow_handler_t         overflow_handler;
>         void                            *overflow_handler_context;
> +#endif /* CONFIG_PERF_EVENTS */
>  #ifdef CONFIG_BPF_SYSCALL
>         perf_overflow_handler_t         orig_overflow_handler;
>         struct bpf_prog                 *prog;
>         u64                             bpf_cookie;
>  #endif
>
> +#ifdef CONFIG_PERF_EVENTS
>  #ifdef CONFIG_EVENT_TRACING
>         struct trace_event_call         *tp_event;
>         struct event_filter             *filter;
> --
> 2.35.2
>
>

Re: [PATCH bpf-next 01/11] bpf, perf: fix bpftool compilation with !CONFIG_PERF_EVENTS

[Song Liu]

On Fri, Apr 15, 2022 at 4:07 PM Song Liu <song@kernel.org> wrote:
>
> On Thu, Apr 14, 2022 at 3:45 PM Alexander Lobakin <alobakin@pm.me> wrote:
> >
> > When CONFIG_PERF_EVENTS is not set, struct perf_event remains empty.
> > However, the structure is being used by bpftool indirectly via BTF.
> > This leads to:
> >
> > skeleton/pid_iter.bpf.c:49:30: error: no member named 'bpf_cookie' in 'struct perf_event'
> >         return BPF_CORE_READ(event, bpf_cookie);
> >                ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
> >
> > ...
> >
> > skeleton/pid_iter.bpf.c:49:9: error: returning 'void' from a function with incompatible result type '__u64' (aka 'unsigned long long')
> >         return BPF_CORE_READ(event, bpf_cookie);
> >                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > Tools and samples can't use any CONFIG_ definitions, so the fields
> > used there should always be present.
> > Move CONFIG_BPF_SYSCALL block out of the CONFIG_PERF_EVENTS block
> > to make it available unconditionally.
> >
> > Fixes: cbdaf71f7e65 ("bpftool: Add bpf_cookie to link output")
> > Signed-off-by: Alexander Lobakin <alobakin@pm.me>
>
> While I can't think of a real failure with this approach, it does feel
> weird to me. Can we fix this with bpf_core_field_exists()?

Hmm.. the error happens at compile time, so I guess it is not very easy.

Andrii,
Do you have some recommendation on this?

Song

Re: [PATCH bpf-next 01/11] bpf, perf: fix bpftool compilation with !CONFIG_PERF_EVENTS

[Peter Zijlstra]

On Thu, Apr 14, 2022 at 10:44:48PM +0000, Alexander Lobakin wrote:
> When CONFIG_PERF_EVENTS is not set, struct perf_event remains empty.
> However, the structure is being used by bpftool indirectly via BTF.
> This leads to:
> 
> skeleton/pid_iter.bpf.c:49:30: error: no member named 'bpf_cookie' in 'struct perf_event'
>         return BPF_CORE_READ(event, bpf_cookie);
>                ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
> 
> ...
> 
> skeleton/pid_iter.bpf.c:49:9: error: returning 'void' from a function with incompatible result type '__u64' (aka 'unsigned long long')
>         return BPF_CORE_READ(event, bpf_cookie);
>                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Tools and samples can't use any CONFIG_ definitions, so the fields
> used there should always be present.
> Move CONFIG_BPF_SYSCALL block out of the CONFIG_PERF_EVENTS block
> to make it available unconditionally.

Urgh, this is nasty.. did you verify nothing relies on that structure
actually being empty?

Also, why are we changing kernel headers to fix some daft userspace
issue?

> Fixes: cbdaf71f7e65 ("bpftool: Add bpf_cookie to link output")
> Signed-off-by: Alexander Lobakin <alobakin@pm.me>
> ---
>  include/linux/perf_event.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index af97dd427501..b1d5715b8b34 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -762,12 +762,14 @@ struct perf_event {
>  	u64				(*clock)(void);
>  	perf_overflow_handler_t		overflow_handler;
>  	void				*overflow_handler_context;
> +#endif /* CONFIG_PERF_EVENTS */
>  #ifdef CONFIG_BPF_SYSCALL
>  	perf_overflow_handler_t		orig_overflow_handler;
>  	struct bpf_prog			*prog;
>  	u64				bpf_cookie;
>  #endif
> 
> +#ifdef CONFIG_PERF_EVENTS
>  #ifdef CONFIG_EVENT_TRACING
>  	struct trace_event_call		*tp_event;
>  	struct event_filter		*filter;
> --
> 2.35.2
> 
> 

Re: [PATCH bpf-next 01/11] bpf, perf: fix bpftool compilation with !CONFIG_PERF_EVENTS

[Andrii Nakryiko]

On Tue, Apr 19, 2022 at 2:04 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Thu, Apr 14, 2022 at 10:44:48PM +0000, Alexander Lobakin wrote:
> > When CONFIG_PERF_EVENTS is not set, struct perf_event remains empty.
> > However, the structure is being used by bpftool indirectly via BTF.
> > This leads to:
> >
> > skeleton/pid_iter.bpf.c:49:30: error: no member named 'bpf_cookie' in 'struct perf_event'
> >         return BPF_CORE_READ(event, bpf_cookie);
> >                ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
> >
> > ...
> >
> > skeleton/pid_iter.bpf.c:49:9: error: returning 'void' from a function with incompatible result type '__u64' (aka 'unsigned long long')
> >         return BPF_CORE_READ(event, bpf_cookie);
> >                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > Tools and samples can't use any CONFIG_ definitions, so the fields
> > used there should always be present.
> > Move CONFIG_BPF_SYSCALL block out of the CONFIG_PERF_EVENTS block
> > to make it available unconditionally.
>
> Urgh, this is nasty.. did you verify nothing relies on that structure
> actually being empty?
>
> Also, why are we changing kernel headers to fix some daft userspace
> issue?
>

I agree, this is quite ugly. And I think it's not necessary at all.

BPF CO-RE, which bpftool relies on here, allows to have bpftool's own
minimal definition of struct perf_event with bpf_cookie field and not
rely on UAPI headers having full definition. Something like this:

struct perf_event___local {
    u64 bpf_cookie;
} __attribute__((preserve_access_index));

Then use `struct perf_event___local` (note the three underscores, they
are important) instead of struct perf_event in BPF code.

And we'll have to do the same for struct bpf_perf_link, I presume?

> > Fixes: cbdaf71f7e65 ("bpftool: Add bpf_cookie to link output")
> > Signed-off-by: Alexander Lobakin <alobakin@pm.me>
> > ---
> >  include/linux/perf_event.h | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> > index af97dd427501..b1d5715b8b34 100644
> > --- a/include/linux/perf_event.h
> > +++ b/include/linux/perf_event.h
> > @@ -762,12 +762,14 @@ struct perf_event {
> >       u64                             (*clock)(void);
> >       perf_overflow_handler_t         overflow_handler;
> >       void                            *overflow_handler_context;
> > +#endif /* CONFIG_PERF_EVENTS */
> >  #ifdef CONFIG_BPF_SYSCALL
> >       perf_overflow_handler_t         orig_overflow_handler;
> >       struct bpf_prog                 *prog;
> >       u64                             bpf_cookie;
> >  #endif
> >
> > +#ifdef CONFIG_PERF_EVENTS
> >  #ifdef CONFIG_EVENT_TRACING
> >       struct trace_event_call         *tp_event;
> >       struct event_filter             *filter;
> > --
> > 2.35.2
> >
> >

[PATCH bpf-next 02/11] bpf: always emit struct bpf_perf_link BTF

[Alexander Lobakin]

When building bpftool with !CONFIG_PERF_EVENTS:

skeleton/pid_iter.bpf.c:47:14: error: incomplete definition of type 'struct bpf_perf_link'
        perf_link = container_of(link, struct bpf_perf_link, link);
                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/bpf/bpftool/bootstrap/libbpf/include/bpf/bpf_helpers.h:74:22: note: expanded from macro 'container_of'
                ((type *)(__mptr - offsetof(type, member)));    \
                                   ^~~~~~~~~~~~~~~~~~~~~~
tools/bpf/bpftool/bootstrap/libbpf/include/bpf/bpf_helpers.h:68:60: note: expanded from macro 'offsetof'
 #define offsetof(TYPE, MEMBER)  ((unsigned long)&((TYPE *)0)->MEMBER)
                                                  ~~~~~~~~~~~^
skeleton/pid_iter.bpf.c:44:9: note: forward declaration of 'struct bpf_perf_link'
        struct bpf_perf_link *perf_link;
               ^

&bpf_perf_link is being defined and used only under the ifdef.
Move it out of the block and explicitly emit a BTF to fix
compilation.

Fixes: cbdaf71f7e65 ("bpftool: Add bpf_cookie to link output")
Signed-off-by: Alexander Lobakin <alobakin@pm.me>
---
 kernel/bpf/syscall.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index e9621cfa09f2..34fdf27d14cf 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -2952,12 +2952,12 @@ static const struct bpf_link_ops bpf_raw_tp_link_lops = {
 	.fill_link_info = bpf_raw_tp_link_fill_link_info,
 };

-#ifdef CONFIG_PERF_EVENTS
 struct bpf_perf_link {
 	struct bpf_link link;
 	struct file *perf_file;
 };

+#ifdef CONFIG_PERF_EVENTS
 static void bpf_perf_link_release(struct bpf_link *link)
 {
 	struct bpf_perf_link *perf_link = container_of(link, struct bpf_perf_link, link);
@@ -4333,6 +4333,7 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr)
 #endif
 	case BPF_PROG_TYPE_PERF_EVENT:
 	case BPF_PROG_TYPE_TRACEPOINT:
+		BTF_TYPE_EMIT(struct bpf_perf_link);
 		ret = bpf_perf_link_attach(attr, prog);
 		break;
 	case BPF_PROG_TYPE_KPROBE:
--
2.35.2

Re: [PATCH bpf-next 02/11] bpf: always emit struct bpf_perf_link BTF

[Song Liu]

On Thu, Apr 14, 2022 at 3:45 PM Alexander Lobakin <alobakin@pm.me> wrote:
>
> When building bpftool with !CONFIG_PERF_EVENTS:
>
> skeleton/pid_iter.bpf.c:47:14: error: incomplete definition of type 'struct bpf_perf_link'
>         perf_link = container_of(link, struct bpf_perf_link, link);
>                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> tools/bpf/bpftool/bootstrap/libbpf/include/bpf/bpf_helpers.h:74:22: note: expanded from macro 'container_of'
>                 ((type *)(__mptr - offsetof(type, member)));    \
>                                    ^~~~~~~~~~~~~~~~~~~~~~
> tools/bpf/bpftool/bootstrap/libbpf/include/bpf/bpf_helpers.h:68:60: note: expanded from macro 'offsetof'
>  #define offsetof(TYPE, MEMBER)  ((unsigned long)&((TYPE *)0)->MEMBER)
>                                                   ~~~~~~~~~~~^
> skeleton/pid_iter.bpf.c:44:9: note: forward declaration of 'struct bpf_perf_link'
>         struct bpf_perf_link *perf_link;
>                ^
>
> &bpf_perf_link is being defined and used only under the ifdef.
> Move it out of the block and explicitly emit a BTF to fix
> compilation.
>
> Fixes: cbdaf71f7e65 ("bpftool: Add bpf_cookie to link output")
> Signed-off-by: Alexander Lobakin <alobakin@pm.me>

Similar to v1, this fix is weird to me. I hope we have can fix it in user
space.

Re: [PATCH bpf-next 02/11] bpf: always emit struct bpf_perf_link BTF

[Alexander Lobakin]

From: Song Liu <song@kernel.org>
Date: Fri, 15 Apr 2022 16:24:41 -0700

> On Thu, Apr 14, 2022 at 3:45 PM Alexander Lobakin <alobakin@pm.me> wrote:
> >
> > When building bpftool with !CONFIG_PERF_EVENTS:
> >
> > skeleton/pid_iter.bpf.c:47:14: error: incomplete definition of type 'struct bpf_perf_link'
> >         perf_link = container_of(link, struct bpf_perf_link, link);
> >                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > tools/bpf/bpftool/bootstrap/libbpf/include/bpf/bpf_helpers.h:74:22: note: expanded from macro 'container_of'
> >                 ((type *)(__mptr - offsetof(type, member)));    \
> >                                    ^~~~~~~~~~~~~~~~~~~~~~
> > tools/bpf/bpftool/bootstrap/libbpf/include/bpf/bpf_helpers.h:68:60: note: expanded from macro 'offsetof'
> >  #define offsetof(TYPE, MEMBER)  ((unsigned long)&((TYPE *)0)->MEMBER)
> >                                                   ~~~~~~~~~~~^
> > skeleton/pid_iter.bpf.c:44:9: note: forward declaration of 'struct bpf_perf_link'
> >         struct bpf_perf_link *perf_link;
> >                ^
> >
> > &bpf_perf_link is being defined and used only under the ifdef.
> > Move it out of the block and explicitly emit a BTF to fix
> > compilation.
> >
> > Fixes: cbdaf71f7e65 ("bpftool: Add bpf_cookie to link output")
> > Signed-off-by: Alexander Lobakin <alobakin@pm.me>
>
> Similar to v1, this fix is weird to me. I hope we have can fix it in user
> space.

I've been thinking on this, but userspace is not provided with any
autoconf.h definitions (only selftests have them), so its code must
be sort of universal.
Both this and 01/11 are compile time and due to imcomplete and/or
absent BTF struct declarations. I'm not familiar with
bpf_core_field_exists(), and it might be that it's able to solve
01/11, but not this one. &bpf_perf_link must be present
unconditionally, otherwise it won't be defined in the generated
vmlinux.h at all.

Thanks,
Al

[PATCH bpf-next 03/11] tools, bpf: fix bpftool build with !CONFIG_BPF_EVENTS

[Alexander Lobakin]

Fix the following error when building bpftool:

  CLANG   profiler.bpf.o
  CLANG   pid_iter.bpf.o
skeleton/profiler.bpf.c:18:21: error: invalid application of 'sizeof' to an incomplete type 'struct bpf_perf_event_value'
        __uint(value_size, sizeof(struct bpf_perf_event_value));
                           ^     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/bpf/bpftool/bootstrap/libbpf/include/bpf/bpf_helpers.h:13:39: note: expanded from macro '__uint'
                                      ^~~
tools/bpf/bpftool/bootstrap/libbpf/include/bpf/bpf_helper_defs.h:7:8: note: forward declaration of 'struct bpf_perf_event_value'
struct bpf_perf_event_value;
       ^

struct bpf_perf_event_value is being used in the kernel only when
CONFIG_BPF_EVENTS is enabled, so it misses a BTF entry then.
Emit the type unconditionally to fix the problem.

Fixes: 47c09d6a9f67 ("bpftool: Introduce "prog profile" command")
Signed-off-by: Alexander Lobakin <alobakin@pm.me>
---
 kernel/bpf/syscall.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 34fdf27d14cf..dd8284a60a8e 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -4286,6 +4286,7 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr)
 		goto out;
 	case BPF_PROG_TYPE_PERF_EVENT:
 	case BPF_PROG_TYPE_TRACEPOINT:
+		BTF_TYPE_EMIT(struct bpf_perf_event_value);
 		if (attr->link_create.attach_type != BPF_PERF_EVENT) {
 			ret = -EINVAL;
 			goto out;
--
2.35.2

Re: [PATCH bpf-next 03/11] tools, bpf: fix bpftool build with !CONFIG_BPF_EVENTS

[Song Liu]

On Thu, Apr 14, 2022 at 3:45 PM Alexander Lobakin <alobakin@pm.me> wrote:
>
> Fix the following error when building bpftool:
>
>   CLANG   profiler.bpf.o
>   CLANG   pid_iter.bpf.o
> skeleton/profiler.bpf.c:18:21: error: invalid application of 'sizeof' to an incomplete type 'struct bpf_perf_event_value'
>         __uint(value_size, sizeof(struct bpf_perf_event_value));
>                            ^     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> tools/bpf/bpftool/bootstrap/libbpf/include/bpf/bpf_helpers.h:13:39: note: expanded from macro '__uint'
>                                       ^~~
> tools/bpf/bpftool/bootstrap/libbpf/include/bpf/bpf_helper_defs.h:7:8: note: forward declaration of 'struct bpf_perf_event_value'
> struct bpf_perf_event_value;
>        ^
>
> struct bpf_perf_event_value is being used in the kernel only when
> CONFIG_BPF_EVENTS is enabled, so it misses a BTF entry then.
> Emit the type unconditionally to fix the problem.
>
> Fixes: 47c09d6a9f67 ("bpftool: Introduce "prog profile" command")
> Signed-off-by: Alexander Lobakin <alobakin@pm.me>

Acked-by: Song Liu <songliubraving@fb.com>

> ---
>  kernel/bpf/syscall.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 34fdf27d14cf..dd8284a60a8e 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -4286,6 +4286,7 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr)
>                 goto out;
>         case BPF_PROG_TYPE_PERF_EVENT:
>         case BPF_PROG_TYPE_TRACEPOINT:
> +               BTF_TYPE_EMIT(struct bpf_perf_event_value);
>                 if (attr->link_create.attach_type != BPF_PERF_EVENT) {
>                         ret = -EINVAL;
>                         goto out;
> --
> 2.35.2
>
>

Re: [PATCH bpf-next 03/11] tools, bpf: fix bpftool build with !CONFIG_BPF_EVENTS

[Andrii Nakryiko]

On Thu, Apr 14, 2022 at 3:45 PM Alexander Lobakin <alobakin@pm.me> wrote:
>
> Fix the following error when building bpftool:
>
>   CLANG   profiler.bpf.o
>   CLANG   pid_iter.bpf.o
> skeleton/profiler.bpf.c:18:21: error: invalid application of 'sizeof' to an incomplete type 'struct bpf_perf_event_value'
>         __uint(value_size, sizeof(struct bpf_perf_event_value));
>                            ^     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> tools/bpf/bpftool/bootstrap/libbpf/include/bpf/bpf_helpers.h:13:39: note: expanded from macro '__uint'
>                                       ^~~
> tools/bpf/bpftool/bootstrap/libbpf/include/bpf/bpf_helper_defs.h:7:8: note: forward declaration of 'struct bpf_perf_event_value'
> struct bpf_perf_event_value;
>        ^
>
> struct bpf_perf_event_value is being used in the kernel only when
> CONFIG_BPF_EVENTS is enabled, so it misses a BTF entry then.
> Emit the type unconditionally to fix the problem.
>
> Fixes: 47c09d6a9f67 ("bpftool: Introduce "prog profile" command")
> Signed-off-by: Alexander Lobakin <alobakin@pm.me>
> ---
>  kernel/bpf/syscall.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 34fdf27d14cf..dd8284a60a8e 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -4286,6 +4286,7 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr)
>                 goto out;
>         case BPF_PROG_TYPE_PERF_EVENT:
>         case BPF_PROG_TYPE_TRACEPOINT:
> +               BTF_TYPE_EMIT(struct bpf_perf_event_value);

same as for previous two patches, if there are types that bpftool
expects and might not be in vmlinux.h due to different kernel
configurations, it's cleaner to just define their minimal local
definitions with __attribute__((preserve_access_index))

>                 if (attr->link_create.attach_type != BPF_PERF_EVENT) {
>                         ret = -EINVAL;
>                         goto out;
> --
> 2.35.2
>
>

[PATCH bpf-next 04/11] samples: bpf: add 'asm/mach-generic' include path for every MIPS

[Alexander Lobakin]

Fix the following:

In file included from samples/bpf/tracex2_kern.c:7:
In file included from ./include/linux/skbuff.h:13:
In file included from ./include/linux/kernel.h:22:
In file included from ./include/linux/bitops.h:33:
In file included from ./arch/mips/include/asm/bitops.h:20:
In file included from ./arch/mips/include/asm/barrier.h:11:
./arch/mips/include/asm/addrspace.h:13:10: fatal error: 'spaces.h' file not found
 #include <spaces.h>
          ^~~~~~~~~~

'arch/mips/include/asm/mach-generic' should always be included as
many other MIPS include files rely on this.
Move it from under CONFIG_MACH_LOONGSON64 to let it be included
for every MIPS.

Fixes: 058107abafc7 ("samples/bpf: Add include dir for MIPS Loongson64 to fix build errors")
Signed-off-by: Alexander Lobakin <alobakin@pm.me>
---
 samples/bpf/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 8fff5ad3444b..97203c0de252 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -193,8 +193,8 @@ ifeq ($(ARCH), mips)
 TPROGS_CFLAGS += -D__SANE_USERSPACE_TYPES__
 ifdef CONFIG_MACH_LOONGSON64
 BPF_EXTRA_CFLAGS += -I$(srctree)/arch/mips/include/asm/mach-loongson64
-BPF_EXTRA_CFLAGS += -I$(srctree)/arch/mips/include/asm/mach-generic
 endif
+BPF_EXTRA_CFLAGS += -I$(srctree)/arch/mips/include/asm/mach-generic
 endif

 TPROGS_CFLAGS += -Wall -O2
--
2.35.2

Re: [PATCH bpf-next 04/11] samples: bpf: add 'asm/mach-generic' include path for every MIPS

[Song Liu]

On Thu, Apr 14, 2022 at 3:45 PM Alexander Lobakin <alobakin@pm.me> wrote:
>
> Fix the following:
>
> In file included from samples/bpf/tracex2_kern.c:7:
> In file included from ./include/linux/skbuff.h:13:
> In file included from ./include/linux/kernel.h:22:
> In file included from ./include/linux/bitops.h:33:
> In file included from ./arch/mips/include/asm/bitops.h:20:
> In file included from ./arch/mips/include/asm/barrier.h:11:
> ./arch/mips/include/asm/addrspace.h:13:10: fatal error: 'spaces.h' file not found
>  #include <spaces.h>
>           ^~~~~~~~~~
>
> 'arch/mips/include/asm/mach-generic' should always be included as
> many other MIPS include files rely on this.
> Move it from under CONFIG_MACH_LOONGSON64 to let it be included
> for every MIPS.
>
> Fixes: 058107abafc7 ("samples/bpf: Add include dir for MIPS Loongson64 to fix build errors")
> Signed-off-by: Alexander Lobakin <alobakin@pm.me>

Acked-by: Song Liu <songliubraving@fb.com>

> ---
>  samples/bpf/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index 8fff5ad3444b..97203c0de252 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -193,8 +193,8 @@ ifeq ($(ARCH), mips)
>  TPROGS_CFLAGS += -D__SANE_USERSPACE_TYPES__
>  ifdef CONFIG_MACH_LOONGSON64
>  BPF_EXTRA_CFLAGS += -I$(srctree)/arch/mips/include/asm/mach-loongson64
> -BPF_EXTRA_CFLAGS += -I$(srctree)/arch/mips/include/asm/mach-generic
>  endif
> +BPF_EXTRA_CFLAGS += -I$(srctree)/arch/mips/include/asm/mach-generic
>  endif
>
>  TPROGS_CFLAGS += -Wall -O2
> --
> 2.35.2
>
>

[PATCH bpf-next 05/11] samples: bpf: use host bpftool to generate vmlinux.h, not target

[Alexander Lobakin]

Use the host build of bpftool (bootstrap) instead of the target one
to generate vmlinux.h/skeletons for the BPF samples. Otherwise, when
host != target, samples compilation fails with:

/bin/sh: line 1: samples/bpf/bpftool/bpftool: failed to exec: Exec
format error

Fixes: 384b6b3bbf0d ("samples: bpf: Add vmlinux.h generation support")
Signed-off-by: Alexander Lobakin <alobakin@pm.me>
---
 samples/bpf/Makefile | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 97203c0de252..02f999a8ef84 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -291,12 +291,13 @@ $(LIBBPF): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OU

 BPFTOOLDIR := $(TOOLS_PATH)/bpf/bpftool
 BPFTOOL_OUTPUT := $(abspath $(BPF_SAMPLES_PATH))/bpftool
-BPFTOOL := $(BPFTOOL_OUTPUT)/bpftool
+BPFTOOL := $(BPFTOOL_OUTPUT)/bootstrap/bpftool
 $(BPFTOOL): $(LIBBPF) $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile) | $(BPFTOOL_OUTPUT)
 	    $(MAKE) -C $(BPFTOOLDIR) srctree=$(BPF_SAMPLES_PATH)/../../ \
 		OUTPUT=$(BPFTOOL_OUTPUT)/ \
 		LIBBPF_OUTPUT=$(LIBBPF_OUTPUT)/ \
-		LIBBPF_DESTDIR=$(LIBBPF_DESTDIR)/
+		LIBBPF_DESTDIR=$(LIBBPF_DESTDIR)/ \
+		bootstrap

 $(LIBBPF_OUTPUT) $(BPFTOOL_OUTPUT):
 	$(call msg,MKDIR,$@)
--
2.35.2

Re: [PATCH bpf-next 05/11] samples: bpf: use host bpftool to generate vmlinux.h, not target

[Kumar Kartikeya Dwivedi]

On Fri, Apr 15, 2022 at 04:15:50AM IST, Alexander Lobakin wrote:
> Use the host build of bpftool (bootstrap) instead of the target one
> to generate vmlinux.h/skeletons for the BPF samples. Otherwise, when
> host != target, samples compilation fails with:
>
> /bin/sh: line 1: samples/bpf/bpftool/bpftool: failed to exec: Exec
> format error
>
> Fixes: 384b6b3bbf0d ("samples: bpf: Add vmlinux.h generation support")
> Signed-off-by: Alexander Lobakin <alobakin@pm.me>
> ---

Acked-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>

>  samples/bpf/Makefile | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index 97203c0de252..02f999a8ef84 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -291,12 +291,13 @@ $(LIBBPF): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OU
>
>  BPFTOOLDIR := $(TOOLS_PATH)/bpf/bpftool
>  BPFTOOL_OUTPUT := $(abspath $(BPF_SAMPLES_PATH))/bpftool
> -BPFTOOL := $(BPFTOOL_OUTPUT)/bpftool
> +BPFTOOL := $(BPFTOOL_OUTPUT)/bootstrap/bpftool
>  $(BPFTOOL): $(LIBBPF) $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile) | $(BPFTOOL_OUTPUT)
>  	    $(MAKE) -C $(BPFTOOLDIR) srctree=$(BPF_SAMPLES_PATH)/../../ \
>  		OUTPUT=$(BPFTOOL_OUTPUT)/ \
>  		LIBBPF_OUTPUT=$(LIBBPF_OUTPUT)/ \
> -		LIBBPF_DESTDIR=$(LIBBPF_DESTDIR)/
> +		LIBBPF_DESTDIR=$(LIBBPF_DESTDIR)/ \
> +		bootstrap
>
>  $(LIBBPF_OUTPUT) $(BPFTOOL_OUTPUT):
>  	$(call msg,MKDIR,$@)
> --
> 2.35.2
>
>

--
Kartikeya

Re: [PATCH bpf-next 05/11] samples: bpf: use host bpftool to generate vmlinux.h, not target

[Song Liu]

On Fri, Apr 15, 2022 at 6:38 AM Kumar Kartikeya Dwivedi
<memxor@gmail.com> wrote:
>
> On Fri, Apr 15, 2022 at 04:15:50AM IST, Alexander Lobakin wrote:
> > Use the host build of bpftool (bootstrap) instead of the target one
> > to generate vmlinux.h/skeletons for the BPF samples. Otherwise, when
> > host != target, samples compilation fails with:
> >
> > /bin/sh: line 1: samples/bpf/bpftool/bpftool: failed to exec: Exec
> > format error
> >
> > Fixes: 384b6b3bbf0d ("samples: bpf: Add vmlinux.h generation support")
> > Signed-off-by: Alexander Lobakin <alobakin@pm.me>
> > ---
>
> Acked-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>

Acked-by: Song Liu <songliubraving@fb.com>

>
> >  samples/bpf/Makefile | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> > index 97203c0de252..02f999a8ef84 100644
> > --- a/samples/bpf/Makefile
> > +++ b/samples/bpf/Makefile
> > @@ -291,12 +291,13 @@ $(LIBBPF): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OU
> >
> >  BPFTOOLDIR := $(TOOLS_PATH)/bpf/bpftool
> >  BPFTOOL_OUTPUT := $(abspath $(BPF_SAMPLES_PATH))/bpftool
> > -BPFTOOL := $(BPFTOOL_OUTPUT)/bpftool
> > +BPFTOOL := $(BPFTOOL_OUTPUT)/bootstrap/bpftool
> >  $(BPFTOOL): $(LIBBPF) $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile) | $(BPFTOOL_OUTPUT)
> >           $(MAKE) -C $(BPFTOOLDIR) srctree=$(BPF_SAMPLES_PATH)/../../ \
> >               OUTPUT=$(BPFTOOL_OUTPUT)/ \
> >               LIBBPF_OUTPUT=$(LIBBPF_OUTPUT)/ \
> > -             LIBBPF_DESTDIR=$(LIBBPF_DESTDIR)/
> > +             LIBBPF_DESTDIR=$(LIBBPF_DESTDIR)/ \
> > +             bootstrap
> >
> >  $(LIBBPF_OUTPUT) $(BPFTOOL_OUTPUT):
> >       $(call msg,MKDIR,$@)
> > --
> > 2.35.2
> >
> >
>
> --
> Kartikeya

[PATCH bpf-next 06/11] tools, bpf: fix fcntl.h include in bpftool

[Alexander Lobakin]

Fix the following (on some libc implementations):

  CC      tracelog.o
In file included from tracelog.c:12:
include/sys/fcntl.h:1:2: warning: #warning redirecting incorrect #include <sys/fcntl.h> to <fcntl.h> [-Wcpp]
    1 | #warning redirecting incorrect #include <sys/fcntl.h> to <fcntl.h>
      |  ^~~~~~~

<sys/fcntl.h> is anyway just a wrapper over <fcntl.h> (backcomp
stuff).

Fixes: 30da46b5dc3a ("tools: bpftool: add a command to dump the trace pipe")
Signed-off-by: Alexander Lobakin <alobakin@pm.me>
---
 tools/bpf/bpftool/tracelog.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/bpf/bpftool/tracelog.c b/tools/bpf/bpftool/tracelog.c
index e80a5c79b38f..bf1f02212797 100644
--- a/tools/bpf/bpftool/tracelog.c
+++ b/tools/bpf/bpftool/tracelog.c
@@ -9,7 +9,7 @@
 #include <string.h>
 #include <unistd.h>
 #include <linux/magic.h>
-#include <sys/fcntl.h>
+#include <fcntl.h>
 #include <sys/vfs.h>

 #include "main.h"
--
2.35.2

Re: [PATCH bpf-next 06/11] tools, bpf: fix fcntl.h include in bpftool

[Song Liu]

On Thu, Apr 14, 2022 at 3:46 PM Alexander Lobakin <alobakin@pm.me> wrote:
>
> Fix the following (on some libc implementations):
>
>   CC      tracelog.o
> In file included from tracelog.c:12:
> include/sys/fcntl.h:1:2: warning: #warning redirecting incorrect #include <sys/fcntl.h> to <fcntl.h> [-Wcpp]
>     1 | #warning redirecting incorrect #include <sys/fcntl.h> to <fcntl.h>
>       |  ^~~~~~~
>
> <sys/fcntl.h> is anyway just a wrapper over <fcntl.h> (backcomp
> stuff).
>
> Fixes: 30da46b5dc3a ("tools: bpftool: add a command to dump the trace pipe")
> Signed-off-by: Alexander Lobakin <alobakin@pm.me>

Acked-by: Song Liu <songliubraving@fb.com>

> ---
>  tools/bpf/bpftool/tracelog.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/bpf/bpftool/tracelog.c b/tools/bpf/bpftool/tracelog.c
> index e80a5c79b38f..bf1f02212797 100644
> --- a/tools/bpf/bpftool/tracelog.c
> +++ b/tools/bpf/bpftool/tracelog.c
> @@ -9,7 +9,7 @@
>  #include <string.h>
>  #include <unistd.h>
>  #include <linux/magic.h>
> -#include <sys/fcntl.h>
> +#include <fcntl.h>
>  #include <sys/vfs.h>
>
>  #include "main.h"
> --
> 2.35.2
>
>

[PATCH bpf-next 07/11] samples: bpf: fix uin64_t format literals

[Alexander Lobakin]

There's a couple places where uin64_t is being passed as an %ld
format argument, which is incorrect (should be %lld). Fix them.

Fixes: 51570a5ab2b7 ("A Sample of using socket cookie and uid for traffic monitoring")
Fixes: 00f660eaf378 ("Sample program using SO_COOKIE")
Signed-off-by: Alexander Lobakin <alobakin@pm.me>
---
 samples/bpf/cookie_uid_helper_example.c | 12 ++++++------
 samples/bpf/lwt_len_hist_user.c         |  4 ++--
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/samples/bpf/cookie_uid_helper_example.c b/samples/bpf/cookie_uid_helper_example.c
index f0df3dda4b1f..1b98debb6019 100644
--- a/samples/bpf/cookie_uid_helper_example.c
+++ b/samples/bpf/cookie_uid_helper_example.c
@@ -207,9 +207,9 @@ static void print_table(void)
 			error(1, errno, "fail to get entry value of Key: %u\n",
 				curN);
 		} else {
-			printf("cookie: %u, uid: 0x%x, Packet Count: %lu,"
-				" Bytes Count: %lu\n", curN, curEntry.uid,
-				curEntry.packets, curEntry.bytes);
+			printf("cookie: %u, uid: 0x%x, Packet Count: %llu, Bytes Count: %llu\n",
+			       curN, curEntry.uid, curEntry.packets,
+			       curEntry.bytes);
 		}
 	}
 }
@@ -265,9 +265,9 @@ static void udp_client(void)
 		if (res < 0)
 			error(1, errno, "lookup sk stat failed, cookie: %lu\n",
 			      cookie);
-		printf("cookie: %lu, uid: 0x%x, Packet Count: %lu,"
-			" Bytes Count: %lu\n\n", cookie, dataEntry.uid,
-			dataEntry.packets, dataEntry.bytes);
+		printf("cookie: %llu, uid: 0x%x, Packet Count: %llu, Bytes Count: %llu\n\n",
+		       cookie, dataEntry.uid, dataEntry.packets,
+		       dataEntry.bytes);
 	}
 	close(s_send);
 	close(s_rcv);
diff --git a/samples/bpf/lwt_len_hist_user.c b/samples/bpf/lwt_len_hist_user.c
index 430a4b7e353e..4ef22571aa67 100644
--- a/samples/bpf/lwt_len_hist_user.c
+++ b/samples/bpf/lwt_len_hist_user.c
@@ -44,7 +44,7 @@ int main(int argc, char **argv)

 	while (bpf_map_get_next_key(map_fd, &key, &next_key) == 0) {
 		if (next_key >= MAX_INDEX) {
-			fprintf(stderr, "Key %lu out of bounds\n", next_key);
+			fprintf(stderr, "Key %llu out of bounds\n", next_key);
 			continue;
 		}

@@ -66,7 +66,7 @@ int main(int argc, char **argv)

 	for (i = 1; i <= max_key + 1; i++) {
 		stars(starstr, data[i - 1], max_value, MAX_STARS);
-		printf("%8ld -> %-8ld : %-8ld |%-*s|\n",
+		printf("%8ld -> %-8ld : %-8lld |%-*s|\n",
 		       (1l << i) >> 1, (1l << i) - 1, data[i - 1],
 		       MAX_STARS, starstr);
 	}
--
2.35.2

Re: [PATCH bpf-next 07/11] samples: bpf: fix uin64_t format literals

[Song Liu]

On Thu, Apr 14, 2022 at 3:46 PM Alexander Lobakin <alobakin@pm.me> wrote:
>
> There's a couple places where uin64_t is being passed as an %ld
> format argument, which is incorrect (should be %lld). Fix them.

This will cause some warning on some 64-bit compiler, no?

Song

>
> Fixes: 51570a5ab2b7 ("A Sample of using socket cookie and uid for traffic monitoring")
> Fixes: 00f660eaf378 ("Sample program using SO_COOKIE")
> Signed-off-by: Alexander Lobakin <alobakin@pm.me>
> ---
>  samples/bpf/cookie_uid_helper_example.c | 12 ++++++------
>  samples/bpf/lwt_len_hist_user.c         |  4 ++--
>  2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/samples/bpf/cookie_uid_helper_example.c b/samples/bpf/cookie_uid_helper_example.c
> index f0df3dda4b1f..1b98debb6019 100644
> --- a/samples/bpf/cookie_uid_helper_example.c
> +++ b/samples/bpf/cookie_uid_helper_example.c
> @@ -207,9 +207,9 @@ static void print_table(void)
>                         error(1, errno, "fail to get entry value of Key: %u\n",
>                                 curN);
>                 } else {
> -                       printf("cookie: %u, uid: 0x%x, Packet Count: %lu,"
> -                               " Bytes Count: %lu\n", curN, curEntry.uid,
> -                               curEntry.packets, curEntry.bytes);
> +                       printf("cookie: %u, uid: 0x%x, Packet Count: %llu, Bytes Count: %llu\n",
> +                              curN, curEntry.uid, curEntry.packets,
> +                              curEntry.bytes);
>                 }
>         }
>  }
> @@ -265,9 +265,9 @@ static void udp_client(void)
>                 if (res < 0)
>                         error(1, errno, "lookup sk stat failed, cookie: %lu\n",
>                               cookie);
> -               printf("cookie: %lu, uid: 0x%x, Packet Count: %lu,"
> -                       " Bytes Count: %lu\n\n", cookie, dataEntry.uid,
> -                       dataEntry.packets, dataEntry.bytes);
> +               printf("cookie: %llu, uid: 0x%x, Packet Count: %llu, Bytes Count: %llu\n\n",
> +                      cookie, dataEntry.uid, dataEntry.packets,
> +                      dataEntry.bytes);
>         }
>         close(s_send);
>         close(s_rcv);
> diff --git a/samples/bpf/lwt_len_hist_user.c b/samples/bpf/lwt_len_hist_user.c
> index 430a4b7e353e..4ef22571aa67 100644
> --- a/samples/bpf/lwt_len_hist_user.c
> +++ b/samples/bpf/lwt_len_hist_user.c
> @@ -44,7 +44,7 @@ int main(int argc, char **argv)
>
>         while (bpf_map_get_next_key(map_fd, &key, &next_key) == 0) {
>                 if (next_key >= MAX_INDEX) {
> -                       fprintf(stderr, "Key %lu out of bounds\n", next_key);
> +                       fprintf(stderr, "Key %llu out of bounds\n", next_key);
>                         continue;
>                 }
>
> @@ -66,7 +66,7 @@ int main(int argc, char **argv)
>
>         for (i = 1; i <= max_key + 1; i++) {
>                 stars(starstr, data[i - 1], max_value, MAX_STARS);
> -               printf("%8ld -> %-8ld : %-8ld |%-*s|\n",
> +               printf("%8ld -> %-8ld : %-8lld |%-*s|\n",
>                        (1l << i) >> 1, (1l << i) - 1, data[i - 1],
>                        MAX_STARS, starstr);
>         }
> --
> 2.35.2
>
>

Re: [PATCH bpf-next 07/11] samples: bpf: fix uin64_t format literals

[Alexander Lobakin]

From: Song Liu <song@kernel.org>
Date: Fri, 15 Apr 2022 16:52:13 -0700

> On Thu, Apr 14, 2022 at 3:46 PM Alexander Lobakin <alobakin@pm.me> wrote:
> >
> > There's a couple places where uin64_t is being passed as an %ld
> > format argument, which is incorrect (should be %lld). Fix them.
>
> This will cause some warning on some 64-bit compiler, no?

Oh wait, I accidentially mentioned %ld and %lld although in fact I
changed %lu to %llu. So there won't be any compiler warnings. I'll
fix the commit message in v2.

>
> Song
>

--- 8< ---

> > --
> > 2.35.2

Thanks,
Al

RE: [PATCH bpf-next 07/11] samples: bpf: fix uin64_t format literals

[David Laight]

From: Alexander Lobakin
> Sent: 16 April 2022 18:55
> To: Song Liu <song@kernel.org>
> 
> From: Song Liu <song@kernel.org>
> Date: Fri, 15 Apr 2022 16:52:13 -0700
> 
> > On Thu, Apr 14, 2022 at 3:46 PM Alexander Lobakin <alobakin@pm.me> wrote:
> > >
> > > There's a couple places where uin64_t is being passed as an %ld
> > > format argument, which is incorrect (should be %lld). Fix them.
> >
> > This will cause some warning on some 64-bit compiler, no?
> 
> Oh wait, I accidentially mentioned %ld and %lld although in fact I
> changed %lu to %llu. So there won't be any compiler warnings. I'll
> fix the commit message in v2.

That won't make any difference.
The correct way to print uint64_t is using PRIu64.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH bpf-next 07/11] samples: bpf: fix uin64_t format literals

[Andrii Nakryiko]

On Thu, Apr 14, 2022 at 3:46 PM Alexander Lobakin <alobakin@pm.me> wrote:
>
> There's a couple places where uin64_t is being passed as an %ld
> format argument, which is incorrect (should be %lld). Fix them.

It depends on architecture, on some it will be %lu, on some it will be
%llu. But instead of PRIu64, just cast it to size_t and use %zu as
formatter

>
> Fixes: 51570a5ab2b7 ("A Sample of using socket cookie and uid for traffic monitoring")
> Fixes: 00f660eaf378 ("Sample program using SO_COOKIE")
> Signed-off-by: Alexander Lobakin <alobakin@pm.me>
> ---
>  samples/bpf/cookie_uid_helper_example.c | 12 ++++++------
>  samples/bpf/lwt_len_hist_user.c         |  4 ++--
>  2 files changed, 8 insertions(+), 8 deletions(-)
>

[...]

[PATCH bpf-next 08/11] samples: bpf: fix shifting unsigned long by 32 positions

[Alexander Lobakin]

On 32 bit systems, shifting an unsigned long by 32 positions
yields the following warning:

samples/bpf/tracex2_kern.c:60:23: warning: shift count >= width of type [-Wshift-count-overflow]
        unsigned int hi = v >> 32;
                            ^  ~~

The usual way to avoid this is to shift by 16 two times (see
upper_32_bits() macro in the kernel). Use it across the BPF sample
code as well.

Fixes: d822a1926849 ("samples/bpf: Add counting example for kfree_skb() function calls and the write() syscall")
Fixes: 0fb1170ee68a ("bpf: BPF based latency tracing")
Fixes: f74599f7c530 ("bpf: Add tests and samples for LWT-BPF")
Signed-off-by: Alexander Lobakin <alobakin@pm.me>
---
 samples/bpf/lathist_kern.c      | 2 +-
 samples/bpf/lwt_len_hist_kern.c | 2 +-
 samples/bpf/tracex2_kern.c      | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/samples/bpf/lathist_kern.c b/samples/bpf/lathist_kern.c
index 4adfcbbe6ef4..9744ed547abe 100644
--- a/samples/bpf/lathist_kern.c
+++ b/samples/bpf/lathist_kern.c
@@ -53,7 +53,7 @@ static unsigned int log2(unsigned int v)

 static unsigned int log2l(unsigned long v)
 {
-	unsigned int hi = v >> 32;
+	unsigned int hi = (v >> 16) >> 16;

 	if (hi)
 		return log2(hi) + 32;
diff --git a/samples/bpf/lwt_len_hist_kern.c b/samples/bpf/lwt_len_hist_kern.c
index 1fa14c54963a..bf32fa04c91f 100644
--- a/samples/bpf/lwt_len_hist_kern.c
+++ b/samples/bpf/lwt_len_hist_kern.c
@@ -49,7 +49,7 @@ static unsigned int log2(unsigned int v)

 static unsigned int log2l(unsigned long v)
 {
-	unsigned int hi = v >> 32;
+	unsigned int hi = (v >> 16) >> 16;
 	if (hi)
 		return log2(hi) + 32;
 	else
diff --git a/samples/bpf/tracex2_kern.c b/samples/bpf/tracex2_kern.c
index 5bc696bac27d..6bf22056ff95 100644
--- a/samples/bpf/tracex2_kern.c
+++ b/samples/bpf/tracex2_kern.c
@@ -57,7 +57,7 @@ static unsigned int log2(unsigned int v)

 static unsigned int log2l(unsigned long v)
 {
-	unsigned int hi = v >> 32;
+	unsigned int hi = (v >> 16) >> 16;
 	if (hi)
 		return log2(hi) + 32;
 	else
--
2.35.2

Re: [PATCH bpf-next 08/11] samples: bpf: fix shifting unsigned long by 32 positions

[Song Liu]

On Thu, Apr 14, 2022 at 3:46 PM Alexander Lobakin <alobakin@pm.me> wrote:
>
> On 32 bit systems, shifting an unsigned long by 32 positions
> yields the following warning:
>
> samples/bpf/tracex2_kern.c:60:23: warning: shift count >= width of type [-Wshift-count-overflow]
>         unsigned int hi = v >> 32;
>                             ^  ~~
>
> The usual way to avoid this is to shift by 16 two times (see
> upper_32_bits() macro in the kernel). Use it across the BPF sample
> code as well.
>
> Fixes: d822a1926849 ("samples/bpf: Add counting example for kfree_skb() function calls and the write() syscall")
> Fixes: 0fb1170ee68a ("bpf: BPF based latency tracing")
> Fixes: f74599f7c530 ("bpf: Add tests and samples for LWT-BPF")
> Signed-off-by: Alexander Lobakin <alobakin@pm.me>

Acked-by: Song Liu <songliubraving@fb.com>
> ---
>  samples/bpf/lathist_kern.c      | 2 +-
>  samples/bpf/lwt_len_hist_kern.c | 2 +-
>  samples/bpf/tracex2_kern.c      | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/samples/bpf/lathist_kern.c b/samples/bpf/lathist_kern.c
> index 4adfcbbe6ef4..9744ed547abe 100644
> --- a/samples/bpf/lathist_kern.c
> +++ b/samples/bpf/lathist_kern.c
> @@ -53,7 +53,7 @@ static unsigned int log2(unsigned int v)
>
>  static unsigned int log2l(unsigned long v)
>  {
> -       unsigned int hi = v >> 32;
> +       unsigned int hi = (v >> 16) >> 16;
>
>         if (hi)
>                 return log2(hi) + 32;
> diff --git a/samples/bpf/lwt_len_hist_kern.c b/samples/bpf/lwt_len_hist_kern.c
> index 1fa14c54963a..bf32fa04c91f 100644
> --- a/samples/bpf/lwt_len_hist_kern.c
> +++ b/samples/bpf/lwt_len_hist_kern.c
> @@ -49,7 +49,7 @@ static unsigned int log2(unsigned int v)
>
>  static unsigned int log2l(unsigned long v)
>  {
> -       unsigned int hi = v >> 32;
> +       unsigned int hi = (v >> 16) >> 16;
>         if (hi)
>                 return log2(hi) + 32;
>         else
> diff --git a/samples/bpf/tracex2_kern.c b/samples/bpf/tracex2_kern.c
> index 5bc696bac27d..6bf22056ff95 100644
> --- a/samples/bpf/tracex2_kern.c
> +++ b/samples/bpf/tracex2_kern.c
> @@ -57,7 +57,7 @@ static unsigned int log2(unsigned int v)
>
>  static unsigned int log2l(unsigned long v)
>  {
> -       unsigned int hi = v >> 32;
> +       unsigned int hi = (v >> 16) >> 16;
>         if (hi)
>                 return log2(hi) + 32;
>         else
> --
> 2.35.2
>
>

Re: [PATCH bpf-next 08/11] samples: bpf: fix shifting unsigned long by 32 positions

[Andrii Nakryiko]

On Thu, Apr 14, 2022 at 3:46 PM Alexander Lobakin <alobakin@pm.me> wrote:
>
> On 32 bit systems, shifting an unsigned long by 32 positions
> yields the following warning:
>
> samples/bpf/tracex2_kern.c:60:23: warning: shift count >= width of type [-Wshift-count-overflow]
>         unsigned int hi = v >> 32;
>                             ^  ~~
>

long is always 64-bit in BPF, but I suspect this is due to
samples/bpf/Makefile still using this clang + llc combo, where clang
is called with native target and llc for -target bpf. Not sure if we
are ready to ditch that complicated combination. Yonghong, do we still
need that or can we just use -target bpf in samples/bpf?


> The usual way to avoid this is to shift by 16 two times (see
> upper_32_bits() macro in the kernel). Use it across the BPF sample
> code as well.
>
> Fixes: d822a1926849 ("samples/bpf: Add counting example for kfree_skb() function calls and the write() syscall")
> Fixes: 0fb1170ee68a ("bpf: BPF based latency tracing")
> Fixes: f74599f7c530 ("bpf: Add tests and samples for LWT-BPF")
> Signed-off-by: Alexander Lobakin <alobakin@pm.me>
> ---
>  samples/bpf/lathist_kern.c      | 2 +-
>  samples/bpf/lwt_len_hist_kern.c | 2 +-
>  samples/bpf/tracex2_kern.c      | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
>

[...]

Re: [PATCH bpf-next 08/11] samples: bpf: fix shifting unsigned long by 32 positions

[Yonghong Song]

On 4/20/22 10:18 AM, Andrii Nakryiko wrote:
> On Thu, Apr 14, 2022 at 3:46 PM Alexander Lobakin <alobakin@pm.me> wrote:
>>
>> On 32 bit systems, shifting an unsigned long by 32 positions
>> yields the following warning:
>>
>> samples/bpf/tracex2_kern.c:60:23: warning: shift count >= width of type [-Wshift-count-overflow]
>>          unsigned int hi = v >> 32;
>>                              ^  ~~
>>
> 
> long is always 64-bit in BPF, but I suspect this is due to
> samples/bpf/Makefile still using this clang + llc combo, where clang
> is called with native target and llc for -target bpf. Not sure if we
> are ready to ditch that complicated combination. Yonghong, do we still
> need that or can we just use -target bpf in samples/bpf?

Current most bpf programs in samples/bpf do not use vmlinux.h and CO-RE.
They direct use kernel header files. That is why clang C -> IR 
compilation still needs to be native.

We could just use -target bpf for the whole compilation but that needs 
to change the code to use vmlinux.h and CO-RE. There are already a 
couple of sample bpf programs did this.

> 
> 
>> The usual way to avoid this is to shift by 16 two times (see
>> upper_32_bits() macro in the kernel). Use it across the BPF sample
>> code as well.
>>
>> Fixes: d822a1926849 ("samples/bpf: Add counting example for kfree_skb() function calls and the write() syscall")
>> Fixes: 0fb1170ee68a ("bpf: BPF based latency tracing")
>> Fixes: f74599f7c530 ("bpf: Add tests and samples for LWT-BPF")
>> Signed-off-by: Alexander Lobakin <alobakin@pm.me>
>> ---
>>   samples/bpf/lathist_kern.c      | 2 +-
>>   samples/bpf/lwt_len_hist_kern.c | 2 +-
>>   samples/bpf/tracex2_kern.c      | 2 +-
>>   3 files changed, 3 insertions(+), 3 deletions(-)
>>
> 
> [...]

Re: [PATCH bpf-next 08/11] samples: bpf: fix shifting unsigned long by 32 positions

[Andrii Nakryiko]

On Wed, Apr 27, 2022 at 8:55 AM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 4/20/22 10:18 AM, Andrii Nakryiko wrote:
> > On Thu, Apr 14, 2022 at 3:46 PM Alexander Lobakin <alobakin@pm.me> wrote:
> >>
> >> On 32 bit systems, shifting an unsigned long by 32 positions
> >> yields the following warning:
> >>
> >> samples/bpf/tracex2_kern.c:60:23: warning: shift count >= width of type [-Wshift-count-overflow]
> >>          unsigned int hi = v >> 32;
> >>                              ^  ~~
> >>
> >
> > long is always 64-bit in BPF, but I suspect this is due to
> > samples/bpf/Makefile still using this clang + llc combo, where clang
> > is called with native target and llc for -target bpf. Not sure if we
> > are ready to ditch that complicated combination. Yonghong, do we still
> > need that or can we just use -target bpf in samples/bpf?
>
> Current most bpf programs in samples/bpf do not use vmlinux.h and CO-RE.
> They direct use kernel header files. That is why clang C -> IR
> compilation still needs to be native.
>
> We could just use -target bpf for the whole compilation but that needs
> to change the code to use vmlinux.h and CO-RE. There are already a
> couple of sample bpf programs did this.

Right, I guess I'm proposing to switch samples/bpf to vmlinux.h. Only
purely networking BPF apps can get away with not using vmlinux.h
because they might avoid dependency on kernel types. But even then a
lot of modern networking apps seem to be gaining elements of more
generic tracing and would rely on CO-RE for staying "portable" between
kernels. So it might be totally fine to just use CO-RE universally in
samples/bpf?

>
> >
> >
> >> The usual way to avoid this is to shift by 16 two times (see
> >> upper_32_bits() macro in the kernel). Use it across the BPF sample
> >> code as well.
> >>
> >> Fixes: d822a1926849 ("samples/bpf: Add counting example for kfree_skb() function calls and the write() syscall")
> >> Fixes: 0fb1170ee68a ("bpf: BPF based latency tracing")
> >> Fixes: f74599f7c530 ("bpf: Add tests and samples for LWT-BPF")
> >> Signed-off-by: Alexander Lobakin <alobakin@pm.me>
> >> ---
> >>   samples/bpf/lathist_kern.c      | 2 +-
> >>   samples/bpf/lwt_len_hist_kern.c | 2 +-
> >>   samples/bpf/tracex2_kern.c      | 2 +-
> >>   3 files changed, 3 insertions(+), 3 deletions(-)
> >>
> >
> > [...]

[PATCH bpf-next 09/11] samples: bpf: fix include order for non-Glibc environments

[Alexander Lobakin]

Some standard C library implementations, e.g. Musl, ship the UAPI
definitions themselves to not be dependent on the UAPI headers and
their versions. Their kernel UAPI counterparts are usually guarded
with some definitions which the formers set in order to avoid
duplicate definitions.
In such cases, include order matters. Change it in two samples: in
the first, kernel UAPI ioctl definitions should go before the libc
ones, and the opposite story with the second, where the kernel
includes should go later to avoid struct redefinitions.

Fixes: b4b8faa1ded7 ("samples/bpf: sample application and documentation for AF_XDP sockets")
Fixes: e55190f26f92 ("samples/bpf: Fix build for task_fd_query_user.c")
Signed-off-by: Alexander Lobakin <alobakin@pm.me>
---
 samples/bpf/task_fd_query_user.c | 2 +-
 samples/bpf/xdpsock_user.c       | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/samples/bpf/task_fd_query_user.c b/samples/bpf/task_fd_query_user.c
index 424718c0872c..5d3a60547f9f 100644
--- a/samples/bpf/task_fd_query_user.c
+++ b/samples/bpf/task_fd_query_user.c
@@ -9,10 +9,10 @@
 #include <stdint.h>
 #include <fcntl.h>
 #include <linux/bpf.h>
+#include <linux/perf_event.h>
 #include <sys/ioctl.h>
 #include <sys/types.h>
 #include <sys/stat.h>
-#include <linux/perf_event.h>

 #include <bpf/bpf.h>
 #include <bpf/libbpf.h>
diff --git a/samples/bpf/xdpsock_user.c b/samples/bpf/xdpsock_user.c
index be7d2572e3e6..399b999fcec2 100644
--- a/samples/bpf/xdpsock_user.c
+++ b/samples/bpf/xdpsock_user.c
@@ -7,14 +7,15 @@
 #include <linux/bpf.h>
 #include <linux/if_link.h>
 #include <linux/if_xdp.h>
-#include <linux/if_ether.h>
 #include <linux/ip.h>
 #include <linux/limits.h>
+#include <linux/net.h>
 #include <linux/udp.h>
 #include <arpa/inet.h>
 #include <locale.h>
 #include <net/ethernet.h>
 #include <netinet/ether.h>
+#include <linux/if_ether.h>
 #include <net/if.h>
 #include <poll.h>
 #include <pthread.h>
--
2.35.2

Re: [PATCH bpf-next 09/11] samples: bpf: fix include order for non-Glibc environments

[Song Liu]

On Thu, Apr 14, 2022 at 3:47 PM Alexander Lobakin <alobakin@pm.me> wrote:
>
> Some standard C library implementations, e.g. Musl, ship the UAPI
> definitions themselves to not be dependent on the UAPI headers and
> their versions. Their kernel UAPI counterparts are usually guarded
> with some definitions which the formers set in order to avoid
> duplicate definitions.
> In such cases, include order matters. Change it in two samples: in
> the first, kernel UAPI ioctl definitions should go before the libc
> ones, and the opposite story with the second, where the kernel
> includes should go later to avoid struct redefinitions.
>
> Fixes: b4b8faa1ded7 ("samples/bpf: sample application and documentation for AF_XDP sockets")
> Fixes: e55190f26f92 ("samples/bpf: Fix build for task_fd_query_user.c")
> Signed-off-by: Alexander Lobakin <alobakin@pm.me>

Acked-by: Song Liu <songliubraving@fb.com>

> ---
>  samples/bpf/task_fd_query_user.c | 2 +-
>  samples/bpf/xdpsock_user.c       | 3 ++-
>  2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/samples/bpf/task_fd_query_user.c b/samples/bpf/task_fd_query_user.c
> index 424718c0872c..5d3a60547f9f 100644
> --- a/samples/bpf/task_fd_query_user.c
> +++ b/samples/bpf/task_fd_query_user.c
> @@ -9,10 +9,10 @@
>  #include <stdint.h>
>  #include <fcntl.h>
>  #include <linux/bpf.h>
> +#include <linux/perf_event.h>
>  #include <sys/ioctl.h>
>  #include <sys/types.h>
>  #include <sys/stat.h>
> -#include <linux/perf_event.h>
>
>  #include <bpf/bpf.h>
>  #include <bpf/libbpf.h>
> diff --git a/samples/bpf/xdpsock_user.c b/samples/bpf/xdpsock_user.c
> index be7d2572e3e6..399b999fcec2 100644
> --- a/samples/bpf/xdpsock_user.c
> +++ b/samples/bpf/xdpsock_user.c
> @@ -7,14 +7,15 @@
>  #include <linux/bpf.h>
>  #include <linux/if_link.h>
>  #include <linux/if_xdp.h>
> -#include <linux/if_ether.h>
>  #include <linux/ip.h>
>  #include <linux/limits.h>
> +#include <linux/net.h>
>  #include <linux/udp.h>
>  #include <arpa/inet.h>
>  #include <locale.h>
>  #include <net/ethernet.h>
>  #include <netinet/ether.h>
> +#include <linux/if_ether.h>
>  #include <net/if.h>
>  #include <poll.h>
>  #include <pthread.h>
> --
> 2.35.2
>
>

[PATCH bpf-next 10/11] samples: bpf: fix -Wsequence-point

[Alexander Lobakin]

In some libc implementations, CPU_SET() may utilize its first
argument several times. When combined with a post-increment, it
leads to:

samples/bpf/test_lru_dist.c:233:36: warning: operation on 'next_to_try' may be undefined [-Wsequence-point]
  233 |                 CPU_SET(next_to_try++, &cpuset);
      |                                    ^

Split the sentence into two standalone operations to fix this.

Fixes: 5db58faf989f ("bpf: Add tests for the LRU bpf_htab")
Signed-off-by: Alexander Lobakin <alobakin@pm.me>
---
 samples/bpf/test_lru_dist.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/samples/bpf/test_lru_dist.c b/samples/bpf/test_lru_dist.c
index be98ccb4952f..191643ec501e 100644
--- a/samples/bpf/test_lru_dist.c
+++ b/samples/bpf/test_lru_dist.c
@@ -229,7 +229,8 @@ static int sched_next_online(int pid, int next_to_try)

 	while (next_to_try < nr_cpus) {
 		CPU_ZERO(&cpuset);
-		CPU_SET(next_to_try++, &cpuset);
+		CPU_SET(next_to_try, &cpuset);
+		next_to_try++;
 		if (!sched_setaffinity(pid, sizeof(cpuset), &cpuset))
 			break;
 	}
--
2.35.2

Re: [PATCH bpf-next 10/11] samples: bpf: fix -Wsequence-point

[Song Liu]

On Thu, Apr 14, 2022 at 3:47 PM Alexander Lobakin <alobakin@pm.me> wrote:
>
> In some libc implementations, CPU_SET() may utilize its first
> argument several times. When combined with a post-increment, it
> leads to:
>
> samples/bpf/test_lru_dist.c:233:36: warning: operation on 'next_to_try' may be undefined [-Wsequence-point]
>   233 |                 CPU_SET(next_to_try++, &cpuset);
>       |                                    ^
>
> Split the sentence into two standalone operations to fix this.
>
> Fixes: 5db58faf989f ("bpf: Add tests for the LRU bpf_htab")
> Signed-off-by: Alexander Lobakin <alobakin@pm.me>

Acked-by: Song Liu <songliubraving@fb.com>

> ---
>  samples/bpf/test_lru_dist.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/samples/bpf/test_lru_dist.c b/samples/bpf/test_lru_dist.c
> index be98ccb4952f..191643ec501e 100644
> --- a/samples/bpf/test_lru_dist.c
> +++ b/samples/bpf/test_lru_dist.c
> @@ -229,7 +229,8 @@ static int sched_next_online(int pid, int next_to_try)
>
>         while (next_to_try < nr_cpus) {
>                 CPU_ZERO(&cpuset);
> -               CPU_SET(next_to_try++, &cpuset);
> +               CPU_SET(next_to_try, &cpuset);
> +               next_to_try++;
>                 if (!sched_setaffinity(pid, sizeof(cpuset), &cpuset))
>                         break;
>         }
> --
> 2.35.2
>
>

[PATCH bpf-next 11/11] samples: bpf: xdpsock: fix -Wmaybe-uninitialized

[Alexander Lobakin]

Fix two sort-of-false-positives in the xdpsock userspace part:

samples/bpf/xdpsock_user.c: In function 'main':
samples/bpf/xdpsock_user.c:1531:47: warning: 'tv_usec' may be used uninitialized in this function [-Wmaybe-uninitialized]
 1531 |                         pktgen_hdr->tv_usec = htonl(tv_usec);
      |                                               ^~~~~~~~~~~~~~
samples/bpf/xdpsock_user.c:1500:26: note: 'tv_usec' was declared here
 1500 |         u32 idx, tv_sec, tv_usec;
      |                          ^~~~~~~
samples/bpf/xdpsock_user.c:1530:46: warning: 'tv_sec' may be used uninitialized in this function [-Wmaybe-uninitialized]
 1530 |                         pktgen_hdr->tv_sec = htonl(tv_sec);
      |                                              ^~~~~~~~~~~~~
samples/bpf/xdpsock_user.c:1500:18: note: 'tv_sec' was declared here
 1500 |         u32 idx, tv_sec, tv_usec;
      |                  ^~~~~~

Both variables are always initialized when @opt_tstamp == true and
they're being used also only when @opt_tstamp == true. However, that
variable comes from the BSS and is being toggled from another
function. They can't be executed simultaneously to actually trigger
undefined behaviour, but purely technically it is a correct warning.
Just initialize them with zeroes.

Fixes: eb68db45b747 ("samples/bpf: xdpsock: Add timestamp for Tx-only operation")
Signed-off-by: Alexander Lobakin <alobakin@pm.me>
---
 samples/bpf/xdpsock_user.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/samples/bpf/xdpsock_user.c b/samples/bpf/xdpsock_user.c
index 399b999fcec2..1dc7ad5dbef4 100644
--- a/samples/bpf/xdpsock_user.c
+++ b/samples/bpf/xdpsock_user.c
@@ -1496,7 +1496,7 @@ static void rx_drop_all(void)
 static int tx_only(struct xsk_socket_info *xsk, u32 *frame_nb,
 		   int batch_size, unsigned long tx_ns)
 {
-	u32 idx, tv_sec, tv_usec;
+	u32 idx, tv_sec = 0, tv_usec = 0;
 	unsigned int i;

 	while (xsk_ring_prod__reserve(&xsk->tx, batch_size, &idx) <
--
2.35.2

Re: [PATCH bpf-next 11/11] samples: bpf: xdpsock: fix -Wmaybe-uninitialized

[Maciej Fijalkowski]

On Thu, Apr 14, 2022 at 10:47:20PM +0000, Alexander Lobakin wrote:
> Fix two sort-of-false-positives in the xdpsock userspace part:
> 
> samples/bpf/xdpsock_user.c: In function 'main':
> samples/bpf/xdpsock_user.c:1531:47: warning: 'tv_usec' may be used uninitialized in this function [-Wmaybe-uninitialized]
>  1531 |                         pktgen_hdr->tv_usec = htonl(tv_usec);
>       |                                               ^~~~~~~~~~~~~~
> samples/bpf/xdpsock_user.c:1500:26: note: 'tv_usec' was declared here
>  1500 |         u32 idx, tv_sec, tv_usec;
>       |                          ^~~~~~~
> samples/bpf/xdpsock_user.c:1530:46: warning: 'tv_sec' may be used uninitialized in this function [-Wmaybe-uninitialized]
>  1530 |                         pktgen_hdr->tv_sec = htonl(tv_sec);
>       |                                              ^~~~~~~~~~~~~
> samples/bpf/xdpsock_user.c:1500:18: note: 'tv_sec' was declared here
>  1500 |         u32 idx, tv_sec, tv_usec;
>       |                  ^~~~~~
> 
> Both variables are always initialized when @opt_tstamp == true and
> they're being used also only when @opt_tstamp == true. However, that
> variable comes from the BSS and is being toggled from another
> function. They can't be executed simultaneously to actually trigger
> undefined behaviour, but purely technically it is a correct warning.
> Just initialize them with zeroes.
> 
> Fixes: eb68db45b747 ("samples/bpf: xdpsock: Add timestamp for Tx-only operation")
> Signed-off-by: Alexander Lobakin <alobakin@pm.me>

Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

Magnus would tell you that you should fix this on libxdp side instead :)

> ---
>  samples/bpf/xdpsock_user.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/samples/bpf/xdpsock_user.c b/samples/bpf/xdpsock_user.c
> index 399b999fcec2..1dc7ad5dbef4 100644
> --- a/samples/bpf/xdpsock_user.c
> +++ b/samples/bpf/xdpsock_user.c
> @@ -1496,7 +1496,7 @@ static void rx_drop_all(void)
>  static int tx_only(struct xsk_socket_info *xsk, u32 *frame_nb,
>  		   int batch_size, unsigned long tx_ns)
>  {
> -	u32 idx, tv_sec, tv_usec;
> +	u32 idx, tv_sec = 0, tv_usec = 0;
>  	unsigned int i;
> 
>  	while (xsk_ring_prod__reserve(&xsk->tx, batch_size, &idx) <
> --
> 2.35.2
> 
> 

Re: [PATCH bpf-next 11/11] samples: bpf: xdpsock: fix -Wmaybe-uninitialized

[Song Liu]

On Thu, Apr 14, 2022 at 3:47 PM Alexander Lobakin <alobakin@pm.me> wrote:
>
> Fix two sort-of-false-positives in the xdpsock userspace part:
>
> samples/bpf/xdpsock_user.c: In function 'main':
> samples/bpf/xdpsock_user.c:1531:47: warning: 'tv_usec' may be used uninitialized in this function [-Wmaybe-uninitialized]
>  1531 |                         pktgen_hdr->tv_usec = htonl(tv_usec);
>       |                                               ^~~~~~~~~~~~~~
> samples/bpf/xdpsock_user.c:1500:26: note: 'tv_usec' was declared here
>  1500 |         u32 idx, tv_sec, tv_usec;
>       |                          ^~~~~~~
> samples/bpf/xdpsock_user.c:1530:46: warning: 'tv_sec' may be used uninitialized in this function [-Wmaybe-uninitialized]
>  1530 |                         pktgen_hdr->tv_sec = htonl(tv_sec);
>       |                                              ^~~~~~~~~~~~~
> samples/bpf/xdpsock_user.c:1500:18: note: 'tv_sec' was declared here
>  1500 |         u32 idx, tv_sec, tv_usec;
>       |                  ^~~~~~
>
> Both variables are always initialized when @opt_tstamp == true and
> they're being used also only when @opt_tstamp == true. However, that
> variable comes from the BSS and is being toggled from another
> function. They can't be executed simultaneously to actually trigger
> undefined behaviour, but purely technically it is a correct warning.
> Just initialize them with zeroes.
>
> Fixes: eb68db45b747 ("samples/bpf: xdpsock: Add timestamp for Tx-only operation")
> Signed-off-by: Alexander Lobakin <alobakin@pm.me>

Acked-by: Song Liu <songliubraving@fb.com>

> ---
>  samples/bpf/xdpsock_user.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/samples/bpf/xdpsock_user.c b/samples/bpf/xdpsock_user.c
> index 399b999fcec2..1dc7ad5dbef4 100644
> --- a/samples/bpf/xdpsock_user.c
> +++ b/samples/bpf/xdpsock_user.c
> @@ -1496,7 +1496,7 @@ static void rx_drop_all(void)
>  static int tx_only(struct xsk_socket_info *xsk, u32 *frame_nb,
>                    int batch_size, unsigned long tx_ns)
>  {
> -       u32 idx, tv_sec, tv_usec;
> +       u32 idx, tv_sec = 0, tv_usec = 0;
>         unsigned int i;
>
>         while (xsk_ring_prod__reserve(&xsk->tx, batch_size, &idx) <
> --
> 2.35.2
>
>

Re: [PATCH bpf-next 00/11] bpf: random unpopular userspace fixes (32 bit et al.)

[Alexei Starovoitov]

On Thu, Apr 14, 2022 at 3:44 PM Alexander Lobakin <alobakin@pm.me> wrote:

Please do not send encrypted patches.
Use plain text.

Also for bpf fixes please use [PATCH bpf] subject.
cc maintainers and bpf@vger only.
There is no need to spam such a huge list of people.

Re: [PATCH bpf-next 00/11] bpf: random unpopular userspace fixes (32 bit et al.)

[Alexander Lobakin]

From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Date: Sat, 16 Apr 2022 00:50:49 +0000

> On Thu, Apr 14, 2022 at 3:44 PM Alexander Lobakin <alobakin@pm.me> wrote:
>
> Please do not send encrypted patches.
> Use plain text.

Oof, weird. I use ProtonMail Bridge and they claim it doesn't
encrypt mails to non-Proton users. That's the first time I hear
such, I was sending several fixes to LKML a couple weeks ago
from this mail address with no issues (and got them accepted).

>
> Also for bpf fixes please use [PATCH bpf] subject.

I decided to go with bpf-next since it changes the layout of
&perf_event a bit when !CONFIG_PERF_EVENTS. But if you're okay
with taking this through the bpf tree, it's even better.

> cc maintainers and bpf@vger only.
> There is no need to spam such a huge list of people.

I usually redirect scripts/get_maintainers.pl to git-send-email
directly, but you're right, sure. Seems like it does overkills
sometimes.

Thanks,
Al

Re: [PATCH bpf-next 00/11] bpf: random unpopular userspace fixes (32 bit et al.)

[Alexei Starovoitov]

On Sat, Apr 16, 2022 at 11:01 AM Alexander Lobakin <alobakin@pm.me> wrote:
>
> From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Date: Sat, 16 Apr 2022 00:50:49 +0000
>
> > On Thu, Apr 14, 2022 at 3:44 PM Alexander Lobakin <alobakin@pm.me> wrote:
> >
> > Please do not send encrypted patches.
> > Use plain text.
>
> Oof, weird. I use ProtonMail Bridge and they claim it doesn't
> encrypt mails to non-Proton users. That's the first time I hear
> such, I was sending several fixes to LKML a couple weeks ago
> from this mail address with no issues (and got them accepted).

They come encrypted to gmail inbox. Shrug.

> >
> > Also for bpf fixes please use [PATCH bpf] subject.
>
> I decided to go with bpf-next since it changes the layout of
> &perf_event a bit when !CONFIG_PERF_EVENTS. But if you're okay
> with taking this through the bpf tree, it's even better.

I didn't see the patches. Only went with 'unpopular fixes' subject.
If they're not then bpf-next is certainly better.

Re: [PATCH bpf-next 00/11] bpf: random unpopular userspace fixes (32 bit et al.)

[Andrii Nakryiko]

On Thu, Apr 14, 2022 at 3:44 PM Alexander Lobakin <alobakin@pm.me> wrote:
>
> This mostly issues the cross build (1) errors for 32 bit (2)
> MIPS (3) with minimal configuration (4) on Musl (5). The majority
> of them aren't yesterday's, so it is a "who does need it outside
> of x86_64 or ARM64?" moment again.
> Trivial stuff in general, not counting the bpf_cookie build fix.
>
> Alexander Lobakin (11):
>   bpf, perf: fix bpftool compilation with !CONFIG_PERF_EVENTS
>   bpf: always emit struct bpf_perf_link BTF
>   tools, bpf: fix bpftool build with !CONFIG_BPF_EVENTS
>   samples: bpf: add 'asm/mach-generic' include path for every MIPS
>   samples: bpf: use host bpftool to generate vmlinux.h, not target
>   tools, bpf: fix fcntl.h include in bpftool
>   samples: bpf: fix uin64_t format literals
>   samples: bpf: fix shifting unsigned long by 32 positions
>   samples: bpf: fix include order for non-Glibc environments
>   samples: bpf: fix -Wsequence-point
>   samples: bpf: xdpsock: fix -Wmaybe-uninitialized
>

For consistency with majority of other commits, can you please use
"samples/bpf: " prefix for samples/bpf changes and "bpftool: " for
bpftool's ones? Thanks!

>  include/linux/perf_event.h              |  2 ++
>  kernel/bpf/syscall.c                    |  4 +++-
>  samples/bpf/Makefile                    |  7 ++++---
>  samples/bpf/cookie_uid_helper_example.c | 12 ++++++------
>  samples/bpf/lathist_kern.c              |  2 +-
>  samples/bpf/lwt_len_hist_kern.c         |  2 +-
>  samples/bpf/lwt_len_hist_user.c         |  4 ++--
>  samples/bpf/task_fd_query_user.c        |  2 +-
>  samples/bpf/test_lru_dist.c             |  3 ++-
>  samples/bpf/tracex2_kern.c              |  2 +-
>  samples/bpf/xdpsock_user.c              |  5 +++--
>  tools/bpf/bpftool/tracelog.c            |  2 +-
>  12 files changed, 27 insertions(+), 20 deletions(-)
>
> --
> 2.35.2
>
>