This series introduces a new tracepoint in bpf_xdp_link_attach(). By this tracepoint, error message will be captured when error happens in dev_xdp_attach(), e.g. invalid attaching flags.
v3 -> v4: * Fix selftest-crashed issue.
Leon Hwang (2): bpf, xdp: Add tracepoint to xdp attaching failure selftests/bpf: Add testcase for xdp attaching failure tracepoint
include/trace/events/xdp.h | 17 +++++ net/core/dev.c | 5 +- .../selftests/bpf/prog_tests/xdp_attach.c | 65 +++++++++++++++++++ .../bpf/progs/test_xdp_attach_fail.c | 54 +++++++++++++++ 4 files changed, 140 insertions(+), 1 deletion(-) create mode 100644 tools/testing/selftests/bpf/progs/test_xdp_attach_fail.c
base-commit: a33d978500acd8fb67efac9773ba0a8502c1ff06
When error happens in dev_xdp_attach(), it should have a way to tell users the error message like the netlink approach.
To avoid breaking uapi, adding a tracepoint in bpf_xdp_link_attach() is an appropriate way to notify users the error message.
Hence, bpf libraries are able to retrieve the error message by this tracepoint, and then report the error message to users.
Signed-off-by: Leon Hwang hffilwlqm@gmail.com --- include/trace/events/xdp.h | 17 +++++++++++++++++ net/core/dev.c | 5 ++++- 2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/include/trace/events/xdp.h b/include/trace/events/xdp.h index c40fc97f94171..cd89f1d5ce7b8 100644 --- a/include/trace/events/xdp.h +++ b/include/trace/events/xdp.h @@ -404,6 +404,23 @@ TRACE_EVENT(mem_return_failed, ) );
+TRACE_EVENT(bpf_xdp_link_attach_failed, + + TP_PROTO(const char *msg), + + TP_ARGS(msg), + + TP_STRUCT__entry( + __string(msg, msg) + ), + + TP_fast_assign( + __assign_str(msg, msg); + ), + + TP_printk("errmsg=%s", __get_str(msg)) +); + #endif /* _TRACE_XDP_H */
#include <trace/define_trace.h> diff --git a/net/core/dev.c b/net/core/dev.c index 8e7d0cb540cdb..49bed890f807e 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -133,6 +133,7 @@ #include <trace/events/net.h> #include <trace/events/skb.h> #include <trace/events/qdisc.h> +#include <trace/events/xdp.h> #include <linux/inetdevice.h> #include <linux/cpu_rmap.h> #include <linux/static_key.h> @@ -9472,6 +9473,7 @@ int bpf_xdp_link_attach(const union bpf_attr *attr, struct bpf_prog *prog) struct bpf_link_primer link_primer; struct bpf_xdp_link *link; struct net_device *dev; + struct netlink_ext_ack extack; int err, fd;
rtnl_lock(); @@ -9497,12 +9499,13 @@ int bpf_xdp_link_attach(const union bpf_attr *attr, struct bpf_prog *prog) goto unlock; }
- err = dev_xdp_attach_link(dev, NULL, link); + err = dev_xdp_attach_link(dev, &extack, link); rtnl_unlock();
if (err) { link->dev = NULL; bpf_link_cleanup(&link_primer); + trace_bpf_xdp_link_attach_failed(extack._msg); goto out_put_dev; }
On Sun, 30 Jul 2023 19:49:50 +0800 Leon Hwang wrote:
@@ -9472,6 +9473,7 @@ int bpf_xdp_link_attach(const union bpf_attr *attr, struct bpf_prog *prog) struct bpf_link_primer link_primer; struct bpf_xdp_link *link; struct net_device *dev;
- struct netlink_ext_ack extack;
This is not initialized. Also, while fixing that, move it up to maintain the line order by decreasing line length.
int err, fd; rtnl_lock(); @@ -9497,12 +9499,13 @@ int bpf_xdp_link_attach(const union bpf_attr *attr, struct bpf_prog *prog) goto unlock; }
- err = dev_xdp_attach_link(dev, NULL, link);
- err = dev_xdp_attach_link(dev, &extack, link); rtnl_unlock();
if (err) { link->dev = NULL; bpf_link_cleanup(&link_primer);
trace_bpf_xdp_link_attach_failed(extack._msg);
On 1/8/23 10:43, Jakub Kicinski wrote:
On Sun, 30 Jul 2023 19:49:50 +0800 Leon Hwang wrote:
@@ -9472,6 +9473,7 @@ int bpf_xdp_link_attach(const union bpf_attr *attr, struct bpf_prog *prog) struct bpf_link_primer link_primer; struct bpf_xdp_link *link; struct net_device *dev;
- struct netlink_ext_ack extack;
This is not initialized. Also, while fixing that, move it up to maintain the line order by decreasing line length.
Thank you for your reviewing.
I'll fix it by initializing extack and moving the line to its appropriate position.
Thanks, Leon
int err, fd; rtnl_lock(); @@ -9497,12 +9499,13 @@ int bpf_xdp_link_attach(const union bpf_attr *attr, struct bpf_prog *prog) goto unlock; }
- err = dev_xdp_attach_link(dev, NULL, link);
- err = dev_xdp_attach_link(dev, &extack, link); rtnl_unlock();
if (err) { link->dev = NULL; bpf_link_cleanup(&link_primer);
trace_bpf_xdp_link_attach_failed(extack._msg);
Add a test case for the tracepoint of xdp attaching failure by bpf tracepoint when attach XDP to a device with invalid flags option.
The bpf tracepoint retrieves error message from the tracepoint, and then put the error message to a perf buffer. The testing code receives error message from perf buffer, and then ASSERT "Invalid XDP flags for BPF link attachment".
Signed-off-by: Leon Hwang hffilwlqm@gmail.com --- .../selftests/bpf/prog_tests/xdp_attach.c | 65 +++++++++++++++++++ .../bpf/progs/test_xdp_attach_fail.c | 54 +++++++++++++++ 2 files changed, 119 insertions(+) create mode 100644 tools/testing/selftests/bpf/progs/test_xdp_attach_fail.c
diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_attach.c b/tools/testing/selftests/bpf/prog_tests/xdp_attach.c index fa3cac5488f5d..8c1cde74e9cd6 100644 --- a/tools/testing/selftests/bpf/prog_tests/xdp_attach.c +++ b/tools/testing/selftests/bpf/prog_tests/xdp_attach.c @@ -1,5 +1,6 @@ // SPDX-License-Identifier: GPL-2.0 #include <test_progs.h> +#include "test_xdp_attach_fail.skel.h"
#define IFINDEX_LO 1 #define XDP_FLAGS_REPLACE (1U << 4) @@ -85,10 +86,74 @@ static void test_xdp_attach(const char *file) bpf_object__close(obj1); }
+#define ERRMSG_LEN 64 + +struct xdp_errmsg { + char msg[ERRMSG_LEN]; +}; + +static void on_xdp_errmsg(void *ctx, int cpu, void *data, __u32 size) +{ + struct xdp_errmsg *ctx_errmg = ctx, *tp_errmsg = data; + + memcpy(&ctx_errmg->msg, &tp_errmsg->msg, ERRMSG_LEN); +} + +static const char tgt_errmsg[] = "Invalid XDP flags for BPF link attachment"; + +static void test_xdp_attach_fail(const char *file) +{ + int err, fd_xdp; + struct bpf_object *obj = NULL; + struct test_xdp_attach_fail *skel = NULL; + struct perf_buffer *pb = NULL; + struct xdp_errmsg errmsg = {}; + + LIBBPF_OPTS(bpf_link_create_opts, opts); + + skel = test_xdp_attach_fail__open_and_load(); + if (!ASSERT_OK_PTR(skel, "test_xdp_attach_fail__open_and_load")) + goto out_close; + + err = test_xdp_attach_fail__attach(skel); + if (!ASSERT_EQ(err, 0, "test_xdp_attach_fail__attach")) + goto out_close; + + /* set up perf buffer */ + pb = perf_buffer__new(bpf_map__fd(skel->maps.xdp_errmsg_pb), 1, + on_xdp_errmsg, NULL, &errmsg, NULL); + if (!ASSERT_OK_PTR(pb, "perf_buffer__new")) + goto out_close; + + err = bpf_prog_test_load(file, BPF_PROG_TYPE_XDP, &obj, &fd_xdp); + if (!ASSERT_EQ(err, 0, "bpf_prog_test_load")) + goto out_close; + + opts.flags = 0xFF; // invalid flags to fail to attach XDP prog + err = bpf_link_create(fd_xdp, IFINDEX_LO, BPF_XDP, &opts); + if (!ASSERT_EQ(err, -EINVAL, "bpf_link_create")) + goto out_close; + + /* read perf buffer */ + err = perf_buffer__poll(pb, 100); + if (!ASSERT_GT(err, -1, "perf_buffer__poll")) + goto out_close; + + ASSERT_STRNEQ((const char *) errmsg.msg, tgt_errmsg, + 42 /* strlen(tgt_errmsg) */, "check error message"); + +out_close: + perf_buffer__free(pb); + bpf_object__close(obj); + test_xdp_attach_fail__destroy(skel); +} + void serial_test_xdp_attach(void) { if (test__start_subtest("xdp_attach")) test_xdp_attach("./test_xdp.bpf.o"); if (test__start_subtest("xdp_attach_dynptr")) test_xdp_attach("./test_xdp_dynptr.bpf.o"); + if (test__start_subtest("xdp_attach_failed")) + test_xdp_attach_fail("./xdp_dummy.bpf.o"); } diff --git a/tools/testing/selftests/bpf/progs/test_xdp_attach_fail.c b/tools/testing/selftests/bpf/progs/test_xdp_attach_fail.c new file mode 100644 index 0000000000000..d7149bbd95f75 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/test_xdp_attach_fail.c @@ -0,0 +1,54 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright Leon Hwang */ + +#include <linux/bpf.h> +#include <bpf/bpf_helpers.h> + +#define ERRMSG_LEN 64 + +struct xdp_errmsg { + char msg[ERRMSG_LEN]; +}; + +struct { + __uint(type, BPF_MAP_TYPE_PERF_EVENT_ARRAY); + __type(key, int); + __type(value, int); +} xdp_errmsg_pb SEC(".maps"); + +struct xdp_attach_error_ctx { + unsigned long unused; + + /* + * bpf does not support tracepoint __data_loc directly. + * + * Actually, this field is a 32 bit integer whose value encodes + * information on where to find the actual data. The first 2 bytes is + * the size of the data. The last 2 bytes is the offset from the start + * of the tracepoint struct where the data begins. + * -- https://github.com/iovisor/bpftrace/pull/1542 + */ + __u32 msg; // __data_loc char[] msg; +}; + +/* + * Catch the error message at the tracepoint. + */ + +SEC("tp/xdp/bpf_xdp_link_attach_failed") +int tp__xdp__bpf_xdp_link_attach_failed(struct xdp_attach_error_ctx *ctx) +{ + struct xdp_errmsg errmsg; + char *msg = (void *)(__u64) ((void *) ctx + (__u16) ctx->msg); + + bpf_probe_read_kernel_str(&errmsg.msg, ERRMSG_LEN, msg); + bpf_perf_event_output(ctx, &xdp_errmsg_pb, BPF_F_CURRENT_CPU, &errmsg, + ERRMSG_LEN); + return 0; +} + +/* + * Reuse the XDP program in xdp_dummy.c. + */ + +char LICENSE[] SEC("license") = "GPL";
This patch is very important to help us to debug the xdp program. At the same time, we can make some monitoring tools to observe the kernel status by using this trace event
李者璈 & Zheaoli
Email: lizheao940510@gmail.com Github: https://github.com/Zheaoli
Leon Hwang hffilwlqm@gmail.com 于2023年7月30日周日 19:50写道:
This series introduces a new tracepoint in bpf_xdp_link_attach(). By this tracepoint, error message will be captured when error happens in dev_xdp_attach(), e.g. invalid attaching flags.
v3 -> v4:
- Fix selftest-crashed issue.
Leon Hwang (2): bpf, xdp: Add tracepoint to xdp attaching failure selftests/bpf: Add testcase for xdp attaching failure tracepoint
include/trace/events/xdp.h | 17 +++++ net/core/dev.c | 5 +- .../selftests/bpf/prog_tests/xdp_attach.c | 65 +++++++++++++++++++ .../bpf/progs/test_xdp_attach_fail.c | 54 +++++++++++++++ 4 files changed, 140 insertions(+), 1 deletion(-) create mode 100644 tools/testing/selftests/bpf/progs/test_xdp_attach_fail.c
base-commit: a33d978500acd8fb67efac9773ba0a8502c1ff06
2.41.0
On 2023/7/30 21:49, Manjusaka wrote:
This patch is very important to help us to debug the xdp program. At the same time, we can make some monitoring tools to observe the kernel status by using this trace event
李者璈 & Zheaoli
Email: lizheao940510@gmail.com Github: https://github.com/Zheaoli
Thank you for your feedback.
I'm glad that it's helpful for you.
Thanks, Leon
linux-kselftest-mirror@lists.linaro.org