Re: [PATCH v2 bpf-next] libbpf: always specify expected_attach_type on program load if supported

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Apr 14, 2020 at 12:04 AM Song Liu <songliubraving@xxxxxx> wrote:
>
>
>
> > On Apr 13, 2020, at 9:56 PM, Andrii Nakryiko <andriin@xxxxxx> wrote:
> >
> > For some types of BPF programs that utilize expected_attach_type, libbpf won't
> > set load_attr.expected_attach_type, even if expected_attach_type is known from
> > section definition. This was done to preserve backwards compatibility with old
> > kernels that didn't recognize expected_attach_type attribute yet (which was
> > added in 5e43f899b03a ("bpf: Check attach type at prog load time"). But this
> > is problematic for some BPF programs that utilize never features that require
> > kernel to know specific expected_attach_type (e.g., extended set of return
> > codes for cgroup_skb/egress programs).
> >
> > This patch makes libbpf specify expected_attach_type by default, but also
> > detect support for this field in kernel and not set it during program load.
> > This allows to have a good metadata for bpf_program
> > (e.g., bpf_program__get_extected_attach_type()), but still work with old
> > kernels (for cases where it can work at all).
> >
> > Additionally, due to expected_attach_type being always set for recognized
> > program types, bpf_program__attach_cgroup doesn't have to do extra checks to
> > determine correct attach type, so remove that additional logic.
> >
> > Also adjust section_names selftest to account for this change.
> >
> > More detailed discussion can be found in [0].
> >
> >  [0] https://lore.kernel.org/bpf/20200412003604.GA15986@xxxxxxxxxxxxxxxxxxxxxxxxxxxxx/
> >
> > Reported-by: Andrey Ignatov <rdna@xxxxxx>
> > Signed-off-by: Andrii Nakryiko <andriin@xxxxxx>
>
> Acked-by: Song Liu <songliubraving@xxxxxx>
>
> With one nit below.
>
> > ---
> > v1->v2:
> > - fixed prog_type/expected_attach_type combo (Andrey);
> > - added comment explaining what we are doing in probe_exp_attach_type (Andrey).
> >
> > tools/lib/bpf/libbpf.c                        | 127 ++++++++++++------
> > .../selftests/bpf/prog_tests/section_names.c  |  42 +++---
> > 2 files changed, 110 insertions(+), 59 deletions(-)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index ff9174282a8c..c7393182e2ae 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -178,6 +178,8 @@ struct bpf_capabilities {
> >       __u32 array_mmap:1;
> >       /* BTF_FUNC_GLOBAL is supported */
> >       __u32 btf_func_global:1;
> > +     /* kernel support for expected_attach_type in BPF_PROG_LOAD */
> > +     __u32 exp_attach_type:1;
> > };
>
> [...]
>
> > -#define BPF_PROG_SEC_IMPL(string, ptype, eatype, is_attachable, btf, atype) \
> > -     { string, sizeof(string) - 1, ptype, eatype, is_attachable, btf, atype }
> > +#define BPF_PROG_SEC_IMPL(string, ptype, eatype, eatype_optional,        \
> > +                       attachable, attach_btf)                           \
> > +     {                                                                   \
> > +             .sec = string,                                              \
> > +             .len = sizeof(string) - 1,                                  \
> > +             .prog_type = ptype,                                         \
> > +             .sec = string,                                              \
>
> Two lines of ".sec = string".


*facepalm*, will fix in next version, once bpf-next is open.

>
> > +             .expected_attach_type = eatype,                             \
> > +             .is_exp_attach_type_optional = eatype_optional,             \
> > +             .is_attachable = attachable,                                \
> > +             .is_attach_btf = attach_btf,                                \
> > +     }
>



[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux