Date:   Fri, 25 Oct 2019 10:28:35 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Jesper Dangaard Brouer <brouer@...hat.com>
Cc:     Toke Høiland-Jørgensen <toke@...hat.com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Alexei Starovoitov <ast@...nel.org>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        David Miller <davem@...emloft.net>,
        Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>
Subject: Re: [PATCH bpf-next v2 3/4] libbpf: Support configurable pinning of
 maps from BTF annotations

On Fri, Oct 25, 2019 at 5:32 AM Jesper Dangaard Brouer
<brouer@...hat.com> wrote:
>
> On Thu, 24 Oct 2019 15:11:40 +0200
> Toke Høiland-Jørgensen <toke@...hat.com> wrote:
>
> > From: Toke Høiland-Jørgensen <toke@...hat.com>
> >
> > This adds support to libbpf for setting map pinning information as part of
> > the BTF map declaration. We introduce a version new
> > bpf_object__map_pin_opts() function to pin maps based on this setting, as
> > well as a getter and setter function for the pin information that callers
> > can use after map load.
> >
> > The pinning type currently only supports a single PIN_BY_NAME mode, where
> > each map will be pinned by its name in a path that can be overridden, but
> > defaults to /sys/fs/bpf.
> >
> > The pinning options supports a 'pin_all' setting, which corresponds to the
> > old bpf_object__map_pin() function with an explicit path. In addition, the
> > function now defaults to just skipping over maps that are already
> > pinned (since the previous commit started recording this in struct
> > bpf_map). This behaviour can be turned off with the 'no_skip_pinned' option.
> >
> > Signed-off-by: Toke Høiland-Jørgensen <toke@...hat.com>
> > ---
> >  tools/lib/bpf/bpf_helpers.h |    6 ++
> >  tools/lib/bpf/libbpf.c      |  134 ++++++++++++++++++++++++++++++++++---------
> >  tools/lib/bpf/libbpf.h      |   26 ++++++++
> >  tools/lib/bpf/libbpf.map    |    3 +
> >  4 files changed, 142 insertions(+), 27 deletions(-)
> >
> > diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
> > index 2203595f38c3..0c7d28292898 100644
> > --- a/tools/lib/bpf/bpf_helpers.h
> > +++ b/tools/lib/bpf/bpf_helpers.h
> > @@ -38,4 +38,10 @@ struct bpf_map_def {
> >       unsigned int map_flags;
> >  };
> >
> > +enum libbpf_pin_type {
> > +     LIBBPF_PIN_NONE,
> > +     /* PIN_BY_NAME: pin maps by name (in /sys/fs/bpf by default) */
> > +     LIBBPF_PIN_BY_NAME,
> > +};
> > +
> >  #endif
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 848e6710b8e6..179c9911458d 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -226,6 +226,7 @@ struct bpf_map {
> >       void *priv;
> >       bpf_map_clear_priv_t clear_priv;
> >       enum libbpf_map_type libbpf_type;
> > +     enum libbpf_pin_type pinning;
> >       char *pin_path;
> >       bool pinned;
> >  };
> > @@ -1271,6 +1272,22 @@ static int bpf_object__init_user_btf_map(struct bpf_object *obj,
> >                       }
> >                       map->def.value_size = sz;
> >                       map->btf_value_type_id = t->type;
> > +             } else if (strcmp(name, "pinning") == 0) {
> > +                     __u32 val;
> > +
> > +                     if (!get_map_field_int(map_name, obj->btf, def, m,
> > +                                            &val))
> > +                             return -EINVAL;
> > +                     pr_debug("map '%s': found pinning = %u.\n",
> > +                              map_name, val);
> > +
> > +                     if (val != LIBBPF_PIN_NONE &&
> > +                         val != LIBBPF_PIN_BY_NAME) {
> > +                             pr_warning("map '%s': invalid pinning value %u.\n",
> > +                                        map_name, val);
> > +                             return -EINVAL;
> > +                     }
> > +                     map->pinning = val;
> >               } else {
> >                       if (strict) {
> >                               pr_warning("map '%s': unknown field '%s'.\n",
> [...]
>
> How does this prepare for being compatible with iproute2 pinning?
>
> iproute2 have these defines (in include/bpf_elf.h):
>
>  /* Object pinning settings */
>  #define PIN_NONE                0
>  #define PIN_OBJECT_NS           1
>  #define PIN_GLOBAL_NS           2
>
> I do know above strcmp(name, "pinning") look at BTF info 'name' and not
> directly at the ELF struct for maps.  I don't know enough about BTF
> (yet), but won't BTF automatically create a "pinning" info 'name' ???
> (with above defines as content/values)

We are not supporting iproute2's BTF map definitions as is, we are
trying to support all the functionality needed to support iproute2's
ways of doing things, but it will require iproute2 to do some gluing,
of course. We don't intend to support any conceivable legacy format
out there in libbpf, rather make libbpf powerful, flexible, and
expressive enough to support those use case, but with the help of
tools like iprout2 to do "translations" necessary.

For "modern" iprout2 BPF programs that are using BTF-defined maps and
stuff - yes, that will work as is without iproute2 having to do much.

>
> --
> Best regards,
>   Jesper Dangaard Brouer
>   MSc.CS, Principal Kernel Engineer at Red Hat
>   LinkedIn: http://www.linkedin.com/in/brouer
>
> From above enum:
>  LIBBPF_PIN_BY_NAME = 1
>
> iproute2 ELF map struct:
>
> /* ELF map definition */
> struct bpf_elf_map {
>         __u32 type;
>         __u32 size_key;
>         __u32 size_value;
>         __u32 max_elem;
>         __u32 flags;
>         __u32 id;
>         __u32 pinning;
>         __u32 inner_id;
>         __u32 inner_idx;
> };
>

Powered by blists - more mailing lists