Re: [PATCH bpf-next 3/3] samples: bpf: refactor XDP kern program maps with BTF-defined map

From: "Daniel T. Lee" <danieltimlee@gmail.com>
To: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>,
	Alexei Starovoitov <ast@kernel.org>,
	Jesper Dangaard Brouer <brouer@redhat.com>,
	Lorenzo Bianconi <lorenzo@kernel.org>, bpf <bpf@vger.kernel.org>,
	Networking <netdev@vger.kernel.org>,
	Xdp <xdp-newbies@vger.kernel.org>
Subject: Re: [PATCH bpf-next 3/3] samples: bpf: refactor XDP kern program maps with BTF-defined map
Date: Sat, 10 Oct 2020 18:55:49 +0900	[thread overview]
Message-ID: <CAEKGpzgPepj2c=tvwgwH93OLDvq_GCDjGeoh_w3VCFwUuE=HPA@mail.gmail.com> (raw)
In-Reply-To: <CAEf4Bzai3y8yHcYAnSBhZ3Wa8nvDcsUw=1o5S-xn42DwPKUndQ@mail.gmail.com>

On Sat, Oct 10, 2020 at 3:25 AM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Fri, Oct 9, 2020 at 9:04 AM Daniel T. Lee <danieltimlee@gmail.com> wrote:
> >
> > Most of the samples were converted to use the new BTF-defined MAP as
> > they moved to libbpf, but some of the samples were missing.
> >
> > Instead of using the previous BPF MAP definition, this commit refactors
> > xdp_monitor and xdp_sample_pkts_kern MAP definition with the new
> > BTF-defined MAP format.
> >
> > Also, this commit removes the max_entries attribute at PERF_EVENT_ARRAY
> > map type. The libbpf's bpf_object__create_map() will automatically
> > set max_entries to the maximum configured number of CPUs on the host.
> >
> > Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com>
> > ---
> >  samples/bpf/xdp_monitor_kern.c     | 60 +++++++++++++++---------------
> >  samples/bpf/xdp_sample_pkts_kern.c | 14 +++----
> >  samples/bpf/xdp_sample_pkts_user.c |  1 -
> >  3 files changed, 36 insertions(+), 39 deletions(-)
> >
>
> [...]
>
> > --- a/samples/bpf/xdp_sample_pkts_kern.c
> > +++ b/samples/bpf/xdp_sample_pkts_kern.c
> > @@ -5,14 +5,12 @@
> >  #include <bpf/bpf_helpers.h>
> >
> >  #define SAMPLE_SIZE 64ul
> > -#define MAX_CPUS 128
> > -
> > -struct bpf_map_def SEC("maps") my_map = {
> > -       .type = BPF_MAP_TYPE_PERF_EVENT_ARRAY,
> > -       .key_size = sizeof(int),
> > -       .value_size = sizeof(u32),
> > -       .max_entries = MAX_CPUS,
> > -};
> > +
> > +struct {
> > +       __uint(type, BPF_MAP_TYPE_PERF_EVENT_ARRAY);
> > +       __type(key, int);
> > +       __type(value, u32);
>
>
> this actually will generate unnecessary libbpf warnings, because
> PERF_EVENT_ARRAY doesn't support BTF types for key/value. So use
> __uint(key_size, sizeof(int)) and __uint(value_size, sizeof(u32))
> instead.
>

Thanks for the great review!
I'll fix it right away and send the next version of patch.

> > +} my_map SEC(".maps");
> >
> >  SEC("xdp_sample")
> >  int xdp_sample_prog(struct xdp_md *ctx)
> > diff --git a/samples/bpf/xdp_sample_pkts_user.c b/samples/bpf/xdp_sample_pkts_user.c
> > index 991ef6f0880b..4b2a300c750c 100644
> > --- a/samples/bpf/xdp_sample_pkts_user.c
> > +++ b/samples/bpf/xdp_sample_pkts_user.c
> > @@ -18,7 +18,6 @@
> >
> >  #include "perf-sys.h"
> >
> > -#define MAX_CPUS 128
> >  static int if_idx;
> >  static char *if_name;
> >  static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
> > --
> > 2.25.1
> >