| 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
| ||
|
Date: Fri, 8 Nov 2019 15:10:47 -0800
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Toke Høiland-Jørgensen <toke@...hat.com>
Cc: Daniel Borkmann <daniel@...earbox.net>,
Alexei Starovoitov <ast@...nel.org>,
Martin KaFai Lau <kafai@...com>,
Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
Jesper Dangaard Brouer <brouer@...hat.com>,
David Miller <davem@...emloft.net>,
Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>
Subject: Re: [PATCH bpf-next v2 5/6] libbpf: Add bpf_get_link_xdp_info()
function to get more XDP information
On Fri, Nov 8, 2019 at 1:33 PM Toke Høiland-Jørgensen <toke@...hat.com> wrote:
>
> From: Toke Høiland-Jørgensen <toke@...hat.com>
>
> Currently, libbpf only provides a function to get a single ID for the XDP
> program attached to the interface. However, it can be useful to get the
> full set of program IDs attached, along with the attachment mode, in one
> go. Add a new getter function to support this, using an extendible
> structure to carry the information. Express the old bpf_get_link_id()
> function in terms of the new function.
>
> Acked-by: Song Liu <songliubraving@...com>
> Signed-off-by: Toke Høiland-Jørgensen <toke@...hat.com>
> ---
> tools/lib/bpf/libbpf.h | 10 ++++++
> tools/lib/bpf/libbpf.map | 1 +
> tools/lib/bpf/netlink.c | 78 ++++++++++++++++++++++++++++++----------------
> 3 files changed, 62 insertions(+), 27 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 6ddc0419337b..f0947cc949d2 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -427,8 +427,18 @@ LIBBPF_API int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
> LIBBPF_API int bpf_prog_load(const char *file, enum bpf_prog_type type,
> struct bpf_object **pobj, int *prog_fd);
>
> +struct xdp_link_info {
> + __u32 prog_id;
> + __u32 drv_prog_id;
> + __u32 hw_prog_id;
> + __u32 skb_prog_id;
> + __u8 attach_mode;
> +};
> +
> LIBBPF_API int bpf_set_link_xdp_fd(int ifindex, int fd, __u32 flags);
> LIBBPF_API int bpf_get_link_xdp_id(int ifindex, __u32 *prog_id, __u32 flags);
> +LIBBPF_API int bpf_get_link_xdp_info(int ifindex, struct xdp_link_info *info,
> + size_t info_size, __u32 flags);
>
> struct perf_buffer;
>
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index 86173cbb159d..d1a782a3a58d 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -193,6 +193,7 @@ LIBBPF_0.0.5 {
>
> LIBBPF_0.0.6 {
> global:
> + bpf_get_link_xdp_info;
> bpf_map__get_pin_path;
> bpf_map__is_pinned;
> bpf_map__set_pin_path;
> diff --git a/tools/lib/bpf/netlink.c b/tools/lib/bpf/netlink.c
> index a261df9cb488..85019da01d3b 100644
> --- a/tools/lib/bpf/netlink.c
> +++ b/tools/lib/bpf/netlink.c
> @@ -25,7 +25,7 @@ typedef int (*__dump_nlmsg_t)(struct nlmsghdr *nlmsg, libbpf_dump_nlmsg_t,
> struct xdp_id_md {
> int ifindex;
> __u32 flags;
> - __u32 id;
> + struct xdp_link_info info;
> };
>
> int libbpf_netlink_open(__u32 *nl_pid)
> @@ -203,26 +203,11 @@ static int __dump_link_nlmsg(struct nlmsghdr *nlh,
> return dump_link_nlmsg(cookie, ifi, tb);
> }
>
> -static unsigned char get_xdp_id_attr(unsigned char mode, __u32 flags)
> -{
> - if (mode != XDP_ATTACHED_MULTI)
> - return IFLA_XDP_PROG_ID;
> - if (flags & XDP_FLAGS_DRV_MODE)
> - return IFLA_XDP_DRV_PROG_ID;
> - if (flags & XDP_FLAGS_HW_MODE)
> - return IFLA_XDP_HW_PROG_ID;
> - if (flags & XDP_FLAGS_SKB_MODE)
> - return IFLA_XDP_SKB_PROG_ID;
> -
> - return IFLA_XDP_UNSPEC;
> -}
> -
> -static int get_xdp_id(void *cookie, void *msg, struct nlattr **tb)
> +static int get_xdp_info(void *cookie, void *msg, struct nlattr **tb)
> {
> struct nlattr *xdp_tb[IFLA_XDP_MAX + 1];
> struct xdp_id_md *xdp_id = cookie;
> struct ifinfomsg *ifinfo = msg;
> - unsigned char mode, xdp_attr;
> int ret;
>
> if (xdp_id->ifindex && xdp_id->ifindex != ifinfo->ifi_index)
> @@ -238,27 +223,40 @@ static int get_xdp_id(void *cookie, void *msg, struct nlattr **tb)
> if (!xdp_tb[IFLA_XDP_ATTACHED])
> return 0;
>
> - mode = libbpf_nla_getattr_u8(xdp_tb[IFLA_XDP_ATTACHED]);
> - if (mode == XDP_ATTACHED_NONE)
> - return 0;
> + xdp_id->info.attach_mode = libbpf_nla_getattr_u8(
> + xdp_tb[IFLA_XDP_ATTACHED]);
>
> - xdp_attr = get_xdp_id_attr(mode, xdp_id->flags);
> - if (!xdp_attr || !xdp_tb[xdp_attr])
> + if (xdp_id->info.attach_mode == XDP_ATTACHED_NONE)
> return 0;
>
> - xdp_id->id = libbpf_nla_getattr_u32(xdp_tb[xdp_attr]);
> + if (xdp_tb[IFLA_XDP_PROG_ID])
> + xdp_id->info.prog_id = libbpf_nla_getattr_u32(
> + xdp_tb[IFLA_XDP_PROG_ID]);
> +
> + if (xdp_tb[IFLA_XDP_SKB_PROG_ID])
> + xdp_id->info.skb_prog_id = libbpf_nla_getattr_u32(
> + xdp_tb[IFLA_XDP_SKB_PROG_ID]);
> +
> + if (xdp_tb[IFLA_XDP_DRV_PROG_ID])
> + xdp_id->info.drv_prog_id = libbpf_nla_getattr_u32(
> + xdp_tb[IFLA_XDP_DRV_PROG_ID]);
> +
> + if (xdp_tb[IFLA_XDP_HW_PROG_ID])
> + xdp_id->info.hw_prog_id = libbpf_nla_getattr_u32(
> + xdp_tb[IFLA_XDP_HW_PROG_ID]);
>
> return 0;
> }
>
> -int bpf_get_link_xdp_id(int ifindex, __u32 *prog_id, __u32 flags)
> +int bpf_get_link_xdp_info(int ifindex, struct xdp_link_info *info,
> + size_t info_size, __u32 flags)
> {
> struct xdp_id_md xdp_id = {};
> int sock, ret;
> __u32 nl_pid;
> __u32 mask;
>
> - if (flags & ~XDP_FLAGS_MASK)
> + if (flags & ~XDP_FLAGS_MASK || info_size != sizeof(*info))
This is not forward-compatible. Newer application can pass bigger
info_size, if xdp_link_info gets extended. We should probably just
zero-fill the part we don't understand.
> return -EINVAL;
>
> /* Check whether the single {HW,DRV,SKB} mode is set */
> @@ -274,14 +272,40 @@ int bpf_get_link_xdp_id(int ifindex, __u32 *prog_id, __u32 flags)
> xdp_id.ifindex = ifindex;
> xdp_id.flags = flags;
>
> - ret = libbpf_nl_get_link(sock, nl_pid, get_xdp_id, &xdp_id);
> + ret = libbpf_nl_get_link(sock, nl_pid, get_xdp_info, &xdp_id);
> if (!ret)
> - *prog_id = xdp_id.id;
> + memcpy(info, &xdp_id.info, sizeof(*info));
>
> close(sock);
> return ret;
> }
>
> +static __u32 get_xdp_id(struct xdp_link_info *info, __u32 flags)
> +{
> + if (info->attach_mode != XDP_ATTACHED_MULTI)
> + return info->prog_id;
> + if (flags & XDP_FLAGS_DRV_MODE)
> + return info->drv_prog_id;
> + if (flags & XDP_FLAGS_HW_MODE)
> + return info->hw_prog_id;
> + if (flags & XDP_FLAGS_SKB_MODE)
> + return info->skb_prog_id;
> +
> + return 0;
> +}
> +
> +int bpf_get_link_xdp_id(int ifindex, __u32 *prog_id, __u32 flags)
> +{
> + struct xdp_link_info info = {};
seems like there is no need to pre-initialize info, it should be
initialized (on success) by bpf_get_link_xdp_info?
> + int ret;
> +
> + ret = bpf_get_link_xdp_info(ifindex, &info, sizeof(info), flags);
> + if (!ret)
> + *prog_id = get_xdp_id(&info, flags);
> +
> + return ret;
> +}
> +
> int libbpf_nl_get_link(int sock, unsigned int nl_pid,
> libbpf_dump_nlmsg_t dump_link_nlmsg, void *cookie)
> {
>
Powered by blists - more mailing lists