* [PATCH bpf-next 01/12] libbpf: Introduce libbpf_bpf_prog_type_str
2022-05-16 17:35 [PATCH bpf-next 00/12] libbpf: Textual representation of enums Daniel Müller
@ 2022-05-16 17:35 ` Daniel Müller
2022-05-16 17:35 ` [PATCH bpf-next 02/12] selftests/bpf: Add test for libbpf_bpf_prog_type_str Daniel Müller
` (12 subsequent siblings)
13 siblings, 0 replies; 24+ messages in thread
From: Daniel Müller @ 2022-05-16 17:35 UTC (permalink / raw)
To: bpf, ast, andrii, daniel, kernel-team, quentin
This change introduces a new function, libbpf_bpf_prog_type_str, to the
public libbpf API. The function allows users to get a string
representation for a bpf_prog_type variant.
Signed-off-by: Daniel Müller <deso@posteo.net>
---
tools/lib/bpf/libbpf.c | 43 ++++++++++++++++++++++++++++++++++++++++
tools/lib/bpf/libbpf.h | 9 +++++++++
tools/lib/bpf/libbpf.map | 1 +
3 files changed, 53 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 9aae886..236c82 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -72,6 +72,41 @@
static struct bpf_map *bpf_object__add_map(struct bpf_object *obj);
static bool prog_is_subprog(const struct bpf_object *obj, const struct bpf_program *prog);
+static const char * const prog_type_name[] = {
+ [BPF_PROG_TYPE_UNSPEC] = "unspec",
+ [BPF_PROG_TYPE_SOCKET_FILTER] = "socket_filter",
+ [BPF_PROG_TYPE_KPROBE] = "kprobe",
+ [BPF_PROG_TYPE_SCHED_CLS] = "sched_cls",
+ [BPF_PROG_TYPE_SCHED_ACT] = "sched_act",
+ [BPF_PROG_TYPE_TRACEPOINT] = "tracepoint",
+ [BPF_PROG_TYPE_XDP] = "xdp",
+ [BPF_PROG_TYPE_PERF_EVENT] = "perf_event",
+ [BPF_PROG_TYPE_CGROUP_SKB] = "cgroup_skb",
+ [BPF_PROG_TYPE_CGROUP_SOCK] = "cgroup_sock",
+ [BPF_PROG_TYPE_LWT_IN] = "lwt_in",
+ [BPF_PROG_TYPE_LWT_OUT] = "lwt_out",
+ [BPF_PROG_TYPE_LWT_XMIT] = "lwt_xmit",
+ [BPF_PROG_TYPE_SOCK_OPS] = "sock_ops",
+ [BPF_PROG_TYPE_SK_SKB] = "sk_skb",
+ [BPF_PROG_TYPE_CGROUP_DEVICE] = "cgroup_device",
+ [BPF_PROG_TYPE_SK_MSG] = "sk_msg",
+ [BPF_PROG_TYPE_RAW_TRACEPOINT] = "raw_tracepoint",
+ [BPF_PROG_TYPE_CGROUP_SOCK_ADDR] = "cgroup_sock_addr",
+ [BPF_PROG_TYPE_LWT_SEG6LOCAL] = "lwt_seg6local",
+ [BPF_PROG_TYPE_LIRC_MODE2] = "lirc_mode2",
+ [BPF_PROG_TYPE_SK_REUSEPORT] = "sk_reuseport",
+ [BPF_PROG_TYPE_FLOW_DISSECTOR] = "flow_dissector",
+ [BPF_PROG_TYPE_CGROUP_SYSCTL] = "cgroup_sysctl",
+ [BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE] = "raw_tracepoint_writable",
+ [BPF_PROG_TYPE_CGROUP_SOCKOPT] = "cgroup_sockopt",
+ [BPF_PROG_TYPE_TRACING] = "tracing",
+ [BPF_PROG_TYPE_STRUCT_OPS] = "struct_ops",
+ [BPF_PROG_TYPE_EXT] = "ext",
+ [BPF_PROG_TYPE_LSM] = "lsm",
+ [BPF_PROG_TYPE_SK_LOOKUP] = "sk_lookup",
+ [BPF_PROG_TYPE_SYSCALL] = "syscall",
+};
+
static int __base_pr(enum libbpf_print_level level, const char *format,
va_list args)
{
@@ -9300,6 +9335,14 @@ int libbpf_prog_type_by_name(const char *name, enum bpf_prog_type *prog_type,
return libbpf_err(-ESRCH);
}
+const char *libbpf_bpf_prog_type_str(enum bpf_prog_type t)
+{
+ if (t < 0 || t >= ARRAY_SIZE(prog_type_name))
+ return NULL;
+
+ return prog_type_name[t];
+}
+
static struct bpf_map *find_struct_ops_map_by_offset(struct bpf_object *obj,
size_t offset)
{
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 9e9a3f..11b5f8c 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -51,6 +51,15 @@ enum libbpf_errno {
LIBBPF_API int libbpf_strerror(int err, char *buf, size_t size);
+/**
+ * @brief **libbpf_bpf_prog_type_str()** converts the provided program type
+ * value into a textual representation.
+ * @param t The program type.
+ * @return Pointer to a static string identifying the program type. NULL is
+ * returned for unknown **bpf_prog_type** values.
+ */
+LIBBPF_API const char *libbpf_bpf_prog_type_str(enum bpf_prog_type t);
+
enum libbpf_print_level {
LIBBPF_WARN,
LIBBPF_INFO,
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 6b36f46..8b93647 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -456,6 +456,7 @@ LIBBPF_0.8.0 {
bpf_program__attach_trace_opts;
bpf_program__attach_usdt;
bpf_program__set_insns;
+ libbpf_bpf_prog_type_str;
libbpf_register_prog_handler;
libbpf_unregister_prog_handler;
} LIBBPF_0.7.0;
--
2.30.2
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH bpf-next 02/12] selftests/bpf: Add test for libbpf_bpf_prog_type_str
2022-05-16 17:35 [PATCH bpf-next 00/12] libbpf: Textual representation of enums Daniel Müller
2022-05-16 17:35 ` [PATCH bpf-next 01/12] libbpf: Introduce libbpf_bpf_prog_type_str Daniel Müller
@ 2022-05-16 17:35 ` Daniel Müller
2022-05-16 17:35 ` [PATCH bpf-next 03/12] bpftool: Use libbpf_bpf_prog_type_str Daniel Müller
` (11 subsequent siblings)
13 siblings, 0 replies; 24+ messages in thread
From: Daniel Müller @ 2022-05-16 17:35 UTC (permalink / raw)
To: bpf, ast, andrii, daniel, kernel-team, quentin
This change adds a test for libbpf_bpf_prog_type_str. The test retrieves
all variants of the bpf_prog_type enumeration using BTF and makes sure
that the function under test works as expected for them.
Signed-off-by: Daniel Müller <deso@posteo.net>
---
.../selftests/bpf/prog_tests/libbpf_str.c | 57 +++++++++++++++++++
1 file changed, 57 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/libbpf_str.c
diff --git a/tools/testing/selftests/bpf/prog_tests/libbpf_str.c b/tools/testing/selftests/bpf/prog_tests/libbpf_str.c
new file mode 100644
index 0000000..3e7a14
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/libbpf_str.c
@@ -0,0 +1,57 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2022 Meta Platforms, Inc. and affiliates. */
+
+#include <ctype.h>
+#include <test_progs.h>
+#include <bpf/btf.h>
+
+/**
+ * Utility function uppercasing an entire string.
+ */
+static void uppercase(char *s)
+{
+ for (; *s != '\0'; s++)
+ *s = toupper(*s);
+}
+
+/**
+ * Test case to check that all bpf_prog_type variants are covered by
+ * libbpf_bpf_prog_type_str.
+ */
+void test_libbpf_bpf_prog_type_str(void)
+{
+ struct btf *btf;
+ const struct btf_type *t;
+ const struct btf_enum *e;
+ int i, n, id;
+
+ btf = btf__parse("/sys/kernel/btf/vmlinux", NULL);
+ if (!ASSERT_OK_PTR(btf, "btf_parse"))
+ return;
+
+ /* find enum bpf_prog_type and enumerate each value */
+ id = btf__find_by_name_kind(btf, "bpf_prog_type", BTF_KIND_ENUM);
+ if (!ASSERT_GT(id, 0, "bpf_prog_type_id"))
+ goto cleanup;
+ t = btf__type_by_id(btf, id);
+ e = btf_enum(t);
+ n = btf_vlen(t);
+ for (i = 0; i < n; e++, i++) {
+ enum bpf_prog_type prog_type = (enum bpf_prog_type)e->val;
+ const char *prog_type_name;
+ const char *prog_type_str;
+ char buf[256];
+
+ prog_type_name = btf__str_by_offset(btf, e->name_off);
+ prog_type_str = libbpf_bpf_prog_type_str(prog_type);
+ ASSERT_OK_PTR(prog_type_str, prog_type_name);
+
+ snprintf(buf, sizeof(buf), "BPF_PROG_TYPE_%s", prog_type_str);
+ uppercase(buf);
+
+ ASSERT_STREQ(buf, prog_type_name, "exp_str_value");
+ }
+
+cleanup:
+ btf__free(btf);
+}
--
2.30.2
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH bpf-next 03/12] bpftool: Use libbpf_bpf_prog_type_str
2022-05-16 17:35 [PATCH bpf-next 00/12] libbpf: Textual representation of enums Daniel Müller
2022-05-16 17:35 ` [PATCH bpf-next 01/12] libbpf: Introduce libbpf_bpf_prog_type_str Daniel Müller
2022-05-16 17:35 ` [PATCH bpf-next 02/12] selftests/bpf: Add test for libbpf_bpf_prog_type_str Daniel Müller
@ 2022-05-16 17:35 ` Daniel Müller
2022-05-17 14:18 ` Quentin Monnet
2022-05-16 17:35 ` [PATCH bpf-next 04/12] libbpf: Introduce libbpf_bpf_map_type_str Daniel Müller
` (10 subsequent siblings)
13 siblings, 1 reply; 24+ messages in thread
From: Daniel Müller @ 2022-05-16 17:35 UTC (permalink / raw)
To: bpf, ast, andrii, daniel, kernel-team, quentin
This change switches bpftool over to using the recently introduced
libbpf_bpf_prog_type_str function instead of maintaining its own string
representation for the bpf_prog_type enum.
Signed-off-by: Daniel Müller <deso@posteo.net>
---
tools/bpf/bpftool/feature.c | 57 +++++++++++++++++++++++--------------
tools/bpf/bpftool/link.c | 19 +++++++------
tools/bpf/bpftool/main.h | 3 --
tools/bpf/bpftool/map.c | 13 +++++----
tools/bpf/bpftool/prog.c | 51 ++++++---------------------------
5 files changed, 64 insertions(+), 79 deletions(-)
diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
index d12f460..a093e1 100644
--- a/tools/bpf/bpftool/feature.c
+++ b/tools/bpf/bpftool/feature.c
@@ -548,8 +548,8 @@ static bool probe_prog_type_ifindex(enum bpf_prog_type prog_type, __u32 ifindex)
}
static void
-probe_prog_type(enum bpf_prog_type prog_type, bool *supported_types,
- const char *define_prefix, __u32 ifindex)
+probe_prog_type(enum bpf_prog_type prog_type, const char *prog_type_str,
+ bool *supported_types, const char *define_prefix, __u32 ifindex)
{
char feat_name[128], plain_desc[128], define_name[128];
const char *plain_comment = "eBPF program_type ";
@@ -580,20 +580,16 @@ probe_prog_type(enum bpf_prog_type prog_type, bool *supported_types,
supported_types[prog_type] |= res;
- if (!prog_type_name[prog_type]) {
- p_info("program type name not found (type %d)", prog_type);
- return;
- }
maxlen = sizeof(plain_desc) - strlen(plain_comment) - 1;
- if (strlen(prog_type_name[prog_type]) > maxlen) {
+ if (strlen(prog_type_str) > maxlen) {
p_info("program type name too long");
return;
}
- sprintf(feat_name, "have_%s_prog_type", prog_type_name[prog_type]);
- sprintf(define_name, "%s_prog_type", prog_type_name[prog_type]);
+ sprintf(feat_name, "have_%s_prog_type", prog_type_str);
+ sprintf(define_name, "%s_prog_type", prog_type_str);
uppercase(define_name, sizeof(define_name));
- sprintf(plain_desc, "%s%s", plain_comment, prog_type_name[prog_type]);
+ sprintf(plain_desc, "%s%s", plain_comment, prog_type_str);
print_bool_feature(feat_name, plain_desc, define_name, res,
define_prefix);
}
@@ -728,10 +724,10 @@ probe_helper_for_progtype(enum bpf_prog_type prog_type, bool supported_type,
}
static void
-probe_helpers_for_progtype(enum bpf_prog_type prog_type, bool supported_type,
+probe_helpers_for_progtype(enum bpf_prog_type prog_type,
+ char const *prog_type_str, bool supported_type,
const char *define_prefix, __u32 ifindex)
{
- const char *ptype_name = prog_type_name[prog_type];
char feat_name[128];
unsigned int id;
bool probe_res = false;
@@ -747,12 +743,12 @@ probe_helpers_for_progtype(enum bpf_prog_type prog_type, bool supported_type,
}
if (json_output) {
- sprintf(feat_name, "%s_available_helpers", ptype_name);
+ sprintf(feat_name, "%s_available_helpers", prog_type_str);
jsonw_name(json_wtr, feat_name);
jsonw_start_array(json_wtr);
} else if (!define_prefix) {
printf("eBPF helpers supported for program type %s:",
- ptype_name);
+ prog_type_str);
}
for (id = 1; id < ARRAY_SIZE(helper_name); id++) {
@@ -768,7 +764,7 @@ probe_helpers_for_progtype(enum bpf_prog_type prog_type, bool supported_type,
/* fallthrough */
default:
probe_res |= probe_helper_for_progtype(prog_type, supported_type,
- define_prefix, id, ptype_name,
+ define_prefix, id, prog_type_str,
ifindex);
}
}
@@ -943,15 +939,24 @@ static void
section_program_types(bool *supported_types, const char *define_prefix,
__u32 ifindex)
{
- unsigned int i;
+ unsigned int prog_type = BPF_PROG_TYPE_UNSPEC;
+ const char *prog_type_str;
print_start_section("program_types",
"Scanning eBPF program types...",
"/*** eBPF program types ***/",
define_prefix);
- for (i = BPF_PROG_TYPE_UNSPEC + 1; i < prog_type_name_size; i++)
- probe_prog_type(i, supported_types, define_prefix, ifindex);
+ while (true) {
+ prog_type++;
+ prog_type_str = libbpf_bpf_prog_type_str(prog_type);
+ /* libbpf will return NULL for variants unknown to it. */
+ if (!prog_type_str)
+ break;
+
+ probe_prog_type(prog_type, prog_type_str, supported_types, define_prefix,
+ ifindex);
+ }
print_end_section();
}
@@ -974,7 +979,8 @@ static void section_map_types(const char *define_prefix, __u32 ifindex)
static void
section_helpers(bool *supported_types, const char *define_prefix, __u32 ifindex)
{
- unsigned int i;
+ unsigned int prog_type = BPF_PROG_TYPE_UNSPEC;
+ const char *prog_type_str;
print_start_section("helpers",
"Scanning eBPF helper functions...",
@@ -996,9 +1002,18 @@ section_helpers(bool *supported_types, const char *define_prefix, __u32 ifindex)
" %sBPF__PROG_TYPE_ ## prog_type ## __HELPER_ ## helper\n",
define_prefix, define_prefix, define_prefix,
define_prefix);
- for (i = BPF_PROG_TYPE_UNSPEC + 1; i < prog_type_name_size; i++)
- probe_helpers_for_progtype(i, supported_types[i], define_prefix,
+ while (true) {
+ prog_type++;
+ prog_type_str = libbpf_bpf_prog_type_str(prog_type);
+ /* libbpf will return NULL for variants unknown to it. */
+ if (!prog_type_str)
+ break;
+
+ probe_helpers_for_progtype(prog_type, prog_type_str,
+ supported_types[prog_type],
+ define_prefix,
ifindex);
+ }
print_end_section();
}
diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
index 6353a78..e27108 100644
--- a/tools/bpf/bpftool/link.c
+++ b/tools/bpf/bpftool/link.c
@@ -121,6 +121,7 @@ static int get_prog_info(int prog_id, struct bpf_prog_info *info)
static int show_link_close_json(int fd, struct bpf_link_info *info)
{
struct bpf_prog_info prog_info;
+ const char *prog_type_str;
int err;
jsonw_start_object(json_wtr);
@@ -137,12 +138,12 @@ static int show_link_close_json(int fd, struct bpf_link_info *info)
if (err)
return err;
- if (prog_info.type < prog_type_name_size)
- jsonw_string_field(json_wtr, "prog_type",
- prog_type_name[prog_info.type]);
+ prog_type_str = libbpf_bpf_prog_type_str(prog_info.type);
+ /* libbpf will return NULL for variants unknown to it. */
+ if (prog_type_str)
+ jsonw_string_field(json_wtr, "prog_type", prog_type_str);
else
- jsonw_uint_field(json_wtr, "prog_type",
- prog_info.type);
+ jsonw_uint_field(json_wtr, "prog_type", prog_info.type);
show_link_attach_type_json(info->tracing.attach_type,
json_wtr);
@@ -214,6 +215,7 @@ static void show_iter_plain(struct bpf_link_info *info)
static int show_link_close_plain(int fd, struct bpf_link_info *info)
{
struct bpf_prog_info prog_info;
+ const char *prog_type_str;
int err;
show_link_header_plain(info);
@@ -228,9 +230,10 @@ static int show_link_close_plain(int fd, struct bpf_link_info *info)
if (err)
return err;
- if (prog_info.type < prog_type_name_size)
- printf("\n\tprog_type %s ",
- prog_type_name[prog_info.type]);
+ prog_type_str = libbpf_bpf_prog_type_str(prog_info.type);
+ /* libbpf will return NULL for variants unknown to it. */
+ if (prog_type_str)
+ printf("\n\tprog_type %s ", prog_type_str);
else
printf("\n\tprog_type %u ", prog_info.type);
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index aa99ff..74204d 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -63,9 +63,6 @@ static inline void *u64_to_ptr(__u64 ptr)
#define HELP_SPEC_LINK \
"LINK := { id LINK_ID | pinned FILE }"
-extern const char * const prog_type_name[];
-extern const size_t prog_type_name_size;
-
extern const char * const attach_type_name[__MAX_BPF_ATTACH_TYPE];
extern const char * const map_type_name[];
diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index 877387..70a1fd5 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -513,10 +513,12 @@ static int show_map_close_json(int fd, struct bpf_map_info *info)
if (owner_prog_type) {
unsigned int prog_type = atoi(owner_prog_type);
+ const char *prog_type_str;
- if (prog_type < prog_type_name_size)
+ prog_type_str = libbpf_bpf_prog_type_str(prog_type);
+ if (prog_type_str)
jsonw_string_field(json_wtr, "owner_prog_type",
- prog_type_name[prog_type]);
+ prog_type_str);
else
jsonw_uint_field(json_wtr, "owner_prog_type",
prog_type);
@@ -597,10 +599,11 @@ static int show_map_close_plain(int fd, struct bpf_map_info *info)
printf("\n\t");
if (owner_prog_type) {
unsigned int prog_type = atoi(owner_prog_type);
+ const char *prog_type_str;
- if (prog_type < prog_type_name_size)
- printf("owner_prog_type %s ",
- prog_type_name[prog_type]);
+ prog_type_str = libbpf_bpf_prog_type_str(prog_type);
+ if (prog_type_str)
+ printf("owner_prog_type %s ", prog_type_str);
else
printf("owner_prog_type %d ", prog_type);
}
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 5c2c63..39e1e71 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -36,43 +36,6 @@
#define BPF_METADATA_PREFIX "bpf_metadata_"
#define BPF_METADATA_PREFIX_LEN (sizeof(BPF_METADATA_PREFIX) - 1)
-const char * const prog_type_name[] = {
- [BPF_PROG_TYPE_UNSPEC] = "unspec",
- [BPF_PROG_TYPE_SOCKET_FILTER] = "socket_filter",
- [BPF_PROG_TYPE_KPROBE] = "kprobe",
- [BPF_PROG_TYPE_SCHED_CLS] = "sched_cls",
- [BPF_PROG_TYPE_SCHED_ACT] = "sched_act",
- [BPF_PROG_TYPE_TRACEPOINT] = "tracepoint",
- [BPF_PROG_TYPE_XDP] = "xdp",
- [BPF_PROG_TYPE_PERF_EVENT] = "perf_event",
- [BPF_PROG_TYPE_CGROUP_SKB] = "cgroup_skb",
- [BPF_PROG_TYPE_CGROUP_SOCK] = "cgroup_sock",
- [BPF_PROG_TYPE_LWT_IN] = "lwt_in",
- [BPF_PROG_TYPE_LWT_OUT] = "lwt_out",
- [BPF_PROG_TYPE_LWT_XMIT] = "lwt_xmit",
- [BPF_PROG_TYPE_SOCK_OPS] = "sock_ops",
- [BPF_PROG_TYPE_SK_SKB] = "sk_skb",
- [BPF_PROG_TYPE_CGROUP_DEVICE] = "cgroup_device",
- [BPF_PROG_TYPE_SK_MSG] = "sk_msg",
- [BPF_PROG_TYPE_RAW_TRACEPOINT] = "raw_tracepoint",
- [BPF_PROG_TYPE_CGROUP_SOCK_ADDR] = "cgroup_sock_addr",
- [BPF_PROG_TYPE_LWT_SEG6LOCAL] = "lwt_seg6local",
- [BPF_PROG_TYPE_LIRC_MODE2] = "lirc_mode2",
- [BPF_PROG_TYPE_SK_REUSEPORT] = "sk_reuseport",
- [BPF_PROG_TYPE_FLOW_DISSECTOR] = "flow_dissector",
- [BPF_PROG_TYPE_CGROUP_SYSCTL] = "cgroup_sysctl",
- [BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE] = "raw_tracepoint_writable",
- [BPF_PROG_TYPE_CGROUP_SOCKOPT] = "cgroup_sockopt",
- [BPF_PROG_TYPE_TRACING] = "tracing",
- [BPF_PROG_TYPE_STRUCT_OPS] = "struct_ops",
- [BPF_PROG_TYPE_EXT] = "ext",
- [BPF_PROG_TYPE_LSM] = "lsm",
- [BPF_PROG_TYPE_SK_LOOKUP] = "sk_lookup",
- [BPF_PROG_TYPE_SYSCALL] = "syscall",
-};
-
-const size_t prog_type_name_size = ARRAY_SIZE(prog_type_name);
-
enum dump_mode {
DUMP_JITED,
DUMP_XLATED,
@@ -428,12 +391,14 @@ static void show_prog_metadata(int fd, __u32 num_maps)
static void print_prog_header_json(struct bpf_prog_info *info, int fd)
{
+ const char *prog_type_str;
char prog_name[MAX_PROG_FULL_NAME];
jsonw_uint_field(json_wtr, "id", info->id);
- if (info->type < ARRAY_SIZE(prog_type_name))
- jsonw_string_field(json_wtr, "type",
- prog_type_name[info->type]);
+ prog_type_str = libbpf_bpf_prog_type_str(info->type);
+
+ if (prog_type_str)
+ jsonw_string_field(json_wtr, "type", prog_type_str);
else
jsonw_uint_field(json_wtr, "type", info->type);
@@ -515,11 +480,13 @@ static void print_prog_json(struct bpf_prog_info *info, int fd)
static void print_prog_header_plain(struct bpf_prog_info *info, int fd)
{
+ const char *prog_type_str;
char prog_name[MAX_PROG_FULL_NAME];
printf("%u: ", info->id);
- if (info->type < ARRAY_SIZE(prog_type_name))
- printf("%s ", prog_type_name[info->type]);
+ prog_type_str = libbpf_bpf_prog_type_str(info->type);
+ if (prog_type_str)
+ printf("%s ", prog_type_str);
else
printf("type %u ", info->type);
--
2.30.2
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH bpf-next 03/12] bpftool: Use libbpf_bpf_prog_type_str
2022-05-16 17:35 ` [PATCH bpf-next 03/12] bpftool: Use libbpf_bpf_prog_type_str Daniel Müller
@ 2022-05-17 14:18 ` Quentin Monnet
2022-05-17 19:09 ` Daniel Müller
0 siblings, 1 reply; 24+ messages in thread
From: Quentin Monnet @ 2022-05-17 14:18 UTC (permalink / raw)
To: Daniel Müller, bpf, ast, andrii, daniel, kernel-team
2022-05-16 17:35 UTC+0000 ~ Daniel Müller <deso@posteo.net>
> This change switches bpftool over to using the recently introduced
> libbpf_bpf_prog_type_str function instead of maintaining its own string
> representation for the bpf_prog_type enum.
>
> Signed-off-by: Daniel Müller <deso@posteo.net>
> ---
> tools/bpf/bpftool/feature.c | 57 +++++++++++++++++++++++--------------
> tools/bpf/bpftool/link.c | 19 +++++++------
> tools/bpf/bpftool/main.h | 3 --
> tools/bpf/bpftool/map.c | 13 +++++----
> tools/bpf/bpftool/prog.c | 51 ++++++---------------------------
> 5 files changed, 64 insertions(+), 79 deletions(-)
>
> diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
> index d12f460..a093e1 100644
> --- a/tools/bpf/bpftool/feature.c
> +++ b/tools/bpf/bpftool/feature.c
> @@ -728,10 +724,10 @@ probe_helper_for_progtype(enum bpf_prog_type prog_type, bool supported_type,
> }
>
> static void
> -probe_helpers_for_progtype(enum bpf_prog_type prog_type, bool supported_type,
> +probe_helpers_for_progtype(enum bpf_prog_type prog_type,
> + char const *prog_type_str, bool supported_type,
Nit: "const char*" for consistency?
> const char *define_prefix, __u32 ifindex)
> {
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH bpf-next 03/12] bpftool: Use libbpf_bpf_prog_type_str
2022-05-17 14:18 ` Quentin Monnet
@ 2022-05-17 19:09 ` Daniel Müller
0 siblings, 0 replies; 24+ messages in thread
From: Daniel Müller @ 2022-05-17 19:09 UTC (permalink / raw)
To: Quentin Monnet; +Cc: bpf, ast, andrii, daniel, kernel-team
On Tue, May 17, 2022 at 03:18:24PM +0100, Quentin Monnet wrote:
> 2022-05-16 17:35 UTC+0000 ~ Daniel Müller <deso@posteo.net>
> > This change switches bpftool over to using the recently introduced
> > libbpf_bpf_prog_type_str function instead of maintaining its own string
> > representation for the bpf_prog_type enum.
> >
> > Signed-off-by: Daniel Müller <deso@posteo.net>
> > ---
> > tools/bpf/bpftool/feature.c | 57 +++++++++++++++++++++++--------------
> > tools/bpf/bpftool/link.c | 19 +++++++------
> > tools/bpf/bpftool/main.h | 3 --
> > tools/bpf/bpftool/map.c | 13 +++++----
> > tools/bpf/bpftool/prog.c | 51 ++++++---------------------------
> > 5 files changed, 64 insertions(+), 79 deletions(-)
> >
> > diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
> > index d12f460..a093e1 100644
> > --- a/tools/bpf/bpftool/feature.c
> > +++ b/tools/bpf/bpftool/feature.c
>
> > @@ -728,10 +724,10 @@ probe_helper_for_progtype(enum bpf_prog_type prog_type, bool supported_type,
> > }
> >
> > static void
> > -probe_helpers_for_progtype(enum bpf_prog_type prog_type, bool supported_type,
> > +probe_helpers_for_progtype(enum bpf_prog_type prog_type,
> > + char const *prog_type_str, bool supported_type,
>
> Nit: "const char*" for consistency?
Good catch. Yes, will make the change.
Thanks,
Daniel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH bpf-next 04/12] libbpf: Introduce libbpf_bpf_map_type_str
2022-05-16 17:35 [PATCH bpf-next 00/12] libbpf: Textual representation of enums Daniel Müller
` (2 preceding siblings ...)
2022-05-16 17:35 ` [PATCH bpf-next 03/12] bpftool: Use libbpf_bpf_prog_type_str Daniel Müller
@ 2022-05-16 17:35 ` Daniel Müller
2022-05-16 17:35 ` [PATCH bpf-next 05/12] selftests/bpf: Add test for libbpf_bpf_map_type_str Daniel Müller
` (9 subsequent siblings)
13 siblings, 0 replies; 24+ messages in thread
From: Daniel Müller @ 2022-05-16 17:35 UTC (permalink / raw)
To: bpf, ast, andrii, daniel, kernel-team, quentin
This change introduces a new function, libbpf_bpf_map_type_str, to the
public libbpf API. The function allows users to get a string
representation for a bpf_map_type enum variant.
Signed-off-by: Daniel Müller <deso@posteo.net>
---
tools/lib/bpf/libbpf.c | 42 ++++++++++++++++++++++++++++++++++++++++
tools/lib/bpf/libbpf.h | 9 +++++++++
tools/lib/bpf/libbpf.map | 1 +
3 files changed, 52 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 236c82..c17f836 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -72,6 +72,40 @@
static struct bpf_map *bpf_object__add_map(struct bpf_object *obj);
static bool prog_is_subprog(const struct bpf_object *obj, const struct bpf_program *prog);
+static const char * const map_type_name[] = {
+ [BPF_MAP_TYPE_UNSPEC] = "unspec",
+ [BPF_MAP_TYPE_HASH] = "hash",
+ [BPF_MAP_TYPE_ARRAY] = "array",
+ [BPF_MAP_TYPE_PROG_ARRAY] = "prog_array",
+ [BPF_MAP_TYPE_PERF_EVENT_ARRAY] = "perf_event_array",
+ [BPF_MAP_TYPE_PERCPU_HASH] = "percpu_hash",
+ [BPF_MAP_TYPE_PERCPU_ARRAY] = "percpu_array",
+ [BPF_MAP_TYPE_STACK_TRACE] = "stack_trace",
+ [BPF_MAP_TYPE_CGROUP_ARRAY] = "cgroup_array",
+ [BPF_MAP_TYPE_LRU_HASH] = "lru_hash",
+ [BPF_MAP_TYPE_LRU_PERCPU_HASH] = "lru_percpu_hash",
+ [BPF_MAP_TYPE_LPM_TRIE] = "lpm_trie",
+ [BPF_MAP_TYPE_ARRAY_OF_MAPS] = "array_of_maps",
+ [BPF_MAP_TYPE_HASH_OF_MAPS] = "hash_of_maps",
+ [BPF_MAP_TYPE_DEVMAP] = "devmap",
+ [BPF_MAP_TYPE_DEVMAP_HASH] = "devmap_hash",
+ [BPF_MAP_TYPE_SOCKMAP] = "sockmap",
+ [BPF_MAP_TYPE_CPUMAP] = "cpumap",
+ [BPF_MAP_TYPE_XSKMAP] = "xskmap",
+ [BPF_MAP_TYPE_SOCKHASH] = "sockhash",
+ [BPF_MAP_TYPE_CGROUP_STORAGE] = "cgroup_storage",
+ [BPF_MAP_TYPE_REUSEPORT_SOCKARRAY] = "reuseport_sockarray",
+ [BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE] = "percpu_cgroup_storage",
+ [BPF_MAP_TYPE_QUEUE] = "queue",
+ [BPF_MAP_TYPE_STACK] = "stack",
+ [BPF_MAP_TYPE_SK_STORAGE] = "sk_storage",
+ [BPF_MAP_TYPE_STRUCT_OPS] = "struct_ops",
+ [BPF_MAP_TYPE_RINGBUF] = "ringbuf",
+ [BPF_MAP_TYPE_INODE_STORAGE] = "inode_storage",
+ [BPF_MAP_TYPE_TASK_STORAGE] = "task_storage",
+ [BPF_MAP_TYPE_BLOOM_FILTER] = "bloom_filter",
+};
+
static const char * const prog_type_name[] = {
[BPF_PROG_TYPE_UNSPEC] = "unspec",
[BPF_PROG_TYPE_SOCKET_FILTER] = "socket_filter",
@@ -9335,6 +9369,14 @@ int libbpf_prog_type_by_name(const char *name, enum bpf_prog_type *prog_type,
return libbpf_err(-ESRCH);
}
+const char *libbpf_bpf_map_type_str(enum bpf_map_type t)
+{
+ if (t < 0 || t >= ARRAY_SIZE(map_type_name))
+ return NULL;
+
+ return map_type_name[t];
+}
+
const char *libbpf_bpf_prog_type_str(enum bpf_prog_type t)
{
if (t < 0 || t >= ARRAY_SIZE(prog_type_name))
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 11b5f8c..6c5b07 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -51,6 +51,15 @@ enum libbpf_errno {
LIBBPF_API int libbpf_strerror(int err, char *buf, size_t size);
+/**
+ * @brief **libbpf_bpf_map_type_str()** converts the provided map type value
+ * into a textual representation.
+ * @param t The map type.
+ * @return Pointer to a static string identifying the map type. NULL is
+ * returned for unknown **bpf_map_type** values.
+ */
+LIBBPF_API const char *libbpf_bpf_map_type_str(enum bpf_map_type t);
+
/**
* @brief **libbpf_bpf_prog_type_str()** converts the provided program type
* value into a textual representation.
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 8b93647..c63624 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -456,6 +456,7 @@ LIBBPF_0.8.0 {
bpf_program__attach_trace_opts;
bpf_program__attach_usdt;
bpf_program__set_insns;
+ libbpf_bpf_map_type_str;
libbpf_bpf_prog_type_str;
libbpf_register_prog_handler;
libbpf_unregister_prog_handler;
--
2.30.2
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH bpf-next 05/12] selftests/bpf: Add test for libbpf_bpf_map_type_str
2022-05-16 17:35 [PATCH bpf-next 00/12] libbpf: Textual representation of enums Daniel Müller
` (3 preceding siblings ...)
2022-05-16 17:35 ` [PATCH bpf-next 04/12] libbpf: Introduce libbpf_bpf_map_type_str Daniel Müller
@ 2022-05-16 17:35 ` Daniel Müller
2022-05-16 17:35 ` [PATCH bpf-next 06/12] bpftool: Use libbpf_bpf_map_type_str Daniel Müller
` (8 subsequent siblings)
13 siblings, 0 replies; 24+ messages in thread
From: Daniel Müller @ 2022-05-16 17:35 UTC (permalink / raw)
To: bpf, ast, andrii, daniel, kernel-team, quentin
This change adds a test for libbpf_bpf_map_type_str. The test retrieves
all variants of the bpf_map_type enumeration using BTF and makes sure
that the function under test works as expected for them.
Signed-off-by: Daniel Müller <deso@posteo.net>
---
.../selftests/bpf/prog_tests/libbpf_str.c | 56 ++++++++++++++++++-
1 file changed, 55 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/libbpf_str.c b/tools/testing/selftests/bpf/prog_tests/libbpf_str.c
index 3e7a14..0f15aaa 100644
--- a/tools/testing/selftests/bpf/prog_tests/libbpf_str.c
+++ b/tools/testing/selftests/bpf/prog_tests/libbpf_str.c
@@ -14,11 +14,53 @@ static void uppercase(char *s)
*s = toupper(*s);
}
+/**
+ * Test case to check that all bpf_map_type variants are covered by
+ * libbpf_bpf_map_type_str.
+ */
+static void test_libbpf_bpf_map_type_str(void)
+{
+ struct btf *btf;
+ const struct btf_type *t;
+ const struct btf_enum *e;
+ int i, n, id;
+
+ btf = btf__parse("/sys/kernel/btf/vmlinux", NULL);
+ if (!ASSERT_OK_PTR(btf, "btf_parse"))
+ return;
+
+ /* find enum bpf_map_type and enumerate each value */
+ id = btf__find_by_name_kind(btf, "bpf_map_type", BTF_KIND_ENUM);
+ if (!ASSERT_GT(id, 0, "bpf_map_type_id"))
+ goto cleanup;
+ t = btf__type_by_id(btf, id);
+ e = btf_enum(t);
+ n = btf_vlen(t);
+ for (i = 0; i < n; e++, i++) {
+ enum bpf_map_type map_type = (enum bpf_map_type)e->val;
+ const char *map_type_name;
+ const char *map_type_str;
+ char buf[256];
+
+ map_type_name = btf__str_by_offset(btf, e->name_off);
+ map_type_str = libbpf_bpf_map_type_str(map_type);
+ ASSERT_OK_PTR(map_type_str, map_type_name);
+
+ snprintf(buf, sizeof(buf), "BPF_MAP_TYPE_%s", map_type_str);
+ uppercase(buf);
+
+ ASSERT_STREQ(buf, map_type_name, "exp_str_value");
+ }
+
+cleanup:
+ btf__free(btf);
+}
+
/**
* Test case to check that all bpf_prog_type variants are covered by
* libbpf_bpf_prog_type_str.
*/
-void test_libbpf_bpf_prog_type_str(void)
+static void test_libbpf_bpf_prog_type_str(void)
{
struct btf *btf;
const struct btf_type *t;
@@ -55,3 +97,15 @@ void test_libbpf_bpf_prog_type_str(void)
cleanup:
btf__free(btf);
}
+
+/**
+ * Run all libbpf str conversion tests.
+ */
+void test_libbpf_str(void)
+{
+ if (test__start_subtest("bpf_map_type_str"))
+ test_libbpf_bpf_map_type_str();
+
+ if (test__start_subtest("bpf_prog_type_str"))
+ test_libbpf_bpf_prog_type_str();
+}
--
2.30.2
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH bpf-next 06/12] bpftool: Use libbpf_bpf_map_type_str
2022-05-16 17:35 [PATCH bpf-next 00/12] libbpf: Textual representation of enums Daniel Müller
` (4 preceding siblings ...)
2022-05-16 17:35 ` [PATCH bpf-next 05/12] selftests/bpf: Add test for libbpf_bpf_map_type_str Daniel Müller
@ 2022-05-16 17:35 ` Daniel Müller
2022-05-16 17:35 ` [PATCH bpf-next 07/12] libbpf: Introduce libbpf_bpf_attach_type_str Daniel Müller
` (7 subsequent siblings)
13 siblings, 0 replies; 24+ messages in thread
From: Daniel Müller @ 2022-05-16 17:35 UTC (permalink / raw)
To: bpf, ast, andrii, daniel, kernel-team, quentin
This change switches bpftool over to using the recently introduced
libbpf_bpf_map_type_str function instead of maintaining its own string
representation for the bpf_map_type enum.
Signed-off-by: Daniel Müller <deso@posteo.net>
---
tools/bpf/bpftool/feature.c | 30 +++++++++-------
tools/bpf/bpftool/main.h | 3 --
tools/bpf/bpftool/map.c | 69 ++++++++++++++-----------------------
3 files changed, 42 insertions(+), 60 deletions(-)
diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
index a093e1..236c53 100644
--- a/tools/bpf/bpftool/feature.c
+++ b/tools/bpf/bpftool/feature.c
@@ -615,8 +615,8 @@ static bool probe_map_type_ifindex(enum bpf_map_type map_type, __u32 ifindex)
}
static void
-probe_map_type(enum bpf_map_type map_type, const char *define_prefix,
- __u32 ifindex)
+probe_map_type(enum bpf_map_type map_type, char const *map_type_str,
+ const char *define_prefix, __u32 ifindex)
{
char feat_name[128], plain_desc[128], define_name[128];
const char *plain_comment = "eBPF map_type ";
@@ -641,20 +641,16 @@ probe_map_type(enum bpf_map_type map_type, const char *define_prefix,
* check required for unprivileged users
*/
- if (!map_type_name[map_type]) {
- p_info("map type name not found (type %d)", map_type);
- return;
- }
maxlen = sizeof(plain_desc) - strlen(plain_comment) - 1;
- if (strlen(map_type_name[map_type]) > maxlen) {
+ if (strlen(map_type_str) > maxlen) {
p_info("map type name too long");
return;
}
- sprintf(feat_name, "have_%s_map_type", map_type_name[map_type]);
- sprintf(define_name, "%s_map_type", map_type_name[map_type]);
+ sprintf(feat_name, "have_%s_map_type", map_type_str);
+ sprintf(define_name, "%s_map_type", map_type_str);
uppercase(define_name, sizeof(define_name));
- sprintf(plain_desc, "%s%s", plain_comment, map_type_name[map_type]);
+ sprintf(plain_desc, "%s%s", plain_comment, map_type_str);
print_bool_feature(feat_name, plain_desc, define_name, res,
define_prefix);
}
@@ -963,15 +959,23 @@ section_program_types(bool *supported_types, const char *define_prefix,
static void section_map_types(const char *define_prefix, __u32 ifindex)
{
- unsigned int i;
+ unsigned int map_type = BPF_MAP_TYPE_UNSPEC;
+ const char *map_type_str;
print_start_section("map_types",
"Scanning eBPF map types...",
"/*** eBPF map types ***/",
define_prefix);
- for (i = BPF_MAP_TYPE_UNSPEC + 1; i < map_type_name_size; i++)
- probe_map_type(i, define_prefix, ifindex);
+ while (true) {
+ map_type++;
+ map_type_str = libbpf_bpf_map_type_str(map_type);
+ /* libbpf will return NULL for variants unknown to it. */
+ if (!map_type_str)
+ break;
+
+ probe_map_type(map_type, map_type_str, define_prefix, ifindex);
+ }
print_end_section();
}
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index 74204d..e4fdaa0 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -65,9 +65,6 @@ static inline void *u64_to_ptr(__u64 ptr)
extern const char * const attach_type_name[__MAX_BPF_ATTACH_TYPE];
-extern const char * const map_type_name[];
-extern const size_t map_type_name_size;
-
/* keep in sync with the definition in skeleton/pid_iter.bpf.c */
enum bpf_obj_type {
BPF_OBJ_UNKNOWN,
diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index 70a1fd5..800834b 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -22,42 +22,6 @@
#include "json_writer.h"
#include "main.h"
-const char * const map_type_name[] = {
- [BPF_MAP_TYPE_UNSPEC] = "unspec",
- [BPF_MAP_TYPE_HASH] = "hash",
- [BPF_MAP_TYPE_ARRAY] = "array",
- [BPF_MAP_TYPE_PROG_ARRAY] = "prog_array",
- [BPF_MAP_TYPE_PERF_EVENT_ARRAY] = "perf_event_array",
- [BPF_MAP_TYPE_PERCPU_HASH] = "percpu_hash",
- [BPF_MAP_TYPE_PERCPU_ARRAY] = "percpu_array",
- [BPF_MAP_TYPE_STACK_TRACE] = "stack_trace",
- [BPF_MAP_TYPE_CGROUP_ARRAY] = "cgroup_array",
- [BPF_MAP_TYPE_LRU_HASH] = "lru_hash",
- [BPF_MAP_TYPE_LRU_PERCPU_HASH] = "lru_percpu_hash",
- [BPF_MAP_TYPE_LPM_TRIE] = "lpm_trie",
- [BPF_MAP_TYPE_ARRAY_OF_MAPS] = "array_of_maps",
- [BPF_MAP_TYPE_HASH_OF_MAPS] = "hash_of_maps",
- [BPF_MAP_TYPE_DEVMAP] = "devmap",
- [BPF_MAP_TYPE_DEVMAP_HASH] = "devmap_hash",
- [BPF_MAP_TYPE_SOCKMAP] = "sockmap",
- [BPF_MAP_TYPE_CPUMAP] = "cpumap",
- [BPF_MAP_TYPE_XSKMAP] = "xskmap",
- [BPF_MAP_TYPE_SOCKHASH] = "sockhash",
- [BPF_MAP_TYPE_CGROUP_STORAGE] = "cgroup_storage",
- [BPF_MAP_TYPE_REUSEPORT_SOCKARRAY] = "reuseport_sockarray",
- [BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE] = "percpu_cgroup_storage",
- [BPF_MAP_TYPE_QUEUE] = "queue",
- [BPF_MAP_TYPE_STACK] = "stack",
- [BPF_MAP_TYPE_SK_STORAGE] = "sk_storage",
- [BPF_MAP_TYPE_STRUCT_OPS] = "struct_ops",
- [BPF_MAP_TYPE_RINGBUF] = "ringbuf",
- [BPF_MAP_TYPE_INODE_STORAGE] = "inode_storage",
- [BPF_MAP_TYPE_TASK_STORAGE] = "task_storage",
- [BPF_MAP_TYPE_BLOOM_FILTER] = "bloom_filter",
-};
-
-const size_t map_type_name_size = ARRAY_SIZE(map_type_name);
-
static struct hashmap *map_table;
static bool map_is_per_cpu(__u32 type)
@@ -81,12 +45,18 @@ static bool map_is_map_of_progs(__u32 type)
static int map_type_from_str(const char *type)
{
+ const char *map_type_str;
unsigned int i;
- for (i = 0; i < ARRAY_SIZE(map_type_name); i++)
+ for (i = 0; ; i++) {
+ map_type_str = libbpf_bpf_map_type_str(i);
+ if (!map_type_str)
+ break;
+
/* Don't allow prefixing in case of possible future shadowing */
- if (map_type_name[i] && !strcmp(map_type_name[i], type))
+ if (!strcmp(map_type_str, type))
return i;
+ }
return -1;
}
@@ -472,9 +442,12 @@ static int parse_elem(char **argv, struct bpf_map_info *info,
static void show_map_header_json(struct bpf_map_info *info, json_writer_t *wtr)
{
+ const char *map_type_str;
+
jsonw_uint_field(wtr, "id", info->id);
- if (info->type < ARRAY_SIZE(map_type_name))
- jsonw_string_field(wtr, "type", map_type_name[info->type]);
+ map_type_str = libbpf_bpf_map_type_str(info->type);
+ if (map_type_str)
+ jsonw_string_field(wtr, "type", map_type_str);
else
jsonw_uint_field(wtr, "type", info->type);
@@ -561,9 +534,13 @@ static int show_map_close_json(int fd, struct bpf_map_info *info)
static void show_map_header_plain(struct bpf_map_info *info)
{
+ const char *map_type_str;
+
printf("%u: ", info->id);
- if (info->type < ARRAY_SIZE(map_type_name))
- printf("%s ", map_type_name[info->type]);
+
+ map_type_str = libbpf_bpf_map_type_str(info->type);
+ if (map_type_str)
+ printf("%s ", map_type_str);
else
printf("type %u ", info->type);
@@ -879,9 +856,13 @@ map_dump(int fd, struct bpf_map_info *info, json_writer_t *wtr,
}
if (info->type == BPF_MAP_TYPE_REUSEPORT_SOCKARRAY &&
- info->value_size != 8)
+ info->value_size != 8) {
+ const char *map_type_str;
+
+ map_type_str = libbpf_bpf_map_type_str(info->type);
p_info("Warning: cannot read values from %s map with value_size != 8",
- map_type_name[info->type]);
+ map_type_str);
+ }
while (true) {
err = bpf_map_get_next_key(fd, prev_key, key);
if (err) {
--
2.30.2
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH bpf-next 07/12] libbpf: Introduce libbpf_bpf_attach_type_str
2022-05-16 17:35 [PATCH bpf-next 00/12] libbpf: Textual representation of enums Daniel Müller
` (5 preceding siblings ...)
2022-05-16 17:35 ` [PATCH bpf-next 06/12] bpftool: Use libbpf_bpf_map_type_str Daniel Müller
@ 2022-05-16 17:35 ` Daniel Müller
2022-05-16 17:35 ` [PATCH bpf-next 08/12] selftests/bpf: Add test for libbpf_bpf_attach_type_str Daniel Müller
` (6 subsequent siblings)
13 siblings, 0 replies; 24+ messages in thread
From: Daniel Müller @ 2022-05-16 17:35 UTC (permalink / raw)
To: bpf, ast, andrii, daniel, kernel-team, quentin
This change introduces a new function, libbpf_bpf_attach_type_str, to
the public libbpf API. The function allows users to get a string
representation for a bpf_attach_type variant.
Signed-off-by: Daniel Müller <deso@posteo.net>
---
tools/lib/bpf/libbpf.c | 54 ++++++++++++++++++++++++++++++++++++++++
tools/lib/bpf/libbpf.h | 9 +++++++
tools/lib/bpf/libbpf.map | 1 +
3 files changed, 64 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index c17f836..a69a752 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -72,6 +72,52 @@
static struct bpf_map *bpf_object__add_map(struct bpf_object *obj);
static bool prog_is_subprog(const struct bpf_object *obj, const struct bpf_program *prog);
+static const char * const attach_type_name[] = {
+ [BPF_CGROUP_INET_INGRESS] = "cgroup_inet_ingress",
+ [BPF_CGROUP_INET_EGRESS] = "cgroup_inet_egress",
+ [BPF_CGROUP_INET_SOCK_CREATE] = "cgroup_inet_sock_create",
+ [BPF_CGROUP_INET_SOCK_RELEASE] = "cgroup_inet_sock_release",
+ [BPF_CGROUP_SOCK_OPS] = "cgroup_sock_ops",
+ [BPF_CGROUP_DEVICE] = "cgroup_device",
+ [BPF_CGROUP_INET4_BIND] = "cgroup_inet4_bind",
+ [BPF_CGROUP_INET6_BIND] = "cgroup_inet6_bind",
+ [BPF_CGROUP_INET4_CONNECT] = "cgroup_inet4_connect",
+ [BPF_CGROUP_INET6_CONNECT] = "cgroup_inet6_connect",
+ [BPF_CGROUP_INET4_POST_BIND] = "cgroup_inet4_post_bind",
+ [BPF_CGROUP_INET6_POST_BIND] = "cgroup_inet6_post_bind",
+ [BPF_CGROUP_INET4_GETPEERNAME] = "cgroup_inet4_getpeername",
+ [BPF_CGROUP_INET6_GETPEERNAME] = "cgroup_inet6_getpeername",
+ [BPF_CGROUP_INET4_GETSOCKNAME] = "cgroup_inet4_getsockname",
+ [BPF_CGROUP_INET6_GETSOCKNAME] = "cgroup_inet6_getsockname",
+ [BPF_CGROUP_UDP4_SENDMSG] = "cgroup_udp4_sendmsg",
+ [BPF_CGROUP_UDP6_SENDMSG] = "cgroup_udp6_sendmsg",
+ [BPF_CGROUP_SYSCTL] = "cgroup_sysctl",
+ [BPF_CGROUP_UDP4_RECVMSG] = "cgroup_udp4_recvmsg",
+ [BPF_CGROUP_UDP6_RECVMSG] = "cgroup_udp6_recvmsg",
+ [BPF_CGROUP_GETSOCKOPT] = "cgroup_getsockopt",
+ [BPF_CGROUP_SETSOCKOPT] = "cgroup_setsockopt",
+ [BPF_SK_SKB_STREAM_PARSER] = "sk_skb_stream_parser",
+ [BPF_SK_SKB_STREAM_VERDICT] = "sk_skb_stream_verdict",
+ [BPF_SK_SKB_VERDICT] = "sk_skb_verdict",
+ [BPF_SK_MSG_VERDICT] = "sk_msg_verdict",
+ [BPF_LIRC_MODE2] = "lirc_mode2",
+ [BPF_FLOW_DISSECTOR] = "flow_dissector",
+ [BPF_TRACE_RAW_TP] = "trace_raw_tp",
+ [BPF_TRACE_FENTRY] = "trace_fentry",
+ [BPF_TRACE_FEXIT] = "trace_fexit",
+ [BPF_MODIFY_RETURN] = "modify_return",
+ [BPF_LSM_MAC] = "lsm_mac",
+ [BPF_SK_LOOKUP] = "sk_lookup",
+ [BPF_TRACE_ITER] = "trace_iter",
+ [BPF_XDP_DEVMAP] = "xdp_devmap",
+ [BPF_XDP_CPUMAP] = "xdp_cpumap",
+ [BPF_XDP] = "xdp",
+ [BPF_SK_REUSEPORT_SELECT] = "sk_reuseport_select",
+ [BPF_SK_REUSEPORT_SELECT_OR_MIGRATE] = "sk_reuseport_select_or_migrate",
+ [BPF_PERF_EVENT] = "perf_event",
+ [BPF_TRACE_KPROBE_MULTI] = "trace_kprobe_multi",
+};
+
static const char * const map_type_name[] = {
[BPF_MAP_TYPE_UNSPEC] = "unspec",
[BPF_MAP_TYPE_HASH] = "hash",
@@ -9369,6 +9415,14 @@ int libbpf_prog_type_by_name(const char *name, enum bpf_prog_type *prog_type,
return libbpf_err(-ESRCH);
}
+const char *libbpf_bpf_attach_type_str(enum bpf_attach_type t)
+{
+ if (t < 0 || t >= ARRAY_SIZE(attach_type_name))
+ return NULL;
+
+ return attach_type_name[t];
+}
+
const char *libbpf_bpf_map_type_str(enum bpf_map_type t)
{
if (t < 0 || t >= ARRAY_SIZE(map_type_name))
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 6c5b07..37a234 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -51,6 +51,15 @@ enum libbpf_errno {
LIBBPF_API int libbpf_strerror(int err, char *buf, size_t size);
+/**
+ * @brief **libbpf_bpf_attach_type_str()** converts the provided attach type
+ * value into a textual representation.
+ * @param t The attach type.
+ * @return Pointer to a static string identifying the attach type. NULL is
+ * returned for unknown **bpf_attach_type** values.
+ */
+LIBBPF_API const char *libbpf_bpf_attach_type_str(enum bpf_attach_type t);
+
/**
* @brief **libbpf_bpf_map_type_str()** converts the provided map type value
* into a textual representation.
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index c63624..d045d6 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -456,6 +456,7 @@ LIBBPF_0.8.0 {
bpf_program__attach_trace_opts;
bpf_program__attach_usdt;
bpf_program__set_insns;
+ libbpf_bpf_attach_type_str;
libbpf_bpf_map_type_str;
libbpf_bpf_prog_type_str;
libbpf_register_prog_handler;
--
2.30.2
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH bpf-next 08/12] selftests/bpf: Add test for libbpf_bpf_attach_type_str
2022-05-16 17:35 [PATCH bpf-next 00/12] libbpf: Textual representation of enums Daniel Müller
` (6 preceding siblings ...)
2022-05-16 17:35 ` [PATCH bpf-next 07/12] libbpf: Introduce libbpf_bpf_attach_type_str Daniel Müller
@ 2022-05-16 17:35 ` Daniel Müller
2022-05-16 17:35 ` [PATCH bpf-next 09/12] bpftool: Use libbpf_bpf_attach_type_str Daniel Müller
` (5 subsequent siblings)
13 siblings, 0 replies; 24+ messages in thread
From: Daniel Müller @ 2022-05-16 17:35 UTC (permalink / raw)
To: bpf, ast, andrii, daniel, kernel-team, quentin
This change adds a test for libbpf_bpf_attach_type_str. The test
retrieves all variants of the bpf_attach_type enumeration using BTF and
makes sure that the function under test works as expected for them.
Signed-off-by: Daniel Müller <deso@posteo.net>
---
.../selftests/bpf/prog_tests/libbpf_str.c | 48 +++++++++++++++++++
1 file changed, 48 insertions(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/libbpf_str.c b/tools/testing/selftests/bpf/prog_tests/libbpf_str.c
index 0f15aaa..f5fa09 100644
--- a/tools/testing/selftests/bpf/prog_tests/libbpf_str.c
+++ b/tools/testing/selftests/bpf/prog_tests/libbpf_str.c
@@ -14,6 +14,51 @@ static void uppercase(char *s)
*s = toupper(*s);
}
+/**
+ * Test case to check that all bpf_attach_type variants are covered by
+ * libbpf_bpf_attach_type_str.
+ */
+static void test_libbpf_bpf_attach_type_str(void)
+{
+ struct btf *btf;
+ const struct btf_type *t;
+ const struct btf_enum *e;
+ int i, n, id;
+
+ btf = btf__parse("/sys/kernel/btf/vmlinux", NULL);
+ if (!ASSERT_OK_PTR(btf, "btf_parse"))
+ return;
+
+ /* find enum bpf_attach_type and enumerate each value */
+ id = btf__find_by_name_kind(btf, "bpf_attach_type", BTF_KIND_ENUM);
+ if (!ASSERT_GT(id, 0, "bpf_attach_type_id"))
+ goto cleanup;
+ t = btf__type_by_id(btf, id);
+ e = btf_enum(t);
+ n = btf_vlen(t);
+ for (i = 0; i < n; e++, i++) {
+ enum bpf_attach_type attach_type = (enum bpf_attach_type)e->val;
+ const char *attach_type_name;
+ const char *attach_type_str;
+ char buf[256];
+
+ if (attach_type == __MAX_BPF_ATTACH_TYPE)
+ continue;
+
+ attach_type_name = btf__str_by_offset(btf, e->name_off);
+ attach_type_str = libbpf_bpf_attach_type_str(attach_type);
+ ASSERT_OK_PTR(attach_type_str, attach_type_name);
+
+ snprintf(buf, sizeof(buf), "BPF_%s", attach_type_str);
+ uppercase(buf);
+
+ ASSERT_STREQ(buf, attach_type_name, "exp_str_value");
+ }
+
+cleanup:
+ btf__free(btf);
+}
+
/**
* Test case to check that all bpf_map_type variants are covered by
* libbpf_bpf_map_type_str.
@@ -103,6 +148,9 @@ static void test_libbpf_bpf_prog_type_str(void)
*/
void test_libbpf_str(void)
{
+ if (test__start_subtest("bpf_attach_type_str"))
+ test_libbpf_bpf_attach_type_str();
+
if (test__start_subtest("bpf_map_type_str"))
test_libbpf_bpf_map_type_str();
--
2.30.2
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH bpf-next 09/12] bpftool: Use libbpf_bpf_attach_type_str
2022-05-16 17:35 [PATCH bpf-next 00/12] libbpf: Textual representation of enums Daniel Müller
` (7 preceding siblings ...)
2022-05-16 17:35 ` [PATCH bpf-next 08/12] selftests/bpf: Add test for libbpf_bpf_attach_type_str Daniel Müller
@ 2022-05-16 17:35 ` Daniel Müller
2022-05-16 23:41 ` Andrii Nakryiko
2022-05-16 17:35 ` [PATCH bpf-next 10/12] libbpf: Introduce libbpf_bpf_link_type_str Daniel Müller
` (4 subsequent siblings)
13 siblings, 1 reply; 24+ messages in thread
From: Daniel Müller @ 2022-05-16 17:35 UTC (permalink / raw)
To: bpf, ast, andrii, daniel, kernel-team, quentin
This change switches bpftool over to using the recently introduced
libbpf_bpf_attach_type_str function instead of maintaining its own
string representation for the bpf_attach_type enum.
Note that contrary to other enum types, the variant names that bpftool
maps bpf_attach_type to do not follow a simple to follow rule. With
bpf_prog_type, for example, the textual representation can easily be
inferred by stripping the BPF_PROG_TYPE_ prefix and lowercasing the
remaining string. bpf_attach_type violates this rule for various
variants. In order to not brake compatibility (these textual
representations appear in JSON and are used to parse user input), we
introduce a program local bpf_attach_type_str that overrides the
variants in question.
We should consider removing this function and expect the libbpf string
representation with the next backwards compatibility breaking release,
if possible.
Signed-off-by: Daniel Müller <deso@posteo.net>
---
tools/bpf/bpftool/cgroup.c | 20 ++++++----
tools/bpf/bpftool/common.c | 82 +++++++++++++++++---------------------
tools/bpf/bpftool/link.c | 15 ++++---
tools/bpf/bpftool/main.h | 15 +++++++
4 files changed, 73 insertions(+), 59 deletions(-)
diff --git a/tools/bpf/bpftool/cgroup.c b/tools/bpf/bpftool/cgroup.c
index effe136..fdaef2 100644
--- a/tools/bpf/bpftool/cgroup.c
+++ b/tools/bpf/bpftool/cgroup.c
@@ -35,11 +35,14 @@ static unsigned int query_flags;
static enum bpf_attach_type parse_attach_type(const char *str)
{
+ const char *attach_type_str;
enum bpf_attach_type type;
- for (type = 0; type < __MAX_BPF_ATTACH_TYPE; type++) {
- if (attach_type_name[type] &&
- is_prefix(str, attach_type_name[type]))
+ for (type = 0; ; type++) {
+ attach_type_str = bpf_attach_type_str(type);
+ if (!attach_type_str)
+ break;
+ if (is_prefix(str, attach_type_str))
return type;
}
@@ -52,6 +55,7 @@ static int show_bpf_prog(int id, enum bpf_attach_type attach_type,
{
char prog_name[MAX_PROG_FULL_NAME];
struct bpf_prog_info info = {};
+ const char *attach_type_str;
__u32 info_len = sizeof(info);
int prog_fd;
@@ -64,13 +68,13 @@ static int show_bpf_prog(int id, enum bpf_attach_type attach_type,
return -1;
}
+ attach_type_str = bpf_attach_type_str(attach_type);
get_prog_full_name(&info, prog_fd, prog_name, sizeof(prog_name));
if (json_output) {
jsonw_start_object(json_wtr);
jsonw_uint_field(json_wtr, "id", info.id);
- if (attach_type < ARRAY_SIZE(attach_type_name))
- jsonw_string_field(json_wtr, "attach_type",
- attach_type_name[attach_type]);
+ if (attach_type_str)
+ jsonw_string_field(json_wtr, "attach_type", attach_type_str);
else
jsonw_uint_field(json_wtr, "attach_type", attach_type);
jsonw_string_field(json_wtr, "attach_flags",
@@ -79,8 +83,8 @@ static int show_bpf_prog(int id, enum bpf_attach_type attach_type,
jsonw_end_object(json_wtr);
} else {
printf("%s%-8u ", level ? " " : "", info.id);
- if (attach_type < ARRAY_SIZE(attach_type_name))
- printf("%-15s", attach_type_name[attach_type]);
+ if (attach_type_str)
+ printf("%-15s", attach_type_str);
else
printf("type %-10u", attach_type);
printf(" %-15s %-15s\n", attach_flags_str, prog_name);
diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c
index c74014..00ab85 100644
--- a/tools/bpf/bpftool/common.c
+++ b/tools/bpf/bpftool/common.c
@@ -31,52 +31,6 @@
#define BPF_FS_MAGIC 0xcafe4a11
#endif
-const char * const attach_type_name[__MAX_BPF_ATTACH_TYPE] = {
- [BPF_CGROUP_INET_INGRESS] = "ingress",
- [BPF_CGROUP_INET_EGRESS] = "egress",
- [BPF_CGROUP_INET_SOCK_CREATE] = "sock_create",
- [BPF_CGROUP_INET_SOCK_RELEASE] = "sock_release",
- [BPF_CGROUP_SOCK_OPS] = "sock_ops",
- [BPF_CGROUP_DEVICE] = "device",
- [BPF_CGROUP_INET4_BIND] = "bind4",
- [BPF_CGROUP_INET6_BIND] = "bind6",
- [BPF_CGROUP_INET4_CONNECT] = "connect4",
- [BPF_CGROUP_INET6_CONNECT] = "connect6",
- [BPF_CGROUP_INET4_POST_BIND] = "post_bind4",
- [BPF_CGROUP_INET6_POST_BIND] = "post_bind6",
- [BPF_CGROUP_INET4_GETPEERNAME] = "getpeername4",
- [BPF_CGROUP_INET6_GETPEERNAME] = "getpeername6",
- [BPF_CGROUP_INET4_GETSOCKNAME] = "getsockname4",
- [BPF_CGROUP_INET6_GETSOCKNAME] = "getsockname6",
- [BPF_CGROUP_UDP4_SENDMSG] = "sendmsg4",
- [BPF_CGROUP_UDP6_SENDMSG] = "sendmsg6",
- [BPF_CGROUP_SYSCTL] = "sysctl",
- [BPF_CGROUP_UDP4_RECVMSG] = "recvmsg4",
- [BPF_CGROUP_UDP6_RECVMSG] = "recvmsg6",
- [BPF_CGROUP_GETSOCKOPT] = "getsockopt",
- [BPF_CGROUP_SETSOCKOPT] = "setsockopt",
- [BPF_SK_SKB_STREAM_PARSER] = "sk_skb_stream_parser",
- [BPF_SK_SKB_STREAM_VERDICT] = "sk_skb_stream_verdict",
- [BPF_SK_SKB_VERDICT] = "sk_skb_verdict",
- [BPF_SK_MSG_VERDICT] = "sk_msg_verdict",
- [BPF_LIRC_MODE2] = "lirc_mode2",
- [BPF_FLOW_DISSECTOR] = "flow_dissector",
- [BPF_TRACE_RAW_TP] = "raw_tp",
- [BPF_TRACE_FENTRY] = "fentry",
- [BPF_TRACE_FEXIT] = "fexit",
- [BPF_MODIFY_RETURN] = "mod_ret",
- [BPF_LSM_MAC] = "lsm_mac",
- [BPF_SK_LOOKUP] = "sk_lookup",
- [BPF_TRACE_ITER] = "trace_iter",
- [BPF_XDP_DEVMAP] = "xdp_devmap",
- [BPF_XDP_CPUMAP] = "xdp_cpumap",
- [BPF_XDP] = "xdp",
- [BPF_SK_REUSEPORT_SELECT] = "sk_skb_reuseport_select",
- [BPF_SK_REUSEPORT_SELECT_OR_MIGRATE] = "sk_skb_reuseport_select_or_migrate",
- [BPF_PERF_EVENT] = "perf_event",
- [BPF_TRACE_KPROBE_MULTI] = "trace_kprobe_multi",
-};
-
void p_err(const char *fmt, ...)
{
va_list ap;
@@ -1009,3 +963,39 @@ bool equal_fn_for_key_as_id(const void *k1, const void *k2, void *ctx)
{
return k1 == k2;
}
+
+const char *bpf_attach_type_str(enum bpf_attach_type t)
+{
+ switch (t) {
+ case BPF_CGROUP_INET_INGRESS: return "ingress";
+ case BPF_CGROUP_INET_EGRESS: return "egress";
+ case BPF_CGROUP_INET_SOCK_CREATE: return "sock_create";
+ case BPF_CGROUP_INET_SOCK_RELEASE: return "sock_release";
+ case BPF_CGROUP_SOCK_OPS: return "sock_ops";
+ case BPF_CGROUP_DEVICE: return "device";
+ case BPF_CGROUP_INET4_BIND: return "bind4";
+ case BPF_CGROUP_INET6_BIND: return "bind6";
+ case BPF_CGROUP_INET4_CONNECT: return "connect4";
+ case BPF_CGROUP_INET6_CONNECT: return "connect6";
+ case BPF_CGROUP_INET4_POST_BIND: return "post_bind4";
+ case BPF_CGROUP_INET6_POST_BIND: return "post_bind6";
+ case BPF_CGROUP_INET4_GETPEERNAME: return "getpeername4";
+ case BPF_CGROUP_INET6_GETPEERNAME: return "getpeername6";
+ case BPF_CGROUP_INET4_GETSOCKNAME: return "getsockname4";
+ case BPF_CGROUP_INET6_GETSOCKNAME: return "getsockname6";
+ case BPF_CGROUP_UDP4_SENDMSG: return "sendmsg4";
+ case BPF_CGROUP_UDP6_SENDMSG: return "sendmsg6";
+ case BPF_CGROUP_SYSCTL: return "sysctl";
+ case BPF_CGROUP_UDP4_RECVMSG: return "recvmsg4";
+ case BPF_CGROUP_UDP6_RECVMSG: return "recvmsg6";
+ case BPF_CGROUP_GETSOCKOPT: return "getsockopt";
+ case BPF_CGROUP_SETSOCKOPT: return "setsockopt";
+ case BPF_TRACE_RAW_TP: return "raw_tp";
+ case BPF_TRACE_FENTRY: return "fentry";
+ case BPF_TRACE_FEXIT: return "fexit";
+ case BPF_MODIFY_RETURN: return "mod_ret";
+ case BPF_SK_REUSEPORT_SELECT: return "sk_skb_reuseport_select";
+ case BPF_SK_REUSEPORT_SELECT_OR_MIGRATE: return "sk_skb_reuseport_select_or_migrate";
+ default: return libbpf_bpf_attach_type_str(t);
+ }
+}
diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
index e27108..2dd0d01 100644
--- a/tools/bpf/bpftool/link.c
+++ b/tools/bpf/bpftool/link.c
@@ -78,9 +78,11 @@ show_link_header_json(struct bpf_link_info *info, json_writer_t *wtr)
static void show_link_attach_type_json(__u32 attach_type, json_writer_t *wtr)
{
- if (attach_type < ARRAY_SIZE(attach_type_name))
- jsonw_string_field(wtr, "attach_type",
- attach_type_name[attach_type]);
+ const char *attach_type_str;
+
+ attach_type_str = bpf_attach_type_str(attach_type);
+ if (attach_type_str)
+ jsonw_string_field(wtr, "attach_type", attach_type_str);
else
jsonw_uint_field(wtr, "attach_type", attach_type);
}
@@ -196,8 +198,11 @@ static void show_link_header_plain(struct bpf_link_info *info)
static void show_link_attach_type_plain(__u32 attach_type)
{
- if (attach_type < ARRAY_SIZE(attach_type_name))
- printf("attach_type %s ", attach_type_name[attach_type]);
+ const char *attach_type_str;
+
+ attach_type_str = bpf_attach_type_str(attach_type);
+ if (attach_type_str)
+ printf("attach_type %s ", attach_type_str);
else
printf("attach_type %u ", attach_type);
}
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index e4fdaa0..e27237c 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -243,6 +243,21 @@ int print_all_levels(__maybe_unused enum libbpf_print_level level,
size_t hash_fn_for_key_as_id(const void *key, void *ctx);
bool equal_fn_for_key_as_id(const void *k1, const void *k2, void *ctx);
+/**
+ * bpf_attach_type_str - convert the provided attach type value into a textual
+ * representation.
+ *
+ * This function acts as a wrapper around libbpf_bpf_attach_type_str which
+ * overrides some of the variant names with ones that have traditionally been
+ * used in the program (and that do not follow the string inference scheme that
+ * libbpf uses).
+ *
+ * @t: The attach type
+ * Returns a pointer to a static string identifying the attach type. NULL is
+ * returned for unknown bpf_attach_type values.
+ */
+const char *bpf_attach_type_str(enum bpf_attach_type t);
+
static inline void *u32_as_hash_field(__u32 x)
{
return (void *)(uintptr_t)x;
--
2.30.2
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH bpf-next 09/12] bpftool: Use libbpf_bpf_attach_type_str
2022-05-16 17:35 ` [PATCH bpf-next 09/12] bpftool: Use libbpf_bpf_attach_type_str Daniel Müller
@ 2022-05-16 23:41 ` Andrii Nakryiko
2022-05-17 14:18 ` Quentin Monnet
0 siblings, 1 reply; 24+ messages in thread
From: Andrii Nakryiko @ 2022-05-16 23:41 UTC (permalink / raw)
To: Daniel Müller, Quentin Monnet
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, Kernel Team
On Mon, May 16, 2022 at 10:36 AM Daniel Müller <deso@posteo.net> wrote:
>
> This change switches bpftool over to using the recently introduced
> libbpf_bpf_attach_type_str function instead of maintaining its own
> string representation for the bpf_attach_type enum.
>
> Note that contrary to other enum types, the variant names that bpftool
> maps bpf_attach_type to do not follow a simple to follow rule. With
> bpf_prog_type, for example, the textual representation can easily be
> inferred by stripping the BPF_PROG_TYPE_ prefix and lowercasing the
> remaining string. bpf_attach_type violates this rule for various
> variants. In order to not brake compatibility (these textual
> representations appear in JSON and are used to parse user input), we
> introduce a program local bpf_attach_type_str that overrides the
> variants in question.
> We should consider removing this function and expect the libbpf string
> representation with the next backwards compatibility breaking release,
> if possible.
>
> Signed-off-by: Daniel Müller <deso@posteo.net>
> ---
Quentin, any opinion on this approach? Should we fallback to libbpf's
API for all the future cases or it's better to keep bpftool's own
attach_type mapping?
> tools/bpf/bpftool/cgroup.c | 20 ++++++----
> tools/bpf/bpftool/common.c | 82 +++++++++++++++++---------------------
> tools/bpf/bpftool/link.c | 15 ++++---
> tools/bpf/bpftool/main.h | 15 +++++++
> 4 files changed, 73 insertions(+), 59 deletions(-)
>
[...]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH bpf-next 09/12] bpftool: Use libbpf_bpf_attach_type_str
2022-05-16 23:41 ` Andrii Nakryiko
@ 2022-05-17 14:18 ` Quentin Monnet
2022-05-17 18:54 ` Daniel Müller
0 siblings, 1 reply; 24+ messages in thread
From: Quentin Monnet @ 2022-05-17 14:18 UTC (permalink / raw)
To: Andrii Nakryiko, Daniel Müller
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, Kernel Team
2022-05-16 16:41 UTC-0700 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com>
> On Mon, May 16, 2022 at 10:36 AM Daniel Müller <deso@posteo.net> wrote:
>>
>> This change switches bpftool over to using the recently introduced
>> libbpf_bpf_attach_type_str function instead of maintaining its own
>> string representation for the bpf_attach_type enum.
>>
>> Note that contrary to other enum types, the variant names that bpftool
>> maps bpf_attach_type to do not follow a simple to follow rule. With
>> bpf_prog_type, for example, the textual representation can easily be
>> inferred by stripping the BPF_PROG_TYPE_ prefix and lowercasing the
>> remaining string. bpf_attach_type violates this rule for various
>> variants. In order to not brake compatibility (these textual
>> representations appear in JSON and are used to parse user input), we
>> introduce a program local bpf_attach_type_str that overrides the
>> variants in question.
>> We should consider removing this function and expect the libbpf string
>> representation with the next backwards compatibility breaking release,
>> if possible.
>>
>> Signed-off-by: Daniel Müller <deso@posteo.net>
>> ---
>
> Quentin, any opinion on this approach? Should we fallback to libbpf's
> API for all the future cases or it's better to keep bpftool's own
> attach_type mapping?
Hi Andrii, Daniel,
Thanks for the ping! I'm unsure what's best, to be honest. Maybe we
should look at bpftool's inputs and outputs separately.
For attach types printed as part of the output:
Thinking about it, I'd say go for the libbpf API, and make the change
now. As much as we all dislike breaking things for user space, I believe
that on the long term, we would benefit from having a more consistent
naming scheme for those strings (prefix + lowercase attach type); and
more importantly, if querying the string from libbpf spreads to other
tools, these will be the reference strings for the attach types and it
will be a pain to convert bpftool's specific exceptions to "regular"
textual representations to interface with other tools.
And if we must break things, I'd as well have it synchronised with the
release of libbpf 1.0, so I'd say let's just change it now? Note that
we're now tagging bpftool releases on the GitHub mirror (I did 6.8.0
earlier today), so at least that's one place where we can have a
changelog and mention breaking changes.
Now for the attach types parsed as input parameters:
I wonder if it would be worth supporting the two values for attach types
where they differ, so that we would avoid breaking bpftool commands
themselves? It's a bit more code, but then the list would be relatively
short, and not expected to grow. We can update the documentation to
mention only the new names, and just keep the short compat list hidden.
Some additional notes on the patch:
There is also attach_type_strings[] in prog.c where strings for attach
types are re-defined, this time for when non cgroup-related programs are
attached (through "bpftool prog attach"). It's used for parsing the
input, so should be treated the same as the attach list in commons.c.
If changing the attach type names, we should also update the following:
- man pages: tools/bpf/bpftool/Documentation/bpftool-{cgroup,prog}.rst
- interactive help (cgroup.c:HELP_SPEC_ATTACH_TYPES + prog.c:do_help())
- bash completion: tools/bpf/bpftool/bash-completion/bpftool
Some of the tests in
tools/testing/selftests/bpf/test_bpftool_synctypes.py, related to
keeping those lists up-to-date, will also need to be modified to parse
the names from libbpf instead of bpftool sources. I can help with that
if necessary.
Quentin
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH bpf-next 09/12] bpftool: Use libbpf_bpf_attach_type_str
2022-05-17 14:18 ` Quentin Monnet
@ 2022-05-17 18:54 ` Daniel Müller
2022-05-18 13:31 ` Quentin Monnet
0 siblings, 1 reply; 24+ messages in thread
From: Daniel Müller @ 2022-05-17 18:54 UTC (permalink / raw)
To: Quentin Monnet
Cc: Andrii Nakryiko, bpf, Alexei Starovoitov, Andrii Nakryiko,
Daniel Borkmann, Kernel Team
On Tue, May 17, 2022 at 03:18:41PM +0100, Quentin Monnet wrote:
> 2022-05-16 16:41 UTC-0700 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com>
> > On Mon, May 16, 2022 at 10:36 AM Daniel Müller <deso@posteo.net> wrote:
> >>
> >> This change switches bpftool over to using the recently introduced
> >> libbpf_bpf_attach_type_str function instead of maintaining its own
> >> string representation for the bpf_attach_type enum.
> >>
> >> Note that contrary to other enum types, the variant names that bpftool
> >> maps bpf_attach_type to do not follow a simple to follow rule. With
> >> bpf_prog_type, for example, the textual representation can easily be
> >> inferred by stripping the BPF_PROG_TYPE_ prefix and lowercasing the
> >> remaining string. bpf_attach_type violates this rule for various
> >> variants. In order to not brake compatibility (these textual
> >> representations appear in JSON and are used to parse user input), we
> >> introduce a program local bpf_attach_type_str that overrides the
> >> variants in question.
> >> We should consider removing this function and expect the libbpf string
> >> representation with the next backwards compatibility breaking release,
> >> if possible.
> >>
> >> Signed-off-by: Daniel Müller <deso@posteo.net>
> >> ---
> >
> > Quentin, any opinion on this approach? Should we fallback to libbpf's
> > API for all the future cases or it's better to keep bpftool's own
> > attach_type mapping?
> Hi Andrii, Daniel,
Hi Quentin,
> Thanks for the ping! I'm unsure what's best, to be honest. Maybe we
> should look at bpftool's inputs and outputs separately.
>
> For attach types printed as part of the output:
>
> Thinking about it, I'd say go for the libbpf API, and make the change
> now. As much as we all dislike breaking things for user space, I believe
> that on the long term, we would benefit from having a more consistent
> naming scheme for those strings (prefix + lowercase attach type); and
> more importantly, if querying the string from libbpf spreads to other
> tools, these will be the reference strings for the attach types and it
> will be a pain to convert bpftool's specific exceptions to "regular"
> textual representations to interface with other tools.
>
> And if we must break things, I'd as well have it synchronised with the
> release of libbpf 1.0, so I'd say let's just change it now? Note that
> we're now tagging bpftool releases on the GitHub mirror (I did 6.8.0
> earlier today), so at least that's one place where we can have a
> changelog and mention breaking changes.
Thanks for the feedback. This sounds good to me. I can make the change. But do
you think we should do it as part of this stack? We could make this very stack a
behavior preserving step (that can reasonably stand on its own). Given the
additional changes to tests & documentation that you mention below, it would
make sense to me to keep individual patches in this series similar in nature.
I'd be happy to author a follow-on, but can also amend this series if that's
preferred. Please let me know your preference.
> Now for the attach types parsed as input parameters:
>
> I wonder if it would be worth supporting the two values for attach types
> where they differ, so that we would avoid breaking bpftool commands
> themselves? It's a bit more code, but then the list would be relatively
> short, and not expected to grow. We can update the documentation to
> mention only the new names, and just keep the short compat list hidden.
Yes, that should be easily possible. I do have a similar question to above,
though (as it involves updating documentation for new preference): can/should
that be a separate patch?
> Some additional notes on the patch:
>
> There is also attach_type_strings[] in prog.c where strings for attach
> types are re-defined, this time for when non cgroup-related programs are
> attached (through "bpftool prog attach"). It's used for parsing the
> input, so should be treated the same as the attach list in commons.c.
Good point. If we want to use libbpf text representations moving forward then I
can adjust this array as well. Do I assume correctly that we would want to keep
the existing variants as hidden fallbacks here too, as you mentioned above?
> If changing the attach type names, we should also update the following:
> - man pages: tools/bpf/bpftool/Documentation/bpftool-{cgroup,prog}.rst
> - interactive help (cgroup.c:HELP_SPEC_ATTACH_TYPES + prog.c:do_help())
> - bash completion: tools/bpf/bpftool/bash-completion/bpftool
>
> Some of the tests in
> tools/testing/selftests/bpf/test_bpftool_synctypes.py, related to
> keeping those lists up-to-date, will also need to be modified to parse
> the names from libbpf instead of bpftool sources. I can help with that
> if necessary.
Sounds good; will do that here or as a follow-on (and reach out to you if I need
assistance), depending on your preference as inquired above.
Thanks,
Daniel
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH bpf-next 09/12] bpftool: Use libbpf_bpf_attach_type_str
2022-05-17 18:54 ` Daniel Müller
@ 2022-05-18 13:31 ` Quentin Monnet
2022-05-18 23:54 ` Daniel Müller
2022-05-19 3:08 ` Dave Thaler
0 siblings, 2 replies; 24+ messages in thread
From: Quentin Monnet @ 2022-05-18 13:31 UTC (permalink / raw)
To: Daniel Müller
Cc: Andrii Nakryiko, bpf, Alexei Starovoitov, Andrii Nakryiko,
Daniel Borkmann, Kernel Team
2022-05-17 18:54 UTC+0000 ~ Daniel Müller <deso@posteo.net>
> On Tue, May 17, 2022 at 03:18:41PM +0100, Quentin Monnet wrote:
>> 2022-05-16 16:41 UTC-0700 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com>
>>> On Mon, May 16, 2022 at 10:36 AM Daniel Müller <deso@posteo.net> wrote:
>>>>
>>>> This change switches bpftool over to using the recently introduced
>>>> libbpf_bpf_attach_type_str function instead of maintaining its own
>>>> string representation for the bpf_attach_type enum.
>>>>
>>>> Note that contrary to other enum types, the variant names that bpftool
>>>> maps bpf_attach_type to do not follow a simple to follow rule. With
>>>> bpf_prog_type, for example, the textual representation can easily be
>>>> inferred by stripping the BPF_PROG_TYPE_ prefix and lowercasing the
>>>> remaining string. bpf_attach_type violates this rule for various
>>>> variants. In order to not brake compatibility (these textual
>>>> representations appear in JSON and are used to parse user input), we
>>>> introduce a program local bpf_attach_type_str that overrides the
>>>> variants in question.
>>>> We should consider removing this function and expect the libbpf string
>>>> representation with the next backwards compatibility breaking release,
>>>> if possible.
>>>>
>>>> Signed-off-by: Daniel Müller <deso@posteo.net>
>>>> ---
>>>
>>> Quentin, any opinion on this approach? Should we fallback to libbpf's
>>> API for all the future cases or it's better to keep bpftool's own
>>> attach_type mapping?
>> Hi Andrii, Daniel,
>
> Hi Quentin,
>
>> Thanks for the ping! I'm unsure what's best, to be honest. Maybe we
>> should look at bpftool's inputs and outputs separately.
>>
>> For attach types printed as part of the output:
>>
>> Thinking about it, I'd say go for the libbpf API, and make the change
>> now. As much as we all dislike breaking things for user space, I believe
>> that on the long term, we would benefit from having a more consistent
>> naming scheme for those strings (prefix + lowercase attach type); and
>> more importantly, if querying the string from libbpf spreads to other
>> tools, these will be the reference strings for the attach types and it
>> will be a pain to convert bpftool's specific exceptions to "regular"
>> textual representations to interface with other tools.
>>
>> And if we must break things, I'd as well have it synchronised with the
>> release of libbpf 1.0, so I'd say let's just change it now? Note that
>> we're now tagging bpftool releases on the GitHub mirror (I did 6.8.0
>> earlier today), so at least that's one place where we can have a
>> changelog and mention breaking changes.
>
> Thanks for the feedback. This sounds good to me. I can make the change. But do
> you think we should do it as part of this stack? We could make this very stack a
> behavior preserving step (that can reasonably stand on its own). Given the
> additional changes to tests & documentation that you mention below, it would
> make sense to me to keep individual patches in this series similar in nature.
> I'd be happy to author a follow-on, but can also amend this series if that's
> preferred. Please let me know your preference.
So I would have a slight preference for having everything together, but
at the same time I understand that the current patchset is already in
a good state and that you don't want to "overgrow" it too much. So
follow-up is fine by me, mostly (see next paragraph), if it lands before
libbpf v1.0 is released. If I understand correctly, you would have the
addition of the new names as an alternative for input parameters in this
set, and follow-up with the breaking changes (using the new names for
output, and switching to new names for input in docs) as a follow-up, is
this correct?
Still, the tests in test_bpftool_synctypes.py should be updated in this
PR, because the bpftool tests in the CI break otherwise [0]. This is due
to the removal of the lists of names in bpftool: the test parses them to
compare the lists with what's present in UAPI bpf.h, bpftool man pages,
etc. I believe it would be best to keep this in a running state.
[0]
https://github.com/kernel-patches/bpf/runs/6459959177?check_suite_focus=true#step:6:678
>
>> Now for the attach types parsed as input parameters:
>>
>> I wonder if it would be worth supporting the two values for attach types
>> where they differ, so that we would avoid breaking bpftool commands
>> themselves? It's a bit more code, but then the list would be relatively
>> short, and not expected to grow. We can update the documentation to
>> mention only the new names, and just keep the short compat list hidden.
>
> Yes, that should be easily possible. I do have a similar question to above,
> though (as it involves updating documentation for new preference): can/should
> that be a separate patch?
>
>> Some additional notes on the patch:
>>
>> There is also attach_type_strings[] in prog.c where strings for attach
>> types are re-defined, this time for when non cgroup-related programs are
>> attached (through "bpftool prog attach"). It's used for parsing the
>> input, so should be treated the same as the attach list in commons.c.
>
> Good point. If we want to use libbpf text representations moving forward then I
> can adjust this array as well. Do I assume correctly that we would want to keep
> the existing variants as hidden fallbacks here too, as you mentioned above?
Yes, exactly.
>> If changing the attach type names, we should also update the following:
>> - man pages: tools/bpf/bpftool/Documentation/bpftool-{cgroup,prog}.rst
>> - interactive help (cgroup.c:HELP_SPEC_ATTACH_TYPES + prog.c:do_help())
>> - bash completion: tools/bpf/bpftool/bash-completion/bpftool
>>
>> Some of the tests in
>> tools/testing/selftests/bpf/test_bpftool_synctypes.py, related to
>> keeping those lists up-to-date, will also need to be modified to parse
>> the names from libbpf instead of bpftool sources. I can help with that
>> if necessary.
>
> Sounds good; will do that here or as a follow-on (and reach out to you if I need
> assistance), depending on your preference as inquired above.
Sounds like a plan :)
Thanks,
Quentin
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH bpf-next 09/12] bpftool: Use libbpf_bpf_attach_type_str
2022-05-18 13:31 ` Quentin Monnet
@ 2022-05-18 23:54 ` Daniel Müller
2022-05-19 3:08 ` Dave Thaler
1 sibling, 0 replies; 24+ messages in thread
From: Daniel Müller @ 2022-05-18 23:54 UTC (permalink / raw)
To: Quentin Monnet
Cc: Andrii Nakryiko, bpf, Alexei Starovoitov, Andrii Nakryiko,
Daniel Borkmann, Kernel Team
On Wed, May 18, 2022 at 02:31:40PM +0100, Quentin Monnet wrote:
> 2022-05-17 18:54 UTC+0000 ~ Daniel Müller <deso@posteo.net>
> > On Tue, May 17, 2022 at 03:18:41PM +0100, Quentin Monnet wrote:
> >> 2022-05-16 16:41 UTC-0700 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com>
> >>> On Mon, May 16, 2022 at 10:36 AM Daniel Müller <deso@posteo.net> wrote:
> >>>>
> >>>> This change switches bpftool over to using the recently introduced
> >>>> libbpf_bpf_attach_type_str function instead of maintaining its own
> >>>> string representation for the bpf_attach_type enum.
> >>>>
> >>>> Note that contrary to other enum types, the variant names that bpftool
> >>>> maps bpf_attach_type to do not follow a simple to follow rule. With
> >>>> bpf_prog_type, for example, the textual representation can easily be
> >>>> inferred by stripping the BPF_PROG_TYPE_ prefix and lowercasing the
> >>>> remaining string. bpf_attach_type violates this rule for various
> >>>> variants. In order to not brake compatibility (these textual
> >>>> representations appear in JSON and are used to parse user input), we
> >>>> introduce a program local bpf_attach_type_str that overrides the
> >>>> variants in question.
> >>>> We should consider removing this function and expect the libbpf string
> >>>> representation with the next backwards compatibility breaking release,
> >>>> if possible.
> >>>>
> >>>> Signed-off-by: Daniel Müller <deso@posteo.net>
> >>>> ---
> >>>
> >>> Quentin, any opinion on this approach? Should we fallback to libbpf's
> >>> API for all the future cases or it's better to keep bpftool's own
> >>> attach_type mapping?
> >> Hi Andrii, Daniel,
> >
> > Hi Quentin,
> >
> >> Thanks for the ping! I'm unsure what's best, to be honest. Maybe we
> >> should look at bpftool's inputs and outputs separately.
> >>
> >> For attach types printed as part of the output:
> >>
> >> Thinking about it, I'd say go for the libbpf API, and make the change
> >> now. As much as we all dislike breaking things for user space, I believe
> >> that on the long term, we would benefit from having a more consistent
> >> naming scheme for those strings (prefix + lowercase attach type); and
> >> more importantly, if querying the string from libbpf spreads to other
> >> tools, these will be the reference strings for the attach types and it
> >> will be a pain to convert bpftool's specific exceptions to "regular"
> >> textual representations to interface with other tools.
> >>
> >> And if we must break things, I'd as well have it synchronised with the
> >> release of libbpf 1.0, so I'd say let's just change it now? Note that
> >> we're now tagging bpftool releases on the GitHub mirror (I did 6.8.0
> >> earlier today), so at least that's one place where we can have a
> >> changelog and mention breaking changes.
> >
> > Thanks for the feedback. This sounds good to me. I can make the change. But do
> > you think we should do it as part of this stack? We could make this very stack a
> > behavior preserving step (that can reasonably stand on its own). Given the
> > additional changes to tests & documentation that you mention below, it would
> > make sense to me to keep individual patches in this series similar in nature.
> > I'd be happy to author a follow-on, but can also amend this series if that's
> > preferred. Please let me know your preference.
>
> So I would have a slight preference for having everything together, but
> at the same time I understand that the current patchset is already in
> a good state and that you don't want to "overgrow" it too much. So
> follow-up is fine by me, mostly (see next paragraph), if it lands before
> libbpf v1.0 is released. If I understand correctly, you would have the
> addition of the new names as an alternative for input parameters in this
> set, and follow-up with the breaking changes (using the new names for
> output, and switching to new names for input in docs) as a follow-up, is
> this correct?
Correct, that was my initial plan. But I had a look at the test code and given
how much it relies on the current source code (without this patch), my suggested
approach is actually no longer tenable (or at least would require significantly
more work, I'd say). I am now adjusting everything in this patch and also added
the fallbacks as you suggested.
> Still, the tests in test_bpftool_synctypes.py should be updated in this
> PR, because the bpftool tests in the CI break otherwise [0]. This is due
> to the removal of the lists of names in bpftool: the test parses them to
> compare the lists with what's present in UAPI bpf.h, bpftool man pages,
> etc. I believe it would be best to keep this in a running state.
>
> [0]
> https://github.com/kernel-patches/bpf/runs/6459959177?check_suite_focus=true#step:6:678
Thanks for the pointer. I've updated the test. It has become a rather large
patch now, though. What I ended up doing is to mostly rely on uapi/linux/bpf.h
enum definitions as the source of truth. Let me know if you want me to poke into
libbpf instead. I'd honestly rather avoid doing that.
Are there additional tests that I should run to give me some more confidence in
my changes? I tried running test_bpftool.sh, but it fails for a reason not
apparent to me. I did not investigate, because it also fails on master and one
of the commands that failed, bpftool feature probe macros, when run directly,
succeeds both with and without my patch set (which strongly suggests to me that
there is something else amiss here).
> >> Now for the attach types parsed as input parameters:
> >>
> >> I wonder if it would be worth supporting the two values for attach types
> >> where they differ, so that we would avoid breaking bpftool commands
> >> themselves? It's a bit more code, but then the list would be relatively
> >> short, and not expected to grow. We can update the documentation to
> >> mention only the new names, and just keep the short compat list hidden.
> >
> > Yes, that should be easily possible. I do have a similar question to above,
> > though (as it involves updating documentation for new preference): can/should
> > that be a separate patch?
> >
> >> Some additional notes on the patch:
> >>
> >> There is also attach_type_strings[] in prog.c where strings for attach
> >> types are re-defined, this time for when non cgroup-related programs are
> >> attached (through "bpftool prog attach"). It's used for parsing the
> >> input, so should be treated the same as the attach list in commons.c.
> >
> > Good point. If we want to use libbpf text representations moving forward then I
> > can adjust this array as well. Do I assume correctly that we would want to keep
> > the existing variants as hidden fallbacks here too, as you mentioned above?
>
> Yes, exactly.
I've adjusted this array now as well.
> >> If changing the attach type names, we should also update the following:
> >> - man pages: tools/bpf/bpftool/Documentation/bpftool-{cgroup,prog}.rst
> >> - interactive help (cgroup.c:HELP_SPEC_ATTACH_TYPES + prog.c:do_help())
> >> - bash completion: tools/bpf/bpftool/bash-completion/bpftool
> >>
> >> Some of the tests in
> >> tools/testing/selftests/bpf/test_bpftool_synctypes.py, related to
> >> keeping those lists up-to-date, will also need to be modified to parse
> >> the names from libbpf instead of bpftool sources. I can help with that
> >> if necessary.
> >
> > Sounds good; will do that here or as a follow-on (and reach out to you if I need
> > assistance), depending on your preference as inquired above.
>
> Sounds like a plan :)
Done. Though I did go with bpf.h as the source of truth. Please let me know if
you want me to use libbpf instead.
Will update to v2 soon.
Thanks,
Daniel
^ permalink raw reply [flat|nested] 24+ messages in thread* RE: [PATCH bpf-next 09/12] bpftool: Use libbpf_bpf_attach_type_str
2022-05-18 13:31 ` Quentin Monnet
2022-05-18 23:54 ` Daniel Müller
@ 2022-05-19 3:08 ` Dave Thaler
1 sibling, 0 replies; 24+ messages in thread
From: Dave Thaler @ 2022-05-19 3:08 UTC (permalink / raw)
To: Quentin Monnet, Daniel Müller
Cc: Andrii Nakryiko, bpf, Alexei Starovoitov, Andrii Nakryiko,
Daniel Borkmann, Kernel Team
> -----Original Message-----
> From: Quentin Monnet <quentin@isovalent.com>
> Sent: Wednesday, May 18, 2022 6:32 AM
> To: Daniel Müller <deso@posteo.net>
> Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>; bpf <bpf@vger.kernel.org>;
> Alexei Starovoitov <ast@kernel.org>; Andrii Nakryiko <andrii@kernel.org>;
> Daniel Borkmann <daniel@iogearbox.net>; Kernel Team <kernel-
> team@fb.com>
> Subject: Re: [PATCH bpf-next 09/12] bpftool: Use libbpf_bpf_attach_type_str
>
> 2022-05-17 18:54 UTC+0000 ~ Daniel Müller <deso@posteo.net>
> > On Tue, May 17, 2022 at 03:18:41PM +0100, Quentin Monnet wrote:
> >> 2022-05-16 16:41 UTC-0700 ~ Andrii Nakryiko
> >> <andrii.nakryiko@gmail.com>
> >>> On Mon, May 16, 2022 at 10:36 AM Daniel Müller <deso@posteo.net>
> wrote:
> >>>>
> >>>> This change switches bpftool over to using the recently introduced
> >>>> libbpf_bpf_attach_type_str function instead of maintaining its own
> >>>> string representation for the bpf_attach_type enum.
> >>>>
> >>>> Note that contrary to other enum types, the variant names that
> >>>> bpftool maps bpf_attach_type to do not follow a simple to follow
> >>>> rule. With bpf_prog_type, for example, the textual representation
> >>>> can easily be inferred by stripping the BPF_PROG_TYPE_ prefix and
> >>>> lowercasing the remaining string. bpf_attach_type violates this
> >>>> rule for various variants. In order to not brake compatibility
> >>>> (these textual representations appear in JSON and are used to parse
> >>>> user input), we introduce a program local bpf_attach_type_str that
> >>>> overrides the variants in question.
> >>>> We should consider removing this function and expect the libbpf
> >>>> string representation with the next backwards compatibility
> >>>> breaking release, if possible.
> >>>>
> >>>> Signed-off-by: Daniel Müller <deso@posteo.net>
> >>>> ---
> >>>
> >>> Quentin, any opinion on this approach? Should we fallback to
> >>> libbpf's API for all the future cases or it's better to keep
> >>> bpftool's own attach_type mapping?
> >> Hi Andrii, Daniel,
> >
> > Hi Quentin,
> >
> >> Thanks for the ping! I'm unsure what's best, to be honest. Maybe we
> >> should look at bpftool's inputs and outputs separately.
> >>
> >> For attach types printed as part of the output:
> >>
> >> Thinking about it, I'd say go for the libbpf API, and make the change
> >> now.
[...]
I mentioned this at LSF/MM/BPF in the discussion of making bpftool & libbpf cross-platform, but yes it is a good direction (in terms of supporting more runtimes) to avoid as much as possible having per-prog/attach type info in bpftool itself. So yes depending on libbpf apis instead of hard coding in bpftool is good.
Dave
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH bpf-next 10/12] libbpf: Introduce libbpf_bpf_link_type_str
2022-05-16 17:35 [PATCH bpf-next 00/12] libbpf: Textual representation of enums Daniel Müller
` (8 preceding siblings ...)
2022-05-16 17:35 ` [PATCH bpf-next 09/12] bpftool: Use libbpf_bpf_attach_type_str Daniel Müller
@ 2022-05-16 17:35 ` Daniel Müller
2022-05-16 17:35 ` [PATCH bpf-next 11/12] selftests/bpf: Add test for libbpf_bpf_link_type_str Daniel Müller
` (3 subsequent siblings)
13 siblings, 0 replies; 24+ messages in thread
From: Daniel Müller @ 2022-05-16 17:35 UTC (permalink / raw)
To: bpf, ast, andrii, daniel, kernel-team, quentin
This change introduces a new function, libbpf_bpf_link_type_str, to the
public libbpf API. The function allows users to get a string
representation for a bpf_link_type enum variant.
Signed-off-by: Daniel Müller <deso@posteo.net>
---
tools/lib/bpf/libbpf.c | 21 +++++++++++++++++++++
tools/lib/bpf/libbpf.h | 9 +++++++++
tools/lib/bpf/libbpf.map | 1 +
3 files changed, 31 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index a69a752..5596679 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -118,6 +118,19 @@ static const char * const attach_type_name[] = {
[BPF_TRACE_KPROBE_MULTI] = "trace_kprobe_multi",
};
+static const char * const link_type_name[] = {
+ [BPF_LINK_TYPE_UNSPEC] = "unspec",
+ [BPF_LINK_TYPE_RAW_TRACEPOINT] = "raw_tracepoint",
+ [BPF_LINK_TYPE_TRACING] = "tracing",
+ [BPF_LINK_TYPE_CGROUP] = "cgroup",
+ [BPF_LINK_TYPE_ITER] = "iter",
+ [BPF_LINK_TYPE_NETNS] = "netns",
+ [BPF_LINK_TYPE_XDP] = "xdp",
+ [BPF_LINK_TYPE_PERF_EVENT] = "perf_event",
+ [BPF_LINK_TYPE_KPROBE_MULTI] = "kprobe_multi",
+ [BPF_LINK_TYPE_STRUCT_OPS] = "struct_ops",
+};
+
static const char * const map_type_name[] = {
[BPF_MAP_TYPE_UNSPEC] = "unspec",
[BPF_MAP_TYPE_HASH] = "hash",
@@ -9423,6 +9436,14 @@ const char *libbpf_bpf_attach_type_str(enum bpf_attach_type t)
return attach_type_name[t];
}
+const char *libbpf_bpf_link_type_str(enum bpf_link_type t)
+{
+ if (t < 0 || t >= ARRAY_SIZE(link_type_name))
+ return NULL;
+
+ return link_type_name[t];
+}
+
const char *libbpf_bpf_map_type_str(enum bpf_map_type t)
{
if (t < 0 || t >= ARRAY_SIZE(map_type_name))
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 37a234..5b34ca 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -60,6 +60,15 @@ LIBBPF_API int libbpf_strerror(int err, char *buf, size_t size);
*/
LIBBPF_API const char *libbpf_bpf_attach_type_str(enum bpf_attach_type t);
+/**
+ * @brief **libbpf_bpf_link_type_str()** converts the provided link type value
+ * into a textual representation.
+ * @param t The link type.
+ * @return Pointer to a static string identifying the link type. NULL is
+ * returned for unknown **bpf_link_type** values.
+ */
+LIBBPF_API const char *libbpf_bpf_link_type_str(enum bpf_link_type t);
+
/**
* @brief **libbpf_bpf_map_type_str()** converts the provided map type value
* into a textual representation.
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index d045d6..c7cf4b6 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -457,6 +457,7 @@ LIBBPF_0.8.0 {
bpf_program__attach_usdt;
bpf_program__set_insns;
libbpf_bpf_attach_type_str;
+ libbpf_bpf_link_type_str;
libbpf_bpf_map_type_str;
libbpf_bpf_prog_type_str;
libbpf_register_prog_handler;
--
2.30.2
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH bpf-next 11/12] selftests/bpf: Add test for libbpf_bpf_link_type_str
2022-05-16 17:35 [PATCH bpf-next 00/12] libbpf: Textual representation of enums Daniel Müller
` (9 preceding siblings ...)
2022-05-16 17:35 ` [PATCH bpf-next 10/12] libbpf: Introduce libbpf_bpf_link_type_str Daniel Müller
@ 2022-05-16 17:35 ` Daniel Müller
2022-05-16 17:35 ` [PATCH bpf-next 12/12] bpftool: Use libbpf_bpf_link_type_str Daniel Müller
` (2 subsequent siblings)
13 siblings, 0 replies; 24+ messages in thread
From: Daniel Müller @ 2022-05-16 17:35 UTC (permalink / raw)
To: bpf, ast, andrii, daniel, kernel-team, quentin
This change adds a test for libbpf_bpf_link_type_str. The test retrieves
all variants of the bpf_link_type enumeration using BTF and makes sure
that the function under test works as expected for them.
Signed-off-by: Daniel Müller <deso@posteo.net>
---
.../selftests/bpf/prog_tests/libbpf_str.c | 48 +++++++++++++++++++
1 file changed, 48 insertions(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/libbpf_str.c b/tools/testing/selftests/bpf/prog_tests/libbpf_str.c
index f5fa09..1e45dd 100644
--- a/tools/testing/selftests/bpf/prog_tests/libbpf_str.c
+++ b/tools/testing/selftests/bpf/prog_tests/libbpf_str.c
@@ -59,6 +59,51 @@ static void test_libbpf_bpf_attach_type_str(void)
btf__free(btf);
}
+/**
+ * Test case to check that all bpf_link_type variants are covered by
+ * libbpf_bpf_link_type_str.
+ */
+static void test_libbpf_bpf_link_type_str(void)
+{
+ struct btf *btf;
+ const struct btf_type *t;
+ const struct btf_enum *e;
+ int i, n, id;
+
+ btf = btf__parse("/sys/kernel/btf/vmlinux", NULL);
+ if (!ASSERT_OK_PTR(btf, "btf_parse"))
+ return;
+
+ /* find enum bpf_link_type and enumerate each value */
+ id = btf__find_by_name_kind(btf, "bpf_link_type", BTF_KIND_ENUM);
+ if (!ASSERT_GT(id, 0, "bpf_link_type_id"))
+ goto cleanup;
+ t = btf__type_by_id(btf, id);
+ e = btf_enum(t);
+ n = btf_vlen(t);
+ for (i = 0; i < n; e++, i++) {
+ enum bpf_link_type link_type = (enum bpf_link_type)e->val;
+ const char *link_type_name;
+ const char *link_type_str;
+ char buf[256];
+
+ if (link_type == MAX_BPF_LINK_TYPE)
+ continue;
+
+ link_type_name = btf__str_by_offset(btf, e->name_off);
+ link_type_str = libbpf_bpf_link_type_str(link_type);
+ ASSERT_OK_PTR(link_type_str, link_type_name);
+
+ snprintf(buf, sizeof(buf), "BPF_LINK_TYPE_%s", link_type_str);
+ uppercase(buf);
+
+ ASSERT_STREQ(buf, link_type_name, "exp_str_value");
+ }
+
+cleanup:
+ btf__free(btf);
+}
+
/**
* Test case to check that all bpf_map_type variants are covered by
* libbpf_bpf_map_type_str.
@@ -151,6 +196,9 @@ void test_libbpf_str(void)
if (test__start_subtest("bpf_attach_type_str"))
test_libbpf_bpf_attach_type_str();
+ if (test__start_subtest("bpf_link_type_str"))
+ test_libbpf_bpf_link_type_str();
+
if (test__start_subtest("bpf_map_type_str"))
test_libbpf_bpf_map_type_str();
--
2.30.2
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH bpf-next 12/12] bpftool: Use libbpf_bpf_link_type_str
2022-05-16 17:35 [PATCH bpf-next 00/12] libbpf: Textual representation of enums Daniel Müller
` (10 preceding siblings ...)
2022-05-16 17:35 ` [PATCH bpf-next 11/12] selftests/bpf: Add test for libbpf_bpf_link_type_str Daniel Müller
@ 2022-05-16 17:35 ` Daniel Müller
2022-05-16 23:43 ` [PATCH bpf-next 00/12] libbpf: Textual representation of enums Andrii Nakryiko
2022-05-17 6:42 ` Yonghong Song
13 siblings, 0 replies; 24+ messages in thread
From: Daniel Müller @ 2022-05-16 17:35 UTC (permalink / raw)
To: bpf, ast, andrii, daniel, kernel-team, quentin
This change switches bpftool over to using the recently introduced
libbpf_bpf_link_type_str function instead of maintaining its own string
representation for the bpf_link_type enum.
Signed-off-by: Daniel Müller <deso@posteo.net>
---
tools/bpf/bpftool/link.c | 27 ++++++++++-----------------
1 file changed, 10 insertions(+), 17 deletions(-)
diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
index 2dd0d01..24ab5a 100644
--- a/tools/bpf/bpftool/link.c
+++ b/tools/bpf/bpftool/link.c
@@ -13,19 +13,6 @@
#include "json_writer.h"
#include "main.h"
-static const char * const link_type_name[] = {
- [BPF_LINK_TYPE_UNSPEC] = "unspec",
- [BPF_LINK_TYPE_RAW_TRACEPOINT] = "raw_tracepoint",
- [BPF_LINK_TYPE_TRACING] = "tracing",
- [BPF_LINK_TYPE_CGROUP] = "cgroup",
- [BPF_LINK_TYPE_ITER] = "iter",
- [BPF_LINK_TYPE_NETNS] = "netns",
- [BPF_LINK_TYPE_XDP] = "xdp",
- [BPF_LINK_TYPE_PERF_EVENT] = "perf_event",
- [BPF_LINK_TYPE_KPROBE_MULTI] = "kprobe_multi",
- [BPF_LINK_TYPE_STRUCT_OPS] = "struct_ops",
-};
-
static struct hashmap *link_table;
static int link_parse_fd(int *argc, char ***argv)
@@ -67,9 +54,12 @@ static int link_parse_fd(int *argc, char ***argv)
static void
show_link_header_json(struct bpf_link_info *info, json_writer_t *wtr)
{
+ const char *link_type_str;
+
jsonw_uint_field(wtr, "id", info->id);
- if (info->type < ARRAY_SIZE(link_type_name))
- jsonw_string_field(wtr, "type", link_type_name[info->type]);
+ link_type_str = libbpf_bpf_link_type_str(info->type);
+ if (link_type_str)
+ jsonw_string_field(wtr, "type", link_type_str);
else
jsonw_uint_field(wtr, "type", info->type);
@@ -187,9 +177,12 @@ static int show_link_close_json(int fd, struct bpf_link_info *info)
static void show_link_header_plain(struct bpf_link_info *info)
{
+ const char *link_type_str;
+
printf("%u: ", info->id);
- if (info->type < ARRAY_SIZE(link_type_name))
- printf("%s ", link_type_name[info->type]);
+ link_type_str = libbpf_bpf_link_type_str(info->type);
+ if (link_type_str)
+ printf("%s ", link_type_str);
else
printf("type %u ", info->type);
--
2.30.2
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH bpf-next 00/12] libbpf: Textual representation of enums
2022-05-16 17:35 [PATCH bpf-next 00/12] libbpf: Textual representation of enums Daniel Müller
` (11 preceding siblings ...)
2022-05-16 17:35 ` [PATCH bpf-next 12/12] bpftool: Use libbpf_bpf_link_type_str Daniel Müller
@ 2022-05-16 23:43 ` Andrii Nakryiko
2022-05-17 18:59 ` Daniel Müller
2022-05-17 6:42 ` Yonghong Song
13 siblings, 1 reply; 24+ messages in thread
From: Andrii Nakryiko @ 2022-05-16 23:43 UTC (permalink / raw)
To: Daniel Müller
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Kernel Team, Quentin Monnet
On Mon, May 16, 2022 at 10:35 AM Daniel Müller <deso@posteo.net> wrote:
>
> This patch set introduces the means for querying a textual representation of
> the following BPF related enum types:
> - enum bpf_map_type
> - enum bpf_prog_type
> - enum bpf_attach_type
> - enum bpf_link_type
>
> To make that possible, we introduce a new public function for each of the types:
> libbpf_bpf_<type>_type_str.
>
> Having a way to query a textual representation has been asked for in the past
> (by systemd, among others). Such representations can generally be useful in
> tracing and logging contexts, among others. At this point, at least one client,
> bpftool, maintains such a mapping manually, which is prone to get out of date as
> new enum variants are introduced. libbpf is arguably best situated to keep this
> list complete and up-to-date. This patch series adds BTF based tests to ensure
> that exhaustiveness is upheld moving forward.
>
> The libbpf provided textual representation can be inferred from the
> corresponding enum variant name by removing the prefix and lowercasing the
> remainder. E.g., BPF_PROG_TYPE_SOCKET_FILTER -> socket_filter. Unfortunately,
> bpftool does not use such a programmatic approach for some of the
> bpf_attach_type variants. We propose a work around keeping the existing behavior
> for the time being in the patch titled "bpftool: Use
> libbpf_bpf_attach_type_str".
>
> The patch series is structured as follows:
> - for each enumeration type in {bpf_prog_type, bpf_map_type, bpf_attach_type,
> bpf_link_type}:
> - we first introduce the corresponding public libbpf API function
> - we then add BTF based self-tests
> - we lastly adjust bpftool to use the libbpf provided functionality
>
> Signed-off-by: Daniel Müller <deso@posteo.net>
>
> Daniel Müller (12):
> libbpf: Introduce libbpf_bpf_prog_type_str
> selftests/bpf: Add test for libbpf_bpf_prog_type_str
> bpftool: Use libbpf_bpf_prog_type_str
> libbpf: Introduce libbpf_bpf_map_type_str
> selftests/bpf: Add test for libbpf_bpf_map_type_str
> bpftool: Use libbpf_bpf_map_type_str
> libbpf: Introduce libbpf_bpf_attach_type_str
> selftests/bpf: Add test for libbpf_bpf_attach_type_str
> bpftool: Use libbpf_bpf_attach_type_str
> libbpf: Introduce libbpf_bpf_link_type_str
> selftests/bpf: Add test for libbpf_bpf_link_type_str
> bpftool: Use libbpf_bpf_link_type_str
>
Looks good to me overall. But keep in mind that libbpf v0.8 was just
released, so these new APIs will have to go into 1.0 section in
libbpf.map. It can't inherit from 0.8, btw, so this is a bit new
procedure, I'll try to get to it in next few days. Meanwhile I'd like
to get some feedback at least from Quentin on bpftool changes.
> tools/bpf/bpftool/cgroup.c | 20 +-
> tools/bpf/bpftool/common.c | 46 ----
> tools/bpf/bpftool/feature.c | 87 +++++---
> tools/bpf/bpftool/link.c | 61 +++---
> tools/bpf/bpftool/main.h | 6 -
> tools/bpf/bpftool/map.c | 82 +++----
> tools/bpf/bpftool/prog.c | 51 +----
> tools/lib/bpf/libbpf.c | 160 ++++++++++++++
> tools/lib/bpf/libbpf.h | 36 ++++
> tools/lib/bpf/libbpf.map | 4 +
> .../selftests/bpf/prog_tests/libbpf_str.c | 201 ++++++++++++++++++
> 11 files changed, 539 insertions(+), 215 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/libbpf_str.c
>
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH bpf-next 00/12] libbpf: Textual representation of enums
2022-05-16 23:43 ` [PATCH bpf-next 00/12] libbpf: Textual representation of enums Andrii Nakryiko
@ 2022-05-17 18:59 ` Daniel Müller
0 siblings, 0 replies; 24+ messages in thread
From: Daniel Müller @ 2022-05-17 18:59 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Kernel Team, Quentin Monnet
On Mon, May 16, 2022 at 04:43:42PM -0700, Andrii Nakryiko wrote:
> On Mon, May 16, 2022 at 10:35 AM Daniel Müller <deso@posteo.net> wrote:
> >
> > This patch set introduces the means for querying a textual representation of
> > the following BPF related enum types:
> > - enum bpf_map_type
> > - enum bpf_prog_type
> > - enum bpf_attach_type
> > - enum bpf_link_type
> >
> > To make that possible, we introduce a new public function for each of the types:
> > libbpf_bpf_<type>_type_str.
> >
> > Having a way to query a textual representation has been asked for in the past
> > (by systemd, among others). Such representations can generally be useful in
> > tracing and logging contexts, among others. At this point, at least one client,
> > bpftool, maintains such a mapping manually, which is prone to get out of date as
> > new enum variants are introduced. libbpf is arguably best situated to keep this
> > list complete and up-to-date. This patch series adds BTF based tests to ensure
> > that exhaustiveness is upheld moving forward.
> >
> > The libbpf provided textual representation can be inferred from the
> > corresponding enum variant name by removing the prefix and lowercasing the
> > remainder. E.g., BPF_PROG_TYPE_SOCKET_FILTER -> socket_filter. Unfortunately,
> > bpftool does not use such a programmatic approach for some of the
> > bpf_attach_type variants. We propose a work around keeping the existing behavior
> > for the time being in the patch titled "bpftool: Use
> > libbpf_bpf_attach_type_str".
> >
> > The patch series is structured as follows:
> > - for each enumeration type in {bpf_prog_type, bpf_map_type, bpf_attach_type,
> > bpf_link_type}:
> > - we first introduce the corresponding public libbpf API function
> > - we then add BTF based self-tests
> > - we lastly adjust bpftool to use the libbpf provided functionality
> >
> > Signed-off-by: Daniel Müller <deso@posteo.net>
> >
> > Daniel Müller (12):
> > libbpf: Introduce libbpf_bpf_prog_type_str
> > selftests/bpf: Add test for libbpf_bpf_prog_type_str
> > bpftool: Use libbpf_bpf_prog_type_str
> > libbpf: Introduce libbpf_bpf_map_type_str
> > selftests/bpf: Add test for libbpf_bpf_map_type_str
> > bpftool: Use libbpf_bpf_map_type_str
> > libbpf: Introduce libbpf_bpf_attach_type_str
> > selftests/bpf: Add test for libbpf_bpf_attach_type_str
> > bpftool: Use libbpf_bpf_attach_type_str
> > libbpf: Introduce libbpf_bpf_link_type_str
> > selftests/bpf: Add test for libbpf_bpf_link_type_str
> > bpftool: Use libbpf_bpf_link_type_str
> >
>
> Looks good to me overall. But keep in mind that libbpf v0.8 was just
> released, so these new APIs will have to go into 1.0 section in
> libbpf.map. It can't inherit from 0.8, btw, so this is a bit new
> procedure, I'll try to get to it in next few days. Meanwhile I'd like
> to get some feedback at least from Quentin on bpftool changes.
Thanks for the heads up (and the review)! I am happy to rebase once we have
figured out the procedure.
[...]
Daniel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH bpf-next 00/12] libbpf: Textual representation of enums
2022-05-16 17:35 [PATCH bpf-next 00/12] libbpf: Textual representation of enums Daniel Müller
` (12 preceding siblings ...)
2022-05-16 23:43 ` [PATCH bpf-next 00/12] libbpf: Textual representation of enums Andrii Nakryiko
@ 2022-05-17 6:42 ` Yonghong Song
13 siblings, 0 replies; 24+ messages in thread
From: Yonghong Song @ 2022-05-17 6:42 UTC (permalink / raw)
To: Daniel Müller, bpf, ast, andrii, daniel, kernel-team, quentin
On 5/16/22 10:35 AM, Daniel Müller wrote:
> This patch set introduces the means for querying a textual representation of
> the following BPF related enum types:
> - enum bpf_map_type
> - enum bpf_prog_type
> - enum bpf_attach_type
> - enum bpf_link_type
>
> To make that possible, we introduce a new public function for each of the types:
> libbpf_bpf_<type>_type_str.
>
> Having a way to query a textual representation has been asked for in the past
> (by systemd, among others). Such representations can generally be useful in
> tracing and logging contexts, among others. At this point, at least one client,
> bpftool, maintains such a mapping manually, which is prone to get out of date as
> new enum variants are introduced. libbpf is arguably best situated to keep this
> list complete and up-to-date. This patch series adds BTF based tests to ensure
> that exhaustiveness is upheld moving forward.
>
> The libbpf provided textual representation can be inferred from the
> corresponding enum variant name by removing the prefix and lowercasing the
> remainder. E.g., BPF_PROG_TYPE_SOCKET_FILTER -> socket_filter. Unfortunately,
> bpftool does not use such a programmatic approach for some of the
> bpf_attach_type variants. We propose a work around keeping the existing behavior
> for the time being in the patch titled "bpftool: Use
> libbpf_bpf_attach_type_str".
>
> The patch series is structured as follows:
> - for each enumeration type in {bpf_prog_type, bpf_map_type, bpf_attach_type,
> bpf_link_type}:
> - we first introduce the corresponding public libbpf API function
> - we then add BTF based self-tests
> - we lastly adjust bpftool to use the libbpf provided functionality
>
> Signed-off-by: Daniel Müller <deso@posteo.net>
>
> Daniel Müller (12):
> libbpf: Introduce libbpf_bpf_prog_type_str
> selftests/bpf: Add test for libbpf_bpf_prog_type_str
> bpftool: Use libbpf_bpf_prog_type_str
> libbpf: Introduce libbpf_bpf_map_type_str
> selftests/bpf: Add test for libbpf_bpf_map_type_str
> bpftool: Use libbpf_bpf_map_type_str
> libbpf: Introduce libbpf_bpf_attach_type_str
> selftests/bpf: Add test for libbpf_bpf_attach_type_str
> bpftool: Use libbpf_bpf_attach_type_str
> libbpf: Introduce libbpf_bpf_link_type_str
> selftests/bpf: Add test for libbpf_bpf_link_type_str
> bpftool: Use libbpf_bpf_link_type_str
>
> tools/bpf/bpftool/cgroup.c | 20 +-
> tools/bpf/bpftool/common.c | 46 ----
> tools/bpf/bpftool/feature.c | 87 +++++---
> tools/bpf/bpftool/link.c | 61 +++---
> tools/bpf/bpftool/main.h | 6 -
> tools/bpf/bpftool/map.c | 82 +++----
> tools/bpf/bpftool/prog.c | 51 +----
> tools/lib/bpf/libbpf.c | 160 ++++++++++++++
> tools/lib/bpf/libbpf.h | 36 ++++
> tools/lib/bpf/libbpf.map | 4 +
> .../selftests/bpf/prog_tests/libbpf_str.c | 201 ++++++++++++++++++
> 11 files changed, 539 insertions(+), 215 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/libbpf_str.c
LGTM. Ack for the whole series.
Acked-by: Yonghong Song <yhs@fb.com>
^ permalink raw reply [flat|nested] 24+ messages in thread