netdev - Re: [v3,2/4] tools: bpftool: add net detach command to detach XDP on interface

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAH3MdRW4LgdLoqSpLsWUOwjnNhJA1sodHqSD2Z14JY6aHMaKxg@mail.gmail.com>
Date:   Wed, 7 Aug 2019 10:02:04 -0700
From:   Y Song <ys114321@...il.com>
To:     "Daniel T. Lee" <danieltimlee@...il.com>
Cc:     Daniel Borkmann <daniel@...earbox.net>,
        Alexei Starovoitov <ast@...nel.org>,
        netdev <netdev@...r.kernel.org>
Subject: Re: [v3,2/4] tools: bpftool: add net detach command to detach XDP on interface

On Tue, Aug 6, 2019 at 7:25 PM Daniel T. Lee <danieltimlee@...il.com> wrote:
>
> By this commit, using `bpftool net detach`, the attached XDP prog can
> be detached. Detaching the BPF prog will be done through libbpf
> 'bpf_set_link_xdp_fd' with the progfd set to -1.
>
> Signed-off-by: Daniel T. Lee <danieltimlee@...il.com>
> ---
>  tools/bpf/bpftool/net.c | 42 ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 41 insertions(+), 1 deletion(-)
>
> diff --git a/tools/bpf/bpftool/net.c b/tools/bpf/bpftool/net.c
> index c05a3fac5cac..7be96acb08e0 100644
> --- a/tools/bpf/bpftool/net.c
> +++ b/tools/bpf/bpftool/net.c
> @@ -343,6 +343,43 @@ static int do_attach(int argc, char **argv)
>         return 0;
>  }
>
> +static int do_detach(int argc, char **argv)
> +{
> +       enum net_attach_type attach_type;
> +       int progfd, ifindex, err = 0;
> +
> +       /* parse detach args */
> +       if (!REQ_ARGS(3))
> +               return -EINVAL;
> +
> +       attach_type = parse_attach_type(*argv);
> +       if (attach_type == max_net_attach_type) {
> +               p_err("invalid net attach/detach type");
> +               return -EINVAL;
> +       }
> +
> +       NEXT_ARG();
> +       ifindex = net_parse_dev(&argc, &argv);
> +       if (ifindex < 1)
> +               return -EINVAL;
> +
> +       /* detach xdp prog */
> +       progfd = -1;
> +       if (is_prefix("xdp", attach_type_strings[attach_type]))
> +               err = do_attach_detach_xdp(progfd, attach_type, ifindex, NULL);

I found an issue here. This is probably related to do_attach_detach_xdp.

-bash-4.4$ sudo ./bpftool net attach x pinned /sys/fs/bpf/xdp_example dev v1
-bash-4.4$ sudo ./bpftool net
xdp:
v1(4) driver id 1172

tc:
eth0(2) clsact/ingress fbflow_icmp id 29 act []
eth0(2) clsact/egress cls_fg_dscp_section id 27 act []
eth0(2) clsact/egress fbflow_egress id 28
eth0(2) clsact/egress fbflow_sslwall_egress id 35

flow_dissector:

-bash-4.4$ sudo ./bpftool net detach x dev v2
-bash-4.4$ sudo ./bpftool net
xdp:
v1(4) driver id 1172

tc:
eth0(2) clsact/ingress fbflow_icmp id 29 act []
eth0(2) clsact/egress cls_fg_dscp_section id 27 act []
eth0(2) clsact/egress fbflow_egress id 28
eth0(2) clsact/egress fbflow_sslwall_egress id 35

flow_dissector:

-bash-4.4$

Basically detaching may fail due to wrong dev name or wrong type, etc.
But the tool did not return an error. Is this expected?
This may be related to this funciton "bpf_set_link_xdp_fd()".
So this patch itself should be okay.

> +
> +       if (err < 0) {
> +               p_err("interface %s detach failed",
> +                     attach_type_strings[attach_type]);
> +               return err;
> +       }
> +
> +       if (json_output)
> +               jsonw_null(json_wtr);
> +
> +       return 0;
> +}
> +
>  static int do_show(int argc, char **argv)
>  {
>         struct bpf_attach_info attach_info = {};
> @@ -419,6 +456,7 @@ static int do_help(int argc, char **argv)
>         fprintf(stderr,
>                 "Usage: %s %s { show | list } [dev <devname>]\n"
>                 "       %s %s attach ATTACH_TYPE PROG dev <devname> [ overwrite ]\n"
> +               "       %s %s detach ATTACH_TYPE dev <devname>\n"
>                 "       %s %s help\n"
>                 "\n"
>                 "       " HELP_SPEC_PROGRAM "\n"
> @@ -429,7 +467,8 @@ static int do_help(int argc, char **argv)
>                 "      to dump program attachments. For program types\n"
>                 "      sk_{filter,skb,msg,reuseport} and lwt/seg6, please\n"
>                 "      consult iproute2.\n",
> -               bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2]);
> +               bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2],
> +               bin_name, argv[-2]);
>
>         return 0;
>  }
> @@ -438,6 +477,7 @@ static const struct cmd cmds[] = {
>         { "show",       do_show },
>         { "list",       do_show },
>         { "attach",     do_attach },
> +       { "detach",     do_detach },
>         { "help",       do_help },
>         { 0 }
>  };
> --
> 2.20.1
>