bpftool: cgroup bpf operations

The bpf_prog_load() function will guess program type if it's not specified explicitly. This functionality will be used to implement loading of different programs without asking a user to specify the program type. In first order it will be used by bpftool.

Signed-off-by Roman Gushchin <[email protected]>

Cc Alexei Starovoitov <[email protected]>

Cc Daniel Borkmann <[email protected]>

Cc Jakub Kicinski <[email protected]>

Cc Martin KaFai Lau <[email protected]>

Cc Quentin Monnet <[email protected]>

Cc David Ahern <[email protected]>

---
 tools/lib/bpf/libbpf.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

tools/lib/bpf/libbpf.c +44 -0

index 5aa45f89da93..205b7822fa0a 100644
@@ -1721,6 +1721,45 @@ BPF_PROG_TYPE_FNS(tracepoint, BPF_PROG_TYPE_TRACEPOINT);
 BPF_PROG_TYPE_FNS(xdp, BPF_PROG_TYPE_XDP);
 BPF_PROG_TYPE_FNS(perf_event, BPF_PROG_TYPE_PERF_EVENT);
 
+#define BPF_PROG_SEC(string, type) { string, sizeof(string), type }
+static const struct {
+	const char *sec;
+	size_t len;
+	enum bpf_prog_type prog_type;
+} section_names[] = {
+	BPF_PROG_SEC("socket",		BPF_PROG_TYPE_SOCKET_FILTER),
+	BPF_PROG_SEC("kprobe/",		BPF_PROG_TYPE_KPROBE),
+	BPF_PROG_SEC("kretprobe/",	BPF_PROG_TYPE_KPROBE),
+	BPF_PROG_SEC("tracepoint/",	BPF_PROG_TYPE_TRACEPOINT),
+	BPF_PROG_SEC("xdp",		BPF_PROG_TYPE_XDP),
+	BPF_PROG_SEC("perf_event",	BPF_PROG_TYPE_PERF_EVENT),
+	BPF_PROG_SEC("cgroup/skb",	BPF_PROG_TYPE_CGROUP_SKB),
+	BPF_PROG_SEC("cgroup/sock",	BPF_PROG_TYPE_CGROUP_SOCK),
+	BPF_PROG_SEC("cgroup/dev",	BPF_PROG_TYPE_CGROUP_DEVICE),
+	BPF_PROG_SEC("sockops",		BPF_PROG_TYPE_SOCK_OPS),
+	BPF_PROG_SEC("sk_skb",		BPF_PROG_TYPE_SK_SKB),
+};
+#undef BPF_PROG_SEC
+
+static enum bpf_prog_type bpf_program__guess_type(struct bpf_program *prog)
+{
+	int i;
+
+	if (!prog->section_name)
+		goto err;
+
+	for (i = 0; i < ARRAY_SIZE(section_names); i++)
+		if (strncmp(prog->section_name, section_names[i].sec,
+			    section_names[i].len) == 0)
+			return section_names[i].prog_type;
+
+err:
+	pr_warning("failed to guess program type based on section name %s\n",
+		   prog->section_name);
+
+	return BPF_PROG_TYPE_UNSPEC;
+}
+
 int bpf_map__fd(struct bpf_map *map)
 {
 	return map ? map->fd : -EINVAL;

@@ -1832,6 +1871,18 @@ int bpf_prog_load(const char *file, enum bpf_prog_type type,
 		return -ENOENT;
 	}
 
+	/*
+	 * If type is not specified, try to guess it based on
+	 * section name.
+	 */
+	if (type == BPF_PROG_TYPE_UNSPEC) {

Add the prog load command to load a bpf program from a specified binary file and pin it to bpffs.

Usage description and examples are given in the corresponding man page.

Syntax:

$ bpftool prog load SOURCE_FILE FILE

FILE is a non-existing file on bpffs.

Signed-off-by Roman Gushchin <[email protected]>

Add the prog load command to load a bpf program from a specified binary file and pin it to bpffs.

Usage description and examples are given in the corresponding man page.

Syntax:

$ bpftool prog load SOURCE_FILE FILE

FILE is a non-existing file on bpffs.

Signed-off-by Roman Gushchin <[email protected]>

Cc Alexei Starovoitov <[email protected]>

Cc Daniel Borkmann <[email protected]>

Cc Jakub Kicinski <[email protected]>

Cc Martin KaFai Lau <[email protected]>

Cc Quentin Monnet <[email protected]>

Cc David Ahern <[email protected]>

---
 tools/bpf/bpftool/Documentation/bpftool-prog.rst | 10 +++-
 tools/bpf/bpftool/Documentation/bpftool.rst      |  2 +-
 tools/bpf/bpftool/common.c                       | 71 +++++++++++++-----------
 tools/bpf/bpftool/main.h                         |  1 +
 tools/bpf/bpftool/prog.c                         | 31 ++++++++++-
 5 files changed, 81 insertions(+), 34 deletions(-)

tools/bpf/bpftool/Documentation/bpftool-prog.rst +7 -1

index 36e8d1c3c40d..827b415f8ab6 100644
@@ -15,7 +15,7 @@ SYNOPSIS
 	*OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-f** | **--bpffs** } }
 
 	*COMMANDS* :=
