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] [day] [month] [year] [list]
Message-ID: <Y8LLN5qiTDlLNQcK@lore-desk>
Date:   Sat, 14 Jan 2023 16:33:11 +0100
From:   Lorenzo Bianconi <lorenzo.bianconi@...hat.com>
To:     Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc:     Lorenzo Bianconi <lorenzo@...nel.org>, 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
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.

ack, I avoided to introduce an extra dependencies since most of the protocol
is already implemented in libbpf and I added just few code.

> 
> > 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).

ack, I extended bpf_xdp_query routine instead of adding a new API.

Regards,
Lorenzo

> 
> > 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;
> > >
> > > [...]
> 

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)