[PATCH bpf-next 0/4] bpftool: Add struct_ops support

[PATCH bpf-next 0/4] bpftool: Add struct_ops support

[Martin KaFai Lau]

This set adds "struct_ops" support to bpftool.

The first two patches improve the btf_dumper in bpftool.
Patch 1: print the enum's name (if it is found) instead of the
         enum's value.
Patch 2: print a char[] as a string if all characters are printable.

"struct_ops" stores the prog_id in a func ptr.
Instead of printing a prog_id,
patch 3 adds an option to btf_dumper to allow a func ptr's value
to be printed with the full func_proto info and the prog_name.

Patch 4 implements the "struct_ops" bpftool command.

Martin KaFai Lau (4):
  bpftool: Print the enum's name instead of value
  bpftool: Print as a string for char array
  bpftool: Translate prog_id to its bpf prog_name
  bpftool: Add struct_ops support

 .../Documentation/bpftool-struct_ops.rst      | 106 ++++
 tools/bpf/bpftool/bash-completion/bpftool     |  28 +
 tools/bpf/bpftool/btf_dumper.c                | 194 +++++-
 tools/bpf/bpftool/main.c                      |   3 +-
 tools/bpf/bpftool/main.h                      |   2 +
 tools/bpf/bpftool/struct_ops.c                | 595 ++++++++++++++++++
 6 files changed, 912 insertions(+), 16 deletions(-)
 create mode 100644 tools/bpf/bpftool/Documentation/bpftool-struct_ops.rst
 create mode 100644 tools/bpf/bpftool/struct_ops.c

-- 
2.17.1

[PATCH bpf-next 1/4] bpftool: Print the enum's name instead of value

[Martin KaFai Lau]

This patch prints the enum's name if there is one found in
the array of btf_enum.

The commit 9eea98497951 ("bpf: fix BTF verification of enums")
has details about an enum could have any power-of-2 size (up to 8 bytes).
This patch also takes this chance to accommodate these non 4 byte
enums.

Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
 tools/bpf/bpftool/btf_dumper.c | 35 +++++++++++++++++++++++++++++++---
 1 file changed, 32 insertions(+), 3 deletions(-)

diff --git a/tools/bpf/bpftool/btf_dumper.c b/tools/bpf/bpftool/btf_dumper.c
index 01cc52b834fa..57bd6c0fafc9 100644
--- a/tools/bpf/bpftool/btf_dumper.c
+++ b/tools/bpf/bpftool/btf_dumper.c
@@ -43,9 +43,38 @@ static int btf_dumper_modifier(const struct btf_dumper *d, __u32 type_id,
 	return btf_dumper_do_type(d, actual_type_id, bit_offset, data);
 }
 
-static void btf_dumper_enum(const void *data, json_writer_t *jw)
+static void btf_dumper_enum(const struct btf_dumper *d,
+			    const struct btf_type *t,
+			    const void *data)
 {
-	jsonw_printf(jw, "%d", *(int *)data);
+	const struct btf_enum *enums = btf_enum(t);
+	__s64 value;
+	__u16 i;
+
+	switch (t->size) {
+	case 8:
+		value = *(__s64 *)data;
+		break;
+	case 4:
+		value = *(__s32 *)data;
+		break;
+	case 2:
+		value = *(__s16 *)data;
+		break;
+	default:
+		value = *(__s8 *)data;
+	}
+
+	for (i = 0; i < btf_vlen(t); i++) {
+		if (value == enums[i].val) {
+			jsonw_string(d->jw,
+				     btf__name_by_offset(d->btf,
+							 enums[i].name_off));
+			return;
+		}
+	}
+
+	jsonw_int(d->jw, value);
 }
 
 static int btf_dumper_array(const struct btf_dumper *d, __u32 type_id,
@@ -366,7 +395,7 @@ static int btf_dumper_do_type(const struct btf_dumper *d, __u32 type_id,
 	case BTF_KIND_ARRAY:
 		return btf_dumper_array(d, type_id, data);
 	case BTF_KIND_ENUM:
-		btf_dumper_enum(data, d->jw);
+		btf_dumper_enum(d, t, data);
 		return 0;
 	case BTF_KIND_PTR:
 		btf_dumper_ptr(data, d->jw, d->is_plain_text);
-- 
2.17.1

[PATCH bpf-next 2/4] bpftool: Print as a string for char array

[Martin KaFai Lau]

A char[] is currently printed as an integer array.
This patch will print it as a string when
1) The array element type is an one byte int
2) The array element type has a BTF_INT_CHAR encoding or
   the array element type's name is "char"
3) All characters is between (0x1f, 0x7f) and it is terminated
   by a null character.

Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
 tools/bpf/bpftool/btf_dumper.c | 41 ++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/tools/bpf/bpftool/btf_dumper.c b/tools/bpf/bpftool/btf_dumper.c
index 57bd6c0fafc9..1d2d8d2cedea 100644
--- a/tools/bpf/bpftool/btf_dumper.c
+++ b/tools/bpf/bpftool/btf_dumper.c
@@ -77,6 +77,42 @@ static void btf_dumper_enum(const struct btf_dumper *d,
 	jsonw_int(d->jw, value);
 }
 