-	{ **show** | **dump xlated** | **dump jited** | **pin** | **help** }
+	{ **show** | **dump xlated** | **dump jited** | **pin** | **load** | **help** }
 
 MAP COMMANDS
 =============

@@ -24,6 +24,7 @@ MAP COMMANDS
 |	**bpftool** **prog dump xlated** *PROG* [{**file** *FILE* | **opcodes**}]
 |	**bpftool** **prog dump jited**  *PROG* [{**file** *FILE* | **opcodes**}]
 |	**bpftool** **prog pin** *PROG* *FILE*
+|	**bpftool** **prog load** *SRC* *FILE*
 |	**bpftool** **prog help**
 |
 |	*PROG* := { **id** *PROG_ID* | **pinned** *FILE* | **tag** *PROG_TAG* }

@@ -57,6 +58,11 @@ DESCRIPTION
 
 		  Note: *FILE* must be located in *bpffs* mount.
 
+	**bpftool prog load** *SRC* *FILE*
+		  Load bpf program from binary *SRC* and pin as *FILE*.
+
+		  Note: *FILE* must be located in *bpffs* mount.
+
 	**bpftool prog help**
 		  Print short help message.
 

tools/bpf/bpftool/Documentation/bpftool-prog.rst +0 -0

 **# bpftool prog dum jited pinned /sys/fs/bpf/prog opcodes**
 

tools/bpf/bpftool/Documentation/bpftool.rst +1 -1

index 926c03d5a8da..f547a0c0aa34 100644
@@ -26,7 +26,7 @@ SYNOPSIS
 	| **pin** | **help** }
 
 	*PROG-COMMANDS* := { **show** | **dump jited** | **dump xlated** | **pin**
-	| **help** }
+	| **load** | **help** }
 
 DESCRIPTION
 ===========

tools/bpf/bpftool/prog.c +1 -0

index ad619b96c276..bac5d81e2ff0 100644
@@ -45,6 +45,7 @@
 #include <sys/stat.h>
 
 #include <bpf.h>
+#include <libbpf.h>
 
 #include "main.h"
 #include "disasm.h"

@@ -635,6 +636,32 @@ static int do_pin(int argc, char **argv)
 	return err;
 }

tools/bpf/bpftool/prog.c +20 -1

+
+	if (bpf_prog_load(argv[0], BPF_PROG_TYPE_UNSPEC, &obj, &prog_fd)) {
+		p_err("failed to load program\n");
+		return -1;
+	}
+
+	if (do_pin_fd(prog_fd, argv[1])) {
+		p_err("failed to pin program\n");
+		return -1;
+	}
+
+	if (json_output)
+		jsonw_null(json_wtr);
+
+	return 0;
+}
+
 static int do_help(int argc, char **argv)
 {
 	if (json_output) {

@@ -647,13 +674,14 @@ static int do_help(int argc, char **argv)
 		"       %s %s dump xlated PROG [{ file FILE | opcodes }]\n"
 		"       %s %s dump jited  PROG [{ file FILE | opcodes }]\n"
 		"       %s %s pin   PROG FILE\n"
+		"       %s %s load  SRC  FILE\n"
 		"       %s %s help\n"
 		"\n"
 		"       " HELP_SPEC_PROGRAM "\n"
 		"       " HELP_SPEC_OPTIONS "\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], bin_name, argv[-2]);
 
 	return 0;
 }

@@ -663,6 +691,7 @@ static const struct cmd cmds[] = {
 	{ "help",	do_help },
 	{ "dump",	do_dump },
 	{ "pin",	do_pin },
+	{ "load",	do_load },
 	{ 0 }
 };
 

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 <[email protected]>

Cc Alexei Starovoitov <[email protected]>

Cc Daniel Borkmann <[email protected]>

Cc Jakub Kicinski <[email protected]>

Cc Martin KaFai Lau <[email protected]>

Cc Quentin Monnet <[email protected]>

Cc David Ahern <[email protected]>

---

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 <[email protected]>

tools/bpf/bpftool/cgroup.c new file +28 -0

new file mode 100644
index 000000000000..88d67f74313f
@@ -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 <[email protected]>
+ */
+
+#include <fcntl.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#include <bpf.h>
+
+#include "main.h"
+
+static const char * const attach_type_strings[] = {
+	[BPF_CGROUP_INET_INGRESS] = "ingress",
+	[BPF_CGROUP_INET_EGRESS] = "egress",
+	[BPF_CGROUP_INET_SOCK_CREATE] = "sock_create",
+	[BPF_CGROUP_SOCK_OPS] = "sock_ops",
+	[BPF_CGROUP_DEVICE] = "device",

tools/bpf/bpftool/cgroup.c new file +11 -0

+	prog_cnt = ARRAY_SIZE(prog_ids);
+	ret = bpf_prog_query(cgroup_fd, type, 0, &attach_flags, prog_ids,
+			     &prog_cnt);
+	if (ret)
+		return ret;
+
+	if (prog_cnt == 0)
+		return 0;
+
+	switch (attach_flags) {
+	case BPF_F_ALLOW_MULTI:

tools/bpf/bpftool/cgroup.c new file +34 -0

+	}
+
+	for (iter = 0; iter < prog_cnt; iter++)
+		list_bpf_prog(prog_ids[iter], attach_type_strings[type],
+			      attach_flags_str);
+
+	return 0;
+}
 
+static int do_attach(int argc, char **argv)
+{
+	int cgroup_fd, prog_fd;
+	enum bpf_attach_type attach_type;
+	int attach_flags = 0;
+	int i;
+	int ret = -1;
+
+	if (argc < 4) {
+		p_err("too few parameters for cgroup attach\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\n");
+		goto exit_cgroup;
+	}
+
+	argc -= 2;

tools/bpf/bpftool/cgroup.c new file +13 -0

+		} else {
+			p_err("unknown option: %s\n", argv[i]);
+			goto exit_cgroup;
+		}
+	}
+
+	if (bpf_prog_attach(prog_fd, cgroup_fd, attach_type, attach_flags)) {
+		p_err("failed to attach program");
+		goto exit_prog;
+	}
+
+	if (json_output)
+		jsonw_null(json_wtr);

Quoted 6 lines · Roman Gushchin

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.

Quoted 6 lines

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 <[email protected]>

Looks good, a few very minor nits/questions below.

Quoted 6 lines

tools/bpf/bpftool/cgroup.c new file +0 -0

new file mode 100644
index 000000000000..88d67f74313f

tools/bpf/bpftool/cgroup.c line 10

Quoted 10 lines

tools/bpf/bpftool/cgroup.c new file +10 -0

@@ -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 <[email protected]>
+ */

Quoted 6 lines · Roman Gushchin

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.

Quoted 6 lines

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 <[email protected]>

Looks good, a few very minor nits/questions below.

Quoted 40 lines

tools/bpf/bpftool/cgroup.c new file +34 -0

new file mode 100644
index 000000000000..88d67f74313f
@@ -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 <[email protected]>
+ */
+
+#include <fcntl.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#include <bpf.h>
+
+#include "main.h"
+
+static const char * const attach_type_strings[] = {
+	[BPF_CGROUP_INET_INGRESS] = "ingress",
+	[BPF_CGROUP_INET_EGRESS] = "egress",
+	[BPF_CGROUP_INET_SOCK_CREATE] = "sock_create",
+	[BPF_CGROUP_SOCK_OPS] = "sock_ops",
+	[BPF_CGROUP_DEVICE] = "device",
+	[__MAX_BPF_ATTACH_TYPE] = NULL,
+};
+
+static enum bpf_attach_type parse_attach_type(const char *str)
+{
+	enum bpf_attach_type type;

tools/bpf/bpftool/cgroup.c line 38

Quoted 4 lines

tools/bpf/bpftool/cgroup.c new file +4 -0

+
+	for (type = 0; type < __MAX_BPF_ATTACH_TYPE; type++) {
+		if (attach_type_strings[type] &&
+		    strcmp(str, attach_type_strings[type]) == 0)

Here and for matching flags you use straight strcmp(), not our locally defined is_prefix(), is this intentional? is_prefix() allows abbreviations, like in iproute2 commands. E.g. this would work:

# bpftool cg att /sys/fs/cgroup/test.slice/ dev id 1 allow_multi

tools/bpf/bpftool/cgroup.c line 87

Quoted 13 lines

tools/bpf/bpftool/cgroup.c new file +12 -0

+			return type;
+	}
+
+	return __MAX_BPF_ATTACH_TYPE;
+}
 
