netdev - Re: [RFC bpf-next 6/8] libbpf: add API to get XDP/XSK supported features

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:   Fri, 13 Jan 2023 10:22:37 -0800
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Lorenzo Bianconi <lorenzo@...nel.org>
Cc:     bpf@...r.kernel.org, netdev@...r.kernel.org, ast@...nel.org,
        daniel@...earbox.net, andrii@...nel.org, davem@...emloft.net,
        kuba@...nel.org, hawk@...nel.org, pabeni@...hat.com,
        edumazet@...gle.com, toke@...hat.com, memxor@...il.com,
        alardam@...il.com, saeedm@...dia.com, anthony.l.nguyen@...el.com,
        gospo@...adcom.com, vladimir.oltean@....com, nbd@....name,
        john@...ozen.org, leon@...nel.org, simon.horman@...igine.com,
        aelior@...vell.com, christophe.jaillet@...adoo.fr,
        ecree.xilinx@...il.com, grygorii.strashko@...com, mst@...hat.com,
        bjorn@...nel.org, magnus.karlsson@...el.com,
        maciej.fijalkowski@...el.com, intel-wired-lan@...ts.osuosl.org,
        lorenzo.bianconi@...hat.com
Subject: Re: [RFC bpf-next 6/8] libbpf: add API to get XDP/XSK supported features

On Tue, Jan 10, 2023 at 9:26 AM Lorenzo Bianconi <lorenzo@...nel.org> wrote:
>
> > On Mon, Dec 19, 2022 at 7:42 AM Lorenzo Bianconi <lorenzo@...nel.org> wrote:
> > >
> > > From: Kumar Kartikeya Dwivedi <memxor@...il.com>
> > >
> > > Add functions to get XDP/XSK supported function of netdev over route
> > > netlink interface. These functions provide functionalities that are
> > > going to be used in upcoming change.
> > >
> > > The newly added bpf_xdp_query_features takes a fflags_cnt parameter,
> > > which denotes the number of elements in the output fflags array. This
> > > must be at least 1 and maybe greater than XDP_FEATURES_WORDS. The
> > > function only writes to words which is min of fflags_cnt and
> > > XDP_FEATURES_WORDS.
> > >
> > > Co-developed-by: Lorenzo Bianconi <lorenzo@...nel.org>
> > > Signed-off-by: Lorenzo Bianconi <lorenzo@...nel.org>
> > > Co-developed-by: Marek Majtyka <alardam@...il.com>
> > > Signed-off-by: Marek Majtyka <alardam@...il.com>
> > > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@...il.com>
> > > ---
> > >  tools/lib/bpf/libbpf.h   |  1 +
> > >  tools/lib/bpf/libbpf.map |  1 +
> > >  tools/lib/bpf/netlink.c  | 62 ++++++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 64 insertions(+)
> > >
> > > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> > > index eee883f007f9..9d102eb5007e 100644
> > > --- a/tools/lib/bpf/libbpf.h
> > > +++ b/tools/lib/bpf/libbpf.h
> > > @@ -967,6 +967,7 @@ LIBBPF_API int bpf_xdp_detach(int ifindex, __u32 flags,
> > >                               const struct bpf_xdp_attach_opts *opts);
> > >  LIBBPF_API int bpf_xdp_query(int ifindex, int flags, struct bpf_xdp_query_opts *opts);
> > >  LIBBPF_API int bpf_xdp_query_id(int ifindex, int flags, __u32 *prog_id);
> > > +LIBBPF_API int bpf_xdp_query_features(int ifindex, __u32 *fflags, __u32 *fflags_cnt);
> >
> > no need to add new API, just extend bpf_xdp_query()?
>
> Hi Andrii,
>
> AFAIK libbpf supports just NETLINK_ROUTE protocol. In order to connect with the
> genl family code shared by Jakub we need to add NETLINK_GENERIC protocol support
> to libbf. Is it ok to introduce a libmnl or libnl dependency in libbpf or do you
> prefer to add open code to just what we need?

I'd very much like to avoid any extra dependencies. But I also have no
clue how much new code we are talking about, tbh. Either way, the less
dependencies, the better, if the result is an acceptable amount of
extra code to maintain.

> I guess we should have a dedicated API to dump xdp features in this case since
> all the other code relies on NETLINK_ROUTE protocol. What do you think?
>

>From API standpoint it looks like an extension to bpf_xdp_query()
family of APIs, which is already extendable through opts. Which is why
I suggested that there is no need for new API. NETLINK_ROUTE vs
NETLINK_GENERIC seems like an internal implementation detail (but
again, I spent literally zero time trying to understand what's going
on here).

> Regards,
> Lorenzo
>
> >
> > >
> > >  /* TC related API */
> > >  enum bpf_tc_attach_point {
> > > diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> > > index 71bf5691a689..9c2abb58fa4b 100644
> > > --- a/tools/lib/bpf/libbpf.map
> > > +++ b/tools/lib/bpf/libbpf.map
> > > @@ -362,6 +362,7 @@ LIBBPF_1.0.0 {
> > >                 bpf_program__set_autoattach;
> > >                 btf__add_enum64;
> > >                 btf__add_enum64_value;
> > > +               bpf_xdp_query_features;
> > >                 libbpf_bpf_attach_type_str;
> > >                 libbpf_bpf_link_type_str;
> > >                 libbpf_bpf_map_type_str;
> >
> > [...]