lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 26 Apr 2022 23:58:54 +0800
From:   Yafang Shao <laoar.shao@...il.com>
To:     Daniel Borkmann <daniel@...earbox.net>
Cc:     Alexei Starovoitov <ast@...nel.org>,
        Andrii Nakryiko <andrii@...nel.org>, Martin Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        john fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...nel.org>, netdev <netdev@...r.kernel.org>,
        bpf <bpf@...r.kernel.org>
Subject: Re: [PATCH bpf-next 2/4] libbpf: Add helpers for pinning bpf prog
 through bpf object skeleton

On Mon, Apr 25, 2022 at 9:57 PM Daniel Borkmann <daniel@...earbox.net> wrote:
>
> On 4/23/22 4:00 PM, Yafang Shao wrote:
> > Currently there're helpers for allowing to open/load/attach BPF object
> > through BPF object skeleton. Let's also add helpers for pinning through
> > BPF object skeleton. It could simplify BPF userspace code which wants to
> > pin the progs into bpffs.
>
> Please elaborate some more on your use case/rationale for the commit message,
> do you have orchestration code that will rely on these specifically?
>

We have a bpf manager on our production environment to maintain the
bpf programs, some of which need to be pinned in bpffs, for example
tracing bpf programs, perf_event programs and other bpf hooks added by
ourselves for performance tuning.  These bpf programs don't need a
user agent, while they really work like a kernel module, that is why
we pin them. For these kinds of bpf programs, the bpf manager can help
to simplify the development and deployment.  Take the improvement on
development for example,  the user doesn't need to write userspace
code while he focuses on the kernel side only, and then bpf manager
will do all the other things. Below is a simple example,
   Step1, gen the skeleton for the user provided bpf object file,
              $ bpftool gen skeleton  test.bpf.o > simple.skel.h
   Step2, Compile the bpf object file into a runnable binary
              #include "simple.skel.h"

              #define SIMPLE_BPF_PIN(name, path)  \
              ({                                                              \
                  struct name##_bpf *obj;                      \
                  int err = 0;                                            \
                                                                              \
                  obj = name##_bpf__open();                \
                   if (!obj) {                                              \
                       err = -errno;                                    \
                       goto cleanup;                                 \
                    }                                                         \
                                                                              \
                    err = name##_bpf__load(obj);           \
                    if (err)                                                 \
                        goto cleanup;                                 \
                                                                               \
                     err = name##_bpf__attach(obj);       \
                     if (err)                                                \
                         goto cleanup;                                \
                                                                               \
                     err = name##_bpf__pin_prog(obj, path);      \
                     if (err)                                                \
                         goto cleanup;                                \
                                                                              \
                      goto end;                                         \
                                                                              \
                  cleanup:                                              \
                      name##_bpf__destroy(obj);            \
                  end:                                                     \
                      err;                                                  \
                   })

                   SIMPLE_BPF_PIN(test, "/sys/fs/bpf");

               As the userspace code of FD-based bpf objects are all
the same,  so we can abstract them as above.  The pathset means to add
the non-exist "name##_bpf__pin_prog(obj, path)" for it.

> > Signed-off-by: Yafang Shao <laoar.shao@...il.com>
> > ---
> >   tools/lib/bpf/libbpf.c   | 59 ++++++++++++++++++++++++++++++++++++++++
> >   tools/lib/bpf/libbpf.h   |  4 +++
> >   tools/lib/bpf/libbpf.map |  2 ++
> >   3 files changed, 65 insertions(+)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 13fcf91e9e0e..e7ed6c53c525 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -12726,6 +12726,65 @@ void bpf_object__detach_skeleton(struct bpf_object_skeleton *s)
> >       }
> >   }
> >
> > +int bpf_object__pin_skeleton_prog(struct bpf_object_skeleton *s,
> > +                               const char *path)
>
> These pin the link, not the prog itself, so the func name is a bit misleading? Also,
> what if is this needs to be more customized in future? It doesn't seem very generic.
>

Ah, it should be bpf_object__pin_skeleton_link().
I'm not sure if it can be extended to work for a non-auto-detachable
bpf program, but I know it is hard for pinning tc-bpf programs, which
is also running on our production environment, in this way.

> > +{
> > +     struct bpf_link *link;
> > +     int err;
> > +     int i;
> > +
> > +     if (!s->prog_cnt)
> > +             return libbpf_err(-EINVAL);
> > +
> > +     if (!path)
> > +             path = DEFAULT_BPFFS;
> > +
> > +     for (i = 0; i < s->prog_cnt; i++) {
> > +             char buf[PATH_MAX];
> > +             int len;
> > +
> > +             len = snprintf(buf, PATH_MAX, "%s/%s", path, s->progs[i].name);
> > +             if (len < 0) {
> > +                     err = -EINVAL;
> > +                     goto err_unpin_prog;
> > +             } else if (len >= PATH_MAX) {
> > +                     err = -ENAMETOOLONG;
> > +                     goto err_unpin_prog;
> > +             }
> > +
> > +             link = *s->progs[i].link;
> > +             if (!link) {
> > +                     err = -EINVAL;
> > +                     goto err_unpin_prog;
> > +             }
> > +
> > +             err = bpf_link__pin(link, buf);
> > +             if (err)
> > +                     goto err_unpin_prog;
> > +     }
> > +
> > +     return 0;
> > +
> > +err_unpin_prog:
> > +     bpf_object__unpin_skeleton_prog(s);
> > +
> > +     return libbpf_err(err);
> > +}
> > +
> > +void bpf_object__unpin_skeleton_prog(struct bpf_object_skeleton *s)
> > +{
> > +     struct bpf_link *link;
> > +     int i;
> > +
> > +     for (i = 0; i < s->prog_cnt; i++) {
> > +             link = *s->progs[i].link;
> > +             if (!link || !link->pin_path)
> > +                     continue;
> > +
> > +             bpf_link__unpin(link);
> > +     }
> > +}
> > +
> >   void bpf_object__destroy_skeleton(struct bpf_object_skeleton *s)
> >   {
> >       if (!s)
> > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> > index 3784867811a4..af44b0968cca 100644
> > --- a/tools/lib/bpf/libbpf.h
> > +++ b/tools/lib/bpf/libbpf.h
> > @@ -1427,6 +1427,10 @@ bpf_object__open_skeleton(struct bpf_object_skeleton *s,
> >   LIBBPF_API int bpf_object__load_skeleton(struct bpf_object_skeleton *s);
> >   LIBBPF_API int bpf_object__attach_skeleton(struct bpf_object_skeleton *s);
> >   LIBBPF_API void bpf_object__detach_skeleton(struct bpf_object_skeleton *s);
> > +LIBBPF_API int
> > +bpf_object__pin_skeleton_prog(struct bpf_object_skeleton *s, const char *path);
> > +LIBBPF_API void
> > +bpf_object__unpin_skeleton_prog(struct bpf_object_skeleton *s);
> >   LIBBPF_API void bpf_object__destroy_skeleton(struct bpf_object_skeleton *s);
>
> Please also add API documentation.
>

Sure.

> >   struct bpf_var_skeleton {
> > diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> > index 82f6d62176dd..4e3e37b84b3a 100644
> > --- a/tools/lib/bpf/libbpf.map
> > +++ b/tools/lib/bpf/libbpf.map
> > @@ -55,6 +55,8 @@ LIBBPF_0.0.1 {
> >               bpf_object__unload;
> >               bpf_object__unpin_maps;
> >               bpf_object__unpin_programs;
> > +             bpf_object__pin_skeleton_prog;
> > +             bpf_object__unpin_skeleton_prog;
>
> This would have to go under LIBBPF_0.8.0 if so.
>

Thanks, I will change it.

> >               bpf_perf_event_read_simple;
> >               bpf_prog_attach;
> >               bpf_prog_detach;
> >
>

-- 
Regards
Yafang

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