netdev - Re: [PATCH bpf-next v2 3/4] libbpf: Add function to set link XDP fd while specifying old fd

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: <CAEf4BzZ17_4tZfMw1DdkXT7hHBpj8nrRufm6WopBnsU7a9-evA@mail.gmail.com>
Date:   Fri, 20 Mar 2020 14:50:43 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Toke Høiland-Jørgensen <toke@...hat.com>
Cc:     Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        Andrii Nakryiko <andriin@...com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Jesper Dangaard Brouer <brouer@...hat.com>,
        John Fastabend <john.fastabend@...il.com>,
        Lorenz Bauer <lmb@...udflare.com>,
        Andrey Ignatov <rdna@...com>,
        Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>
Subject: Re: [PATCH bpf-next v2 3/4] libbpf: Add function to set link XDP fd
 while specifying old fd

On Fri, Mar 20, 2020 at 9:57 AM Toke Høiland-Jørgensen <toke@...hat.com> wrote:
>
> From: Toke Høiland-Jørgensen <toke@...hat.com>
>
> This adds a new function to set the XDP fd while specifying the old fd to
> replace, using the newly added IFLA_XDP_EXPECTED_FD netlink parameter.
>
> Signed-off-by: Toke Høiland-Jørgensen <toke@...hat.com>
> ---
>  tools/lib/bpf/libbpf.h   |    2 ++
>  tools/lib/bpf/libbpf.map |    1 +
>  tools/lib/bpf/netlink.c  |   22 +++++++++++++++++++++-
>  3 files changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index d38d7a629417..b5ca4f741e28 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -445,6 +445,8 @@ struct xdp_link_info {
>  };
>
>  LIBBPF_API int bpf_set_link_xdp_fd(int ifindex, int fd, __u32 flags);
> +LIBBPF_API int bpf_set_link_xdp_fd_replace(int ifindex, int fd, int old_fd,
> +                                          __u32 flags);

If we end up doing it in XDP-specific way, I'd argue for using OPTS to
specify optional expected FD. That will allow to extend this API
further without adding many small variations of the same API.

>  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);
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index 5129283c0284..154f1d94fa63 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -244,4 +244,5 @@ LIBBPF_0.0.8 {
>                 bpf_link__pin_path;
>                 bpf_link__unpin;
>                 bpf_program__set_attach_target;
> +               bpf_set_link_xdp_fd_replace;
>  } LIBBPF_0.0.7;
> diff --git a/tools/lib/bpf/netlink.c b/tools/lib/bpf/netlink.c
> index 431bd25c6cdb..39bd0ead1546 100644
> --- a/tools/lib/bpf/netlink.c
> +++ b/tools/lib/bpf/netlink.c
> @@ -132,7 +132,8 @@ static int bpf_netlink_recv(int sock, __u32 nl_pid, int seq,
>         return ret;
>  }
>
> -int bpf_set_link_xdp_fd(int ifindex, int fd, __u32 flags)
> +static int __bpf_set_link_xdp_fd_replace(int ifindex, int fd, int old_fd,
> +                                        __u32 flags)
>  {
>         int sock, seq = 0, ret;
>         struct nlattr *nla, *nla_xdp;
> @@ -178,6 +179,14 @@ int bpf_set_link_xdp_fd(int ifindex, int fd, __u32 flags)
>                 nla->nla_len += nla_xdp->nla_len;
>         }
>
> +       if (flags & XDP_FLAGS_EXPECT_FD) {
> +               nla_xdp = (struct nlattr *)((char *)nla + nla->nla_len);
> +               nla_xdp->nla_type = IFLA_XDP_EXPECTED_FD;
> +               nla_xdp->nla_len = NLA_HDRLEN + sizeof(int);
> +               memcpy((char *)nla_xdp + NLA_HDRLEN, &old_fd, sizeof(old_fd));
> +               nla->nla_len += nla_xdp->nla_len;
> +       }
> +
>         req.nh.nlmsg_len += NLA_ALIGN(nla->nla_len);
>
>         if (send(sock, &req, req.nh.nlmsg_len, 0) < 0) {
> @@ -191,6 +200,17 @@ int bpf_set_link_xdp_fd(int ifindex, int fd, __u32 flags)
>         return ret;
>  }
>
> +int bpf_set_link_xdp_fd_replace(int ifindex, int fd, int old_fd, __u32 flags)
> +{
> +       return __bpf_set_link_xdp_fd_replace(ifindex, fd, old_fd,
> +                                            flags | XDP_FLAGS_EXPECT_FD);
> +}
> +
> +int bpf_set_link_xdp_fd(int ifindex, int fd, __u32 flags)
> +{
> +       return __bpf_set_link_xdp_fd_replace(ifindex, fd, -1, flags);
> +}
> +
>  static int __dump_link_nlmsg(struct nlmsghdr *nlh,
>                              libbpf_dump_nlmsg_t dump_link_nlmsg, void *cookie)
>  {
>