+static int list_attached_bpf_progs(int cgroup_fd, enum bpf_attach_type type)
+{
+	__u32 attach_flags;
+	__u32 prog_ids[1024] = {0};
+	__u32 prog_cnt, iter;
+	char *attach_flags_str;
+	int ret;

nit: could you reorder the variables so they're listed longest to
shortest (reverse christmas tree)?

Quoted 11 lines

tools/bpf/bpftool/cgroup.c new file +11 -0

+	prog_cnt = ARRAY_SIZE(prog_ids);
+	ret = bpf_prog_query(cgroup_fd, type, 0, &attach_flags, prog_ids,
+			     &prog_cnt);
+	if (ret)
+		return ret;
+
+	if (prog_cnt == 0)
+		return 0;
+
+	switch (attach_flags) {
+	case BPF_F_ALLOW_MULTI:

tools/bpf/bpftool/cgroup.c line 109

Quoted 10 lines

tools/bpf/bpftool/cgroup.c new file +10 -0

+		attach_flags_str = "allow_multi";
+		break;
+	case BPF_F_ALLOW_OVERRIDE:
+		attach_flags_str = "allow_override";
+		break;
+	case 0:
+		attach_flags_str = "";
+		break;
+	default:
+		attach_flags_str = "unknown";

nit: would it make sense to perhaps print flags in hex format in this
case?

Quoted 39 lines

tools/bpf/bpftool/cgroup.c new file +38 -0

+	}
+
+	for (iter = 0; iter < prog_cnt; iter++)
+		list_bpf_prog(prog_ids[iter], attach_type_strings[type],
+			      attach_flags_str);
+
+	return 0;
+}
 
+static int do_attach(int argc, char **argv)
+{
+	int cgroup_fd, prog_fd;
+	enum bpf_attach_type attach_type;
+	int attach_flags = 0;
+	int i;
+	int ret = -1;
+
+	if (argc < 4) {
+		p_err("too few parameters for cgroup attach\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\n");
+		goto exit_cgroup;
+	}
+
+	argc -= 2;
+	argv = &argv[2];
+	prog_fd = prog_parse_fd(&argc, &argv);
+	if (prog_fd < 0)
+		goto exit_cgroup;

tools/bpf/bpftool/cgroup.c line 199

Quoted 6 lines

tools/bpf/bpftool/cgroup.c new file +6 -0

+
+	for (i = 0; i < argc; i++) {
+		if (strcmp(argv[i], "allow_multi") == 0) {
+			attach_flags |= BPF_F_ALLOW_MULTI;
+		} else if (strcmp(argv[i], "allow_override") == 0) {
+			attach_flags |= BPF_F_ALLOW_OVERRIDE;

This is the other potential place for is_prefix() I referred to.

That reminds me - would you care to also update Quentin's bash completions (tools/bpf/bpftool/bash-completion/bpftool)? They are _very_ nice to have in day to day use!

Otherwise looks really nice, thanks for working on it!

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 <[email protected]>

Cc Alexei Starovoitov <[email protected]>

Cc Daniel Borkmann <[email protected]>

Cc Jakub Kicinski <[email protected]>

Cc Martin KaFai Lau <[email protected]>

Cc Quentin Monnet <[email protected]>

Cc David Ahern <[email protected]>

---
 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

tools/bpf/bpftool/Documentation/bpftool-cgroup.rst new file +18 -0

new file mode 100644
index 000000000000..61ded613aee1
@@ -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** }

tools/bpf/bpftool/Documentation/bpftool-cgroup.rst new file +19 -0

+
+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.

tools/bpf/bpftool/cgroup.c new file +11 -0

new file mode 100644
index 000000000000..88d67f74313f
@@ -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 <[email protected]>
+ */
+

tools/bpf/bpftool/cgroup.c new file +21 -0

+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;

tools/bpf/bpftool/cgroup.c new file +5 -0

+		goto exit_prog;
+	}
+
+	if (json_output)
+		jsonw_null(json_wtr);

2017-12-07 18:39 UTC+0000 ~ Roman Gushchin <[email protected]>

Quoted 2 lines

This patch adds basic cgroup bpf operations to bpftool:
cgroup list, attach and detach commands.

Quoted 6 lines

tools/bpf/bpftool/cgroup.c new file +3 -0

@@ -0,0 +1,305 @@
+/*
+ * Copyright (C) 2017 Facebook
+ *

tools/bpf/bpftool/cgroup.c line 11

Quoted 8 lines

tools/bpf/bpftool/cgroup.c new file +8 -0

+ * 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.
+ *
+ *
+ */
+

Quoted 3 lines · Quentin Monnet

2017-12-07 18:39 UTC+0000 ~ Roman Gushchin <[email protected]>

Quoted 2 lines

This patch adds basic cgroup bpf operations to bpftool:
cgroup list, attach and detach commands.

[...]

Quoted 6 lines

Quoted 6 lines

tools/bpf/bpftool/cgroup.c new file +3 -0

@@ -0,0 +1,305 @@
+/*
+ * Copyright (C) 2017 Facebook
+ *

Quoted 8 lines

tools/bpf/bpftool/cgroup.c line 11

Quoted 8 lines

tools/bpf/bpftool/cgroup.c new file +8 -0

+ * 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.
+ *
+ *
+ */
+

Roman,
Have you considered using the simpler and new SPDX ids instead? e.g.:

// SPDX-License-Identifier: GPL-2.0+
// Copyright (C) 2017 Facebook
// Author: Roman Gushchin <[email protected]>

This would boost your code/comments ratio nicely IMHO.

2017-12-07 18:39 UTC+0000 ~ Roman Gushchin <[email protected]>

Quoted 5 lines

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.

Quoted 4 lines

tools/bpf/bpftool/Documentation/bpftool-cgroup.rst new file +4 -0

+MAP COMMANDS
+=============
+
+|	**bpftool** **cgroup list** *CGROUP*

Quoted 5 lines

+|	**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?

Quoted 13 lines

tools/bpf/bpftool/Documentation/bpftool-cgroup.rst new file +12 -0

+
+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*.

Quoted 9 lines

tools/bpf/bpftool/cgroup.c new file +9 -0

+
+	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];

