* [PATCH bpf-next v2 1/7] tools/bpf: selftests : explain bpf_iter test failures with llvm 10.0.0
2020-05-13 18:02 [PATCH bpf-next v2 0/7] bpf: misc fixes for bpf_iter Yonghong Song
@ 2020-05-13 18:02 ` Yonghong Song
2020-05-13 18:02 ` [PATCH bpf-next v2 2/7] bpf: change btf_iter func proto prefix to "bpf_iter_" Yonghong Song
` (7 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Yonghong Song @ 2020-05-13 18:02 UTC (permalink / raw)
To: Andrii Nakryiko, bpf, Martin KaFai Lau, netdev
Cc: Alexei Starovoitov, Daniel Borkmann, kernel-team, Alexei Starovoitov
Commit 6879c042e105 ("tools/bpf: selftests: Add bpf_iter selftests")
added self tests for bpf_iter feature. But two subtests
ipv6_route and netlink needs llvm latest 10.x release branch
or trunk due to a bug in llvm BPF backend. This patch added
the file README.rst to document these two failures
so people using llvm 10.0.0 can be aware of them.
Suggested-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Yonghong Song <yhs@fb.com>
---
tools/testing/selftests/bpf/README.rst | 43 ++++++++++++++++++++++++++
1 file changed, 43 insertions(+)
create mode 100644 tools/testing/selftests/bpf/README.rst
diff --git a/tools/testing/selftests/bpf/README.rst b/tools/testing/selftests/bpf/README.rst
new file mode 100644
index 000000000000..0f67f1b470b0
--- /dev/null
+++ b/tools/testing/selftests/bpf/README.rst
@@ -0,0 +1,43 @@
+==================
+BPF Selftest Notes
+==================
+
+Additional information about selftest failures are
+documented here.
+
+bpf_iter test failures with clang/llvm 10.0.0
+=============================================
+
+With clang/llvm 10.0.0, the following two bpf_iter tests failed:
+ * ``bpf_iter/ipv6_route``
+ * ``bpf_iter/netlink``
+
+The symptom for ``bpf_iter/ipv6_route`` looks like
+
+.. code-block:: c
+
+ 2: (79) r8 = *(u64 *)(r1 +8)
+ ...
+ 14: (bf) r2 = r8
+ 15: (0f) r2 += r1
+ ; BPF_SEQ_PRINTF(seq, "%pi6 %02x ", &rt->fib6_dst.addr, rt->fib6_dst.plen);
+ 16: (7b) *(u64 *)(r8 +64) = r2
+ only read is supported
+
+The symptom for ``bpf_iter/netlink`` looks like
+
+.. code-block:: c
+
+ ; struct netlink_sock *nlk = ctx->sk;
+ 2: (79) r7 = *(u64 *)(r1 +8)
+ ...
+ 15: (bf) r2 = r7
+ 16: (0f) r2 += r1
+ ; BPF_SEQ_PRINTF(seq, "%pK %-3d ", s, s->sk_protocol);
+ 17: (7b) *(u64 *)(r7 +0) = r2
+ only read is supported
+
+This is due to a llvm BPF backend bug. The fix
+ https://reviews.llvm.org/D78466
+has been pushed to llvm 10.x release branch and will be
+available in 10.0.1. The fix is available in llvm 11.0.0 trunk.
--
2.24.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH bpf-next v2 2/7] bpf: change btf_iter func proto prefix to "bpf_iter_"
2020-05-13 18:02 [PATCH bpf-next v2 0/7] bpf: misc fixes for bpf_iter Yonghong Song
2020-05-13 18:02 ` [PATCH bpf-next v2 1/7] tools/bpf: selftests : explain bpf_iter test failures with llvm 10.0.0 Yonghong Song
@ 2020-05-13 18:02 ` Yonghong Song
2020-05-13 18:02 ` [PATCH bpf-next v2 3/7] bpf: add comments to interpret bpf_prog return values Yonghong Song
` (6 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Yonghong Song @ 2020-05-13 18:02 UTC (permalink / raw)
To: Andrii Nakryiko, bpf, Martin KaFai Lau, netdev
Cc: Alexei Starovoitov, Daniel Borkmann, kernel-team, Alexei Starovoitov
This is to be consistent with tracing and lsm programs
which have prefix "bpf_trace_" and "bpf_lsm_" respectively.
Suggested-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Andrii Nakryiko <andriin@fb.com>
Signed-off-by: Yonghong Song <yhs@fb.com>
---
include/linux/bpf.h | 6 +++---
tools/lib/bpf/libbpf.c | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index cf4b6e44f2bc..ab94dfd8826f 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1131,10 +1131,10 @@ struct bpf_link *bpf_link_get_from_fd(u32 ufd);
int bpf_obj_pin_user(u32 ufd, const char __user *pathname);
int bpf_obj_get_user(const char __user *pathname, int flags);
-#define BPF_ITER_FUNC_PREFIX "__bpf_iter__"
+#define BPF_ITER_FUNC_PREFIX "bpf_iter_"
#define DEFINE_BPF_ITER_FUNC(target, args...) \
- extern int __bpf_iter__ ## target(args); \
- int __init __bpf_iter__ ## target(args) { return 0; }
+ extern int bpf_iter_ ## target(args); \
+ int __init bpf_iter_ ## target(args) { return 0; }
typedef int (*bpf_iter_init_seq_priv_t)(void *private_data);
typedef void (*bpf_iter_fini_seq_priv_t)(void *private_data);
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index fd882616ab52..292257995487 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -6919,7 +6919,7 @@ static int bpf_object__collect_st_ops_relos(struct bpf_object *obj,
#define BTF_TRACE_PREFIX "btf_trace_"
#define BTF_LSM_PREFIX "bpf_lsm_"
-#define BTF_ITER_PREFIX "__bpf_iter__"
+#define BTF_ITER_PREFIX "bpf_iter_"
#define BTF_MAX_NAME_SIZE 128
static int find_btf_by_prefix_kind(const struct btf *btf, const char *prefix,
--
2.24.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH bpf-next v2 3/7] bpf: add comments to interpret bpf_prog return values
2020-05-13 18:02 [PATCH bpf-next v2 0/7] bpf: misc fixes for bpf_iter Yonghong Song
2020-05-13 18:02 ` [PATCH bpf-next v2 1/7] tools/bpf: selftests : explain bpf_iter test failures with llvm 10.0.0 Yonghong Song
2020-05-13 18:02 ` [PATCH bpf-next v2 2/7] bpf: change btf_iter func proto prefix to "bpf_iter_" Yonghong Song
@ 2020-05-13 18:02 ` Yonghong Song
2020-05-13 18:02 ` [PATCH bpf-next v2 4/7] bpf: net: refactor bpf_iter target registration Yonghong Song
` (5 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Yonghong Song @ 2020-05-13 18:02 UTC (permalink / raw)
To: Andrii Nakryiko, bpf, Martin KaFai Lau, netdev
Cc: Alexei Starovoitov, Daniel Borkmann, kernel-team
Add a short comment in bpf_iter_run_prog() function to
explain how bpf_prog return value is converted to
seq_ops->show() return value:
bpf_prog return seq_ops()->show() return
0 0
1 -EAGAIN
When show() return value is -EAGAIN, the current
bpf_seq_read() will end. If the current seq_file buffer
is empty, -EAGAIN will return to user space. Otherwise,
the buffer will be copied to user space.
In both cases, the next bpf_seq_read() call will
try to show the same object which returned -EAGAIN
previously.
Acked-by: Andrii Nakryiko <andriin@fb.com>
Signed-off-by: Yonghong Song <yhs@fb.com>
---
kernel/bpf/bpf_iter.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/kernel/bpf/bpf_iter.c b/kernel/bpf/bpf_iter.c
index 30efd15cd4a0..0a45a6cdfabd 100644
--- a/kernel/bpf/bpf_iter.c
+++ b/kernel/bpf/bpf_iter.c
@@ -526,5 +526,11 @@ int bpf_iter_run_prog(struct bpf_prog *prog, void *ctx)
migrate_enable();
rcu_read_unlock();
+ /* bpf program can only return 0 or 1:
+ * 0 : okay
+ * 1 : retry the same object
+ * The bpf_iter_run_prog() return value
+ * will be seq_ops->show() return value.
+ */
return ret == 0 ? 0 : -EAGAIN;
}
--
2.24.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH bpf-next v2 4/7] bpf: net: refactor bpf_iter target registration
2020-05-13 18:02 [PATCH bpf-next v2 0/7] bpf: misc fixes for bpf_iter Yonghong Song
` (2 preceding siblings ...)
2020-05-13 18:02 ` [PATCH bpf-next v2 3/7] bpf: add comments to interpret bpf_prog return values Yonghong Song
@ 2020-05-13 18:02 ` Yonghong Song
2020-05-13 18:02 ` [PATCH bpf-next v2 5/7] bpf: change func bpf_iter_unreg_target() signature Yonghong Song
` (4 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Yonghong Song @ 2020-05-13 18:02 UTC (permalink / raw)
To: Andrii Nakryiko, bpf, Martin KaFai Lau, netdev
Cc: Alexei Starovoitov, Daniel Borkmann, kernel-team
Currently bpf_iter_reg_target takes parameters from target
and allocates memory to save them. This is really not
necessary, esp. in the future we may grow information
passed from targets to bpf_iter manager.
The patch refactors the code so target reg_info
becomes static and bpf_iter manager can just take
a reference to it.
Signed-off-by: Yonghong Song <yhs@fb.com>
---
include/linux/bpf.h | 2 +-
kernel/bpf/bpf_iter.c | 36 +++++++++++++++++-------------------
kernel/bpf/map_iter.c | 18 +++++++++---------
kernel/bpf/task_iter.c | 30 ++++++++++++++++--------------
net/ipv6/route.c | 18 +++++++++---------
net/netlink/af_netlink.c | 18 +++++++++---------
6 files changed, 61 insertions(+), 61 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index ab94dfd8826f..6fa773e2d1bf 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1153,7 +1153,7 @@ struct bpf_iter_meta {
u64 seq_num;
};
-int bpf_iter_reg_target(struct bpf_iter_reg *reg_info);
+int bpf_iter_reg_target(const struct bpf_iter_reg *reg_info);
void bpf_iter_unreg_target(const char *target);
bool bpf_iter_prog_supported(struct bpf_prog *prog);
int bpf_iter_link_attach(const union bpf_attr *attr, struct bpf_prog *prog);
diff --git a/kernel/bpf/bpf_iter.c b/kernel/bpf/bpf_iter.c
index 0a45a6cdfabd..051fb8cab62a 100644
--- a/kernel/bpf/bpf_iter.c
+++ b/kernel/bpf/bpf_iter.c
@@ -8,11 +8,7 @@
struct bpf_iter_target_info {
struct list_head list;
- const char *target;
- const struct seq_operations *seq_ops;
- bpf_iter_init_seq_priv_t init_seq_private;
- bpf_iter_fini_seq_priv_t fini_seq_private;
- u32 seq_priv_size;
+ const struct bpf_iter_reg *reg_info;
u32 btf_id; /* cached value */
};
@@ -222,8 +218,8 @@ static int iter_release(struct inode *inode, struct file *file)
iter_priv = container_of(seq->private, struct bpf_iter_priv_data,
target_private);
- if (iter_priv->tinfo->fini_seq_private)
- iter_priv->tinfo->fini_seq_private(seq->private);
+ if (iter_priv->tinfo->reg_info->fini_seq_private)
+ iter_priv->tinfo->reg_info->fini_seq_private(seq->private);
bpf_prog_put(iter_priv->prog);
seq->private = iter_priv;
@@ -238,7 +234,12 @@ const struct file_operations bpf_iter_fops = {
.release = iter_release,
};
-int bpf_iter_reg_target(struct bpf_iter_reg *reg_info)
+/* The argument reg_info will be cached in bpf_iter_target_info.
+ * The common practice is to declare target reg_info as
+ * a const static variable and passed as an argument to
+ * bpf_iter_reg_target().
+ */
+int bpf_iter_reg_target(const struct bpf_iter_reg *reg_info)
{
struct bpf_iter_target_info *tinfo;
@@ -246,11 +247,7 @@ int bpf_iter_reg_target(struct bpf_iter_reg *reg_info)
if (!tinfo)
return -ENOMEM;
- tinfo->target = reg_info->target;
- tinfo->seq_ops = reg_info->seq_ops;
- tinfo->init_seq_private = reg_info->init_seq_private;
- tinfo->fini_seq_private = reg_info->fini_seq_private;
- tinfo->seq_priv_size = reg_info->seq_priv_size;
+ tinfo->reg_info = reg_info;
INIT_LIST_HEAD(&tinfo->list);
mutex_lock(&targets_mutex);
@@ -267,7 +264,7 @@ void bpf_iter_unreg_target(const char *target)
mutex_lock(&targets_mutex);
list_for_each_entry(tinfo, &targets, list) {
- if (!strcmp(target, tinfo->target)) {
+ if (!strcmp(target, tinfo->reg_info->target)) {
list_del(&tinfo->list);
kfree(tinfo);
found = true;
@@ -303,7 +300,7 @@ bool bpf_iter_prog_supported(struct bpf_prog *prog)
supported = true;
break;
}
- if (!strcmp(attach_fname + prefix_len, tinfo->target)) {
+ if (!strcmp(attach_fname + prefix_len, tinfo->reg_info->target)) {
cache_btf_id(tinfo, prog);
supported = true;
break;
@@ -431,15 +428,16 @@ static int prepare_seq_file(struct file *file, struct bpf_iter_link *link)
tinfo = link->tinfo;
total_priv_dsize = offsetof(struct bpf_iter_priv_data, target_private) +
- tinfo->seq_priv_size;
- priv_data = __seq_open_private(file, tinfo->seq_ops, total_priv_dsize);
+ tinfo->reg_info->seq_priv_size;
+ priv_data = __seq_open_private(file, tinfo->reg_info->seq_ops,
+ total_priv_dsize);
if (!priv_data) {
err = -ENOMEM;
goto release_prog;
}
- if (tinfo->init_seq_private) {
- err = tinfo->init_seq_private(priv_data->target_private);
+ if (tinfo->reg_info->init_seq_private) {
+ err = tinfo->reg_info->init_seq_private(priv_data->target_private);
if (err)
goto release_seq_file;
}
diff --git a/kernel/bpf/map_iter.c b/kernel/bpf/map_iter.c
index 8162e0c00b9f..c6216a5fe56e 100644
--- a/kernel/bpf/map_iter.c
+++ b/kernel/bpf/map_iter.c
@@ -81,17 +81,17 @@ static const struct seq_operations bpf_map_seq_ops = {
.show = bpf_map_seq_show,
};
+static const struct bpf_iter_reg bpf_map_reg_info = {
+ .target = "bpf_map",
+ .seq_ops = &bpf_map_seq_ops,
+ .init_seq_private = NULL,
+ .fini_seq_private = NULL,
+ .seq_priv_size = sizeof(struct bpf_iter_seq_map_info),
+};
+
static int __init bpf_map_iter_init(void)
{
- struct bpf_iter_reg reg_info = {
- .target = "bpf_map",
- .seq_ops = &bpf_map_seq_ops,
- .init_seq_private = NULL,
- .fini_seq_private = NULL,
- .seq_priv_size = sizeof(struct bpf_iter_seq_map_info),
- };
-
- return bpf_iter_reg_target(®_info);
+ return bpf_iter_reg_target(&bpf_map_reg_info);
}
late_initcall(bpf_map_iter_init);
diff --git a/kernel/bpf/task_iter.c b/kernel/bpf/task_iter.c
index aeed662d8451..bd7bfd83d9e0 100644
--- a/kernel/bpf/task_iter.c
+++ b/kernel/bpf/task_iter.c
@@ -306,22 +306,24 @@ static const struct seq_operations task_file_seq_ops = {
.show = task_file_seq_show,
};
+static const struct bpf_iter_reg task_reg_info = {
+ .target = "task",
+ .seq_ops = &task_seq_ops,
+ .init_seq_private = init_seq_pidns,
+ .fini_seq_private = fini_seq_pidns,
+ .seq_priv_size = sizeof(struct bpf_iter_seq_task_info),
+};
+
+static const struct bpf_iter_reg task_file_reg_info = {
+ .target = "task_file",
+ .seq_ops = &task_file_seq_ops,
+ .init_seq_private = init_seq_pidns,
+ .fini_seq_private = fini_seq_pidns,
+ .seq_priv_size = sizeof(struct bpf_iter_seq_task_file_info),
+};
+
static int __init task_iter_init(void)
{
- struct bpf_iter_reg task_file_reg_info = {
- .target = "task_file",
- .seq_ops = &task_file_seq_ops,
- .init_seq_private = init_seq_pidns,
- .fini_seq_private = fini_seq_pidns,
- .seq_priv_size = sizeof(struct bpf_iter_seq_task_file_info),
- };
- struct bpf_iter_reg task_reg_info = {
- .target = "task",
- .seq_ops = &task_seq_ops,
- .init_seq_private = init_seq_pidns,
- .fini_seq_private = fini_seq_pidns,
- .seq_priv_size = sizeof(struct bpf_iter_seq_task_info),
- };
int ret;
ret = bpf_iter_reg_target(&task_reg_info);
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 25f6d3e619d0..6ad2fa51a23a 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -6397,17 +6397,17 @@ void __init ip6_route_init_special_entries(void)
#if defined(CONFIG_BPF_SYSCALL) && defined(CONFIG_PROC_FS)
DEFINE_BPF_ITER_FUNC(ipv6_route, struct bpf_iter_meta *meta, struct fib6_info *rt)
+static const struct bpf_iter_reg ipv6_route_reg_info = {
+ .target = "ipv6_route",
+ .seq_ops = &ipv6_route_seq_ops,
+ .init_seq_private = bpf_iter_init_seq_net,
+ .fini_seq_private = bpf_iter_fini_seq_net,
+ .seq_priv_size = sizeof(struct ipv6_route_iter),
+};
+
static int __init bpf_iter_register(void)
{
- struct bpf_iter_reg reg_info = {
- .target = "ipv6_route",
- .seq_ops = &ipv6_route_seq_ops,
- .init_seq_private = bpf_iter_init_seq_net,
- .fini_seq_private = bpf_iter_fini_seq_net,
- .seq_priv_size = sizeof(struct ipv6_route_iter),
- };
-
- return bpf_iter_reg_target(®_info);
+ return bpf_iter_reg_target(&ipv6_route_reg_info);
}
static void bpf_iter_unregister(void)
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 33cda9baa979..839827227e98 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -2803,17 +2803,17 @@ static const struct rhashtable_params netlink_rhashtable_params = {
};
#if defined(CONFIG_BPF_SYSCALL) && defined(CONFIG_PROC_FS)
+static const struct bpf_iter_reg netlink_reg_info = {
+ .target = "netlink",
+ .seq_ops = &netlink_seq_ops,
+ .init_seq_private = bpf_iter_init_seq_net,
+ .fini_seq_private = bpf_iter_fini_seq_net,
+ .seq_priv_size = sizeof(struct nl_seq_iter),
+};
+
static int __init bpf_iter_register(void)
{
- struct bpf_iter_reg reg_info = {
- .target = "netlink",
- .seq_ops = &netlink_seq_ops,
- .init_seq_private = bpf_iter_init_seq_net,
- .fini_seq_private = bpf_iter_fini_seq_net,
- .seq_priv_size = sizeof(struct nl_seq_iter),
- };
-
- return bpf_iter_reg_target(®_info);
+ return bpf_iter_reg_target(&netlink_reg_info);
}
#endif
--
2.24.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH bpf-next v2 5/7] bpf: change func bpf_iter_unreg_target() signature
2020-05-13 18:02 [PATCH bpf-next v2 0/7] bpf: misc fixes for bpf_iter Yonghong Song
` (3 preceding siblings ...)
2020-05-13 18:02 ` [PATCH bpf-next v2 4/7] bpf: net: refactor bpf_iter target registration Yonghong Song
@ 2020-05-13 18:02 ` Yonghong Song
2020-05-13 18:02 ` [PATCH bpf-next v2 6/7] bpf: enable bpf_iter targets registering ctx argument types Yonghong Song
` (3 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Yonghong Song @ 2020-05-13 18:02 UTC (permalink / raw)
To: Andrii Nakryiko, bpf, Martin KaFai Lau, netdev
Cc: Alexei Starovoitov, Daniel Borkmann, kernel-team
Change func bpf_iter_unreg_target() parameter from target
name to target reg_info, similar to bpf_iter_reg_target().
Acked-by: Andrii Nakryiko <andriin@fb.com>
Signed-off-by: Yonghong Song <yhs@fb.com>
---
include/linux/bpf.h | 2 +-
kernel/bpf/bpf_iter.c | 4 ++--
net/ipv6/route.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 6fa773e2d1bf..534174eca86b 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1154,7 +1154,7 @@ struct bpf_iter_meta {
};
int bpf_iter_reg_target(const struct bpf_iter_reg *reg_info);
-void bpf_iter_unreg_target(const char *target);
+void bpf_iter_unreg_target(const struct bpf_iter_reg *reg_info);
bool bpf_iter_prog_supported(struct bpf_prog *prog);
int bpf_iter_link_attach(const union bpf_attr *attr, struct bpf_prog *prog);
int bpf_iter_new_fd(struct bpf_link *link);
diff --git a/kernel/bpf/bpf_iter.c b/kernel/bpf/bpf_iter.c
index 051fb8cab62a..644f8626b2c0 100644
--- a/kernel/bpf/bpf_iter.c
+++ b/kernel/bpf/bpf_iter.c
@@ -257,14 +257,14 @@ int bpf_iter_reg_target(const struct bpf_iter_reg *reg_info)
return 0;
}
-void bpf_iter_unreg_target(const char *target)
+void bpf_iter_unreg_target(const struct bpf_iter_reg *reg_info)
{
struct bpf_iter_target_info *tinfo;
bool found = false;
mutex_lock(&targets_mutex);
list_for_each_entry(tinfo, &targets, list) {
- if (!strcmp(target, tinfo->reg_info->target)) {
+ if (reg_info == tinfo->reg_info) {
list_del(&tinfo->list);
kfree(tinfo);
found = true;
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 6ad2fa51a23a..22bf4e36c093 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -6412,7 +6412,7 @@ static int __init bpf_iter_register(void)
static void bpf_iter_unregister(void)
{
- bpf_iter_unreg_target("ipv6_route");
+ bpf_iter_unreg_target(&ipv6_route_reg_info);
}
#endif
#endif
--
2.24.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH bpf-next v2 6/7] bpf: enable bpf_iter targets registering ctx argument types
2020-05-13 18:02 [PATCH bpf-next v2 0/7] bpf: misc fixes for bpf_iter Yonghong Song
` (4 preceding siblings ...)
2020-05-13 18:02 ` [PATCH bpf-next v2 5/7] bpf: change func bpf_iter_unreg_target() signature Yonghong Song
@ 2020-05-13 18:02 ` Yonghong Song
2020-05-13 18:02 ` [PATCH bpf-next v2 7/7] samples/bpf: remove compiler warnings Yonghong Song
` (2 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Yonghong Song @ 2020-05-13 18:02 UTC (permalink / raw)
To: Andrii Nakryiko, bpf, Martin KaFai Lau, netdev
Cc: Alexei Starovoitov, Daniel Borkmann, kernel-team
Commit b121b341e598 ("bpf: Add PTR_TO_BTF_ID_OR_NULL
support") adds a field btf_id_or_null_non0_off to
bpf_prog->aux structure to indicate that the
first ctx argument is PTR_TO_BTF_ID reg_type and
all others are PTR_TO_BTF_ID_OR_NULL.
This approach does not really scale if we have
other different reg types in the future, e.g.,
a pointer to a buffer.
This patch enables bpf_iter targets registering ctx argument
reg types which may be different from the default one.
For example, for pointers to structures, the default reg_type
is PTR_TO_BTF_ID for tracing program. The target can register
a particular pointer type as PTR_TO_BTF_ID_OR_NULL which can
be used by the verifier to enforce accesses.
Acked-by: Andrii Nakryiko <andriin@fb.com>
Signed-off-by: Yonghong Song <yhs@fb.com>
---
include/linux/bpf.h | 12 +++++++++++-
include/net/ip6_fib.h | 7 +++++++
kernel/bpf/bpf_iter.c | 5 +++++
kernel/bpf/btf.c | 15 ++++++++++-----
kernel/bpf/map_iter.c | 5 +++++
kernel/bpf/task_iter.c | 12 ++++++++++++
kernel/bpf/verifier.c | 1 -
net/ipv6/ip6_fib.c | 5 -----
net/ipv6/route.c | 5 +++++
net/netlink/af_netlink.c | 5 +++++
10 files changed, 60 insertions(+), 12 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 534174eca86b..c45d198ac38c 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -643,6 +643,12 @@ struct bpf_jit_poke_descriptor {
u16 reason;
};
+/* reg_type info for ctx arguments */
+struct bpf_ctx_arg_aux {
+ u32 offset;
+ enum bpf_reg_type reg_type;
+};
+
struct bpf_prog_aux {
atomic64_t refcnt;
u32 used_map_cnt;
@@ -654,12 +660,13 @@ struct bpf_prog_aux {
u32 func_cnt; /* used by non-func prog as the number of func progs */
u32 func_idx; /* 0 for non-func prog, the index in func array for func prog */
u32 attach_btf_id; /* in-kernel BTF type id to attach to */
+ u32 ctx_arg_info_size;
+ const struct bpf_ctx_arg_aux *ctx_arg_info;
struct bpf_prog *linked_prog;
bool verifier_zext; /* Zero extensions has been inserted by verifier. */
bool offload_requested;
bool attach_btf_trace; /* true if attaching to BTF-enabled raw tp */
bool func_proto_unreliable;
- bool btf_id_or_null_non0_off;
enum bpf_tramp_prog_type trampoline_prog_type;
struct bpf_trampoline *trampoline;
struct hlist_node tramp_hlist;
@@ -1139,12 +1146,15 @@ int bpf_obj_get_user(const char __user *pathname, int flags);
typedef int (*bpf_iter_init_seq_priv_t)(void *private_data);
typedef void (*bpf_iter_fini_seq_priv_t)(void *private_data);
+#define BPF_ITER_CTX_ARG_MAX 2
struct bpf_iter_reg {
const char *target;
const struct seq_operations *seq_ops;
bpf_iter_init_seq_priv_t init_seq_private;
bpf_iter_fini_seq_priv_t fini_seq_private;
u32 seq_priv_size;
+ u32 ctx_arg_info_size;
+ struct bpf_ctx_arg_aux ctx_arg_info[BPF_ITER_CTX_ARG_MAX];
};
struct bpf_iter_meta {
diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
index 80262d2980f5..870b646c5797 100644
--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -540,6 +540,13 @@ static inline bool fib6_metric_locked(struct fib6_info *f6i, int metric)
return !!(f6i->fib6_metrics->metrics[RTAX_LOCK - 1] & (1 << metric));
}
+#if IS_BUILTIN(CONFIG_IPV6) && defined(CONFIG_BPF_SYSCALL)
+struct bpf_iter__ipv6_route {
+ __bpf_md_ptr(struct bpf_iter_meta *, meta);
+ __bpf_md_ptr(struct fib6_info *, rt);
+};
+#endif
+
#ifdef CONFIG_IPV6_MULTIPLE_TABLES
static inline bool fib6_has_custom_rules(const struct net *net)
{
diff --git a/kernel/bpf/bpf_iter.c b/kernel/bpf/bpf_iter.c
index 644f8626b2c0..dd612b80b9fe 100644
--- a/kernel/bpf/bpf_iter.c
+++ b/kernel/bpf/bpf_iter.c
@@ -308,6 +308,11 @@ bool bpf_iter_prog_supported(struct bpf_prog *prog)
}
mutex_unlock(&targets_mutex);
+ if (supported) {
+ prog->aux->ctx_arg_info_size = tinfo->reg_info->ctx_arg_info_size;
+ prog->aux->ctx_arg_info = tinfo->reg_info->ctx_arg_info;
+ }
+
return supported;
}
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index dcd233139294..58c9af1d4808 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -3694,7 +3694,7 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
struct bpf_verifier_log *log = info->log;
const struct btf_param *args;
u32 nr_args, arg;
- int ret;
+ int i, ret;
if (off % 8) {
bpf_log(log, "func '%s' offset %d is not multiple of 8\n",
@@ -3790,10 +3790,15 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
return true;
/* this is a pointer to another type */
- if (off != 0 && prog->aux->btf_id_or_null_non0_off)
- info->reg_type = PTR_TO_BTF_ID_OR_NULL;
- else
- info->reg_type = PTR_TO_BTF_ID;
+ info->reg_type = PTR_TO_BTF_ID;
+ for (i = 0; i < prog->aux->ctx_arg_info_size; i++) {
+ const struct bpf_ctx_arg_aux *ctx_arg_info = &prog->aux->ctx_arg_info[i];
+
+ if (ctx_arg_info->offset == off) {
+ info->reg_type = ctx_arg_info->reg_type;
+ break;
+ }
+ }
if (tgt_prog) {
ret = btf_translate_to_vmlinux(log, btf, t, tgt_prog->type, arg);
diff --git a/kernel/bpf/map_iter.c b/kernel/bpf/map_iter.c
index c6216a5fe56e..c69071e334bf 100644
--- a/kernel/bpf/map_iter.c
+++ b/kernel/bpf/map_iter.c
@@ -87,6 +87,11 @@ static const struct bpf_iter_reg bpf_map_reg_info = {
.init_seq_private = NULL,
.fini_seq_private = NULL,
.seq_priv_size = sizeof(struct bpf_iter_seq_map_info),
+ .ctx_arg_info_size = 1,
+ .ctx_arg_info = {
+ { offsetof(struct bpf_iter__bpf_map, map),
+ PTR_TO_BTF_ID_OR_NULL },
+ },
};
static int __init bpf_map_iter_init(void)
diff --git a/kernel/bpf/task_iter.c b/kernel/bpf/task_iter.c
index bd7bfd83d9e0..a9b7264dda08 100644
--- a/kernel/bpf/task_iter.c
+++ b/kernel/bpf/task_iter.c
@@ -312,6 +312,11 @@ static const struct bpf_iter_reg task_reg_info = {
.init_seq_private = init_seq_pidns,
.fini_seq_private = fini_seq_pidns,
.seq_priv_size = sizeof(struct bpf_iter_seq_task_info),
+ .ctx_arg_info_size = 1,
+ .ctx_arg_info = {
+ { offsetof(struct bpf_iter__task, task),
+ PTR_TO_BTF_ID_OR_NULL },
+ },
};
static const struct bpf_iter_reg task_file_reg_info = {
@@ -320,6 +325,13 @@ static const struct bpf_iter_reg task_file_reg_info = {
.init_seq_private = init_seq_pidns,
.fini_seq_private = fini_seq_pidns,
.seq_priv_size = sizeof(struct bpf_iter_seq_task_file_info),
+ .ctx_arg_info_size = 2,
+ .ctx_arg_info = {
+ { offsetof(struct bpf_iter__task_file, task),
+ PTR_TO_BTF_ID_OR_NULL },
+ { offsetof(struct bpf_iter__task_file, file),
+ PTR_TO_BTF_ID_OR_NULL },
+ },
};
static int __init task_iter_init(void)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 2a1826c76bb6..a3f2af756fd6 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -10652,7 +10652,6 @@ static int check_attach_btf_id(struct bpf_verifier_env *env)
prog->aux->attach_func_proto = t;
if (!bpf_iter_prog_supported(prog))
return -EINVAL;
- prog->aux->btf_id_or_null_non0_off = true;
ret = btf_distill_func_proto(&env->log, btf, t,
tname, &fmodel);
return ret;
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index a1fcc0ca21af..250ff52c674e 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -2638,11 +2638,6 @@ static void ipv6_route_native_seq_stop(struct seq_file *seq, void *v)
}
#if IS_BUILTIN(CONFIG_IPV6) && defined(CONFIG_BPF_SYSCALL)
-struct bpf_iter__ipv6_route {
- __bpf_md_ptr(struct bpf_iter_meta *, meta);
- __bpf_md_ptr(struct fib6_info *, rt);
-};
-
static int ipv6_route_prog_seq_show(struct bpf_prog *prog,
struct bpf_iter_meta *meta,
void *v)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 22bf4e36c093..22e56465f14d 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -6403,6 +6403,11 @@ static const struct bpf_iter_reg ipv6_route_reg_info = {
.init_seq_private = bpf_iter_init_seq_net,
.fini_seq_private = bpf_iter_fini_seq_net,
.seq_priv_size = sizeof(struct ipv6_route_iter),
+ .ctx_arg_info_size = 1,
+ .ctx_arg_info = {
+ { offsetof(struct bpf_iter__ipv6_route, rt),
+ PTR_TO_BTF_ID_OR_NULL },
+ },
};
static int __init bpf_iter_register(void)
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 839827227e98..4f2c3b14ddbf 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -2809,6 +2809,11 @@ static const struct bpf_iter_reg netlink_reg_info = {
.init_seq_private = bpf_iter_init_seq_net,
.fini_seq_private = bpf_iter_fini_seq_net,
.seq_priv_size = sizeof(struct nl_seq_iter),
+ .ctx_arg_info_size = 1,
+ .ctx_arg_info = {
+ { offsetof(struct bpf_iter__netlink, sk),
+ PTR_TO_BTF_ID_OR_NULL },
+ },
};
static int __init bpf_iter_register(void)
--
2.24.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH bpf-next v2 7/7] samples/bpf: remove compiler warnings
2020-05-13 18:02 [PATCH bpf-next v2 0/7] bpf: misc fixes for bpf_iter Yonghong Song
` (5 preceding siblings ...)
2020-05-13 18:02 ` [PATCH bpf-next v2 6/7] bpf: enable bpf_iter targets registering ctx argument types Yonghong Song
@ 2020-05-13 18:02 ` Yonghong Song
2020-05-13 18:19 ` [PATCH bpf-next v2 0/7] bpf: misc fixes for bpf_iter Andrii Nakryiko
2020-05-13 19:38 ` Alexei Starovoitov
8 siblings, 0 replies; 10+ messages in thread
From: Yonghong Song @ 2020-05-13 18:02 UTC (permalink / raw)
To: Andrii Nakryiko, bpf, Martin KaFai Lau, netdev
Cc: Alexei Starovoitov, Daniel Borkmann, kernel-team
Commit 5fbc220862fc ("tools/libpf: Add offsetof/container_of macro
in bpf_helpers.h") added macros offsetof/container_of to
bpf_helpers.h. Unfortunately, it caused compilation warnings
below for a few samples/bpf programs:
In file included from /data/users/yhs/work/net-next/samples/bpf/sockex2_kern.c:4:
In file included from /data/users/yhs/work/net-next/include/uapi/linux/in.h:24:
In file included from /data/users/yhs/work/net-next/include/linux/socket.h:8:
In file included from /data/users/yhs/work/net-next/include/linux/uio.h:8:
/data/users/yhs/work/net-next/include/linux/kernel.h:992:9: warning: 'container_of' macro redefined [-Wmacro-redefined]
^
/data/users/yhs/work/net-next/tools/lib/bpf/bpf_helpers.h:46:9: note: previous definition is here
^
1 warning generated.
CLANG-bpf samples/bpf/sockex3_kern.o
In all these cases, bpf_helpers.h is included first, followed by other
standard headers. The macro container_of is defined unconditionally
in kernel.h, causing the compiler warning.
The fix is to move bpf_helpers.h after standard headers.
Acked-by: Andrii Nakryiko <andriin@fb.com>
Signed-off-by: Yonghong Song <yhs@fb.com>
---
samples/bpf/offwaketime_kern.c | 4 ++--
samples/bpf/sockex2_kern.c | 4 ++--
samples/bpf/sockex3_kern.c | 4 ++--
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/samples/bpf/offwaketime_kern.c b/samples/bpf/offwaketime_kern.c
index c4ec10dbfc3b..d459f73412a4 100644
--- a/samples/bpf/offwaketime_kern.c
+++ b/samples/bpf/offwaketime_kern.c
@@ -5,12 +5,12 @@
* License as published by the Free Software Foundation.
*/
#include <uapi/linux/bpf.h>
-#include <bpf/bpf_helpers.h>
-#include <bpf/bpf_tracing.h>
#include <uapi/linux/ptrace.h>
#include <uapi/linux/perf_event.h>
#include <linux/version.h>
#include <linux/sched.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
#define _(P) ({typeof(P) val; bpf_probe_read(&val, sizeof(val), &P); val;})
diff --git a/samples/bpf/sockex2_kern.c b/samples/bpf/sockex2_kern.c
index a41dd520bc53..b7997541f7ee 100644
--- a/samples/bpf/sockex2_kern.c
+++ b/samples/bpf/sockex2_kern.c
@@ -1,12 +1,12 @@
#include <uapi/linux/bpf.h>
-#include <bpf/bpf_helpers.h>
-#include "bpf_legacy.h"
#include <uapi/linux/in.h>
#include <uapi/linux/if.h>
#include <uapi/linux/if_ether.h>
#include <uapi/linux/ip.h>
#include <uapi/linux/ipv6.h>
#include <uapi/linux/if_tunnel.h>
+#include <bpf/bpf_helpers.h>
+#include "bpf_legacy.h"
#define IP_MF 0x2000
#define IP_OFFSET 0x1FFF
diff --git a/samples/bpf/sockex3_kern.c b/samples/bpf/sockex3_kern.c
index 36d4dac23549..779a5249c418 100644
--- a/samples/bpf/sockex3_kern.c
+++ b/samples/bpf/sockex3_kern.c
@@ -5,8 +5,6 @@
* License as published by the Free Software Foundation.
*/
#include <uapi/linux/bpf.h>
-#include <bpf/bpf_helpers.h>
-#include "bpf_legacy.h"
#include <uapi/linux/in.h>
#include <uapi/linux/if.h>
#include <uapi/linux/if_ether.h>
@@ -14,6 +12,8 @@
#include <uapi/linux/ipv6.h>
#include <uapi/linux/if_tunnel.h>
#include <uapi/linux/mpls.h>
+#include <bpf/bpf_helpers.h>
+#include "bpf_legacy.h"
#define IP_MF 0x2000
#define IP_OFFSET 0x1FFF
--
2.24.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH bpf-next v2 0/7] bpf: misc fixes for bpf_iter
2020-05-13 18:02 [PATCH bpf-next v2 0/7] bpf: misc fixes for bpf_iter Yonghong Song
` (6 preceding siblings ...)
2020-05-13 18:02 ` [PATCH bpf-next v2 7/7] samples/bpf: remove compiler warnings Yonghong Song
@ 2020-05-13 18:19 ` Andrii Nakryiko
2020-05-13 19:38 ` Alexei Starovoitov
8 siblings, 0 replies; 10+ messages in thread
From: Andrii Nakryiko @ 2020-05-13 18:19 UTC (permalink / raw)
To: Yonghong Song
Cc: Andrii Nakryiko, bpf, Martin KaFai Lau, Networking,
Alexei Starovoitov, Daniel Borkmann, Kernel Team
On Wed, May 13, 2020 at 11:03 AM Yonghong Song <yhs@fb.com> wrote:
>
> Commit ae24345da54e ("bpf: Implement an interface to register
> bpf_iter targets") and its subsequent commits in the same patch set
> introduced bpf iterator, a way to run bpf program when iterating
> kernel data structures.
>
> This patch set addressed some followup issues. One big change
> is to allow target to pass ctx arg register types to verifier
> for verification purpose. Please see individual patch for details.
>
> Changelogs:
> v1 -> v2:
> . add "const" qualifier to struct bpf_iter_reg for
> bpf_iter_[un]reg_target, and this results in
> additional "const" qualifiers in some other places
> . drop the patch which will issue WARN_ONCE if
> seq_ops->show() returns a positive value.
> If this does happen, code review should spot
> this or author does know what he is doing.
> In the future, we do want to implement a
> mechanism to find out all registered targets
> so we will be aware of new additions.
>
> Yonghong Song (7):
> tools/bpf: selftests : explain bpf_iter test failures with llvm 10.0.0
> bpf: change btf_iter func proto prefix to "bpf_iter_"
> bpf: add comments to interpret bpf_prog return values
> bpf: net: refactor bpf_iter target registration
> bpf: change func bpf_iter_unreg_target() signature
> bpf: enable bpf_iter targets registering ctx argument types
> samples/bpf: remove compiler warnings
>
> include/linux/bpf.h | 22 ++++++++----
> include/net/ip6_fib.h | 7 ++++
> kernel/bpf/bpf_iter.c | 49 +++++++++++++++-----------
> kernel/bpf/btf.c | 15 +++++---
> kernel/bpf/map_iter.c | 23 +++++++-----
> kernel/bpf/task_iter.c | 42 ++++++++++++++--------
> kernel/bpf/verifier.c | 1 -
> net/ipv6/ip6_fib.c | 5 ---
> net/ipv6/route.c | 25 +++++++------
> net/netlink/af_netlink.c | 23 +++++++-----
> samples/bpf/offwaketime_kern.c | 4 +--
> samples/bpf/sockex2_kern.c | 4 +--
> samples/bpf/sockex3_kern.c | 4 +--
> tools/lib/bpf/libbpf.c | 2 +-
> tools/testing/selftests/bpf/README.rst | 43 ++++++++++++++++++++++
> 15 files changed, 183 insertions(+), 86 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/README.rst
>
> --
> 2.24.1
>
For the series:
Acked-by: Andrii Nakryiko <andriin@fb.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH bpf-next v2 0/7] bpf: misc fixes for bpf_iter
2020-05-13 18:02 [PATCH bpf-next v2 0/7] bpf: misc fixes for bpf_iter Yonghong Song
` (7 preceding siblings ...)
2020-05-13 18:19 ` [PATCH bpf-next v2 0/7] bpf: misc fixes for bpf_iter Andrii Nakryiko
@ 2020-05-13 19:38 ` Alexei Starovoitov
8 siblings, 0 replies; 10+ messages in thread
From: Alexei Starovoitov @ 2020-05-13 19:38 UTC (permalink / raw)
To: Yonghong Song
Cc: Andrii Nakryiko, bpf, Martin KaFai Lau, netdev,
Alexei Starovoitov, Daniel Borkmann, kernel-team
On Wed, May 13, 2020 at 11:02:15AM -0700, Yonghong Song wrote:
> Commit ae24345da54e ("bpf: Implement an interface to register
> bpf_iter targets") and its subsequent commits in the same patch set
> introduced bpf iterator, a way to run bpf program when iterating
> kernel data structures.
>
> This patch set addressed some followup issues. One big change
> is to allow target to pass ctx arg register types to verifier
> for verification purpose. Please see individual patch for details.
>
> Changelogs:
> v1 -> v2:
> . add "const" qualifier to struct bpf_iter_reg for
> bpf_iter_[un]reg_target, and this results in
> additional "const" qualifiers in some other places
> . drop the patch which will issue WARN_ONCE if
> seq_ops->show() returns a positive value.
> If this does happen, code review should spot
> this or author does know what he is doing.
> In the future, we do want to implement a
> mechanism to find out all registered targets
> so we will be aware of new additions.
Applied, Thanks
^ permalink raw reply [flat|nested] 10+ messages in thread