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  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
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