tools/bpf/bpftool/cgroup.c line 254

Quoted 6 lines

tools/bpf/bpftool/cgroup.c new file +6 -0

+	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”.

Quoted 16 lines

tools/bpf/bpftool/cgroup.c new file +15 -0

+		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

Quoted 6 lines

2017-12-07 18:39 UTC+0000 ~ Roman Gushchin <[email protected]>

Quoted 5 lines

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.

[...]

Quoted 4 lines

Quoted 4 lines

tools/bpf/bpftool/Documentation/bpftool-cgroup.rst new file +4 -0

+MAP COMMANDS
+=============
+
+|	**bpftool** **cgroup list** *CGROUP*

Quoted 8 lines

Quoted 5 lines

+|	**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?

I intentionally didn't include the list of possible values, as it depends on the exact kernel version, and other bpftool docs are carefully avoiding specifying such things.

It would be nice to have a way to ask the kernel about provided bpf program types, attach types, etc; but I'm not sure that hardcoding it in bpftool docs is a good idea.

I don't believe compatibility is an issue here, since the program and its documentation come together (so they should stay in sync) and are part of the kernel tree (so the tool should be compatible with the kernel sources it comes with). My concern is that there is no way to guess from the current description what the values for ATTACH_FLAG or ATTACH_TYPE can be, without reading the source code of the program—which is not exactly user-friendly.

Quoted 7 lines · Quentin Monnet

I don't believe compatibility is an issue here, since the program and its documentation come together (so they should stay in sync) and are part of the kernel tree (so the tool should be compatible with the kernel sources it comes with). My concern is that there is no way to guess from the current description what the values for ATTACH_FLAG or ATTACH_TYPE can be, without reading the source code of the program—which is not exactly user-friendly.

The tool should be backward and forward compatible across kernel versions. Running a newer command on an older kernel should fail in a deterministic. While the tool is in the kernel tree for ease of development, that should not be confused with having a direct tie to any kernel version.

I believe man pages do include kernel version descriptions in flags (e.g., man 7 socket -- flags are denoted with "since Linux x.y") which is one way to handle it with the usual caveat that vendors might have backported support to earlier kernels.

2017-12-08 14:12 UTC+0000 ~ Roman Gushchin <[email protected]>

Quoted 21 lines · Quentin Monnet

Quoted 6 lines

2017-12-07 18:39 UTC+0000 ~ Roman Gushchin <[email protected]>

Quoted 5 lines

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.

[...]

Quoted 12 lines

tools/bpf/bpftool/Documentation/bpftool-cgroup.rst line 28

Quoted 9 lines

tools/bpf/bpftool/Documentation/bpftool-cgroup.rst new file +9 -0

+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?

Quoted 3 lines

I intentionally didn't include the list of possible values, as it depends on the exact kernel version, and other bpftool docs are carefully avoiding specifying such things.

Do they? As far as I can tell the only other bpftool command that uses flags is the bpftool map update, and it does specify the possible values for UPDATE_FLAGS (and document them) in the man page.