+static bool is_str_array(const struct btf *btf, const struct btf_array *arr,
+			 const char *s)
+{
+	const struct btf_type *elem_type;
+	const char *end_s;
+
+	if (!arr->nelems)
+		return false;
+
+	elem_type = btf__type_by_id(btf, arr->type);
+	/* Not skipping typedef.  typedef to char does not count as
+	 * a string now.
+	 */
+	while (elem_type && btf_is_mod(elem_type))
+		elem_type = btf__type_by_id(btf, elem_type->type);
+
+	if (!elem_type || !btf_is_int(elem_type) || elem_type->size != 1)
+		return false;
+
+	if (btf_int_encoding(elem_type) != BTF_INT_CHAR &&
+	    strcmp("char", btf__name_by_offset(btf, elem_type->name_off)))
+		return false;
+
+	end_s = s + arr->nelems;
+	while (s < end_s) {
+		if (!*s)
+			return true;
+		if (*s <= 0x1f || *s >= 0x7f)
+			return false;
+		s++;
+	}
+
+	/* '\0' is not found */
+	return false;
+}
+
 static int btf_dumper_array(const struct btf_dumper *d, __u32 type_id,
 			    const void *data)
 {
@@ -86,6 +122,11 @@ static int btf_dumper_array(const struct btf_dumper *d, __u32 type_id,
 	int ret = 0;
 	__u32 i;
 
+	if (is_str_array(d->btf, arr, data)) {
+		jsonw_string(d->jw, data);
+		return 0;
+	}
+
 	elem_size = btf__resolve_size(d->btf, arr->type);
 	if (elem_size < 0)
 		return elem_size;
-- 
2.17.1

[PATCH bpf-next 3/4] bpftool: Translate prog_id to its bpf prog_name

[Martin KaFai Lau]

The kernel struct_ops obj has kernel's func ptrs implemented by bpf_progs.
The bpf prog_id is stored as the value of the func ptr for introspection
purpose.  In the latter patch, a struct_ops dump subcmd will be added
to introspect these func ptrs.  It is desired to print the actual bpf
prog_name instead of only printing the prog_id.

Since struct_ops is the only usecase storing prog_id in the func ptr,
this patch adds a prog_id_as_func_ptr bool (default is false) to
"struct btf_dumper" in order not to mis-interpret the ptr value
for the other existing use-cases.

While printing a func_ptr as a bpf prog_name,
this patch also prefix the bpf prog_name with the ptr's func_proto.
[ Note that it is the ptr's func_proto instead of the bpf prog's
  func_proto ]
It reuses the current btf_dump_func() to obtain the ptr's func_proto
string.

Here is an example from the bpf_cubic.c:
"void (struct sock *, u32, u32) bictcp_cong_avoid/prog_id:140"

Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
 tools/bpf/bpftool/btf_dumper.c | 118 +++++++++++++++++++++++++++++----
 tools/bpf/bpftool/main.h       |   1 +
 2 files changed, 107 insertions(+), 12 deletions(-)

diff --git a/tools/bpf/bpftool/btf_dumper.c b/tools/bpf/bpftool/btf_dumper.c
index 1d2d8d2cedea..2897a6c28a91 100644
--- a/tools/bpf/bpftool/btf_dumper.c
+++ b/tools/bpf/bpftool/btf_dumper.c
@@ -4,11 +4,13 @@
 #include <ctype.h>
 #include <stdio.h> /* for (FILE *) used by json_writer */
 #include <string.h>
+#include <unistd.h>
 #include <asm/byteorder.h>
 #include <linux/bitops.h>
 #include <linux/btf.h>
 #include <linux/err.h>
 #include <bpf/btf.h>
+#include <bpf/bpf.h>
 
 #include "json_writer.h"
 #include "main.h"
@@ -22,13 +24,102 @@
 static int btf_dumper_do_type(const struct btf_dumper *d, __u32 type_id,
 			      __u8 bit_offset, const void *data);
 
-static void btf_dumper_ptr(const void *data, json_writer_t *jw,
-			   bool is_plain_text)
+static int btf_dump_func(const struct btf *btf, char *func_sig,
+			 const struct btf_type *func_proto,
+			 const struct btf_type *func, int pos, int size);
+
+static int dump_prog_id_as_func_ptr(const struct btf_dumper *d,
+				    const struct btf_type *func_proto,
+				    __u32 prog_id)
 {
-	if (is_plain_text)
-		jsonw_printf(jw, "%p", *(void **)data);
+	struct bpf_prog_info_linear *prog_info = NULL;
+	const struct btf_type *func_type;
+	const char *prog_name = NULL;
+	struct bpf_func_info *finfo;
+	struct btf *prog_btf = NULL;
+	struct bpf_prog_info *info;
+	int prog_fd, func_sig_len;
+	char prog_str[1024];
+
+	/* Get the ptr's func_proto */
+	func_sig_len = btf_dump_func(d->btf, prog_str, func_proto, NULL, 0,
+				     sizeof(prog_str));
+	if (func_sig_len == -1)
+		return -1;
+
+	if (!prog_id)
+		goto print;
+
+	/* Get the bpf_prog's name.  Obtain from func_info. */
+	prog_fd = bpf_prog_get_fd_by_id(prog_id);
+	if (prog_fd == -1)
+		goto print;
+
+	prog_info = bpf_program__get_prog_info_linear(prog_fd,
+						1UL << BPF_PROG_INFO_FUNC_INFO);
+	close(prog_fd);
+	if (IS_ERR(prog_info)) {
+		prog_info = NULL;
+		goto print;
+	}
+	info = &prog_info->info;
+
+	if (!info->btf_id || !info->nr_func_info ||
+	    btf__get_from_id(info->btf_id, &prog_btf))
+		goto print;
+	finfo = (struct bpf_func_info *)info->func_info;
+	func_type = btf__type_by_id(prog_btf, finfo->type_id);
+	if (!func_type || !btf_is_func(func_type))
+		goto print;
+
+	prog_name = btf__name_by_offset(prog_btf, func_type->name_off);
+
+print:
+	if (!prog_id)
+		snprintf(&prog_str[func_sig_len],
+			 sizeof(prog_str) - func_sig_len, " 0");
+	else if (prog_name)
+		snprintf(&prog_str[func_sig_len],
+			 sizeof(prog_str) - func_sig_len,
+			 " %s/prog_id:%u", prog_name, prog_id);
 	else
-		jsonw_printf(jw, "%lu", *(unsigned long *)data);
+		snprintf(&prog_str[func_sig_len],
+			 sizeof(prog_str) - func_sig_len,
+			 " <unknown_prog_name>/prog_id:%u", prog_id);
+
+	prog_str[sizeof(prog_str) - 1] = '\0';
+	jsonw_string(d->jw, prog_str);
+	btf__free(prog_btf);
+	free(prog_info);
+	return 0;
+}
+
+static void btf_dumper_ptr(const struct btf_dumper *d,
+			   const struct btf_type *t,
+			   const void *data)
+{
+	unsigned long value = *(unsigned long *)data;
+	const struct btf_type *ptr_type;
+	__s32 ptr_type_id;
+
+	if (!d->prog_id_as_func_ptr || value > UINT32_MAX)
+		goto print_ptr_value;
+
+	ptr_type_id = btf__resolve_type(d->btf, t->type);
+	if (ptr_type_id < 0)
+		goto print_ptr_value;
+	ptr_type = btf__type_by_id(d->btf, ptr_type_id);
+	if (!ptr_type || !btf_is_func_proto(ptr_type))
+		goto print_ptr_value;
+
+	if (!dump_prog_id_as_func_ptr(d, ptr_type, value))
+		return;
+
+print_ptr_value:
+	if (d->is_plain_text)
+		jsonw_printf(d->jw, "%p", (void *)value);
+	else
+		jsonw_printf(d->jw, "%lu", value);
 }
 
 static int btf_dumper_modifier(const struct btf_dumper *d, __u32 type_id,
@@ -439,7 +530,7 @@ static int btf_dumper_do_type(const struct btf_dumper *d, __u32 type_id,
 		btf_dumper_enum(d, t, data);
 		return 0;
 	case BTF_KIND_PTR:
-		btf_dumper_ptr(data, d->jw, d->is_plain_text);
+		btf_dumper_ptr(d, t, data);
 		return 0;
 	case BTF_KIND_UNKN:
 		jsonw_printf(d->jw, "(unknown)");
@@ -484,10 +575,6 @@ int btf_dumper_type(const struct btf_dumper *d, __u32 type_id,
 			return -1;					\
 	} while (0)
 
-static int btf_dump_func(const struct btf *btf, char *func_sig,
-			 const struct btf_type *func_proto,
-			 const struct btf_type *func, int pos, int size);
-
 static int __btf_dumper_type_only(const struct btf *btf, __u32 type_id,
 				  char *func_sig, int pos, int size)
 {
@@ -596,8 +683,15 @@ static int btf_dump_func(const struct btf *btf, char *func_sig,
 			BTF_PRINT_ARG(", ");
 		if (arg->type) {
 			BTF_PRINT_TYPE(arg->type);
-			BTF_PRINT_ARG("%s",
-				      btf__name_by_offset(btf, arg->name_off));
+			if (arg->name_off)
+				BTF_PRINT_ARG("%s",
+					      btf__name_by_offset(btf, arg->name_off));
+			else if (pos && func_sig[pos - 1] == ' ')
+				/* Remove unnecessary space for
+				 * FUNC_PROTO that does not have
+				 * arg->name_off
+				 */
+				func_sig[--pos] = '\0';
 		} else {
 			BTF_PRINT_ARG("...");
 		}
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index 5f6dccd43622..6db2398ae7e9 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -205,6 +205,7 @@ struct btf_dumper {
 	const struct btf *btf;
 	json_writer_t *jw;
 	bool is_plain_text;
+	bool prog_id_as_func_ptr;
 };
 
 /* btf_dumper_type - print data along with type information
-- 
2.17.1

[PATCH bpf-next 4/4] bpftool: Add struct_ops support

[Martin KaFai Lau]

This patch adds struct_ops support to the bpftool.

To recap a bit on the recent bpf_struct_ops feature on the kernel side:
It currently supports "struct tcp_congestion_ops" to be implemented
in bpf.  At a high level, bpf_struct_ops is struct_ops map populated
with a number of bpf progs.  bpf_struct_ops currently supports the
"struct tcp_congestion_ops".  However, the bpf_struct_ops design is
generic enough that other kernel struct ops can be supported in
the future.

Although struct_ops is map+progs at a high lever, there are differences
in details.  For example,
1) After registering a struct_ops, the struct_ops is held by the kernel
   subsystem (e.g. tcp-cc).  Thus, there is no need to pin a
   struct_ops map or its progs in order to keep them around.
2) To iterate all struct_ops in a system, it iterates all maps
   in type BPF_MAP_TYPE_STRUCT_OPS.  BPF_MAP_TYPE_STRUCT_OPS is
   the current usual filter.  In the future, it may need to
   filter by other struct_ops specific properties.  e.g. filter by
   tcp_congestion_ops or other kernel subsystem ops in the future.
3) struct_ops requires the running kernel having BTF info.  That allows
   more flexibility in handling other kernel structs.  e.g. it can
   always dump the latest bpf_map_info.
4) Also, "struct_ops" command is not intended to repeat all features
   already provided by "map" or "prog".  For example, if there really
   is a need to pin the struct_ops map, the user can use the "map" cmd
   to do that.

While the first attempt was to reuse parts from map/prog.c,  it ended up
not a lot to share.  The only obvious item is the map_parse_fds() but
that still requires modifications to accommodate struct_ops map specific
filtering (for the immediate and the future needs).  Together with the
earlier mentioned differences, it is better to part away from map/prog.c.

The initial set of subcmds are, register, unregister, show, and dump.

For register, it registers all struct_ops maps that can be found in an
obj file.  Option can be added in the future to specify a particular
struct_ops map.  Also, the common bpf_tcp_cc is stateless (e.g.
bpf_cubic.c and bpf_dctcp.c).  The "reuse map" feature is not
implemented in this patch and it can be considered later also.

For other subcmds, please see the man doc for details.

A sample output of dump:
[root@arch-fb-vm1 bpf]# bpftool struct_ops dump name cubic
[{
        "bpf_map_info": {
            "type": 26,
            "id": 64,
            "key_size": 4,
            "value_size": 256,
            "max_entries": 1,
            "map_flags": 0,
            "name": "cubic",
            "ifindex": 0,
            "btf_vmlinux_value_type_id": 18452,
            "netns_dev": 0,
            "netns_ino": 0,
            "btf_id": 52,
            "btf_key_type_id": 0,
            "btf_value_type_id": 0
        }
    },{
        "bpf_struct_ops_tcp_congestion_ops": {
            "refcnt": {
                "refs": {
                    "counter": 1
                }
            },
            "state": "BPF_STRUCT_OPS_STATE_INUSE",
            "data": {
                "list": {
                    "next": 0,
                    "prev": 0
                },
                "key": 0,
                "flags": 0,
                "init": "void (struct sock *) bictcp_init/prog_id:138",
                "release": "void (struct sock *) 0",
                "ssthresh": "u32 (struct sock *) bictcp_recalc_ssthresh/prog_id:141",
                "cong_avoid": "void (struct sock *, u32, u32) bictcp_cong_avoid/prog_id:140",
                "set_state": "void (struct sock *, u8) bictcp_state/prog_id:142",
                "cwnd_event": "void (struct sock *, enum tcp_ca_event) bictcp_cwnd_event/prog_id:139",
                "in_ack_event": "void (struct sock *, u32) 0",
                "undo_cwnd": "u32 (struct sock *) tcp_reno_undo_cwnd/prog_id:144",
                "pkts_acked": "void (struct sock *, const struct ack_sample *) bictcp_acked/prog_id:143",
                "min_tso_segs": "u32 (struct sock *) 0",
                "sndbuf_expand": "u32 (struct sock *) 0",
                "cong_control": "void (struct sock *, const struct rate_sample *) 0",
                "get_info": "size_t (struct sock *, u32, int *, union tcp_cc_info *) 0",
                "name": "bpf_cubic",
                "owner": 0
            }
        }
    }
]

Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
 .../Documentation/bpftool-struct_ops.rst      | 106 ++++
 tools/bpf/bpftool/bash-completion/bpftool     |  28 +
 tools/bpf/bpftool/main.c                      |   3 +-
 tools/bpf/bpftool/main.h                      |   1 +
 tools/bpf/bpftool/struct_ops.c                | 595 ++++++++++++++++++
 5 files changed, 732 insertions(+), 1 deletion(-)
 create mode 100644 tools/bpf/bpftool/Documentation/bpftool-struct_ops.rst
 create mode 100644 tools/bpf/bpftool/struct_ops.c

diff --git a/tools/bpf/bpftool/Documentation/bpftool-struct_ops.rst b/tools/bpf/bpftool/Documentation/bpftool-struct_ops.rst
new file mode 100644
index 000000000000..27aae5bc632e
--- /dev/null
+++ b/tools/bpf/bpftool/Documentation/bpftool-struct_ops.rst
@@ -0,0 +1,106 @@
+==================
+bpftool-struct_ops
+==================
+-------------------------------------------------------------------------------
+tool to register/unregister/introspect BPF struct_ops
+-------------------------------------------------------------------------------
+
+:Manual section: 8
+
+SYNOPSIS
+========
+
+	**bpftool** [*OPTIONS*] **struct_ops** *COMMAND*
+
+	*OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] }
+
+	*COMMANDS* :=
+	{ **show** | **list** | **dump** | **register** | **unregister** | **help** }
+
+STRUCT_OPS COMMANDS
+===================
+
+|	**bpftool** **struct_ops { show | list }** [*STRUCT_OPS_MAP*]
+|	**bpftool** **struct_ops dump** [*STRUCT_OPS_MAP*]
+|	**bpftool** **struct_ops register** *OBJ*
+|	**bpftool** **struct_ops unregister** *STRUCT_OPS_MAP*
+|	**bpftool** **struct_ops help**
+|
+|	*STRUCT_OPS_MAP* := { **id** *STRUCT_OPS_MAP_ID* | **name** *STRUCT_OPS_MAP_NAME* }
+|	*OBJ* := /a/file/of/bpf_struct_ops.o
+
+
+DESCRIPTION
+===========
+	**bpftool struct_ops { show | list }** [*STRUCT_OPS_MAP*]
+		  Show brief information about the struct_ops in the system.
+		  If *STRUCT_OPS_MAP* is specified, it shows information only
+		  for the given struct_ops.  Otherwise, it lists all struct_ops
+		  currently exists in the system.
+
+		  Output will start with struct_ops map ID, followed by its map
+		  name and its struct_ops's kernel type.
+
+	**bpftool struct_ops dump** [*STRUCT_OPS_MAP*]
+		  Dump details information about the struct_ops in the system.
+		  If *STRUCT_OPS_MAP* is specified, it dumps information only
+		  for the given struct_ops.  Otherwise, it dumps all struct_ops
+		  currently exists in the system.
+
+	**bpftool struct_ops register** *OBJ*
+		  Register bpf struct_ops from *OBJ*.  All struct_ops under
+		  the ELF section ".struct_ops" will be registered to
+		  its kernel subsystem.
+
+	**bpftool struct_ops unregister**  *STRUCT_OPS_MAP*
+		  Unregister the *STRUCT_OPS_MAP* from the kernel subsystem.
+
+	**bpftool struct_ops help**
+		  Print short help message.
+
+OPTIONS
+=======
+	-h, --help
+		  Print short generic help message (similar to **bpftool help**).
+
+	-V, --version
+		  Print version number (similar to **bpftool version**).
+
+	-j, --json
+		  Generate JSON output. For commands that cannot produce JSON, this
+		  option has no effect.
+
+	-p, --pretty
+		  Generate human-readable JSON output. Implies **-j**.
+
+	-d, --debug
+		  Print all logs available, even debug-level information. This
+		  includes logs from libbpf as well as from the verifier, when
+		  attempting to load programs.
+
+EXAMPLES
+========
+**# bpftool struct_ops show**
+
+::
+
+    100: dctcp           tcp_congestion_ops
+    105: cubic           tcp_congestion_ops
+
+**# bpftool struct_ops unregister id 105**
+
+::
+
+   Unregistered tcp_congestion_ops cubic id 105
+
+**# bpftool struct_ops register bpf_cubic.o**
+
+::
+
+   Registered tcp_congestion_ops cubic id 110
+
+
+SEE ALSO
+========
+	**bpftool-map**\ (8)
+	**bpftool-prog**\ (8)
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index 9b0534f558f1..45ee99b159e2 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -576,6 +576,34 @@ _bpftool()
                     ;;
             esac
             ;;
+        struct_ops)
+            local STRUCT_OPS_TYPE='id name'
+            case $command in
+                show|list|dump|unregister)
+                    case $prev in
+                        $command)
+                            COMPREPLY=( $( compgen -W "$STRUCT_OPS_TYPE" -- "$cur" ) )
+                            ;;
+                        id)
+                            _bpftool_get_map_ids_for_type struct_ops
+                            ;;
+                        name)
+                            _bpftool_get_map_names_for_type struct_ops
+                            ;;
+                    esac
+                    return 0
+                    ;;
+                register)
+                    _filedir
+                    return 0
+                    ;;
+                *)
+                    [[ $prev == $object ]] && \
+                        COMPREPLY=( $( compgen -W 'register unregister show list dump help' \
+                            -- "$cur" ) )
+                    ;;
+            esac
+            ;;
         map)
             local MAP_TYPE='id pinned name'
             case $command in
diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
index 06449e846e4b..466c269eabdd 100644
--- a/tools/bpf/bpftool/main.c
+++ b/tools/bpf/bpftool/main.c
@@ -58,7 +58,7 @@ static int do_help(int argc, char **argv)
 		"       %s batch file FILE\n"
 		"       %s version\n"
 		"\n"
-		"       OBJECT := { prog | map | cgroup | perf | net | feature | btf | gen }\n"
+		"       OBJECT := { prog | map | cgroup | perf | net | feature | btf | gen | struct_ops }\n"
 		"       " HELP_SPEC_OPTIONS "\n"
 		"",
 		bin_name, bin_name, bin_name);
@@ -221,6 +221,7 @@ static const struct cmd cmds[] = {
 	{ "feature",	do_feature },
 	{ "btf",	do_btf },
 	{ "gen",	do_gen },
+	{ "struct_ops",	do_struct_ops },
 	{ "version",	do_version },
 	{ 0 }
 };
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index 6db2398ae7e9..86f14ce26fd7 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -161,6 +161,7 @@ int do_tracelog(int argc, char **arg);
 int do_feature(int argc, char **argv);
 int do_btf(int argc, char **argv);
 int do_gen(int argc, char **argv);
+int do_struct_ops(int argc, char **argv);
 
 int parse_u32_arg(int *argc, char ***argv, __u32 *val, const char *what);
 int prog_parse_fd(int *argc, char ***argv);
diff --git a/tools/bpf/bpftool/struct_ops.c b/tools/bpf/bpftool/struct_ops.c
new file mode 100644
index 000000000000..ba145e3d0d5d
--- /dev/null
+++ b/tools/bpf/bpftool/struct_ops.c
@@ -0,0 +1,595 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+/* Copyright (C) 2020 Facebook */
+
+#include <unistd.h>
+#include <stdio.h>
+#include <errno.h>
+
+#include <linux/err.h>
+#include <bpf/libbpf.h>
+#include <bpf/bpf.h>
+#include <bpf/btf.h>
+
+#include "json_writer.h"
+#include "main.h"
+
+#define STRUCT_OPS_VALUE_PREFIX "bpf_struct_ops_"
+
+static const struct btf_type *map_info_type;
+static __u32 map_info_alloc_len;
+static struct btf *btf_vmlinux;
+static __s32 map_info_type_id;
+
+struct res {
+	unsigned int nr_maps;
+	unsigned int nr_errs;
+};
+
+static const struct btf *get_btf_vmlinux(void)
+{
+	if (btf_vmlinux)
+		return btf_vmlinux;
+
+	btf_vmlinux = libbpf_find_kernel_btf();
+	if (IS_ERR(btf_vmlinux))
+		p_err("struct_ops requires kernel CONFIG_DEBUG_INFO_BTF=y");
+
+	return btf_vmlinux;
+}
+
+static const char *get_kern_struct_ops_name(const struct bpf_map_info *info)
+{
+	const struct btf *kern_btf;
+	const struct btf_type *t;
+	const char *st_ops_name;
+
+	kern_btf = get_btf_vmlinux();
+	if (IS_ERR(kern_btf))
+		return "<btf_vmlinux_not_found>";
+
+	t = btf__type_by_id(kern_btf, info->btf_vmlinux_value_type_id);
+	st_ops_name = btf__name_by_offset(kern_btf, t->name_off);
+	st_ops_name += strlen(STRUCT_OPS_VALUE_PREFIX);
+
+	return st_ops_name;
+}
+
+static __s32 get_map_info_type_id(void)
+{
+	const struct btf *kern_btf;
+
+	if (map_info_type_id)
+		return map_info_type_id;
+
+	kern_btf = get_btf_vmlinux();
+	if (IS_ERR(kern_btf)) {
+		map_info_type_id = PTR_ERR(kern_btf);
+		return map_info_type_id;
+	}
+
+	map_info_type_id = btf__find_by_name_kind(kern_btf, "bpf_map_info",
+						  BTF_KIND_STRUCT);
+	if (map_info_type_id < 0) {
+		p_err("can't find bpf_map_info from btf_vmlinux");
+		return map_info_type_id;
+	}
+	map_info_type = btf__type_by_id(kern_btf, map_info_type_id);
+
+	/* Ensure map_info_alloc() has at least what the bpftool needs */
+	map_info_alloc_len = map_info_type->size;
+	if (map_info_alloc_len < sizeof(struct bpf_map_info))
+		map_info_alloc_len = sizeof(struct bpf_map_info);
+
+	return map_info_type_id;
+}
+
+/* If the subcmd needs to print out the bpf_map_info,
+ * it should always call map_info_alloc to allocate
+ * a bpf_map_info object instead of allocating it
+ * on the stack.
+ *
+ * map_info_alloc() will take the running kernel's btf
+ * into account.  i.e. it will consider the
+ * sizeof(struct bpf_map_info) of the running kernel.
+ *
+ * It will enable the "struct_ops" cmd to print the latest
+ * "struct bpf_map_info".
+ *
+ * [ Recall that "struct_ops" requires the kernel's btf to
+ *   be available ]
+ */
+static struct bpf_map_info *map_info_alloc(__u32 *alloc_len)
+{
+	struct bpf_map_info *info;
+
+	if (get_map_info_type_id() < 0)
+		return NULL;
+
+	info = calloc(1, map_info_alloc_len);
+	if (!info)
+		p_err("mem alloc failed");
+	else
+		*alloc_len = map_info_alloc_len;
+
+	return info;
+}
+
+/* It iterates all struct_ops maps of the system.
+ * It returns the fd in "*res_fd" and map_info in "*info".
+ * In the very first iteration, info->id should be 0.
+ * An optional map "*name" filter can be specified.
+ * The filter can be made more flexibile in the future.
+ * e.g. filter by kernel-struct-ops-name, regex-name, glob-name, ...etc.
+ *
+ * Return value:
+ *     1: A struct_ops map found.  It is returned in "*res_fd" and "*info".
+ *	  The caller can continue to call get_next in the future.
+ *     0: No struct_ops map is returned.
+ *        All struct_ops map has been found.
+ *    -1: Error and the caller should abort the iteration.
+ */
+static int get_next_struct_ops_map(const char *name, int *res_fd,
+				   struct bpf_map_info *info, __u32 info_len)
+{
+	__u32 id = info->id;
+	int err, fd;
+
+	while (true) {
+		err = bpf_map_get_next_id(id, &id);
+		if (err) {
+			if (errno == ENOENT)
+				return 0;
+			p_err("can't get next map %s", strerror(errno));
+			return -1;
+		}
+
+		fd = bpf_map_get_fd_by_id(id);
+		if (fd < 0) {
+			if (errno == ENOENT)
+				continue;
+			p_err("can't get map by id (%u): %s",
+			      id, strerror(errno));
+			return -1;
+		}
+
+		err = bpf_obj_get_info_by_fd(fd, info, &info_len);
+		if (err) {
+			p_err("can't get map info: %s", strerror(errno));
+			close(fd);
+			return -1;
+		}
+
+		if (info->type == BPF_MAP_TYPE_STRUCT_OPS &&
+		    (!name || !strcmp(name, info->name))) {
+			*res_fd = fd;
+			return 1;
+		}
+		close(fd);
+	}
+}
+
+static int cmd_retval(const struct res *res, bool must_have_one_map)
+{
+	if (res->nr_errs || (!res->nr_maps && must_have_one_map))
+		return -1;
+
+	return 0;
+}
+
+/* "data" is the work_func private storage */
+typedef int (*work_func)(int fd, const struct bpf_map_info *info, void *data,
+			 struct json_writer *wtr);
+
+/* Find all struct_ops map in the system.
+ * Filter out by "name" (if specified).
+ * Then call "func(fd, info, data, wtr)" on each struct_ops map found.
+ */
+static struct res do_search(const char *name, work_func func, void *data,
+			    struct json_writer *wtr)
+{
+	struct bpf_map_info *info;
+	struct res res = {};
+	__u32 info_len;
+	int fd, err;
+
+	info = map_info_alloc(&info_len);
+	if (!info) {
+		res.nr_errs++;
+		return res;
+	}
+
+	if (wtr)
+		jsonw_start_array(wtr);
+	while ((err = get_next_struct_ops_map(name, &fd, info, info_len)) == 1) {
+		res.nr_maps++;
+		err = func(fd, info, data, wtr);
+		if (err)
+			res.nr_errs++;
+		close(fd);
+	}
+	if (wtr)
+		jsonw_end_array(wtr);
+
+	if (err)
+		res.nr_errs++;
+
+	if (!wtr && name && !res.nr_errs && !res.nr_maps)
+		/* It is not printing empty [].
+		 * Thus, needs to specifically say nothing found
+		 * for "name" here.
+		 */
+		p_err("no struct_ops found for %s", name);
+	else if (!wtr && json_output && !res.nr_errs)
+		/* The "func()" above is not writing any json (i.e. !wtr
+		 * test here).
+		 *
+		 * However, "-j" is enabled and there is no errs here,
+		 * so call json_null() as the current convention of
+		 * other cmds.
+		 */
+		jsonw_null(json_wtr);
+
+	free(info);
+	return res;
+}
+
+static struct res do_one_id(const char *id_str, work_func func, void *data,
+			    struct json_writer *wtr)
+{
+	struct bpf_map_info *info;
+	struct res res = {};
+	unsigned long id;
+	__u32 info_len;
+	char *endptr;
+	int fd;
+
+	id = strtoul(id_str, &endptr, 0);
+	if (*endptr || !id || id > UINT32_MAX) {
+		p_err("invalid id %s", id_str);
+		res.nr_errs++;
+		return res;
+	}
+
+	fd = bpf_map_get_fd_by_id(id);
+	if (fd == -1) {
+		p_err("can't get map by id (%lu): %s", id, strerror(errno));
+		res.nr_errs++;
+		return res;
+	}
+
+	info = map_info_alloc(&info_len);
+	if (!info) {
+		res.nr_errs++;
+		goto done;
+	}
+
+	if (bpf_obj_get_info_by_fd(fd, info, &info_len)) {
+		p_err("can't get map info: %s", strerror(errno));
+		res.nr_errs++;
+		goto done;
+	}
+
+	if (info->type != BPF_MAP_TYPE_STRUCT_OPS) {
+		p_err("%s id %u is not a struct_ops map", info->name, info->id);
+		res.nr_errs++;
+		goto done;
+	}
+
+	res.nr_maps++;
+
+	if (func(fd, info, data, wtr))
+		res.nr_errs++;
+	else if (!wtr && json_output)
+		/* The "func()" above is not writing any json (i.e. !wtr
+		 * test here).
+		 *
+		 * However, "-j" is enabled and there is no errs here,
+		 * so call json_null() as the current convention of
+		 * other cmds.
+		 */
+		jsonw_null(json_wtr);
+
+done:
+	free(info);
+	close(fd);
+
+	return res;
+}
+
+static struct res do_work_on_struct_ops(const char *search_type,
+					const char *search_term,
+					work_func func, void *data,
+					struct json_writer *wtr)
+{
+	if (search_type) {
+		if (is_prefix(search_type, "id"))
+			return do_one_id(search_term, func, data, wtr);
+		else if (!is_prefix(search_type, "name"))
+			usage();
+	}
+
+	return do_search(search_term, func, data, wtr);
+}
+
+static int __do_show(int fd, const struct bpf_map_info *info, void *data,
+		     struct json_writer *wtr)
+{
+	if (wtr) {
+		jsonw_start_object(wtr);
+		jsonw_uint_field(wtr, "id", info->id);
+		jsonw_string_field(wtr, "name", info->name);
+		jsonw_string_field(wtr, "kernel_struct_ops",
+				   get_kern_struct_ops_name(info));
+		jsonw_end_object(wtr);
+	} else {
+		printf("%u: %-15s %-32s\n", info->id, info->name,
+		       get_kern_struct_ops_name(info));
+	}
+
+	return 0;
+}
+
+static int do_show(int argc, char **argv)
+{
+	const char *search_type = NULL, *search_term = NULL;
+	struct res res;
+
+	if (argc && argc != 2)
+		usage();
+
+	if (argc == 2) {
+		search_type = argv[0];
+		search_term = argv[1];
+	}
+
+	res = do_work_on_struct_ops(search_type, search_term, __do_show,
+				    NULL, json_wtr);
+
+	return cmd_retval(&res, !!search_term);
+}
+
+static int __do_dump(int fd, const struct bpf_map_info *info, void *data,
+		     struct json_writer *wtr)
+{
+	struct btf_dumper *d = (struct btf_dumper *)data;
+	const struct btf_type *struct_ops_type;
+	const struct btf *kern_btf = d->btf;
+	const char *struct_ops_name;
+	int zero = 0;
+	void *value;
+
+	/* note: d->jw == wtr */
+
+	kern_btf = d->btf;
+
+	/* The kernel supporting BPF_MAP_TYPE_STRUCT_OPS must have
+	 * btf_vmlinux_value_type_id.
+	 */
+	struct_ops_type = btf__type_by_id(kern_btf,
+					  info->btf_vmlinux_value_type_id);
+	struct_ops_name = btf__name_by_offset(kern_btf,
+					      struct_ops_type->name_off);
+	value = calloc(1, info->value_size);
+	if (!value) {
+		p_err("mem alloc failed");
+		return -1;
+	}
+
+	if (bpf_map_lookup_elem(fd, &zero, value)) {
+		p_err("can't lookup struct_ops map %s id %u",
+		      info->name, info->id);
+		free(value);
+		return -1;
+	}
+
+	jsonw_start_object(wtr);
+	jsonw_name(wtr, "bpf_map_info");
+	btf_dumper_type(d, map_info_type_id, (void *)info);
+	jsonw_end_object(wtr);
+
+	jsonw_start_object(wtr);
+	jsonw_name(wtr, struct_ops_name);
+	btf_dumper_type(d, info->btf_vmlinux_value_type_id, value);
+	jsonw_end_object(wtr);
+
+	free(value);
+
+	return 0;
+}
+
+static int do_dump(int argc, char **argv)
+{
+	const char *search_type = NULL, *search_term = NULL;
+	json_writer_t *wtr = json_wtr;
+	const struct btf *kern_btf;
+	struct btf_dumper d = {};
+	struct res res;
+
+	if (argc && argc != 2)
+		usage();
+
+	if (argc == 2) {
+		search_type = argv[0];
+		search_term = argv[1];
+	}
+
+	kern_btf = get_btf_vmlinux();
+	if (IS_ERR(kern_btf))
+		return -1;
+
+	if (!json_output) {
+		wtr = jsonw_new(stdout);
+		if (!wtr) {
+			p_err("can't create json writer");
+			return -1;
+		}
+		jsonw_pretty(wtr, true);
+	}
+
+	d.btf = kern_btf;
+	d.jw = wtr;
+	d.is_plain_text = !json_output;
+	d.prog_id_as_func_ptr = true;
+
+	res = do_work_on_struct_ops(search_type, search_term, __do_dump, &d,
+				    wtr);
+
+	if (!json_output)
+		jsonw_destroy(&wtr);
+
+	return cmd_retval(&res, !!search_term);
+}
+
+static int __do_unregister(int fd, const struct bpf_map_info *info, void *data,
+			   struct json_writer *wtr)
+{
+	int zero = 0;
+
+	if (bpf_map_delete_elem(fd, &zero)) {
+		p_err("can't unload %s %s id %u: %s",
+		      get_kern_struct_ops_name(info), info->name,
+		      info->id, strerror(errno));
+		return -1;
+	}
+
+	p_info("Unregistered %s %s id %u",
+	       get_kern_struct_ops_name(info), info->name,
+	       info->id);
+
+	return 0;
+}
+
+static int do_unregister(int argc, char **argv)
+{
+	const char *search_type, *search_term;
+	struct res res;
+
+	if (argc != 2)
+		usage();
+
+	search_type = argv[0];
+	search_term = argv[1];
+
+	res = do_work_on_struct_ops(search_type, search_term,
+				    __do_unregister, NULL, NULL);
+
+	return cmd_retval(&res, true);
+}
+
+static int do_register(int argc, char **argv)
+{
+	const struct bpf_map_def *def;
+	struct bpf_map_info info = {};
+	__u32 info_len = sizeof(info);
+	int nr_errs = 0, nr_maps = 0;
+	struct bpf_object *obj;
+	struct bpf_link *link;
+	struct bpf_map *map;
+	const char *file;
+
+	if (argc != 1)
+		usage();
+
+	file = argv[0];
+
+	obj = bpf_object__open(file);
+	if (IS_ERR_OR_NULL(obj))
+		return -1;
+
+	set_max_rlimit();
+
+	if (bpf_object__load(obj)) {
+		bpf_object__close(obj);
+		return -1;
+	}
+
+	bpf_object__for_each_map(map, obj) {
+		def = bpf_map__def(map);
+		if (def->type != BPF_MAP_TYPE_STRUCT_OPS)
+			continue;
+
+		link = bpf_map__attach_struct_ops(map);
+		if (IS_ERR(link)) {
+			p_err("can't register struct_ops %s: %s",
+			      bpf_map__name(map),
+			      strerror(-PTR_ERR(link)));
+			nr_errs++;
+			continue;
+		}
+		nr_maps++;
+
+		bpf_link__disconnect(link);
+		bpf_link__destroy(link);
+
+		if (!bpf_obj_get_info_by_fd(bpf_map__fd(map), &info,
+					    &info_len))
+			p_info("Registered %s %s id %u",
+			       get_kern_struct_ops_name(&info),
+			       bpf_map__name(map),
+			       info.id);
+		else
+			/* Not p_err.  The struct_ops was attached
+			 * successfully.
+			 */
+			p_info("Registered %s but can't find id: %s",
+			       bpf_map__name(map), strerror(errno));
+	}
+
+	bpf_object__close(obj);
+
+	if (nr_errs)
+		return -1;
+
+	if (!nr_maps) {
+		p_err("no struct_ops found in %s", file);
+		return -1;
+	}
+
+	if (json_output)
+		jsonw_null(json_wtr);
+
+	return 0;
+}
+
+static int do_help(int argc, char **argv)
+{
+	if (json_output) {
+		jsonw_null(json_wtr);
+		return 0;
+	}
+
+	fprintf(stderr,
+		"Usage: %s %s { show | list } [STRUCT_OPS_MAP]\n"
+		"       %s %s dump [STRUCT_OPS_MAP]\n"
+		"       %s %s register OBJ\n"
+		"       %s %s unregister STRUCT_OPS_MAP\n"
+		"       %s %s help\n"
+		"\n"
+		"       OPTIONS := { {-j|--json} [{-p|--pretty}] }\n"
+		"       STRUCT_OPS_MAP := [ id STRUCT_OPS_MAP_ID | name STRUCT_OPS_MAP_NAME ]\n",
+		bin_name, argv[-2], bin_name, argv[-2],
+		bin_name, argv[-2], bin_name, argv[-2],
+		bin_name, argv[-2]);
+
+	return 0;
+}
+
+static const struct cmd cmds[] = {
+	{ "show",	do_show },
+	{ "list",	do_show },
+	{ "register",	do_register },
+	{ "unregister",	do_unregister },
+	{ "dump",	do_dump },
+	{ "help",	do_help },
+	{ 0 }
+};
+
+int do_struct_ops(int argc, char **argv)
+{
+	int err;
+
+	err = cmd_select(cmds, argc, argv, do_help);
+
+	btf__free(btf_vmlinux);
+	return err;
+}
-- 
2.17.1

Re: [PATCH bpf-next 4/4] bpftool: Add struct_ops support

[Quentin Monnet]

2020-03-15 17:56 UTC-0700 ~ Martin KaFai Lau <kafai@fb.com>
> This patch adds struct_ops support to the bpftool.

[...]

> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> ---
>  .../Documentation/bpftool-struct_ops.rst      | 106 ++++
>  tools/bpf/bpftool/bash-completion/bpftool     |  28 +
>  tools/bpf/bpftool/main.c                      |   3 +-
>  tools/bpf/bpftool/main.h                      |   1 +
>  tools/bpf/bpftool/struct_ops.c                | 595 ++++++++++++++++++
>  5 files changed, 732 insertions(+), 1 deletion(-)
>  create mode 100644 tools/bpf/bpftool/Documentation/bpftool-struct_ops.rst
>  create mode 100644 tools/bpf/bpftool/struct_ops.c
> 
> diff --git a/tools/bpf/bpftool/Documentation/bpftool-struct_ops.rst b/tools/bpf/bpftool/Documentation/bpftool-struct_ops.rst
> new file mode 100644
> index 000000000000..27aae5bc632e
> --- /dev/null
> +++ b/tools/bpf/bpftool/Documentation/bpftool-struct_ops.rst
> @@ -0,0 +1,106 @@
> +==================
> +bpftool-struct_ops
> +==================
> +-------------------------------------------------------------------------------
> +tool to register/unregister/introspect BPF struct_ops
> +-------------------------------------------------------------------------------
> +
> +:Manual section: 8
> +
> +SYNOPSIS
> +========
> +
> +	**bpftool** [*OPTIONS*] **struct_ops** *COMMAND*
> +
> +	*OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] }
> +
> +	*COMMANDS* :=
> +	{ **show** | **list** | **dump** | **register** | **unregister** | **help** }
> +
> +STRUCT_OPS COMMANDS
> +===================
> +
> +|	**bpftool** **struct_ops { show | list }** [*STRUCT_OPS_MAP*]
> +|	**bpftool** **struct_ops dump** [*STRUCT_OPS_MAP*]
> +|	**bpftool** **struct_ops register** *OBJ*
> +|	**bpftool** **struct_ops unregister** *STRUCT_OPS_MAP*
> +|	**bpftool** **struct_ops help**
> +|
> +|	*STRUCT_OPS_MAP* := { **id** *STRUCT_OPS_MAP_ID* | **name** *STRUCT_OPS_MAP_NAME* }
> +|	*OBJ* := /a/file/of/bpf_struct_ops.o
> +
> +
> +DESCRIPTION
> +===========
> +	**bpftool struct_ops { show | list }** [*STRUCT_OPS_MAP*]
> +		  Show brief information about the struct_ops in the system.
> +		  If *STRUCT_OPS_MAP* is specified, it shows information only
> +		  for the given struct_ops.  Otherwise, it lists all struct_ops
> +		  currently exists in the system.

Typo: s/exists/existing/

> +
> +		  Output will start with struct_ops map ID, followed by its map
> +		  name and its struct_ops's kernel type.
> +
> +	**bpftool struct_ops dump** [*STRUCT_OPS_MAP*]
> +		  Dump details information about the struct_ops in the system.
> +		  If *STRUCT_OPS_MAP* is specified, it dumps information only
> +		  for the given struct_ops.  Otherwise, it dumps all struct_ops
> +		  currently exists in the system.

Same here.

> +
> +	**bpftool struct_ops register** *OBJ*
> +		  Register bpf struct_ops from *OBJ*.  All struct_ops under
> +		  the ELF section ".struct_ops" will be registered to
> +		  its kernel subsystem.
> +
> +	**bpftool struct_ops unregister**  *STRUCT_OPS_MAP*
> +		  Unregister the *STRUCT_OPS_MAP* from the kernel subsystem.
> +
> +	**bpftool struct_ops help**
> +		  Print short help message.
> +
> +OPTIONS
> +=======
> +	-h, --help
> +		  Print short generic help message (similar to **bpftool help**).
> +
> +	-V, --version
> +		  Print version number (similar to **bpftool version**).
> +
> +	-j, --json
> +		  Generate JSON output. For commands that cannot produce JSON, this
> +		  option has no effect.
> +
> +	-p, --pretty
> +		  Generate human-readable JSON output. Implies **-j**.
> +
> +	-d, --debug
> +		  Print all logs available, even debug-level information. This
> +		  includes logs from libbpf as well as from the verifier, when
> +		  attempting to load programs.
> +
> +EXAMPLES
> +========
> +**# bpftool struct_ops show**
> +
> +::
> +
> +    100: dctcp           tcp_congestion_ops
> +    105: cubic           tcp_congestion_ops
> +
> +**# bpftool struct_ops unregister id 105**
> +
> +::
> +
> +   Unregistered tcp_congestion_ops cubic id 105
> +
> +**# bpftool struct_ops register bpf_cubic.o**
> +
> +::
> +
> +   Registered tcp_congestion_ops cubic id 110
> +
> +
> +SEE ALSO
> +========
> +	**bpftool-map**\ (8)
> +	**bpftool-prog**\ (8)

Other man pages link to all available bpftool-* pages. If you do not
want to do that, could you at least link to bpf(2) and bpftool(8) please?

[...]

> diff --git a/tools/bpf/bpftool/struct_ops.c b/tools/bpf/bpftool/struct_ops.c
> new file mode 100644
> index 000000000000..ba145e3d0d5d
> --- /dev/null
> +++ b/tools/bpf/bpftool/struct_ops.c
> @@ -0,0 +1,595 @@
> +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +/* Copyright (C) 2020 Facebook */
> +
> +#include <unistd.h>
> +#include <stdio.h>
> +#include <errno.h>
> +
> +#include <linux/err.h>

Nit: line break here, please

> +#include <bpf/libbpf.h>
> +#include <bpf/bpf.h>
> +#include <bpf/btf.h>

Nit again: Could you please have the includes in each block ordered
alphabetically?

> +
> +#include "json_writer.h"
> +#include "main.h"
> +
> +#define STRUCT_OPS_VALUE_PREFIX "bpf_struct_ops_"
> +

[...]

> +static struct bpf_map_info *map_info_alloc(__u32 *alloc_len)
> +{
> +	struct bpf_map_info *info;
> +
> +	if (get_map_info_type_id() < 0)
> +		return NULL;
> +
> +	info = calloc(1, map_info_alloc_len);
> +	if (!info)
> +		p_err("mem alloc failed");
> +	else
> +		*alloc_len = map_info_alloc_len;
> +
> +	return info;
> +}
> +
> +/* It iterates all struct_ops maps of the system.
> + * It returns the fd in "*res_fd" and map_info in "*info".
> + * In the very first iteration, info->id should be 0.
> + * An optional map "*name" filter can be specified.
> + * The filter can be made more flexibile in the future.

Typo: flexible

> + * e.g. filter by kernel-struct-ops-name, regex-name, glob-name, ...etc.
> + *
> + * Return value:
> + *     1: A struct_ops map found.  It is returned in "*res_fd" and "*info".
> + *	  The caller can continue to call get_next in the future.
> + *     0: No struct_ops map is returned.
> + *        All struct_ops map has been found.
> + *    -1: Error and the caller should abort the iteration.
> + */
> +static int get_next_struct_ops_map(const char *name, int *res_fd,
> +				   struct bpf_map_info *info, __u32 info_len)
> +{
> +	__u32 id = info->id;
> +	int err, fd;
> +
> +	while (true) {
> +		err = bpf_map_get_next_id(id, &id);
> +		if (err) {
> +			if (errno == ENOENT)
> +				return 0;
> +			p_err("can't get next map %s", strerror(errno));

Nit: Add a colon before "%s"?

> +			return -1;
> +		}
> +
> +		fd = bpf_map_get_fd_by_id(id);
> +		if (fd < 0) {
> +			if (errno == ENOENT)
> +				continue;
> +			p_err("can't get map by id (%u): %s",
> +			      id, strerror(errno));
> +			return -1;
> +		}
> +
> +		err = bpf_obj_get_info_by_fd(fd, info, &info_len);
> +		if (err) {
> +			p_err("can't get map info: %s", strerror(errno));
> +			close(fd);
> +			return -1;
> +		}
> +
> +		if (info->type == BPF_MAP_TYPE_STRUCT_OPS &&
> +		    (!name || !strcmp(name, info->name))) {
> +			*res_fd = fd;
> +			return 1;
> +		}
> +		close(fd);
> +	}
> +}

[...]

> +static int do_unregister(int argc, char **argv)
> +{
> +	const char *search_type, *search_term;
> +	struct res res;
> +
> +	if (argc != 2)
> +		usage();

Or you could reuse the macros in main.h, for more consistency with other
subcommands:

	if (!REQ_ARGS(2))
		return -1;

> +
> +	search_type = argv[0];
> +	search_term = argv[1];

	search_type = GET_ARG();
	search_term = GET_ARG();

> +
> +	res = do_work_on_struct_ops(search_type, search_term,
> +				    __do_unregister, NULL, NULL);
> +
> +	return cmd_retval(&res, true);
> +}
> +
> +static int do_register(int argc, char **argv)
> +{
> +	const struct bpf_map_def *def;
> +	struct bpf_map_info info = {};
> +	__u32 info_len = sizeof(info);
> +	int nr_errs = 0, nr_maps = 0;
> +	struct bpf_object *obj;
> +	struct bpf_link *link;
> +	struct bpf_map *map;
> +	const char *file;
> +
> +	if (argc != 1)
> +		usage();

(Same remark here.)

> +
> +	file = argv[0];
> +
> +	obj = bpf_object__open(file);
> +	if (IS_ERR_OR_NULL(obj))
> +		return -1;

[...]

Re: [PATCH bpf-next 0/4] bpftool: Add struct_ops support

[Quentin Monnet]

2020-03-15 17:55 UTC-0700 ~ Martin KaFai Lau <kafai@fb.com>
> This set adds "struct_ops" support to bpftool.
> 
> The first two patches improve the btf_dumper in bpftool.
> Patch 1: print the enum's name (if it is found) instead of the
>          enum's value.
> Patch 2: print a char[] as a string if all characters are printable.
> 
> "struct_ops" stores the prog_id in a func ptr.
> Instead of printing a prog_id,
> patch 3 adds an option to btf_dumper to allow a func ptr's value
> to be printed with the full func_proto info and the prog_name.
> 
> Patch 4 implements the "struct_ops" bpftool command.

Hi Martin, I have a few very small nits on patch 4 -- please see related
message -- but other than this your series looks good to me.

Acked-by: Quentin Monnet <quentin@isovalent.com>

(I did not test it, though.)

Thanks,
Quentin

Re: [PATCH bpf-next 4/4] bpftool: Add struct_ops support

[Martin KaFai Lau]

On Mon, Mar 16, 2020 at 11:54:28AM +0000, Quentin Monnet wrote:

> [...]
> 
> > +static int do_unregister(int argc, char **argv)
> > +{
> > +	const char *search_type, *search_term;
> > +	struct res res;
> > +
> > +	if (argc != 2)
> > +		usage();
> 
> Or you could reuse the macros in main.h, for more consistency with other
> subcommands:
> 
> 	if (!REQ_ARGS(2))
> 		return -1;
Thanks for the review!

I prefer to print out "usage();" whenever possible but then "-j" gave
me a 'null' after a json error mesage ...

# bpftool -j struct_ops unregister
{"error":"'unregister' needs at least 2 arguments, 0 found"},null

Then I went without REQ_ARGS(2) which is similar to a few existing
cases like do_dump(), do_updaate()...etc in map.c.

That was my consideration.  However, I can go back to use REQ_ARGS(2)
and return -1 without printing usage.  no strong preference here.

Re: [PATCH bpf-next 4/4] bpftool: Add struct_ops support

[Martin KaFai Lau]

On Mon, Mar 16, 2020 at 05:24:52PM -0700, Martin KaFai Lau wrote:
> On Mon, Mar 16, 2020 at 11:54:28AM +0000, Quentin Monnet wrote:
> 
> > [...]
> > 
> > > +static int do_unregister(int argc, char **argv)
> > > +{
> > > +	const char *search_type, *search_term;
> > > +	struct res res;
> > > +
> > > +	if (argc != 2)
> > > +		usage();
> > 
> > Or you could reuse the macros in main.h, for more consistency with other
> > subcommands:
> > 
> > 	if (!REQ_ARGS(2))
> > 		return -1;
> Thanks for the review!
> 
> I prefer to print out "usage();" whenever possible but then "-j" gave
> me a 'null' after a json error mesage ...
> 
> # bpftool -j struct_ops unregister
> {"error":"'unregister' needs at least 2 arguments, 0 found"},null
> 
> Then I went without REQ_ARGS(2) which is similar to a few existing
> cases like do_dump(), do_updaate()...etc in map.c.
> 
> That was my consideration.  However, I can go back to use REQ_ARGS(2)
> and return -1 without printing usage.  no strong preference here.
After another look,  I will keep it as is since REQ_ARGS() is a "<"
check.  "argc != 2" is the correct check here.  Otherwise,
allowing 'bpftool struct_ops unregister name cubic dctcp' looks weird.

Re: [PATCH bpf-next 4/4] bpftool: Add struct_ops support

[Quentin Monnet]

2020-03-16 17:57 UTC-0700 ~ Martin KaFai Lau <kafai@fb.com>
> On Mon, Mar 16, 2020 at 05:24:52PM -0700, Martin KaFai Lau wrote:
>> On Mon, Mar 16, 2020 at 11:54:28AM +0000, Quentin Monnet wrote:
>>
>>> [...]
>>>
>>>> +static int do_unregister(int argc, char **argv)
>>>> +{
>>>> +	const char *search_type, *search_term;
>>>> +	struct res res;
>>>> +
>>>> +	if (argc != 2)
>>>> +		usage();
>>>
>>> Or you could reuse the macros in main.h, for more consistency with other
>>> subcommands:
>>>
>>> 	if (!REQ_ARGS(2))
>>> 		return -1;
>> Thanks for the review!
>>
>> I prefer to print out "usage();" whenever possible but then "-j" gave
>> me a 'null' after a json error mesage ...
>>
>> # bpftool -j struct_ops unregister
>> {"error":"'unregister' needs at least 2 arguments, 0 found"},null
>>
>> Then I went without REQ_ARGS(2) which is similar to a few existing
>> cases like do_dump(), do_updaate()...etc in map.c.
>>
>> That was my consideration.  However, I can go back to use REQ_ARGS(2)
>> and return -1 without printing usage.  no strong preference here.
> After another look,  I will keep it as is since REQ_ARGS() is a "<"
> check.  "argc != 2" is the correct check here.  Otherwise,
> allowing 'bpftool struct_ops unregister name cubic dctcp' looks weird.
> 

Ah right, fair enough. I'm good with v2, thanks for the changes!
Quentin

Re: [PATCH bpf-next 1/4] bpftool: Print the enum's name instead of value

[Andrii Nakryiko]

On Sun, Mar 15, 2020 at 5:56 PM Martin KaFai Lau <kafai@fb.com> wrote:
>
> This patch prints the enum's name if there is one found in
> the array of btf_enum.
>
> The commit 9eea98497951 ("bpf: fix BTF verification of enums")
> has details about an enum could have any power-of-2 size (up to 8 bytes).
> This patch also takes this chance to accommodate these non 4 byte
> enums.
>
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> ---
>  tools/bpf/bpftool/btf_dumper.c | 35 +++++++++++++++++++++++++++++++---
>  1 file changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/tools/bpf/bpftool/btf_dumper.c b/tools/bpf/bpftool/btf_dumper.c
> index 01cc52b834fa..57bd6c0fafc9 100644
> --- a/tools/bpf/bpftool/btf_dumper.c
> +++ b/tools/bpf/bpftool/btf_dumper.c
> @@ -43,9 +43,38 @@ static int btf_dumper_modifier(const struct btf_dumper *d, __u32 type_id,
>         return btf_dumper_do_type(d, actual_type_id, bit_offset, data);
>  }
>
> -static void btf_dumper_enum(const void *data, json_writer_t *jw)
> +static void btf_dumper_enum(const struct btf_dumper *d,
> +                           const struct btf_type *t,
> +                           const void *data)
>  {
> -       jsonw_printf(jw, "%d", *(int *)data);
> +       const struct btf_enum *enums = btf_enum(t);
> +       __s64 value;
> +       __u16 i;
> +
> +       switch (t->size) {
> +       case 8:
> +               value = *(__s64 *)data;
> +               break;
> +       case 4:
> +               value = *(__s32 *)data;
> +               break;
> +       case 2:
> +               value = *(__s16 *)data;
> +               break;
> +       default:
> +               value = *(__s8 *)data;
> +       }

I'm scared of catch-all defaults like this. Let's do `case 1: `
explicitly and error out on anything else here?

> +
> +       for (i = 0; i < btf_vlen(t); i++) {
> +               if (value == enums[i].val) {
> +                       jsonw_string(d->jw,
> +                                    btf__name_by_offset(d->btf,
> +                                                        enums[i].name_off));
> +                       return;
> +               }
> +       }
> +
> +       jsonw_int(d->jw, value);
>  }
>
>  static int btf_dumper_array(const struct btf_dumper *d, __u32 type_id,
> @@ -366,7 +395,7 @@ static int btf_dumper_do_type(const struct btf_dumper *d, __u32 type_id,
>         case BTF_KIND_ARRAY:
>                 return btf_dumper_array(d, type_id, data);
>         case BTF_KIND_ENUM:
> -               btf_dumper_enum(data, d->jw);
> +               btf_dumper_enum(d, t, data);
>                 return 0;
>         case BTF_KIND_PTR:
>                 btf_dumper_ptr(data, d->jw, d->is_plain_text);
> --
> 2.17.1
>

Re: [PATCH bpf-next 2/4] bpftool: Print as a string for char array

[Andrii Nakryiko]

On Sun, Mar 15, 2020 at 5:57 PM Martin KaFai Lau <kafai@fb.com> wrote:
>
> A char[] is currently printed as an integer array.
> This patch will print it as a string when
> 1) The array element type is an one byte int
> 2) The array element type has a BTF_INT_CHAR encoding or
>    the array element type's name is "char"
> 3) All characters is between (0x1f, 0x7f) and it is terminated
>    by a null character.
>
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> ---
>  tools/bpf/bpftool/btf_dumper.c | 41 ++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
>
> diff --git a/tools/bpf/bpftool/btf_dumper.c b/tools/bpf/bpftool/btf_dumper.c
> index 57bd6c0fafc9..1d2d8d2cedea 100644
> --- a/tools/bpf/bpftool/btf_dumper.c
> +++ b/tools/bpf/bpftool/btf_dumper.c
> @@ -77,6 +77,42 @@ static void btf_dumper_enum(const struct btf_dumper *d,
>         jsonw_int(d->jw, value);
>  }
>
> +static bool is_str_array(const struct btf *btf, const struct btf_array *arr,
> +                        const char *s)
> +{
> +       const struct btf_type *elem_type;
> +       const char *end_s;
> +
> +       if (!arr->nelems)
> +               return false;
> +
> +       elem_type = btf__type_by_id(btf, arr->type);
> +       /* Not skipping typedef.  typedef to char does not count as
> +        * a string now.
> +        */
> +       while (elem_type && btf_is_mod(elem_type))
> +               elem_type = btf__type_by_id(btf, elem_type->type);
> +
> +       if (!elem_type || !btf_is_int(elem_type) || elem_type->size != 1)
> +               return false;
> +
> +       if (btf_int_encoding(elem_type) != BTF_INT_CHAR &&
> +           strcmp("char", btf__name_by_offset(btf, elem_type->name_off)))
> +               return false;
> +
> +       end_s = s + arr->nelems;
> +       while (s < end_s) {
> +               if (!*s)
> +                       return true;
> +               if (*s <= 0x1f || *s >= 0x7f)
> +                       return false;
> +               s++;
> +       }
> +
> +       /* '\0' is not found */
> +       return false;
> +}
> +
>  static int btf_dumper_array(const struct btf_dumper *d, __u32 type_id,
>                             const void *data)
>  {
> @@ -86,6 +122,11 @@ static int btf_dumper_array(const struct btf_dumper *d, __u32 type_id,
>         int ret = 0;
>         __u32 i;
>
> +       if (is_str_array(d->btf, arr, data)) {
> +               jsonw_string(d->jw, data);
> +               return 0;
> +       }
> +

Looks good, but curious how the string that contains ' or " will be
output in json? Will it be escaped properly or will result in
malformed JSON?

Acked-by: Andrii Nakryiko <andriin@fb.com>

>         elem_size = btf__resolve_size(d->btf, arr->type);
>         if (elem_size < 0)
>                 return elem_size;
> --
> 2.17.1
>

Re: [PATCH bpf-next 2/4] bpftool: Print as a string for char array

[Martin KaFai Lau]

On Tue, Mar 17, 2020 at 01:08:01PM -0700, Andrii Nakryiko wrote:
> On Sun, Mar 15, 2020 at 5:57 PM Martin KaFai Lau <kafai@fb.com> wrote:
> >
> > A char[] is currently printed as an integer array.
> > This patch will print it as a string when
> > 1) The array element type is an one byte int
> > 2) The array element type has a BTF_INT_CHAR encoding or
> >    the array element type's name is "char"
> > 3) All characters is between (0x1f, 0x7f) and it is terminated
> >    by a null character.
> >
> > Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> > ---
> >  tools/bpf/bpftool/btf_dumper.c | 41 ++++++++++++++++++++++++++++++++++
> >  1 file changed, 41 insertions(+)
> >
> > diff --git a/tools/bpf/bpftool/btf_dumper.c b/tools/bpf/bpftool/btf_dumper.c
> > index 57bd6c0fafc9..1d2d8d2cedea 100644
> > --- a/tools/bpf/bpftool/btf_dumper.c
> > +++ b/tools/bpf/bpftool/btf_dumper.c
> > @@ -77,6 +77,42 @@ static void btf_dumper_enum(const struct btf_dumper *d,
> >         jsonw_int(d->jw, value);
> >  }
> >
> > +static bool is_str_array(const struct btf *btf, const struct btf_array *arr,
> > +                        const char *s)
> > +{
> > +       const struct btf_type *elem_type;
> > +       const char *end_s;
> > +
> > +       if (!arr->nelems)
> > +               return false;
> > +
> > +       elem_type = btf__type_by_id(btf, arr->type);
> > +       /* Not skipping typedef.  typedef to char does not count as
> > +        * a string now.
> > +        */
> > +       while (elem_type && btf_is_mod(elem_type))
> > +               elem_type = btf__type_by_id(btf, elem_type->type);
> > +
> > +       if (!elem_type || !btf_is_int(elem_type) || elem_type->size != 1)
> > +               return false;
> > +
> > +       if (btf_int_encoding(elem_type) != BTF_INT_CHAR &&
> > +           strcmp("char", btf__name_by_offset(btf, elem_type->name_off)))
> > +               return false;
> > +
> > +       end_s = s + arr->nelems;
> > +       while (s < end_s) {
> > +               if (!*s)
> > +                       return true;
> > +               if (*s <= 0x1f || *s >= 0x7f)
> > +                       return false;
> > +               s++;
> > +       }
> > +
> > +       /* '\0' is not found */
> > +       return false;
> > +}
> > +
> >  static int btf_dumper_array(const struct btf_dumper *d, __u32 type_id,
> >                             const void *data)
> >  {
> > @@ -86,6 +122,11 @@ static int btf_dumper_array(const struct btf_dumper *d, __u32 type_id,
> >         int ret = 0;
> >         __u32 i;
> >
> > +       if (is_str_array(d->btf, arr, data)) {
> > +               jsonw_string(d->jw, data);
> > +               return 0;
> > +       }
> > +
> 
> Looks good, but curious how the string that contains ' or " will be
> output in json? Will it be escaped properly or will result in
> malformed JSON?
They will be escaped.