* [PATCH bpf-next v3 01/12] libbpf: Introduce libbpf_bpf_prog_type_str
2022-05-19 21:29 [PATCH bpf-next v3 00/12] libbpf: Textual representation of enums Daniel Müller
@ 2022-05-19 21:29 ` Daniel Müller
2022-05-19 21:29 ` [PATCH bpf-next v3 02/12] selftests/bpf: Add test for libbpf_bpf_prog_type_str Daniel Müller
` (11 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: Daniel Müller @ 2022-05-19 21:29 UTC (permalink / raw)
To: bpf, ast, andrii, daniel, kernel-team; +Cc: yhs, 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 | 3 +++
3 files changed, 55 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index ef7f30..6b9c0f 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 52973c..b12d290 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -461,5 +461,8 @@ LIBBPF_0.8.0 {
} LIBBPF_0.7.0;
LIBBPF_1.0.0 {
+ global:
+ libbpf_bpf_prog_type_str;
+
local: *;
};
--
2.30.2
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH bpf-next v3 02/12] selftests/bpf: Add test for libbpf_bpf_prog_type_str
2022-05-19 21:29 [PATCH bpf-next v3 00/12] libbpf: Textual representation of enums Daniel Müller
2022-05-19 21:29 ` [PATCH bpf-next v3 01/12] libbpf: Introduce libbpf_bpf_prog_type_str Daniel Müller
@ 2022-05-19 21:29 ` Daniel Müller
2022-05-19 21:29 ` [PATCH bpf-next v3 03/12] bpftool: Use libbpf_bpf_prog_type_str Daniel Müller
` (10 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: Daniel Müller @ 2022-05-19 21:29 UTC (permalink / raw)
To: bpf, ast, andrii, daniel, kernel-team; +Cc: yhs, 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] 20+ messages in thread* [PATCH bpf-next v3 03/12] bpftool: Use libbpf_bpf_prog_type_str
2022-05-19 21:29 [PATCH bpf-next v3 00/12] libbpf: Textual representation of enums Daniel Müller
2022-05-19 21:29 ` [PATCH bpf-next v3 01/12] libbpf: Introduce libbpf_bpf_prog_type_str Daniel Müller
2022-05-19 21:29 ` [PATCH bpf-next v3 02/12] selftests/bpf: Add test for libbpf_bpf_prog_type_str Daniel Müller
@ 2022-05-19 21:29 ` Daniel Müller
2022-05-19 21:29 ` [PATCH bpf-next v3 04/12] libbpf: Introduce libbpf_bpf_map_type_str Daniel Müller
` (9 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: Daniel Müller @ 2022-05-19 21:29 UTC (permalink / raw)
To: bpf, ast, andrii, daniel, kernel-team; +Cc: yhs, 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..02753f 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,
+ const char *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] 20+ messages in thread* [PATCH bpf-next v3 04/12] libbpf: Introduce libbpf_bpf_map_type_str
2022-05-19 21:29 [PATCH bpf-next v3 00/12] libbpf: Textual representation of enums Daniel Müller
` (2 preceding siblings ...)
2022-05-19 21:29 ` [PATCH bpf-next v3 03/12] bpftool: Use libbpf_bpf_prog_type_str Daniel Müller
@ 2022-05-19 21:29 ` Daniel Müller
2022-05-19 21:29 ` [PATCH bpf-next v3 05/12] selftests/bpf: Add test for libbpf_bpf_map_type_str Daniel Müller
` (8 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: Daniel Müller @ 2022-05-19 21:29 UTC (permalink / raw)
To: bpf, ast, andrii, daniel, kernel-team; +Cc: yhs, 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 6b9c0f..1cbe90 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 b12d290..16065ac 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -462,6 +462,7 @@ LIBBPF_0.8.0 {
LIBBPF_1.0.0 {
global:
+ libbpf_bpf_map_type_str;
libbpf_bpf_prog_type_str;
local: *;
--
2.30.2
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH bpf-next v3 05/12] selftests/bpf: Add test for libbpf_bpf_map_type_str
2022-05-19 21:29 [PATCH bpf-next v3 00/12] libbpf: Textual representation of enums Daniel Müller
` (3 preceding siblings ...)
2022-05-19 21:29 ` [PATCH bpf-next v3 04/12] libbpf: Introduce libbpf_bpf_map_type_str Daniel Müller
@ 2022-05-19 21:29 ` Daniel Müller
2022-05-19 21:29 ` [PATCH bpf-next v3 06/12] bpftool: Use libbpf_bpf_map_type_str Daniel Müller
` (7 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: Daniel Müller @ 2022-05-19 21:29 UTC (permalink / raw)
To: bpf, ast, andrii, daniel, kernel-team; +Cc: yhs, 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] 20+ messages in thread* [PATCH bpf-next v3 06/12] bpftool: Use libbpf_bpf_map_type_str
2022-05-19 21:29 [PATCH bpf-next v3 00/12] libbpf: Textual representation of enums Daniel Müller
` (4 preceding siblings ...)
2022-05-19 21:29 ` [PATCH bpf-next v3 05/12] selftests/bpf: Add test for libbpf_bpf_map_type_str Daniel Müller
@ 2022-05-19 21:29 ` Daniel Müller
2022-05-19 21:29 ` [PATCH bpf-next v3 07/12] libbpf: Introduce libbpf_bpf_attach_type_str Daniel Müller
` (6 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: Daniel Müller @ 2022-05-19 21:29 UTC (permalink / raw)
To: bpf, ast, andrii, daniel, kernel-team; +Cc: yhs, 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 02753f..cc9e4d 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] 20+ messages in thread* [PATCH bpf-next v3 07/12] libbpf: Introduce libbpf_bpf_attach_type_str
2022-05-19 21:29 [PATCH bpf-next v3 00/12] libbpf: Textual representation of enums Daniel Müller
` (5 preceding siblings ...)
2022-05-19 21:29 ` [PATCH bpf-next v3 06/12] bpftool: Use libbpf_bpf_map_type_str Daniel Müller
@ 2022-05-19 21:29 ` Daniel Müller
2022-05-19 21:29 ` [PATCH bpf-next v3 08/12] selftests/bpf: Add test for libbpf_bpf_attach_type_str Daniel Müller
` (5 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: Daniel Müller @ 2022-05-19 21:29 UTC (permalink / raw)
To: bpf, ast, andrii, daniel, kernel-team; +Cc: yhs, 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 1cbe90..bc56e9c 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 16065ac..44c536 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -462,6 +462,7 @@ LIBBPF_0.8.0 {
LIBBPF_1.0.0 {
global:
+ libbpf_bpf_attach_type_str;
libbpf_bpf_map_type_str;
libbpf_bpf_prog_type_str;
--
2.30.2
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH bpf-next v3 08/12] selftests/bpf: Add test for libbpf_bpf_attach_type_str
2022-05-19 21:29 [PATCH bpf-next v3 00/12] libbpf: Textual representation of enums Daniel Müller
` (6 preceding siblings ...)
2022-05-19 21:29 ` [PATCH bpf-next v3 07/12] libbpf: Introduce libbpf_bpf_attach_type_str Daniel Müller
@ 2022-05-19 21:29 ` Daniel Müller
2022-05-19 21:29 ` [PATCH bpf-next v3 09/12] bpftool: Use libbpf_bpf_attach_type_str Daniel Müller
` (4 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: Daniel Müller @ 2022-05-19 21:29 UTC (permalink / raw)
To: bpf, ast, andrii, daniel, kernel-team; +Cc: yhs, 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] 20+ messages in thread* [PATCH bpf-next v3 09/12] bpftool: Use libbpf_bpf_attach_type_str
2022-05-19 21:29 [PATCH bpf-next v3 00/12] libbpf: Textual representation of enums Daniel Müller
` (7 preceding siblings ...)
2022-05-19 21:29 ` [PATCH bpf-next v3 08/12] selftests/bpf: Add test for libbpf_bpf_attach_type_str Daniel Müller
@ 2022-05-19 21:29 ` Daniel Müller
2022-05-23 11:48 ` Quentin Monnet
2022-05-19 21:29 ` [PATCH bpf-next v3 10/12] libbpf: Introduce libbpf_bpf_link_type_str Daniel Müller
` (3 subsequent siblings)
12 siblings, 1 reply; 20+ messages in thread
From: Daniel Müller @ 2022-05-19 21:29 UTC (permalink / raw)
To: bpf, ast, andrii, daniel, kernel-team; +Cc: yhs, 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.
We decided to fix up this deficiency with this change, meaning that
bpftool uses the same textual representations as libbpf. Supporting
test, completion scripts, and man pages have been adjusted accordingly.
However, we did add support for accepting (the now undocumented)
original attach type names when they are provided by users.
For the test (test_bpftool_synctypes.py), I have removed the enum
representation checks, because we no longer mirror the various enum
variant names in bpftool source code. For the man page, help text, and
completion script checks we are now using enum definitions from
uapi/linux/bpf.h as the source of truth directly.
Signed-off-by: Daniel Müller <deso@posteo.net>
---
.../bpftool/Documentation/bpftool-cgroup.rst | 16 +-
.../bpftool/Documentation/bpftool-prog.rst | 5 +-
tools/bpf/bpftool/bash-completion/bpftool | 18 +-
tools/bpf/bpftool/cgroup.c | 49 ++++--
tools/bpf/bpftool/common.c | 82 ++++-----
tools/bpf/bpftool/link.c | 15 +-
tools/bpf/bpftool/main.h | 17 ++
tools/bpf/bpftool/prog.c | 26 ++-
.../selftests/bpf/test_bpftool_synctypes.py | 163 ++++++++----------
9 files changed, 213 insertions(+), 178 deletions(-)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst b/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
index a17e9a..bd015e 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
@@ -31,11 +31,17 @@ CGROUP COMMANDS
| **bpftool** **cgroup help**
|
| *PROG* := { **id** *PROG_ID* | **pinned** *FILE* | **tag** *PROG_TAG* }
-| *ATTACH_TYPE* := { **ingress** | **egress** | **sock_create** | **sock_ops** | **device** |
-| **bind4** | **bind6** | **post_bind4** | **post_bind6** | **connect4** | **connect6** |
-| **getpeername4** | **getpeername6** | **getsockname4** | **getsockname6** | **sendmsg4** |
-| **sendmsg6** | **recvmsg4** | **recvmsg6** | **sysctl** | **getsockopt** | **setsockopt** |
-| **sock_release** }
+| *ATTACH_TYPE* := { **cgroup_inet_ingress** | **cgroup_inet_egress** |
+| **cgroup_inet_sock_create** | **cgroup_sock_ops** |
+| **cgroup_device** | **cgroup_inet4_bind** | **cgroup_inet6_bind** |
+| **cgroup_inet4_post_bind** | **cgroup_inet6_post_bind** |
+| **cgroup_inet4_connect** | **cgroup_inet6_connect** |
+| **cgroup_inet4_getpeername** | **cgroup_inet6_getpeername** |
+| **cgroup_inet4_getsockname** | **cgroup_inet6_getsockname** |
+| **cgroup_udp4_sendmsg** | **cgroup_udp6_sendmsg** |
+| **cgroup_udp4_recvmsg** | **cgroup_udp6_recvmsg** |
+| **cgroup_sysctl** | **cgroup_getsockopt** | **cgroup_setsockopt** |
+| **cgroup_inet_sock_release** }
| *ATTACH_FLAGS* := { **multi** | **override** }
DESCRIPTION
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index a2e935..eb1b2a 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -53,8 +53,9 @@ PROG COMMANDS
| **cgroup/getsockopt** | **cgroup/setsockopt** | **cgroup/sock_release** |
| **struct_ops** | **fentry** | **fexit** | **freplace** | **sk_lookup**
| }
-| *ATTACH_TYPE* := {
-| **msg_verdict** | **skb_verdict** | **stream_verdict** | **stream_parser** | **flow_dissector**
+| *ATTACH_TYPE* := {
+| **sk_msg_verdict** | **sk_skb_verdict** | **sk_skb_stream_verdict** |
+| **sk_skb_stream_parser** | **flow_dissector**
| }
| *METRICs* := {
| **cycles** | **instructions** | **l1d_loads** | **llc_misses** |
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index 5df8d7..91f89a 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -407,8 +407,8 @@ _bpftool()
return 0
;;
5)
- local BPFTOOL_PROG_ATTACH_TYPES='msg_verdict \
- skb_verdict stream_verdict stream_parser \
+ local BPFTOOL_PROG_ATTACH_TYPES='sk_msg_verdict \
+ sk_skb_verdict sk_skb_stream_verdict sk_skb_stream_parser \
flow_dissector'
COMPREPLY=( $( compgen -W "$BPFTOOL_PROG_ATTACH_TYPES" -- "$cur" ) )
return 0
@@ -1039,12 +1039,14 @@ _bpftool()
return 0
;;
attach|detach)
- local BPFTOOL_CGROUP_ATTACH_TYPES='ingress egress \
- sock_create sock_ops device \
- bind4 bind6 post_bind4 post_bind6 connect4 connect6 \
- getpeername4 getpeername6 getsockname4 getsockname6 \
- sendmsg4 sendmsg6 recvmsg4 recvmsg6 sysctl getsockopt \
- setsockopt sock_release'
+ local BPFTOOL_CGROUP_ATTACH_TYPES='cgroup_inet_ingress cgroup_inet_egress \
+ cgroup_inet_sock_create cgroup_sock_ops cgroup_device cgroup_inet4_bind \
+ cgroup_inet6_bind cgroup_inet4_post_bind cgroup_inet6_post_bind \
+ cgroup_inet4_connect cgroup_inet6_connect cgroup_inet4_getpeername \
+ cgroup_inet6_getpeername cgroup_inet4_getsockname cgroup_inet6_getsockname \
+ cgroup_udp4_sendmsg cgroup_udp6_sendmsg cgroup_udp4_recvmsg \
+ cgroup_udp6_recvmsg cgroup_sysctl cgroup_getsockopt cgroup_setsockopt \
+ cgroup_inet_sock_release'
local ATTACH_FLAGS='multi override'
local PROG_TYPE='id pinned tag name'
# Check for $prev = $command first
diff --git a/tools/bpf/bpftool/cgroup.c b/tools/bpf/bpftool/cgroup.c
index effe136..c111a5 100644
--- a/tools/bpf/bpftool/cgroup.c
+++ b/tools/bpf/bpftool/cgroup.c
@@ -21,25 +21,39 @@
#define HELP_SPEC_ATTACH_FLAGS \
"ATTACH_FLAGS := { multi | override }"
-#define HELP_SPEC_ATTACH_TYPES \
- " ATTACH_TYPE := { ingress | egress | sock_create |\n" \
- " sock_ops | device | bind4 | bind6 |\n" \
- " post_bind4 | post_bind6 | connect4 |\n" \
- " connect6 | getpeername4 | getpeername6 |\n" \
- " getsockname4 | getsockname6 | sendmsg4 |\n" \
- " sendmsg6 | recvmsg4 | recvmsg6 |\n" \
- " sysctl | getsockopt | setsockopt |\n" \
- " sock_release }"
+#define HELP_SPEC_ATTACH_TYPES \
+ " ATTACH_TYPE := { cgroup_inet_ingress | cgroup_inet_egress |\n" \
+ " cgroup_inet_sock_create | cgroup_sock_ops |\n" \
+ " cgroup_device | cgroup_inet4_bind |\n" \
+ " cgroup_inet6_bind | cgroup_inet4_post_bind |\n" \
+ " cgroup_inet6_post_bind | cgroup_inet4_connect |\n" \
+ " cgroup_inet6_connect | cgroup_inet4_getpeername |\n" \
+ " cgroup_inet6_getpeername | cgroup_inet4_getsockname |\n" \
+ " cgroup_inet6_getsockname | cgroup_udp4_sendmsg |\n" \
+ " cgroup_udp6_sendmsg | cgroup_udp4_recvmsg |\n" \
+ " cgroup_udp6_recvmsg | cgroup_sysctl |\n" \
+ " cgroup_getsockopt | cgroup_setsockopt |\n" \
+ " cgroup_inet_sock_release }"
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 = libbpf_bpf_attach_type_str(type);
+ if (!attach_type_str)
+ break;
+ if (is_prefix(str, attach_type_str))
+ return type;
+
+ /* Also check traditionally used attach type strings. */
+ attach_type_str = bpf_attach_type_input_str(type);
+ if (!attach_type_str)
+ continue;
+ if (is_prefix(str, attach_type_str))
return type;
}
@@ -52,6 +66,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 +79,13 @@ static int show_bpf_prog(int id, enum bpf_attach_type attach_type,
return -1;
}
+ attach_type_str = libbpf_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 +94,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..b659eac 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_input_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 NULL;
+ }
+}
diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
index e27108..66a254 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 = libbpf_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 = libbpf_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..a686b2c 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -243,6 +243,23 @@ 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_input_str - convert the provided attach type value into a
+ * textual representation that we accept for input purposes.
+ *
+ * This function acts is similar in nature to libbpf_bpf_attach_type_str, but
+ * only recognizes some of attach type names that have been used by the program
+ * in the past and which do not follow the string inference scheme that libbpf
+ * uses.
+ * These textual representations should only be used for user input.
+ *
+ * @t: The attach type
+ * Returns a pointer to a static string identifying the attach type. NULL is
+ * returned for unknown bpf_attach_type values or those whose representation
+ * does not differ from that in libbpf itself.
+ */
+const char *bpf_attach_type_input_str(enum bpf_attach_type t);
+
static inline void *u32_as_hash_field(__u32 x)
{
return (void *)(uintptr_t)x;
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 39e1e71..a45fa3 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -41,12 +41,24 @@ enum dump_mode {
DUMP_XLATED,
};
+static const bool attach_types[] = {
+ [BPF_SK_SKB_STREAM_PARSER] = true,
+ [BPF_SK_SKB_STREAM_VERDICT] = true,
+ [BPF_SK_SKB_VERDICT] = true,
+ [BPF_SK_MSG_VERDICT] = true,
+ [BPF_FLOW_DISSECTOR] = true,
+ [__MAX_BPF_ATTACH_TYPE] = false,
+};
+
+/*
+ * Textual representations traditionally used by the program and kept around for
+ * the sake of backwards compatibility.
+ */
static const char * const attach_type_strings[] = {
[BPF_SK_SKB_STREAM_PARSER] = "stream_parser",
[BPF_SK_SKB_STREAM_VERDICT] = "stream_verdict",
[BPF_SK_SKB_VERDICT] = "skb_verdict",
[BPF_SK_MSG_VERDICT] = "msg_verdict",
- [BPF_FLOW_DISSECTOR] = "flow_dissector",
[__MAX_BPF_ATTACH_TYPE] = NULL,
};
@@ -57,6 +69,14 @@ static enum bpf_attach_type parse_attach_type(const char *str)
enum bpf_attach_type type;
for (type = 0; type < __MAX_BPF_ATTACH_TYPE; type++) {
+ if (attach_types[type]) {
+ const char *attach_type_str;
+
+ attach_type_str = libbpf_bpf_attach_type_str(type);
+ if (is_prefix(str, attach_type_str))
+ return type;
+ }
+
if (attach_type_strings[type] &&
is_prefix(str, attach_type_strings[type]))
return type;
@@ -2341,8 +2361,8 @@ static int do_help(int argc, char **argv)
" cgroup/sendmsg6 | cgroup/recvmsg4 | cgroup/recvmsg6 |\n"
" cgroup/getsockopt | cgroup/setsockopt | cgroup/sock_release |\n"
" struct_ops | fentry | fexit | freplace | sk_lookup }\n"
- " ATTACH_TYPE := { msg_verdict | skb_verdict | stream_verdict |\n"
- " stream_parser | flow_dissector }\n"
+ " ATTACH_TYPE := { sk_msg_verdict | sk_skb_verdict | sk_skb_stream_verdict |\n"
+ " sk_skb_stream_parser | flow_dissector }\n"
" METRIC := { cycles | instructions | l1d_loads | llc_misses | itlb_misses | dtlb_misses }\n"
" " HELP_SPEC_OPTIONS " |\n"
" {-f|--bpffs} | {-m|--mapcompat} | {-n|--nomount} |\n"
diff --git a/tools/testing/selftests/bpf/test_bpftool_synctypes.py b/tools/testing/selftests/bpf/test_bpftool_synctypes.py
index c0e7acd..0ca3c1 100755
--- a/tools/testing/selftests/bpf/test_bpftool_synctypes.py
+++ b/tools/testing/selftests/bpf/test_bpftool_synctypes.py
@@ -58,7 +58,7 @@ class BlockParser(object):
class ArrayParser(BlockParser):
"""
- A parser for extracting dicionaries of values from some BPF-related arrays.
+ A parser for extracting a set of values from some BPF-related arrays.
@reader: a pointer to the open file to parse
@array_name: name of the array to parse
"""
@@ -66,7 +66,7 @@ class ArrayParser(BlockParser):
def __init__(self, reader, array_name):
self.array_name = array_name
- self.start_marker = re.compile(f'(static )?const char \* const {self.array_name}\[.*\] = {{\n')
+ self.start_marker = re.compile(f'(static )?const bool {self.array_name}\[.*\] = {{\n')
super().__init__(reader)
def search_block(self):
@@ -80,15 +80,15 @@ class ArrayParser(BlockParser):
Parse a block and return data as a dictionary. Items to extract must be
on separate lines in the file.
"""
- pattern = re.compile('\[(BPF_\w*)\]\s*= "(.*)",?$')
- entries = {}
+ pattern = re.compile('\[(BPF_\w*)\]\s*= (true|false),?$')
+ entries = set()
while True:
line = self.reader.readline()
if line == '' or re.match(self.end_marker, line):
break
capture = pattern.search(line)
if capture:
- entries[capture.group(1)] = capture.group(2)
+ entries |= {capture.group(1)}
return entries
class InlineListParser(BlockParser):
@@ -115,7 +115,7 @@ class InlineListParser(BlockParser):
class FileExtractor(object):
"""
A generic reader for extracting data from a given file. This class contains
- several helper methods that wrap arround parser objects to extract values
+ several helper methods that wrap around parser objects to extract values
from different structures.
This class does not offer a way to set a filename, which is expected to be
defined in children classes.
@@ -139,21 +139,20 @@ class FileExtractor(object):
def get_types_from_array(self, array_name):
"""
- Search for and parse an array associating names to BPF_* enum members,
- for example:
+ Search for and parse an array white-listing BPF_* enum members, for
+ example:
- const char * const prog_type_name[] = {
- [BPF_PROG_TYPE_UNSPEC] = "unspec",
- [BPF_PROG_TYPE_SOCKET_FILTER] = "socket_filter",
- [BPF_PROG_TYPE_KPROBE] = "kprobe",
+ const bool prog_type_name[] = {
+ [BPF_PROG_TYPE_UNSPEC] = true,
+ [BPF_PROG_TYPE_SOCKET_FILTER] = true,
+ [BPF_PROG_TYPE_KPROBE] = true,
};
- Return a dictionary with the enum member names as keys and the
- associated names as values, for example:
+ Return a set of the enum members, for example:
- {'BPF_PROG_TYPE_UNSPEC': 'unspec',
- 'BPF_PROG_TYPE_SOCKET_FILTER': 'socket_filter',
- 'BPF_PROG_TYPE_KPROBE': 'kprobe'}
+ {'BPF_PROG_TYPE_UNSPEC',
+ 'BPF_PROG_TYPE_SOCKET_FILTER',
+ 'BPF_PROG_TYPE_KPROBE'}
@array_name: name of the array to parse
"""
@@ -186,6 +185,27 @@ class FileExtractor(object):
parser.search_block(start_marker)
return parser.parse(pattern, end_marker)
+ def make_enum_map(self, names, enum_prefix):
+ """
+ Search for and parse an enum containing BPF_* members, just as get_enum
+ does. However, instead of just returning a set of the variant names,
+ also generate a textual representation from them by (assuming and)
+ removing a provided prefix and lowercasing the remainder. Then return a
+ dict mapping from name to textual representation.
+
+ @enum_values: a set of enum values; e.g., as retrieved by get_enum
+ @enum_prefix: the prefix to remove from each of the variants to infer
+ textual representation
+ """
+ mapping = {}
+ for name in names:
+ if not name.startswith(enum_prefix):
+ raise Exception(f"enum variant {name} does not start with {enum_prefix}")
+ text = name[len(enum_prefix):].lower()
+ mapping[name] = text
+
+ return mapping
+
def __get_description_list(self, start_marker, pattern, end_marker):
parser = InlineListParser(self.reader)
parser.search_block(start_marker)
@@ -333,11 +353,9 @@ class ProgFileExtractor(SourceFileExtractor):
"""
filename = os.path.join(BPFTOOL_DIR, 'prog.c')
- def get_prog_types(self):
- return self.get_types_from_array('prog_type_name')
-
def get_attach_types(self):
- return self.get_types_from_array('attach_type_strings')
+ types = self.get_types_from_array('attach_types')
+ return self.make_enum_map(types, 'BPF_')
def get_prog_attach_help(self):
return self.get_help_list('ATTACH_TYPE')
@@ -348,9 +366,6 @@ class MapFileExtractor(SourceFileExtractor):
"""
filename = os.path.join(BPFTOOL_DIR, 'map.c')
- def get_map_types(self):
- return self.get_types_from_array('map_type_name')
-
def get_map_help(self):
return self.get_help_list('TYPE')
@@ -363,30 +378,6 @@ class CgroupFileExtractor(SourceFileExtractor):
def get_prog_attach_help(self):
return self.get_help_list('ATTACH_TYPE')
-class CommonFileExtractor(SourceFileExtractor):
- """
- An extractor for bpftool's common.c.
- """
- filename = os.path.join(BPFTOOL_DIR, 'common.c')
-
- def __init__(self):
- super().__init__()
- self.attach_types = {}
-
- def get_attach_types(self):
- if not self.attach_types:
- self.attach_types = self.get_types_from_array('attach_type_name')
- return self.attach_types
-
- def get_cgroup_attach_types(self):
- if not self.attach_types:
- self.get_attach_types()
- cgroup_types = {}
- for (key, value) in self.attach_types.items():
- if key.find('BPF_CGROUP') != -1:
- cgroup_types[key] = value
- return cgroup_types
-
class GenericSourceExtractor(SourceFileExtractor):
"""
An extractor for generic source code files.
@@ -403,14 +394,28 @@ class BpfHeaderExtractor(FileExtractor):
"""
filename = os.path.join(INCLUDE_DIR, 'uapi/linux/bpf.h')
+ def __init__(self):
+ super().__init__()
+ self.attach_types = {}
+
def get_prog_types(self):
return self.get_enum('bpf_prog_type')
- def get_map_types(self):
- return self.get_enum('bpf_map_type')
+ def get_map_type_map(self):
+ names = self.get_enum('bpf_map_type')
+ return self.make_enum_map(names, 'BPF_MAP_TYPE_')
- def get_attach_types(self):
- return self.get_enum('bpf_attach_type')
+ def get_attach_type_map(self):
+ if not self.attach_types:
+ names = self.get_enum('bpf_attach_type')
+ self.attach_types = self.make_enum_map(names, 'BPF_')
+ return self.attach_types
+
+ def get_cgroup_attach_type_map(self):
+ if not self.attach_types:
+ self.get_attach_type_map()
+ return {name: text for name, text in self.attach_types.items()
+ if name.startswith('BPF_CGROUP')}
class ManPageExtractor(FileExtractor):
"""
@@ -495,21 +500,12 @@ def main():
""")
args = argParser.parse_args()
- # Map types (enum)
-
bpf_info = BpfHeaderExtractor()
- ref = bpf_info.get_map_types()
-
- map_info = MapFileExtractor()
- source_map_items = map_info.get_map_types()
- map_types_enum = set(source_map_items.keys())
-
- verify(ref, map_types_enum,
- f'Comparing BPF header (enum bpf_map_type) and {MapFileExtractor.filename} (map_type_name):')
# Map types (names)
- source_map_types = set(source_map_items.values())
+ map_info = MapFileExtractor()
+ source_map_types = set(bpf_info.get_map_type_map().values())
source_map_types.discard('unspec')
help_map_types = map_info.get_map_help()
@@ -525,34 +521,18 @@ def main():
bashcomp_map_types = bashcomp_info.get_map_types()
verify(source_map_types, help_map_types,
- f'Comparing {MapFileExtractor.filename} (map_type_name) and {MapFileExtractor.filename} (do_help() TYPE):')
+ f'Comparing {BpfHeaderExtractor.filename} (bpf_map_type) and {MapFileExtractor.filename} (do_help() TYPE):')
verify(source_map_types, man_map_types,
- f'Comparing {MapFileExtractor.filename} (map_type_name) and {ManMapExtractor.filename} (TYPE):')
+ f'Comparing {BpfHeaderExtractor.filename} (bpf_map_type) and {ManMapExtractor.filename} (TYPE):')
verify(help_map_options, man_map_options,
f'Comparing {MapFileExtractor.filename} (do_help() OPTIONS) and {ManMapExtractor.filename} (OPTIONS):')
verify(source_map_types, bashcomp_map_types,
- f'Comparing {MapFileExtractor.filename} (map_type_name) and {BashcompExtractor.filename} (BPFTOOL_MAP_CREATE_TYPES):')
+ f'Comparing {BpfHeaderExtractor.filename} (bpf_map_type) and {BashcompExtractor.filename} (BPFTOOL_MAP_CREATE_TYPES):')
# Program types (enum)
- ref = bpf_info.get_prog_types()
-
prog_info = ProgFileExtractor()
- prog_types = set(prog_info.get_prog_types().keys())
-
- verify(ref, prog_types,
- f'Comparing BPF header (enum bpf_prog_type) and {ProgFileExtractor.filename} (prog_type_name):')
-
- # Attach types (enum)
-
- ref = bpf_info.get_attach_types()
- bpf_info.close()
-
- common_info = CommonFileExtractor()
- attach_types = common_info.get_attach_types()
-
- verify(ref, attach_types,
- f'Comparing BPF header (enum bpf_attach_type) and {CommonFileExtractor.filename} (attach_type_name):')
+ prog_types = bpf_info.get_prog_types()
# Attach types (names)
@@ -571,18 +551,17 @@ def main():
bashcomp_prog_attach_types = bashcomp_info.get_prog_attach_types()
verify(source_prog_attach_types, help_prog_attach_types,
- f'Comparing {ProgFileExtractor.filename} (attach_type_strings) and {ProgFileExtractor.filename} (do_help() ATTACH_TYPE):')
+ f'Comparing {ProgFileExtractor.filename} (bpf_attach_type) and {ProgFileExtractor.filename} (do_help() ATTACH_TYPE):')
verify(source_prog_attach_types, man_prog_attach_types,
- f'Comparing {ProgFileExtractor.filename} (attach_type_strings) and {ManProgExtractor.filename} (ATTACH_TYPE):')
+ f'Comparing {ProgFileExtractor.filename} (bpf_attach_type) and {ManProgExtractor.filename} (ATTACH_TYPE):')
verify(help_prog_options, man_prog_options,
f'Comparing {ProgFileExtractor.filename} (do_help() OPTIONS) and {ManProgExtractor.filename} (OPTIONS):')
verify(source_prog_attach_types, bashcomp_prog_attach_types,
- f'Comparing {ProgFileExtractor.filename} (attach_type_strings) and {BashcompExtractor.filename} (BPFTOOL_PROG_ATTACH_TYPES):')
+ f'Comparing {ProgFileExtractor.filename} (bpf_attach_type) and {BashcompExtractor.filename} (BPFTOOL_PROG_ATTACH_TYPES):')
# Cgroup attach types
-
- source_cgroup_attach_types = set(common_info.get_cgroup_attach_types().values())
- common_info.close()
+ source_cgroup_attach_types = set(bpf_info.get_cgroup_attach_type_map().values())
+ bpf_info.close()
cgroup_info = CgroupFileExtractor()
help_cgroup_attach_types = cgroup_info.get_prog_attach_help()
@@ -598,13 +577,13 @@ def main():
bashcomp_info.close()
verify(source_cgroup_attach_types, help_cgroup_attach_types,
- f'Comparing {CommonFileExtractor.filename} (attach_type_strings) and {CgroupFileExtractor.filename} (do_help() ATTACH_TYPE):')
+ f'Comparing {BpfHeaderExtractor.filename} (bpf_attach_type) and {CgroupFileExtractor.filename} (do_help() ATTACH_TYPE):')
verify(source_cgroup_attach_types, man_cgroup_attach_types,
- f'Comparing {CommonFileExtractor.filename} (attach_type_strings) and {ManCgroupExtractor.filename} (ATTACH_TYPE):')
+ f'Comparing {BpfHeaderExtractor.filename} (bpf_attach_type) and {ManCgroupExtractor.filename} (ATTACH_TYPE):')
verify(help_cgroup_options, man_cgroup_options,
f'Comparing {CgroupFileExtractor.filename} (do_help() OPTIONS) and {ManCgroupExtractor.filename} (OPTIONS):')
verify(source_cgroup_attach_types, bashcomp_cgroup_attach_types,
- f'Comparing {CommonFileExtractor.filename} (attach_type_strings) and {BashcompExtractor.filename} (BPFTOOL_CGROUP_ATTACH_TYPES):')
+ f'Comparing {BpfHeaderExtractor.filename} (bpf_attach_type) and {BashcompExtractor.filename} (BPFTOOL_CGROUP_ATTACH_TYPES):')
# Options for remaining commands
--
2.30.2
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH bpf-next v3 09/12] bpftool: Use libbpf_bpf_attach_type_str
2022-05-19 21:29 ` [PATCH bpf-next v3 09/12] bpftool: Use libbpf_bpf_attach_type_str Daniel Müller
@ 2022-05-23 11:48 ` Quentin Monnet
2022-05-23 18:05 ` Andrii Nakryiko
2022-05-23 20:14 ` Daniel Müller
0 siblings, 2 replies; 20+ messages in thread
From: Quentin Monnet @ 2022-05-23 11:48 UTC (permalink / raw)
To: Daniel Müller, bpf, ast, andrii, daniel, kernel-team; +Cc: yhs
2022-05-19 21:29 UTC+0000 ~ Daniel Müller <deso@posteo.net>
> 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.
> We decided to fix up this deficiency with this change, meaning that
> bpftool uses the same textual representations as libbpf. Supporting
> test, completion scripts, and man pages have been adjusted accordingly.
> However, we did add support for accepting (the now undocumented)
> original attach type names when they are provided by users.
>
> For the test (test_bpftool_synctypes.py), I have removed the enum
> representation checks, because we no longer mirror the various enum
> variant names in bpftool source code. For the man page, help text, and
> completion script checks we are now using enum definitions from
> uapi/linux/bpf.h as the source of truth directly.
>
> Signed-off-by: Daniel Müller <deso@posteo.net>
> ---
> .../bpftool/Documentation/bpftool-cgroup.rst | 16 +-
> .../bpftool/Documentation/bpftool-prog.rst | 5 +-
> tools/bpf/bpftool/bash-completion/bpftool | 18 +-
> tools/bpf/bpftool/cgroup.c | 49 ++++--
> tools/bpf/bpftool/common.c | 82 ++++-----
> tools/bpf/bpftool/link.c | 15 +-
> tools/bpf/bpftool/main.h | 17 ++
> tools/bpf/bpftool/prog.c | 26 ++-
> .../selftests/bpf/test_bpftool_synctypes.py | 163 ++++++++----------
> 9 files changed, 213 insertions(+), 178 deletions(-)
> diff --git a/tools/bpf/bpftool/cgroup.c b/tools/bpf/bpftool/cgroup.c
> index effe136..c111a5 100644
> --- a/tools/bpf/bpftool/cgroup.c
> +++ b/tools/bpf/bpftool/cgroup.c
> @@ -21,25 +21,39 @@
> #define HELP_SPEC_ATTACH_FLAGS \
> "ATTACH_FLAGS := { multi | override }"
>
> -#define HELP_SPEC_ATTACH_TYPES \
> - " ATTACH_TYPE := { ingress | egress | sock_create |\n" \
> - " sock_ops | device | bind4 | bind6 |\n" \
> - " post_bind4 | post_bind6 | connect4 |\n" \
> - " connect6 | getpeername4 | getpeername6 |\n" \
> - " getsockname4 | getsockname6 | sendmsg4 |\n" \
> - " sendmsg6 | recvmsg4 | recvmsg6 |\n" \
> - " sysctl | getsockopt | setsockopt |\n" \
> - " sock_release }"
> +#define HELP_SPEC_ATTACH_TYPES \
> + " ATTACH_TYPE := { cgroup_inet_ingress | cgroup_inet_egress |\n" \
> + " cgroup_inet_sock_create | cgroup_sock_ops |\n" \
> + " cgroup_device | cgroup_inet4_bind |\n" \
> + " cgroup_inet6_bind | cgroup_inet4_post_bind |\n" \
> + " cgroup_inet6_post_bind | cgroup_inet4_connect |\n" \
> + " cgroup_inet6_connect | cgroup_inet4_getpeername |\n" \
> + " cgroup_inet6_getpeername | cgroup_inet4_getsockname |\n" \
> + " cgroup_inet6_getsockname | cgroup_udp4_sendmsg |\n" \
> + " cgroup_udp6_sendmsg | cgroup_udp4_recvmsg |\n" \
> + " cgroup_udp6_recvmsg | cgroup_sysctl |\n" \
> + " cgroup_getsockopt | cgroup_setsockopt |\n" \
> + " cgroup_inet_sock_release }"
>
> 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 = libbpf_bpf_attach_type_str(type);
> + if (!attach_type_str)
> + break;
> + if (is_prefix(str, attach_type_str))
With so many shared prefixes here, I'm wondering if it would make more
sense to compare the whole string instead? Otherwise it's hard to guess
which type “bpftool c a <cgroup> cgroup_ <prog>” will use. At the same
time we allow prefixing arguments everywhere else, so maybe not worth
changing it here. Or we could maybe error out if the string length is <=
strlen("cgroup_")? Let's see for a follow-up maybe.
> + return type;
> +
> + /* Also check traditionally used attach type strings. */
> + attach_type_str = bpf_attach_type_input_str(type);
> + if (!attach_type_str)
> + continue;
> + if (is_prefix(str, attach_type_str))
> return type;
> }
>
> diff --git a/tools/testing/selftests/bpf/test_bpftool_synctypes.py b/tools/testing/selftests/bpf/test_bpftool_synctypes.py
> index c0e7acd..0ca3c1 100755
> --- a/tools/testing/selftests/bpf/test_bpftool_synctypes.py
> +++ b/tools/testing/selftests/bpf/test_bpftool_synctypes.py
> @@ -139,21 +139,20 @@ class FileExtractor(object):
>
> def get_types_from_array(self, array_name):
> """
> - Search for and parse an array associating names to BPF_* enum members,
> - for example:
> + Search for and parse an array white-listing BPF_* enum members, for
The coding style now recommends against the “white-listing”. Maybe
“[...] a list of allowed BPF_* enum members”?
> + example:
>
> - const char * const prog_type_name[] = {
> - [BPF_PROG_TYPE_UNSPEC] = "unspec",
> - [BPF_PROG_TYPE_SOCKET_FILTER] = "socket_filter",
> - [BPF_PROG_TYPE_KPROBE] = "kprobe",
> + const bool prog_type_name[] = {
> + [BPF_PROG_TYPE_UNSPEC] = true,
> + [BPF_PROG_TYPE_SOCKET_FILTER] = true,
> + [BPF_PROG_TYPE_KPROBE] = true,
> };
>
> - Return a dictionary with the enum member names as keys and the
> - associated names as values, for example:
> + Return a set of the enum members, for example:
>
> - {'BPF_PROG_TYPE_UNSPEC': 'unspec',
> - 'BPF_PROG_TYPE_SOCKET_FILTER': 'socket_filter',
> - 'BPF_PROG_TYPE_KPROBE': 'kprobe'}
> + {'BPF_PROG_TYPE_UNSPEC',
> + 'BPF_PROG_TYPE_SOCKET_FILTER',
> + 'BPF_PROG_TYPE_KPROBE'}
>
> @array_name: name of the array to parse
> """
> @@ -525,34 +521,18 @@ def main():
> bashcomp_map_types = bashcomp_info.get_map_types()
>
> verify(source_map_types, help_map_types,
> - f'Comparing {MapFileExtractor.filename} (map_type_name) and {MapFileExtractor.filename} (do_help() TYPE):')
> + f'Comparing {BpfHeaderExtractor.filename} (bpf_map_type) and {MapFileExtractor.filename} (do_help() TYPE):')
> verify(source_map_types, man_map_types,
> - f'Comparing {MapFileExtractor.filename} (map_type_name) and {ManMapExtractor.filename} (TYPE):')
> + f'Comparing {BpfHeaderExtractor.filename} (bpf_map_type) and {ManMapExtractor.filename} (TYPE):')
> verify(help_map_options, man_map_options,
> f'Comparing {MapFileExtractor.filename} (do_help() OPTIONS) and {ManMapExtractor.filename} (OPTIONS):')
> verify(source_map_types, bashcomp_map_types,
> - f'Comparing {MapFileExtractor.filename} (map_type_name) and {BashcompExtractor.filename} (BPFTOOL_MAP_CREATE_TYPES):')
> + f'Comparing {BpfHeaderExtractor.filename} (bpf_map_type) and {BashcompExtractor.filename} (BPFTOOL_MAP_CREATE_TYPES):')
>
> # Program types (enum)
>
> - ref = bpf_info.get_prog_types()
> -
> prog_info = ProgFileExtractor()
Nit: Let's remove "# Program types (enum)" and move "prog_info = ..."
under "# Attach types"?
> - prog_types = set(prog_info.get_prog_types().keys())
> -
> - verify(ref, prog_types,
> - f'Comparing BPF header (enum bpf_prog_type) and {ProgFileExtractor.filename} (prog_type_name):')
> -
> - # Attach types (enum)
> -
> - ref = bpf_info.get_attach_types()
> - bpf_info.close()
> -
> - common_info = CommonFileExtractor()
> - attach_types = common_info.get_attach_types()
> -
> - verify(ref, attach_types,
> - f'Comparing BPF header (enum bpf_attach_type) and {CommonFileExtractor.filename} (attach_type_name):')
> + prog_types = bpf_info.get_prog_types()
It looks like prog_types is unused? I suspect the intention was to
compare with the program types that bpftool supports in prog.c. Looking
at this script, it seems there is no such check currently, which is
likely an ommission on my side. We should add it eventually, but given
that this is beyond the scope of this PR, let's remove "prog_types" for now?
>
> # Attach types (names)
>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH bpf-next v3 09/12] bpftool: Use libbpf_bpf_attach_type_str
2022-05-23 11:48 ` Quentin Monnet
@ 2022-05-23 18:05 ` Andrii Nakryiko
2022-05-23 20:17 ` Daniel Müller
2022-05-23 20:14 ` Daniel Müller
1 sibling, 1 reply; 20+ messages in thread
From: Andrii Nakryiko @ 2022-05-23 18:05 UTC (permalink / raw)
To: Quentin Monnet
Cc: Daniel Müller, bpf, Alexei Starovoitov, Andrii Nakryiko,
Daniel Borkmann, Kernel Team, Yonghong Song
On Mon, May 23, 2022 at 4:48 AM Quentin Monnet <quentin@isovalent.com> wrote:
>
> 2022-05-19 21:29 UTC+0000 ~ Daniel Müller <deso@posteo.net>
> > 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.
> > We decided to fix up this deficiency with this change, meaning that
> > bpftool uses the same textual representations as libbpf. Supporting
> > test, completion scripts, and man pages have been adjusted accordingly.
> > However, we did add support for accepting (the now undocumented)
> > original attach type names when they are provided by users.
> >
> > For the test (test_bpftool_synctypes.py), I have removed the enum
> > representation checks, because we no longer mirror the various enum
> > variant names in bpftool source code. For the man page, help text, and
> > completion script checks we are now using enum definitions from
> > uapi/linux/bpf.h as the source of truth directly.
> >
> > Signed-off-by: Daniel Müller <deso@posteo.net>
> > ---
> > .../bpftool/Documentation/bpftool-cgroup.rst | 16 +-
> > .../bpftool/Documentation/bpftool-prog.rst | 5 +-
> > tools/bpf/bpftool/bash-completion/bpftool | 18 +-
> > tools/bpf/bpftool/cgroup.c | 49 ++++--
> > tools/bpf/bpftool/common.c | 82 ++++-----
> > tools/bpf/bpftool/link.c | 15 +-
> > tools/bpf/bpftool/main.h | 17 ++
> > tools/bpf/bpftool/prog.c | 26 ++-
> > .../selftests/bpf/test_bpftool_synctypes.py | 163 ++++++++----------
> > 9 files changed, 213 insertions(+), 178 deletions(-)
[...]
> > 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 = libbpf_bpf_attach_type_str(type);
> > + if (!attach_type_str)
> > + break;
> > + if (is_prefix(str, attach_type_str))
>
> With so many shared prefixes here, I'm wondering if it would make more
> sense to compare the whole string instead? Otherwise it's hard to guess
> which type “bpftool c a <cgroup> cgroup_ <prog>” will use. At the same
> time we allow prefixing arguments everywhere else, so maybe not worth
> changing it here. Or we could maybe error out if the string length is <=
> strlen("cgroup_")? Let's see for a follow-up maybe.
>
Let's make string match exact for new strings and keep is_prefix()
logic for legacy values? It's better to split this loop into two then,
one over new-style strings and then over legacy strings.
> > + return type;
> > +
> > + /* Also check traditionally used attach type strings. */
> > + attach_type_str = bpf_attach_type_input_str(type);
> > + if (!attach_type_str)
> > + continue;
> > + if (is_prefix(str, attach_type_str))
> > return type;
> > }
> >
>
[...]
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH bpf-next v3 09/12] bpftool: Use libbpf_bpf_attach_type_str
2022-05-23 18:05 ` Andrii Nakryiko
@ 2022-05-23 20:17 ` Daniel Müller
0 siblings, 0 replies; 20+ messages in thread
From: Daniel Müller @ 2022-05-23 20:17 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Quentin Monnet, bpf, Alexei Starovoitov, Andrii Nakryiko,
Daniel Borkmann, Kernel Team, Yonghong Song
On Mon, May 23, 2022 at 11:05:08AM -0700, Andrii Nakryiko wrote:
> On Mon, May 23, 2022 at 4:48 AM Quentin Monnet <quentin@isovalent.com> wrote:
> >
> > 2022-05-19 21:29 UTC+0000 ~ Daniel Müller <deso@posteo.net>
> > > 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.
> > > We decided to fix up this deficiency with this change, meaning that
> > > bpftool uses the same textual representations as libbpf. Supporting
> > > test, completion scripts, and man pages have been adjusted accordingly.
> > > However, we did add support for accepting (the now undocumented)
> > > original attach type names when they are provided by users.
> > >
> > > For the test (test_bpftool_synctypes.py), I have removed the enum
> > > representation checks, because we no longer mirror the various enum
> > > variant names in bpftool source code. For the man page, help text, and
> > > completion script checks we are now using enum definitions from
> > > uapi/linux/bpf.h as the source of truth directly.
> > >
> > > Signed-off-by: Daniel Müller <deso@posteo.net>
> > > ---
> > > .../bpftool/Documentation/bpftool-cgroup.rst | 16 +-
> > > .../bpftool/Documentation/bpftool-prog.rst | 5 +-
> > > tools/bpf/bpftool/bash-completion/bpftool | 18 +-
> > > tools/bpf/bpftool/cgroup.c | 49 ++++--
> > > tools/bpf/bpftool/common.c | 82 ++++-----
> > > tools/bpf/bpftool/link.c | 15 +-
> > > tools/bpf/bpftool/main.h | 17 ++
> > > tools/bpf/bpftool/prog.c | 26 ++-
> > > .../selftests/bpf/test_bpftool_synctypes.py | 163 ++++++++----------
> > > 9 files changed, 213 insertions(+), 178 deletions(-)
>
> [...]
>
> > > 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 = libbpf_bpf_attach_type_str(type);
> > > + if (!attach_type_str)
> > > + break;
> > > + if (is_prefix(str, attach_type_str))
> >
> > With so many shared prefixes here, I'm wondering if it would make more
> > sense to compare the whole string instead? Otherwise it's hard to guess
> > which type “bpftool c a <cgroup> cgroup_ <prog>” will use. At the same
> > time we allow prefixing arguments everywhere else, so maybe not worth
> > changing it here. Or we could maybe error out if the string length is <=
> > strlen("cgroup_")? Let's see for a follow-up maybe.
> >
>
> Let's make string match exact for new strings and keep is_prefix()
> logic for legacy values? It's better to split this loop into two then,
> one over new-style strings and then over legacy strings.
Okay, let's do that then.
Thanks,
Daniel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH bpf-next v3 09/12] bpftool: Use libbpf_bpf_attach_type_str
2022-05-23 11:48 ` Quentin Monnet
2022-05-23 18:05 ` Andrii Nakryiko
@ 2022-05-23 20:14 ` Daniel Müller
1 sibling, 0 replies; 20+ messages in thread
From: Daniel Müller @ 2022-05-23 20:14 UTC (permalink / raw)
To: Quentin Monnet; +Cc: bpf, ast, andrii, daniel, kernel-team, yhs
On Mon, May 23, 2022 at 12:48:09PM +0100, Quentin Monnet wrote:
> 2022-05-19 21:29 UTC+0000 ~ Daniel Müller <deso@posteo.net>
> > 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.
> > We decided to fix up this deficiency with this change, meaning that
> > bpftool uses the same textual representations as libbpf. Supporting
> > test, completion scripts, and man pages have been adjusted accordingly.
> > However, we did add support for accepting (the now undocumented)
> > original attach type names when they are provided by users.
> >
> > For the test (test_bpftool_synctypes.py), I have removed the enum
> > representation checks, because we no longer mirror the various enum
> > variant names in bpftool source code. For the man page, help text, and
> > completion script checks we are now using enum definitions from
> > uapi/linux/bpf.h as the source of truth directly.
> >
> > Signed-off-by: Daniel Müller <deso@posteo.net>
> > ---
> > .../bpftool/Documentation/bpftool-cgroup.rst | 16 +-
> > .../bpftool/Documentation/bpftool-prog.rst | 5 +-
> > tools/bpf/bpftool/bash-completion/bpftool | 18 +-
> > tools/bpf/bpftool/cgroup.c | 49 ++++--
> > tools/bpf/bpftool/common.c | 82 ++++-----
> > tools/bpf/bpftool/link.c | 15 +-
> > tools/bpf/bpftool/main.h | 17 ++
> > tools/bpf/bpftool/prog.c | 26 ++-
> > .../selftests/bpf/test_bpftool_synctypes.py | 163 ++++++++----------
> > 9 files changed, 213 insertions(+), 178 deletions(-)
>
> > diff --git a/tools/bpf/bpftool/cgroup.c b/tools/bpf/bpftool/cgroup.c
> > index effe136..c111a5 100644
> > --- a/tools/bpf/bpftool/cgroup.c
> > +++ b/tools/bpf/bpftool/cgroup.c
> > @@ -21,25 +21,39 @@
> > #define HELP_SPEC_ATTACH_FLAGS \
> > "ATTACH_FLAGS := { multi | override }"
> >
> > -#define HELP_SPEC_ATTACH_TYPES \
> > - " ATTACH_TYPE := { ingress | egress | sock_create |\n" \
> > - " sock_ops | device | bind4 | bind6 |\n" \
> > - " post_bind4 | post_bind6 | connect4 |\n" \
> > - " connect6 | getpeername4 | getpeername6 |\n" \
> > - " getsockname4 | getsockname6 | sendmsg4 |\n" \
> > - " sendmsg6 | recvmsg4 | recvmsg6 |\n" \
> > - " sysctl | getsockopt | setsockopt |\n" \
> > - " sock_release }"
> > +#define HELP_SPEC_ATTACH_TYPES \
> > + " ATTACH_TYPE := { cgroup_inet_ingress | cgroup_inet_egress |\n" \
> > + " cgroup_inet_sock_create | cgroup_sock_ops |\n" \
> > + " cgroup_device | cgroup_inet4_bind |\n" \
> > + " cgroup_inet6_bind | cgroup_inet4_post_bind |\n" \
> > + " cgroup_inet6_post_bind | cgroup_inet4_connect |\n" \
> > + " cgroup_inet6_connect | cgroup_inet4_getpeername |\n" \
> > + " cgroup_inet6_getpeername | cgroup_inet4_getsockname |\n" \
> > + " cgroup_inet6_getsockname | cgroup_udp4_sendmsg |\n" \
> > + " cgroup_udp6_sendmsg | cgroup_udp4_recvmsg |\n" \
> > + " cgroup_udp6_recvmsg | cgroup_sysctl |\n" \
> > + " cgroup_getsockopt | cgroup_setsockopt |\n" \
> > + " cgroup_inet_sock_release }"
> >
> > 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 = libbpf_bpf_attach_type_str(type);
> > + if (!attach_type_str)
> > + break;
> > + if (is_prefix(str, attach_type_str))
>
> With so many shared prefixes here, I'm wondering if it would make more
> sense to compare the whole string instead? Otherwise it's hard to guess
> which type “bpftool c a <cgroup> cgroup_ <prog>” will use. At the same
> time we allow prefixing arguments everywhere else, so maybe not worth
> changing it here. Or we could maybe error out if the string length is <=
> strlen("cgroup_")? Let's see for a follow-up maybe.
It is true that it could get confusing, but I'd think it's mostly for write-once
cases where this functionality is used. And there I really see value in
supporting prefixes. I also agree that it's what we do elsewhere. What I think
we should consider fixing, though, is just doing short-circuited first-matches
check. In my opinion we should error out if there are multiple matches instead.
After all, what is first depends on numeric values and these are not really
obvious to the user, I think (and certainly nothing I would want to be bothered
with at this point).
As that is an existing problem, I'd suggest we leave the existing behavior until
we address that.
> > + return type;
> > +
> > + /* Also check traditionally used attach type strings. */
> > + attach_type_str = bpf_attach_type_input_str(type);
> > + if (!attach_type_str)
> > + continue;
> > + if (is_prefix(str, attach_type_str))
> > return type;
> > }
> >
>
> > diff --git a/tools/testing/selftests/bpf/test_bpftool_synctypes.py b/tools/testing/selftests/bpf/test_bpftool_synctypes.py
> > index c0e7acd..0ca3c1 100755
> > --- a/tools/testing/selftests/bpf/test_bpftool_synctypes.py
> > +++ b/tools/testing/selftests/bpf/test_bpftool_synctypes.py
>
> > @@ -139,21 +139,20 @@ class FileExtractor(object):
> >
> > def get_types_from_array(self, array_name):
> > """
> > - Search for and parse an array associating names to BPF_* enum members,
> > - for example:
> > + Search for and parse an array white-listing BPF_* enum members, for
>
> The coding style now recommends against the “white-listing”. Maybe
> “[...] a list of allowed BPF_* enum members”?
Ah, good catch, thanks. Will fix it.
[...]
> > @@ -525,34 +521,18 @@ def main():
> > bashcomp_map_types = bashcomp_info.get_map_types()
> >
> > verify(source_map_types, help_map_types,
> > - f'Comparing {MapFileExtractor.filename} (map_type_name) and {MapFileExtractor.filename} (do_help() TYPE):')
> > + f'Comparing {BpfHeaderExtractor.filename} (bpf_map_type) and {MapFileExtractor.filename} (do_help() TYPE):')
> > verify(source_map_types, man_map_types,
> > - f'Comparing {MapFileExtractor.filename} (map_type_name) and {ManMapExtractor.filename} (TYPE):')
> > + f'Comparing {BpfHeaderExtractor.filename} (bpf_map_type) and {ManMapExtractor.filename} (TYPE):')
> > verify(help_map_options, man_map_options,
> > f'Comparing {MapFileExtractor.filename} (do_help() OPTIONS) and {ManMapExtractor.filename} (OPTIONS):')
> > verify(source_map_types, bashcomp_map_types,
> > - f'Comparing {MapFileExtractor.filename} (map_type_name) and {BashcompExtractor.filename} (BPFTOOL_MAP_CREATE_TYPES):')
> > + f'Comparing {BpfHeaderExtractor.filename} (bpf_map_type) and {BashcompExtractor.filename} (BPFTOOL_MAP_CREATE_TYPES):')
> >
> > # Program types (enum)
> >
> > - ref = bpf_info.get_prog_types()
> > -
> > prog_info = ProgFileExtractor()
>
> Nit: Let's remove "# Program types (enum)" and move "prog_info = ..."
> under "# Attach types"?
Sounds good. Done.
> > - prog_types = set(prog_info.get_prog_types().keys())
> > -
> > - verify(ref, prog_types,
> > - f'Comparing BPF header (enum bpf_prog_type) and {ProgFileExtractor.filename} (prog_type_name):')
> > -
> > - # Attach types (enum)
> > -
> > - ref = bpf_info.get_attach_types()
> > - bpf_info.close()
> > -
> > - common_info = CommonFileExtractor()
> > - attach_types = common_info.get_attach_types()
> > -
> > - verify(ref, attach_types,
> > - f'Comparing BPF header (enum bpf_attach_type) and {CommonFileExtractor.filename} (attach_type_name):')
> > + prog_types = bpf_info.get_prog_types()
>
> It looks like prog_types is unused? I suspect the intention was to
> compare with the program types that bpftool supports in prog.c. Looking
> at this script, it seems there is no such check currently, which is
> likely an ommission on my side. We should add it eventually, but given
> that this is beyond the scope of this PR, let's remove "prog_types" for now?
Yep, unsure how I missed it. Thanks for spotting.
Thanks for the review.
Daniel
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH bpf-next v3 10/12] libbpf: Introduce libbpf_bpf_link_type_str
2022-05-19 21:29 [PATCH bpf-next v3 00/12] libbpf: Textual representation of enums Daniel Müller
` (8 preceding siblings ...)
2022-05-19 21:29 ` [PATCH bpf-next v3 09/12] bpftool: Use libbpf_bpf_attach_type_str Daniel Müller
@ 2022-05-19 21:29 ` Daniel Müller
2022-05-19 21:30 ` [PATCH bpf-next v3 11/12] selftests/bpf: Add test for libbpf_bpf_link_type_str Daniel Müller
` (2 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: Daniel Müller @ 2022-05-19 21:29 UTC (permalink / raw)
To: bpf, ast, andrii, daniel, kernel-team; +Cc: yhs, 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 bc56e9c..74f375 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 44c536..38e284 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -463,6 +463,7 @@ LIBBPF_0.8.0 {
LIBBPF_1.0.0 {
global:
libbpf_bpf_attach_type_str;
+ libbpf_bpf_link_type_str;
libbpf_bpf_map_type_str;
libbpf_bpf_prog_type_str;
--
2.30.2
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH bpf-next v3 11/12] selftests/bpf: Add test for libbpf_bpf_link_type_str
2022-05-19 21:29 [PATCH bpf-next v3 00/12] libbpf: Textual representation of enums Daniel Müller
` (9 preceding siblings ...)
2022-05-19 21:29 ` [PATCH bpf-next v3 10/12] libbpf: Introduce libbpf_bpf_link_type_str Daniel Müller
@ 2022-05-19 21:30 ` Daniel Müller
2022-05-19 21:30 ` [PATCH bpf-next v3 12/12] bpftool: Use libbpf_bpf_link_type_str Daniel Müller
2022-05-20 23:45 ` [PATCH bpf-next v3 00/12] libbpf: Textual representation of enums Andrii Nakryiko
12 siblings, 0 replies; 20+ messages in thread
From: Daniel Müller @ 2022-05-19 21:30 UTC (permalink / raw)
To: bpf, ast, andrii, daniel, kernel-team; +Cc: yhs, 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] 20+ messages in thread* [PATCH bpf-next v3 12/12] bpftool: Use libbpf_bpf_link_type_str
2022-05-19 21:29 [PATCH bpf-next v3 00/12] libbpf: Textual representation of enums Daniel Müller
` (10 preceding siblings ...)
2022-05-19 21:30 ` [PATCH bpf-next v3 11/12] selftests/bpf: Add test for libbpf_bpf_link_type_str Daniel Müller
@ 2022-05-19 21:30 ` Daniel Müller
2022-05-20 23:45 ` [PATCH bpf-next v3 00/12] libbpf: Textual representation of enums Andrii Nakryiko
12 siblings, 0 replies; 20+ messages in thread
From: Daniel Müller @ 2022-05-19 21:30 UTC (permalink / raw)
To: bpf, ast, andrii, daniel, kernel-team; +Cc: yhs, 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 66a254..7a2093 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] 20+ messages in thread* Re: [PATCH bpf-next v3 00/12] libbpf: Textual representation of enums
2022-05-19 21:29 [PATCH bpf-next v3 00/12] libbpf: Textual representation of enums Daniel Müller
` (11 preceding siblings ...)
2022-05-19 21:30 ` [PATCH bpf-next v3 12/12] bpftool: Use libbpf_bpf_link_type_str Daniel Müller
@ 2022-05-20 23:45 ` Andrii Nakryiko
2022-05-23 11:48 ` Quentin Monnet
2022-05-23 20:59 ` Daniel Müller
12 siblings, 2 replies; 20+ messages in thread
From: Andrii Nakryiko @ 2022-05-20 23:45 UTC (permalink / raw)
To: Daniel Müller
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Kernel Team, Yonghong Song, Quentin Monnet
On Thu, May 19, 2022 at 2:30 PM 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 decided changing its behavior to work with libbpf
> representations. However, for user inputs, specifically, we do keep support for
> the traditionally used names around (please see patch "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
>
> Changelog:
> v2 -> v3:
> - use LIBBPF_1.0.0 section in libbpf.map for newly added exports
>
> v1 -> v2:
> - adjusted bpftool to work with algorithmically determined attach types as
> libbpf now uses (just removed prefix from enum name and lowercased the rest)
> - adjusted tests, man page, and completion script to work with the new names
> - renamed bpf_attach_type_str -> bpf_attach_type_input_str
> - for input: added special cases that accept the traditionally used strings as
> well
> - changed 'char const *' -> 'const char *'
>
> Signed-off-by: Daniel Müller <deso@posteo.net>
> Acked-by: Yonghong Song <yhs@fb.com>
> Cc: Quentin Monnet <quentin@isovalent.com>
>
So this looks good to me for libbpf and selftests/bpf changes. I'll
wait for Quentin to give his acks at least for bpftool changes.
Quention, please take a look when you get a chance.
Few small nits, please accommodate them in next version, if you happen
to send another one. If not, I'll try to remember to fix it up when
applying.
1. Received Acked-by/Reviewed-by tags should be added to each
individual patch, not cover letter.
2. You are using /** ... */ comments, which are considered to be kdoc
comments and they have some additional formatting, which some of the
tooling run on patches in Patchworks complains about [0]. Please use
just /* ... */ style everywhere where it's not actual kdoc (or libbpf
API documentation).
[0] https://patchwork.hopto.org/static/nipa/643335/12856068/kdoc/summary
> 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
>
> .../bpftool/Documentation/bpftool-cgroup.rst | 16 +-
> .../bpftool/Documentation/bpftool-prog.rst | 5 +-
> tools/bpf/bpftool/bash-completion/bpftool | 18 +-
> tools/bpf/bpftool/cgroup.c | 49 +++--
> tools/bpf/bpftool/common.c | 82 +++----
> tools/bpf/bpftool/feature.c | 87 +++++---
> tools/bpf/bpftool/link.c | 61 +++---
> tools/bpf/bpftool/main.h | 23 +-
> tools/bpf/bpftool/map.c | 82 +++----
> tools/bpf/bpftool/prog.c | 77 +++----
> tools/lib/bpf/libbpf.c | 160 ++++++++++++++
> tools/lib/bpf/libbpf.h | 36 +++
> tools/lib/bpf/libbpf.map | 6 +
> .../selftests/bpf/prog_tests/libbpf_str.c | 207 ++++++++++++++++++
> .../selftests/bpf/test_bpftool_synctypes.py | 163 ++++++--------
> 15 files changed, 738 insertions(+), 334 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/libbpf_str.c
>
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH bpf-next v3 00/12] libbpf: Textual representation of enums
2022-05-20 23:45 ` [PATCH bpf-next v3 00/12] libbpf: Textual representation of enums Andrii Nakryiko
@ 2022-05-23 11:48 ` Quentin Monnet
2022-05-23 20:59 ` Daniel Müller
1 sibling, 0 replies; 20+ messages in thread
From: Quentin Monnet @ 2022-05-23 11:48 UTC (permalink / raw)
To: Andrii Nakryiko, Daniel Müller
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Kernel Team, Yonghong Song
2022-05-20 16:45 UTC-0700 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com>
> On Thu, May 19, 2022 at 2:30 PM 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 decided changing its behavior to work with libbpf
>> representations. However, for user inputs, specifically, we do keep support for
>> the traditionally used names around (please see patch "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
>>
>> Changelog:
>> v2 -> v3:
>> - use LIBBPF_1.0.0 section in libbpf.map for newly added exports
>>
>> v1 -> v2:
>> - adjusted bpftool to work with algorithmically determined attach types as
>> libbpf now uses (just removed prefix from enum name and lowercased the rest)
>> - adjusted tests, man page, and completion script to work with the new names
>> - renamed bpf_attach_type_str -> bpf_attach_type_input_str
>> - for input: added special cases that accept the traditionally used strings as
>> well
>> - changed 'char const *' -> 'const char *'
>>
>> Signed-off-by: Daniel Müller <deso@posteo.net>
>> Acked-by: Yonghong Song <yhs@fb.com>
>> Cc: Quentin Monnet <quentin@isovalent.com>
>>
>
> So this looks good to me for libbpf and selftests/bpf changes. I'll
> wait for Quentin to give his acks at least for bpftool changes.
> Quention, please take a look when you get a chance.
>
> Few small nits, please accommodate them in next version, if you happen
> to send another one. If not, I'll try to remember to fix it up when
> applying.
>
> 1. Received Acked-by/Reviewed-by tags should be added to each
> individual patch, not cover letter.
>
> 2. You are using /** ... */ comments, which are considered to be kdoc
> comments and they have some additional formatting, which some of the
> tooling run on patches in Patchworks complains about [0]. Please use
> just /* ... */ style everywhere where it's not actual kdoc (or libbpf
> API documentation).
>
> [0] https://patchwork.hopto.org/static/nipa/643335/12856068/kdoc/summary
>
Apologies, didn't have time to go through the Python changes last week.
This looks all good for me as well! With a few additional nitpicks on
patch 9, see my reply for that one. For all other patches, please feel
free to add:
Acked-by: Quentin Monnet <quentin@isovalent.com>
Nice work, thanks a lot Daniel for diving into the Python script!
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH bpf-next v3 00/12] libbpf: Textual representation of enums
2022-05-20 23:45 ` [PATCH bpf-next v3 00/12] libbpf: Textual representation of enums Andrii Nakryiko
2022-05-23 11:48 ` Quentin Monnet
@ 2022-05-23 20:59 ` Daniel Müller
1 sibling, 0 replies; 20+ messages in thread
From: Daniel Müller @ 2022-05-23 20:59 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Kernel Team, Yonghong Song, Quentin Monnet
On Fri, May 20, 2022 at 04:45:42PM -0700, Andrii Nakryiko wrote:
> On Thu, May 19, 2022 at 2:30 PM 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 decided changing its behavior to work with libbpf
> > representations. However, for user inputs, specifically, we do keep support for
> > the traditionally used names around (please see patch "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
> >
> > Changelog:
> > v2 -> v3:
> > - use LIBBPF_1.0.0 section in libbpf.map for newly added exports
> >
> > v1 -> v2:
> > - adjusted bpftool to work with algorithmically determined attach types as
> > libbpf now uses (just removed prefix from enum name and lowercased the rest)
> > - adjusted tests, man page, and completion script to work with the new names
> > - renamed bpf_attach_type_str -> bpf_attach_type_input_str
> > - for input: added special cases that accept the traditionally used strings as
> > well
> > - changed 'char const *' -> 'const char *'
> >
> > Signed-off-by: Daniel Müller <deso@posteo.net>
> > Acked-by: Yonghong Song <yhs@fb.com>
> > Cc: Quentin Monnet <quentin@isovalent.com>
> >
>
> So this looks good to me for libbpf and selftests/bpf changes. I'll
> wait for Quentin to give his acks at least for bpftool changes.
> Quention, please take a look when you get a chance.
>
> Few small nits, please accommodate them in next version, if you happen
> to send another one. If not, I'll try to remember to fix it up when
> applying.
>
> 1. Received Acked-by/Reviewed-by tags should be added to each
> individual patch, not cover letter.
Thanks for pointing that out. Will fix that up.
> 2. You are using /** ... */ comments, which are considered to be kdoc
> comments and they have some additional formatting, which some of the
> tooling run on patches in Patchworks complains about [0]. Please use
> just /* ... */ style everywhere where it's not actual kdoc (or libbpf
> API documentation).
Force of habit. Fixed.
Thanks for taking a look!
Daniel
[...]
^ permalink raw reply [flat|nested] 20+ messages in thread