Re: [PATCH v2 net-next 4/4] bpftool: implement cgroup bpf operations

[Quentin Monnet <quentin.monnet@netronome.com>]

2017-12-07 18:39 UTC+0000 ~ Roman Gushchin <guro@fb.com>
> This patch adds basic cgroup bpf operations to bpftool:
> cgroup list, attach and detach commands.
> 
> Usage is described in the corresponding man pages,
> and examples are provided.
> 
> Syntax:
> $ bpftool cgroup list CGROUP
> $ bpftool cgroup attach CGROUP ATTACH_TYPE PROG [ATTACH_FLAGS]
> $ bpftool cgroup detach CGROUP ATTACH_TYPE PROG
> 
> Signed-off-by: Roman Gushchin <guro@fb.com>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
> Cc: Martin KaFai Lau <kafai@fb.com>
> Cc: Quentin Monnet <quentin.monnet@netronome.com>
> Cc: David Ahern <dsahern@gmail.com>
> ---
>  tools/bpf/bpftool/Documentation/bpftool-cgroup.rst |  92 +++++++
>  tools/bpf/bpftool/Documentation/bpftool-map.rst    |   2 +-
>  tools/bpf/bpftool/Documentation/bpftool-prog.rst   |   2 +-
>  tools/bpf/bpftool/Documentation/bpftool.rst        |   6 +-
>  tools/bpf/bpftool/cgroup.c                         | 305 +++++++++++++++++++++
>  tools/bpf/bpftool/main.c                           |   3 +-
>  tools/bpf/bpftool/main.h                           |   1 +
>  7 files changed, 406 insertions(+), 5 deletions(-)
>  create mode 100644 tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
>  create mode 100644 tools/bpf/bpftool/cgroup.c
> 
> diff --git a/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst b/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
> new file mode 100644
> index 000000000000..61ded613aee1
> --- /dev/null
> +++ b/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
> @@ -0,0 +1,92 @@
> +================
> +bpftool-cgroup
> +================
> +-------------------------------------------------------------------------------
> +tool for inspection and simple manipulation of eBPF progs
> +-------------------------------------------------------------------------------
> +
> +:Manual section: 8
> +
> +SYNOPSIS
> +========
> +
> +	**bpftool** [*OPTIONS*] **cgroup** *COMMAND*
> +
> +	*OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-f** | **--bpffs** } }
> +
> +	*COMMANDS* :=
> +	{ **list** | **attach** | **detach** | **help** }
> +
> +MAP COMMANDS
> +=============
> +
> +|	**bpftool** **cgroup list** *CGROUP*
> +|	**bpftool** **cgroup attach** *CGROUP* *ATTACH_TYPE* *PROG* [*ATTACH_FLAGS*]
> +|	**bpftool** **cgroup detach** *CGROUP* *ATTACH_TYPE* *PROG*
> +|	**bpftool** **cgroup help**
> +|
> +|	*PROG* := { **id** *PROG_ID* | **pinned** *FILE* | **tag** *PROG_TAG* }

Could you please give the different possible values for ATTACH_TYPE and
ATTACH_FLAGS, and provide some documentation for the flags?

> +
> +DESCRIPTION
> +===========
> +	**bpftool cgroup list** *CGROUP*
> +		  List all programs attached to the cgroup *CGROUP*.
> +
> +		  Output will start with program ID followed by attach type,
> +		  attach flags and program name.
> +
> +	**bpftool cgroup attach** *CGROUP* *ATTACH_TYPE* *PROG* [*ATTACH_FLAGS*]
> +		  Attach program *PROG* to the cgroup *CGROUP* with attach type
> +		  *ATTACH_TYPE* and optional *ATTACH_FLAGS*.
> +
> +	**bpftool cgroup detach** *CGROUP* *ATTACH_TYPE* *PROG*
> +		  Detach *PROG* from the cgroup *CGROUP* and attach type
> +		  *ATTACH_TYPE*.
> +
> +	**bpftool prog help**
> +		  Print short help message.

[…]

> diff --git a/tools/bpf/bpftool/cgroup.c b/tools/bpf/bpftool/cgroup.c
> new file mode 100644
> index 000000000000..88d67f74313f
> --- /dev/null
> +++ b/tools/bpf/bpftool/cgroup.c
> @@ -0,0 +1,305 @@
> +/*
> + * Copyright (C) 2017 Facebook
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + *
> + * Author: Roman Gushchin <guro@fb.com>
> + */
> +

[…]

> +static int do_detach(int argc, char **argv)
> +{
> +	int prog_fd, cgroup_fd;
> +	enum bpf_attach_type attach_type;
> +	int ret = -1;
> +
> +	if (argc < 4) {
> +		p_err("too few parameters for cgroup detach\n");
> +		goto exit;
> +	}
> +
> +	cgroup_fd = open(argv[0], O_RDONLY);
> +	if (cgroup_fd < 0) {
> +		p_err("can't open cgroup %s\n", argv[1]);
> +		goto exit;
> +	}
> +
> +	attach_type = parse_attach_type(argv[1]);
> +	if (attach_type == __MAX_BPF_ATTACH_TYPE) {
> +		p_err("invalid attach type");
> +		goto exit_cgroup;
> +	}
> +
> +	argc -= 2;
> +	argv = &argv[2];
> +	prog_fd = prog_parse_fd(&argc, &argv);
> +	if (prog_fd < 0)
> +		goto exit_cgroup;
> +
> +	if (bpf_prog_detach2(prog_fd, cgroup_fd, attach_type)) {
> +		p_err("failed to attach program");

Failed to *detach* instead of “attach”.

> +		goto exit_prog;
> +	}
> +
> +	if (json_output)
> +		jsonw_null(json_wtr);
> +
> +	ret = 0;
> +
> +exit_prog:
> +	close(prog_fd);
> +exit_cgroup:
> +	close(cgroup_fd);
> +exit:
> +	return ret;
> +}

[…]

Very nice work on this v2, thanks a lot!
Quentin