| Message ID | a7e6e8da5b2ba24f44f0d5b44a234e2bf90220fd.1674737592.git.lorenzo@kernel.org |
|---|---|
| State | Awaiting Upstream |
| Headers | show |
| Series | xdp: introduce xdp-feature support | expand |
On Thu, Jan 26, 2023 at 4:59 AM Lorenzo Bianconi <lorenzo@kernel.org> wrote: > > Extend bpf_xdp_query routine in order to get XDP/XSK supported features > of netdev over route netlink interface. > Extend libbpf netlink implementation in order to support netlink_generic > protocol. > > Co-developed-by: Kumar Kartikeya Dwivedi <memxor@gmail.com> > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com> > Co-developed-by: Marek Majtyka <alardam@gmail.com> > Signed-off-by: Marek Majtyka <alardam@gmail.com> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > --- > tools/lib/bpf/libbpf.h | 3 +- > tools/lib/bpf/netlink.c | 99 +++++++++++++++++++++++++++++++++++++++++ > tools/lib/bpf/nlattr.h | 12 +++++ > 3 files changed, 113 insertions(+), 1 deletion(-) > > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h > index 898db26e42e9..29cb7040fa77 100644 > --- a/tools/lib/bpf/libbpf.h > +++ b/tools/lib/bpf/libbpf.h > @@ -982,9 +982,10 @@ struct bpf_xdp_query_opts { > __u32 hw_prog_id; /* output */ > __u32 skb_prog_id; /* output */ > __u8 attach_mode; /* output */ > + __u64 fflags; /* output */ > size_t :0; > }; > -#define bpf_xdp_query_opts__last_field attach_mode > +#define bpf_xdp_query_opts__last_field fflags is "fflags" an obvious name in this context? I'd expect "feature_flags", especially that there are already "flags". Is saving a few characters worth the confusion? > > LIBBPF_API int bpf_xdp_attach(int ifindex, int prog_fd, __u32 flags, > const struct bpf_xdp_attach_opts *opts); > diff --git a/tools/lib/bpf/netlink.c b/tools/lib/bpf/netlink.c > index d2468a04a6c3..674e4d61e67e 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/netdev.h> > #include <sys/socket.h> > #include <errno.h> > #include <time.h> > @@ -39,6 +40,12 @@ struct xdp_id_md { > int ifindex; > __u32 flags; > struct xdp_link_info info; > + __u64 fflags; > +}; > + > +struct xdp_features_md { > + int ifindex; > + __u64 flags; > }; > > static int libbpf_netlink_open(__u32 *nl_pid, int proto) [...] > int bpf_xdp_query(int ifindex, int xdp_flags, struct bpf_xdp_query_opts *opts) > { > struct libbpf_nla_req req = { > @@ -393,6 +460,38 @@ int bpf_xdp_query(int ifindex, int xdp_flags, struct bpf_xdp_query_opts *opts) > OPTS_SET(opts, skb_prog_id, xdp_id.info.skb_prog_id); > OPTS_SET(opts, attach_mode, xdp_id.info.attach_mode); > > + if (OPTS_HAS(opts, fflags)) { maybe invert condition, return early, reduce nesting of the following code? > + struct xdp_features_md md = { > + .ifindex = ifindex, > + }; > + __u16 id; > + > + err = libbpf_netlink_resolve_genl_family_id("netdev", > + sizeof("netdev"), > + &id); nit: if it fits under 100 characters, let's leave it on a single line > + if (err < 0) > + return libbpf_err(err); > + > + memset(&req, 0, sizeof(req)); > + req.nh.nlmsg_len = NLMSG_LENGTH(GENL_HDRLEN); > + req.nh.nlmsg_flags = NLM_F_REQUEST; > + req.nh.nlmsg_type = id; > + req.gnl.cmd = NETDEV_CMD_DEV_GET; > + req.gnl.version = 2; > + > + err = nlattr_add(&req, NETDEV_A_DEV_IFINDEX, &ifindex, > + sizeof(ifindex)); > + if (err < 0) > + return err; > + > + err = libbpf_netlink_send_recv(&req, NETLINK_GENERIC, > + parse_xdp_features, NULL, &md); > + if (err) > + return libbpf_err(err); > + > + opts->fflags = md.flags; > + } > + > return 0; > } > [...]
> On Thu, Jan 26, 2023 at 4:59 AM Lorenzo Bianconi <lorenzo@kernel.org> wrote: > > > > Extend bpf_xdp_query routine in order to get XDP/XSK supported features > > of netdev over route netlink interface. > > Extend libbpf netlink implementation in order to support netlink_generic > > protocol. > > > > Co-developed-by: Kumar Kartikeya Dwivedi <memxor@gmail.com> > > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com> > > Co-developed-by: Marek Majtyka <alardam@gmail.com> > > Signed-off-by: Marek Majtyka <alardam@gmail.com> > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > > --- > > tools/lib/bpf/libbpf.h | 3 +- > > tools/lib/bpf/netlink.c | 99 +++++++++++++++++++++++++++++++++++++++++ > > tools/lib/bpf/nlattr.h | 12 +++++ > > 3 files changed, 113 insertions(+), 1 deletion(-) > > > > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h > > index 898db26e42e9..29cb7040fa77 100644 > > --- a/tools/lib/bpf/libbpf.h > > +++ b/tools/lib/bpf/libbpf.h > > @@ -982,9 +982,10 @@ struct bpf_xdp_query_opts { > > __u32 hw_prog_id; /* output */ > > __u32 skb_prog_id; /* output */ > > __u8 attach_mode; /* output */ > > + __u64 fflags; /* output */ > > size_t :0; > > }; > > -#define bpf_xdp_query_opts__last_field attach_mode > > +#define bpf_xdp_query_opts__last_field fflags > > is "fflags" an obvious name in this context? I'd expect > "feature_flags", especially that there are already "flags". Is saving > a few characters worth the confusion? ack, I will fix it. > > > > > > LIBBPF_API int bpf_xdp_attach(int ifindex, int prog_fd, __u32 flags, > > const struct bpf_xdp_attach_opts *opts); > > diff --git a/tools/lib/bpf/netlink.c b/tools/lib/bpf/netlink.c > > index d2468a04a6c3..674e4d61e67e 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/netdev.h> > > #include <sys/socket.h> > > #include <errno.h> > > #include <time.h> > > @@ -39,6 +40,12 @@ struct xdp_id_md { > > int ifindex; > > __u32 flags; > > struct xdp_link_info info; > > + __u64 fflags; > > +}; > > + > > +struct xdp_features_md { > > + int ifindex; > > + __u64 flags; > > }; > > > > static int libbpf_netlink_open(__u32 *nl_pid, int proto) > > [...] > > > int bpf_xdp_query(int ifindex, int xdp_flags, struct bpf_xdp_query_opts *opts) > > { > > struct libbpf_nla_req req = { > > @@ -393,6 +460,38 @@ int bpf_xdp_query(int ifindex, int xdp_flags, struct bpf_xdp_query_opts *opts) > > OPTS_SET(opts, skb_prog_id, xdp_id.info.skb_prog_id); > > OPTS_SET(opts, attach_mode, xdp_id.info.attach_mode); > > > > + if (OPTS_HAS(opts, fflags)) { > > maybe invert condition, return early, reduce nesting of the following code? ack, fine, I will fix it. > > > + struct xdp_features_md md = { > > + .ifindex = ifindex, > > + }; > > + __u16 id; > > + > > + err = libbpf_netlink_resolve_genl_family_id("netdev", > > + sizeof("netdev"), > > + &id); > > nit: if it fits under 100 characters, let's leave it on a single line ack, fine, I will fix it (I am still used to 79 char limits :)) Regards, Lorenzo > > > + if (err < 0) > > + return libbpf_err(err); > > + > > + memset(&req, 0, sizeof(req)); > > + req.nh.nlmsg_len = NLMSG_LENGTH(GENL_HDRLEN); > > + req.nh.nlmsg_flags = NLM_F_REQUEST; > > + req.nh.nlmsg_type = id; > > + req.gnl.cmd = NETDEV_CMD_DEV_GET; > > + req.gnl.version = 2; > > + > > + err = nlattr_add(&req, NETDEV_A_DEV_IFINDEX, &ifindex, > > + sizeof(ifindex)); > > + if (err < 0) > > + return err; > > + > > + err = libbpf_netlink_send_recv(&req, NETLINK_GENERIC, > > + parse_xdp_features, NULL, &md); > > + if (err) > > + return libbpf_err(err); > > + > > + opts->fflags = md.flags; > > + } > > + > > return 0; > > } > > > > [...]
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h index 898db26e42e9..29cb7040fa77 100644 --- a/tools/lib/bpf/libbpf.h +++ b/tools/lib/bpf/libbpf.h @@ -982,9 +982,10 @@ struct bpf_xdp_query_opts { __u32 hw_prog_id; /* output */ __u32 skb_prog_id; /* output */ __u8 attach_mode; /* output */ + __u64 fflags; /* output */ size_t :0; }; -#define bpf_xdp_query_opts__last_field attach_mode +#define bpf_xdp_query_opts__last_field fflags LIBBPF_API int bpf_xdp_attach(int ifindex, int prog_fd, __u32 flags, const struct bpf_xdp_attach_opts *opts); diff --git a/tools/lib/bpf/netlink.c b/tools/lib/bpf/netlink.c index d2468a04a6c3..674e4d61e67e 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/netdev.h> #include <sys/socket.h> #include <errno.h> #include <time.h> @@ -39,6 +40,12 @@ struct xdp_id_md { int ifindex; __u32 flags; struct xdp_link_info info; + __u64 fflags; +}; + +struct xdp_features_md { + int ifindex; + __u64 flags; }; static int libbpf_netlink_open(__u32 *nl_pid, int proto) @@ -238,6 +245,43 @@ static int libbpf_netlink_send_recv(struct libbpf_nla_req *req, return ret; } +static int parse_genl_family_id(struct nlmsghdr *nh, libbpf_dump_nlmsg_t fn, + void *cookie) +{ + struct genlmsghdr *gnl = NLMSG_DATA(nh); + struct nlattr *na = (struct nlattr *)((void *)gnl + GENL_HDRLEN); + struct nlattr *tb[CTRL_ATTR_FAMILY_ID + 1]; + __u16 *id = cookie; + + libbpf_nla_parse(tb, CTRL_ATTR_FAMILY_ID, na, + NLMSG_PAYLOAD(nh, sizeof(*gnl)), NULL); + if (!tb[CTRL_ATTR_FAMILY_ID]) + return NL_CONT; + + *id = libbpf_nla_getattr_u16(tb[CTRL_ATTR_FAMILY_ID]); + return NL_DONE; +} + +static int libbpf_netlink_resolve_genl_family_id(const char *name, + __u16 len, __u16 *id) +{ + struct libbpf_nla_req req = { + .nh.nlmsg_len = NLMSG_LENGTH(GENL_HDRLEN), + .nh.nlmsg_type = GENL_ID_CTRL, + .nh.nlmsg_flags = NLM_F_REQUEST, + .gnl.cmd = CTRL_CMD_GETFAMILY, + .gnl.version = 2, + }; + int err; + + err = nlattr_add(&req, CTRL_ATTR_FAMILY_NAME, name, len); + if (err < 0) + return err; + + return libbpf_netlink_send_recv(&req, NETLINK_GENERIC, + parse_genl_family_id, NULL, id); +} + static int __bpf_set_link_xdp_fd_replace(int ifindex, int fd, int old_fd, __u32 flags) { @@ -357,6 +401,29 @@ static int get_xdp_info(void *cookie, void *msg, struct nlattr **tb) return 0; } +static int parse_xdp_features(struct nlmsghdr *nh, libbpf_dump_nlmsg_t fn, + void *cookie) +{ + struct genlmsghdr *gnl = NLMSG_DATA(nh); + struct nlattr *na = (struct nlattr *)((void *)gnl + GENL_HDRLEN); + struct nlattr *tb[NETDEV_CMD_MAX + 1]; + struct xdp_features_md *md = cookie; + __u32 ifindex; + + libbpf_nla_parse(tb, NETDEV_CMD_MAX, na, + NLMSG_PAYLOAD(nh, sizeof(*gnl)), NULL); + + if (!tb[NETDEV_A_DEV_IFINDEX] || !tb[NETDEV_A_DEV_XDP_FEATURES]) + return NL_CONT; + + ifindex = libbpf_nla_getattr_u32(tb[NETDEV_A_DEV_IFINDEX]); + if (ifindex != md->ifindex) + return NL_CONT; + + md->flags = libbpf_nla_getattr_u64(tb[NETDEV_A_DEV_XDP_FEATURES]); + return NL_DONE; +} + int bpf_xdp_query(int ifindex, int xdp_flags, struct bpf_xdp_query_opts *opts) { struct libbpf_nla_req req = { @@ -393,6 +460,38 @@ int bpf_xdp_query(int ifindex, int xdp_flags, struct bpf_xdp_query_opts *opts) OPTS_SET(opts, skb_prog_id, xdp_id.info.skb_prog_id); OPTS_SET(opts, attach_mode, xdp_id.info.attach_mode); + if (OPTS_HAS(opts, fflags)) { + struct xdp_features_md md = { + .ifindex = ifindex, + }; + __u16 id; + + err = libbpf_netlink_resolve_genl_family_id("netdev", + sizeof("netdev"), + &id); + if (err < 0) + return libbpf_err(err); + + memset(&req, 0, sizeof(req)); + req.nh.nlmsg_len = NLMSG_LENGTH(GENL_HDRLEN); + req.nh.nlmsg_flags = NLM_F_REQUEST; + req.nh.nlmsg_type = id; + req.gnl.cmd = NETDEV_CMD_DEV_GET; + req.gnl.version = 2; + + err = nlattr_add(&req, NETDEV_A_DEV_IFINDEX, &ifindex, + sizeof(ifindex)); + if (err < 0) + return err; + + err = libbpf_netlink_send_recv(&req, NETLINK_GENERIC, + parse_xdp_features, NULL, &md); + if (err) + return libbpf_err(err); + + opts->fflags = md.flags; + } + return 0; } diff --git a/tools/lib/bpf/nlattr.h b/tools/lib/bpf/nlattr.h index 4d15ae2ff812..d92d1c1de700 100644 --- a/tools/lib/bpf/nlattr.h +++ b/tools/lib/bpf/nlattr.h @@ -14,6 +14,7 @@ #include <errno.h> #include <linux/netlink.h> #include <linux/rtnetlink.h> +#include <linux/genetlink.h> /* avoid multiple definition of netlink features */ #define __LINUX_NETLINK_H @@ -58,6 +59,7 @@ struct libbpf_nla_req { union { struct ifinfomsg ifinfo; struct tcmsg tc; + struct genlmsghdr gnl; }; char buf[128]; }; @@ -89,11 +91,21 @@ static inline uint8_t libbpf_nla_getattr_u8(const struct nlattr *nla) return *(uint8_t *)libbpf_nla_data(nla); } +static inline uint16_t libbpf_nla_getattr_u16(const struct nlattr *nla) +{ + return *(uint16_t *)libbpf_nla_data(nla); +} + static inline uint32_t libbpf_nla_getattr_u32(const struct nlattr *nla) { return *(uint32_t *)libbpf_nla_data(nla); } +static inline uint64_t libbpf_nla_getattr_u64(const struct nlattr *nla) +{ + return *(uint64_t *)libbpf_nla_data(nla); +} + static inline const char *libbpf_nla_getattr_str(const struct nlattr *nla) { return (const char *)libbpf_nla_data(nla);