| Message ID | 6cce9b15a57345402bb94366434a5ac5609583b8.1671462951.git.lorenzo@kernel.org (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | BPF |
| Headers | show |
| Series | xdp: introduce xdp-feature support | expand |
On Mon, Dec 19, 2022 at 7:42 AM Lorenzo Bianconi <lorenzo@kernel.org> wrote: > > From: Kumar Kartikeya Dwivedi <memxor@gmail.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@kernel.org> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > Co-developed-by: Marek Majtyka <alardam@gmail.com> > Signed-off-by: Marek Majtyka <alardam@gmail.com> > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.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()? > > /* 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; [...]
> On Mon, Dec 19, 2022 at 7:42 AM Lorenzo Bianconi <lorenzo@kernel.org> wrote: > > > > From: Kumar Kartikeya Dwivedi <memxor@gmail.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@kernel.org> > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > > Co-developed-by: Marek Majtyka <alardam@gmail.com> > > Signed-off-by: Marek Majtyka <alardam@gmail.com> > > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.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 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? 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; > > [...]
On Tue, Jan 10, 2023 at 9:26 AM Lorenzo Bianconi <lorenzo@kernel.org> wrote: > > > On Mon, Dec 19, 2022 at 7:42 AM Lorenzo Bianconi <lorenzo@kernel.org> wrote: > > > > > > From: Kumar Kartikeya Dwivedi <memxor@gmail.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@kernel.org> > > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > > > Co-developed-by: Marek Majtyka <alardam@gmail.com> > > > Signed-off-by: Marek Majtyka <alardam@gmail.com> > > > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.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; > > > > [...]
> On Tue, Jan 10, 2023 at 9:26 AM Lorenzo Bianconi <lorenzo@kernel.org> wrote: > > > > > On Mon, Dec 19, 2022 at 7:42 AM Lorenzo Bianconi <lorenzo@kernel.org> wrote: > > > > > > > > From: Kumar Kartikeya Dwivedi <memxor@gmail.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@kernel.org> > > > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > > > > Co-developed-by: Marek Majtyka <alardam@gmail.com> > > > > Signed-off-by: Marek Majtyka <alardam@gmail.com> > > > > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.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; > > > > > > [...] >
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); /* 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; diff --git a/tools/lib/bpf/netlink.c b/tools/lib/bpf/netlink.c index 35104580870c..6fd424cde58b 100644 --- a/tools/lib/bpf/netlink.c +++ b/tools/lib/bpf/netlink.c @@ -9,6 +9,7 @@ #include <linux/if_ether.h> #include <linux/pkt_cls.h> #include <linux/rtnetlink.h> +#include <linux/xdp_features.h> #include <sys/socket.h> #include <errno.h> #include <time.h> @@ -41,6 +42,12 @@ struct xdp_id_md { struct xdp_link_info info; }; +struct xdp_features_md { + int ifindex; + __u32 *flags; + __u32 *flags_cnt; +}; + static int libbpf_netlink_open(__u32 *nl_pid) { struct sockaddr_nl sa; @@ -357,6 +364,39 @@ static int get_xdp_info(void *cookie, void *msg, struct nlattr **tb) return 0; } +static int bpf_get_xdp_features(void *cookie, void *msg, struct nlattr **tb) +{ + struct nlattr *xdp_tb[IFLA_XDP_FEATURES_BITS_WORD + 1]; + struct xdp_features_md *md = cookie; + struct ifinfomsg *ifinfo = msg; + int ret, i, words; + + if (md->ifindex && md->ifindex != ifinfo->ifi_index) + return 0; + + if (!tb[IFLA_XDP_FEATURES]) + return 0; + + words = min(XDP_FEATURES_WORDS, *md->flags_cnt); + for (i = 0; i < words; ++i) + md->flags[i] = 0; + + ret = libbpf_nla_parse_nested(xdp_tb, XDP_FEATURES_WORDS, + tb[IFLA_XDP_FEATURES], NULL); + if (ret) + return ret; + + *md->flags_cnt = words; + for (i = 0; i < words; ++i) { + if (!xdp_tb[IFLA_XDP_FEATURES_BITS_WORD]) + continue; + + md->flags[i] = libbpf_nla_getattr_u32(xdp_tb[IFLA_XDP_FEATURES_BITS_WORD]); + } + + return 0; +} + int bpf_xdp_query(int ifindex, int xdp_flags, struct bpf_xdp_query_opts *opts) { struct libbpf_nla_req req = { @@ -421,6 +461,28 @@ int bpf_xdp_query_id(int ifindex, int flags, __u32 *prog_id) return 0; } +int bpf_xdp_query_features(int ifindex, __u32 *xdp_fflags, __u32 *fflags_cnt) +{ + struct libbpf_nla_req req = { + .nh.nlmsg_len = NLMSG_LENGTH(sizeof(struct ifinfomsg)), + .nh.nlmsg_type = RTM_GETLINK, + .nh.nlmsg_flags = NLM_F_DUMP | NLM_F_REQUEST, + .ifinfo.ifi_family = AF_PACKET, + }; + struct xdp_features_md md = {}; + int ret; + + if (!xdp_fflags || !fflags_cnt) + return -EINVAL; + + md.ifindex = ifindex; + md.flags = xdp_fflags; + md.flags_cnt = fflags_cnt; + + ret = libbpf_netlink_send_recv(&req, __dump_link_nlmsg, + bpf_get_xdp_features, &md); + return libbpf_err(ret); +} typedef int (*qdisc_config_t)(struct libbpf_nla_req *req);