* [PATCH bpf-next v2 1/2] bpf: Move skb->len == 0 checks into __bpf_redirect
@ 2022-11-21 18:03 Stanislav Fomichev
2022-11-21 18:03 ` [PATCH bpf-next v2 2/2] selftests/bpf: Make sure zero-len skbs aren't redirectable Stanislav Fomichev
2022-11-21 21:00 ` [PATCH bpf-next v2 1/2] bpf: Move skb->len == 0 checks into __bpf_redirect patchwork-bot+netdevbpf
0 siblings, 2 replies; 11+ messages in thread
From: Stanislav Fomichev @ 2022-11-21 18:03 UTC (permalink / raw)
To: bpf
Cc: ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, sdf, haoluo, jolsa
To avoid potentially breaking existing users.
Both mac/no-mac cases have to be amended; mac_header >= network_header
is not enough (verified with a new test, see next patch).
Fixes: fd1894224407 ("bpf: Don't redirect packets with invalid pkt_len")
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
net/bpf/test_run.c | 3 ---
net/core/filter.c | 11 ++++++-----
2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index 13d578ce2a09..6fba440efc40 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -979,9 +979,6 @@ static int convert___skb_to_skb(struct sk_buff *skb, struct __sk_buff *__skb)
{
struct qdisc_skb_cb *cb = (struct qdisc_skb_cb *)skb->cb;
- if (!skb->len)
- return -EINVAL;
-
if (!__skb)
return 0;
diff --git a/net/core/filter.c b/net/core/filter.c
index 2a105fb1ceb2..b6e1b81cdfae 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2124,12 +2124,13 @@ static int __bpf_redirect_no_mac(struct sk_buff *skb, struct net_device *dev,
{
unsigned int mlen = skb_network_offset(skb);
+ if (unlikely(skb->len <= mlen)) {
+ kfree_skb(skb);
+ return -ERANGE;
+ }
+
if (mlen) {
__skb_pull(skb, mlen);
- if (unlikely(!skb->len)) {
- kfree_skb(skb);
- return -ERANGE;
- }
/* At ingress, the mac header has already been pulled once.
* At egress, skb_pospull_rcsum has to be done in case that
@@ -2149,7 +2150,7 @@ static int __bpf_redirect_common(struct sk_buff *skb, struct net_device *dev,
u32 flags)
{
/* Verify that a link layer header is carried */
- if (unlikely(skb->mac_header >= skb->network_header)) {
+ if (unlikely(skb->mac_header >= skb->network_header || skb->len == 0)) {
kfree_skb(skb);
return -ERANGE;
}
--
2.38.1.584.g0f3c55d4c2-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH bpf-next v2 2/2] selftests/bpf: Make sure zero-len skbs aren't redirectable
2022-11-21 18:03 [PATCH bpf-next v2 1/2] bpf: Move skb->len == 0 checks into __bpf_redirect Stanislav Fomichev
@ 2022-11-21 18:03 ` Stanislav Fomichev
2022-11-23 12:23 ` Jiri Olsa
2023-09-08 16:54 ` Daniel Borkmann
2022-11-21 21:00 ` [PATCH bpf-next v2 1/2] bpf: Move skb->len == 0 checks into __bpf_redirect patchwork-bot+netdevbpf
1 sibling, 2 replies; 11+ messages in thread
From: Stanislav Fomichev @ 2022-11-21 18:03 UTC (permalink / raw)
To: bpf
Cc: ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, sdf, haoluo, jolsa
LWT_XMIT to test L3 case, TC to test L2 case.
v2:
- s/veth_ifindex/ipip_ifindex/ in two places (Martin)
- add comment about which condition triggers the rejection (Martin)
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
.../selftests/bpf/prog_tests/empty_skb.c | 146 ++++++++++++++++++
tools/testing/selftests/bpf/progs/empty_skb.c | 37 +++++
2 files changed, 183 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/empty_skb.c
create mode 100644 tools/testing/selftests/bpf/progs/empty_skb.c
diff --git a/tools/testing/selftests/bpf/prog_tests/empty_skb.c b/tools/testing/selftests/bpf/prog_tests/empty_skb.c
new file mode 100644
index 000000000000..32dd731e9070
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/empty_skb.c
@@ -0,0 +1,146 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <test_progs.h>
+#include <network_helpers.h>
+#include <net/if.h>
+#include "empty_skb.skel.h"
+
+#define SYS(cmd) ({ \
+ if (!ASSERT_OK(system(cmd), (cmd))) \
+ goto out; \
+})
+
+void test_empty_skb(void)
+{
+ LIBBPF_OPTS(bpf_test_run_opts, tattr);
+ struct empty_skb *bpf_obj = NULL;
+ struct nstoken *tok = NULL;
+ struct bpf_program *prog;
+ char eth_hlen_pp[15];
+ char eth_hlen[14];
+ int veth_ifindex;
+ int ipip_ifindex;
+ int err;
+ int i;
+
+ struct {
+ const char *msg;
+ const void *data_in;
+ __u32 data_size_in;
+ int *ifindex;
+ int err;
+ int ret;
+ bool success_on_tc;
+ } tests[] = {
+ /* Empty packets are always rejected. */
+
+ {
+ /* BPF_PROG_RUN ETH_HLEN size check */
+ .msg = "veth empty ingress packet",
+ .data_in = NULL,
+ .data_size_in = 0,
+ .ifindex = &veth_ifindex,
+ .err = -EINVAL,
+ },
+ {
+ /* BPF_PROG_RUN ETH_HLEN size check */
+ .msg = "ipip empty ingress packet",
+ .data_in = NULL,
+ .data_size_in = 0,
+ .ifindex = &ipip_ifindex,
+ .err = -EINVAL,
+ },
+
+ /* ETH_HLEN-sized packets:
+ * - can not be redirected at LWT_XMIT
+ * - can be redirected at TC to non-tunneling dest
+ */
+
+ {
+ /* __bpf_redirect_common */
+ .msg = "veth ETH_HLEN packet ingress",
+ .data_in = eth_hlen,
+ .data_size_in = sizeof(eth_hlen),
+ .ifindex = &veth_ifindex,
+ .ret = -ERANGE,
+ .success_on_tc = true,
+ },
+ {
+ /* __bpf_redirect_no_mac
+ *
+ * lwt: skb->len=0 <= skb_network_offset=0
+ * tc: skb->len=14 <= skb_network_offset=14
+ */
+ .msg = "ipip ETH_HLEN packet ingress",
+ .data_in = eth_hlen,
+ .data_size_in = sizeof(eth_hlen),
+ .ifindex = &ipip_ifindex,
+ .ret = -ERANGE,
+ },
+
+ /* ETH_HLEN+1-sized packet should be redirected. */
+
+ {
+ .msg = "veth ETH_HLEN+1 packet ingress",
+ .data_in = eth_hlen_pp,
+ .data_size_in = sizeof(eth_hlen_pp),
+ .ifindex = &veth_ifindex,
+ },
+ {
+ .msg = "ipip ETH_HLEN+1 packet ingress",
+ .data_in = eth_hlen_pp,
+ .data_size_in = sizeof(eth_hlen_pp),
+ .ifindex = &ipip_ifindex,
+ },
+ };
+
+ SYS("ip netns add empty_skb");
+ tok = open_netns("empty_skb");
+ SYS("ip link add veth0 type veth peer veth1");
+ SYS("ip link set dev veth0 up");
+ SYS("ip link set dev veth1 up");
+ SYS("ip addr add 10.0.0.1/8 dev veth0");
+ SYS("ip addr add 10.0.0.2/8 dev veth1");
+ veth_ifindex = if_nametoindex("veth0");
+
+ SYS("ip link add ipip0 type ipip local 10.0.0.1 remote 10.0.0.2");
+ SYS("ip link set ipip0 up");
+ SYS("ip addr add 192.168.1.1/16 dev ipip0");
+ ipip_ifindex = if_nametoindex("ipip0");
+
+ bpf_obj = empty_skb__open_and_load();
+ if (!ASSERT_OK_PTR(bpf_obj, "open skeleton"))
+ goto out;
+
+ for (i = 0; i < ARRAY_SIZE(tests); i++) {
+ bpf_object__for_each_program(prog, bpf_obj->obj) {
+ char buf[128];
+ bool at_tc = !strncmp(bpf_program__section_name(prog), "tc", 2);
+
+ tattr.data_in = tests[i].data_in;
+ tattr.data_size_in = tests[i].data_size_in;
+
+ tattr.data_size_out = 0;
+ bpf_obj->bss->ifindex = *tests[i].ifindex;
+ bpf_obj->bss->ret = 0;
+ err = bpf_prog_test_run_opts(bpf_program__fd(prog), &tattr);
+ sprintf(buf, "err: %s [%s]", tests[i].msg, bpf_program__name(prog));
+
+ if (at_tc && tests[i].success_on_tc)
+ ASSERT_GE(err, 0, buf);
+ else
+ ASSERT_EQ(err, tests[i].err, buf);
+ sprintf(buf, "ret: %s [%s]", tests[i].msg, bpf_program__name(prog));
+ if (at_tc && tests[i].success_on_tc)
+ ASSERT_GE(bpf_obj->bss->ret, 0, buf);
+ else
+ ASSERT_EQ(bpf_obj->bss->ret, tests[i].ret, buf);
+ }
+ }
+
+out:
+ if (bpf_obj)
+ empty_skb__destroy(bpf_obj);
+ if (tok)
+ close_netns(tok);
+ system("ip netns del empty_skb");
+}
diff --git a/tools/testing/selftests/bpf/progs/empty_skb.c b/tools/testing/selftests/bpf/progs/empty_skb.c
new file mode 100644
index 000000000000..4b0cd6753251
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/empty_skb.c
@@ -0,0 +1,37 @@
+// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_endian.h>
+
+char _license[] SEC("license") = "GPL";
+
+int ifindex;
+int ret;
+
+SEC("lwt_xmit")
+int redirect_ingress(struct __sk_buff *skb)
+{
+ ret = bpf_clone_redirect(skb, ifindex, BPF_F_INGRESS);
+ return 0;
+}
+
+SEC("lwt_xmit")
+int redirect_egress(struct __sk_buff *skb)
+{
+ ret = bpf_clone_redirect(skb, ifindex, 0);
+ return 0;
+}
+
+SEC("tc")
+int tc_redirect_ingress(struct __sk_buff *skb)
+{
+ ret = bpf_clone_redirect(skb, ifindex, BPF_F_INGRESS);
+ return 0;
+}
+
+SEC("tc")
+int tc_redirect_egress(struct __sk_buff *skb)
+{
+ ret = bpf_clone_redirect(skb, ifindex, 0);
+ return 0;
+}
--
2.38.1.584.g0f3c55d4c2-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next v2 1/2] bpf: Move skb->len == 0 checks into __bpf_redirect
2022-11-21 18:03 [PATCH bpf-next v2 1/2] bpf: Move skb->len == 0 checks into __bpf_redirect Stanislav Fomichev
2022-11-21 18:03 ` [PATCH bpf-next v2 2/2] selftests/bpf: Make sure zero-len skbs aren't redirectable Stanislav Fomichev
@ 2022-11-21 21:00 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 11+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-11-21 21:00 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: bpf, ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, haoluo, jolsa
Hello:
This series was applied to bpf/bpf-next.git (master)
by Martin KaFai Lau <martin.lau@kernel.org>:
On Mon, 21 Nov 2022 10:03:39 -0800 you wrote:
> To avoid potentially breaking existing users.
>
> Both mac/no-mac cases have to be amended; mac_header >= network_header
> is not enough (verified with a new test, see next patch).
>
> Fixes: fd1894224407 ("bpf: Don't redirect packets with invalid pkt_len")
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
>
> [...]
Here is the summary with links:
- [bpf-next,v2,1/2] bpf: Move skb->len == 0 checks into __bpf_redirect
https://git.kernel.org/bpf/bpf-next/c/114039b34201
- [bpf-next,v2,2/2] selftests/bpf: Make sure zero-len skbs aren't redirectable
https://git.kernel.org/bpf/bpf-next/c/68f8e3d4b916
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next v2 2/2] selftests/bpf: Make sure zero-len skbs aren't redirectable
2022-11-21 18:03 ` [PATCH bpf-next v2 2/2] selftests/bpf: Make sure zero-len skbs aren't redirectable Stanislav Fomichev
@ 2022-11-23 12:23 ` Jiri Olsa
2022-11-23 16:18 ` Yonghong Song
2023-09-08 16:54 ` Daniel Borkmann
1 sibling, 1 reply; 11+ messages in thread
From: Jiri Olsa @ 2022-11-23 12:23 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: bpf, ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, haoluo
On Mon, Nov 21, 2022 at 10:03:40AM -0800, Stanislav Fomichev wrote:
> LWT_XMIT to test L3 case, TC to test L2 case.
>
> v2:
> - s/veth_ifindex/ipip_ifindex/ in two places (Martin)
> - add comment about which condition triggers the rejection (Martin)
>
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
hi,
I'm getting selftest fails and it looks like it's because of this test:
[root@qemu bpf]# ./test_progs -n 62,98
#62 empty_skb:OK
execute_one_variant:PASS:skel_open 0 nsec
execute_one_variant:PASS:my_pid_map_update 0 nsec
libbpf: failed to determine tracepoint 'raw_syscalls/sys_enter' perf event ID: No such file or directory
libbpf: prog 'handle_legacy': failed to create tracepoint 'raw_syscalls/sys_enter' perf event: No such file or directory
libbpf: prog 'handle_legacy': failed to auto-attach: -2
execute_one_variant:FAIL:skel_attach unexpected error: -2 (errno 2)
test_legacy_printk:FAIL:legacy_case unexpected error: -2 (errno 2)
execute_one_variant:PASS:skel_open 0 nsec
libbpf: failed to determine tracepoint 'raw_syscalls/sys_enter' perf event ID: No such file or directory
libbpf: prog 'handle_modern': failed to create tracepoint 'raw_syscalls/sys_enter' perf event: No such file or directory
libbpf: prog 'handle_modern': failed to auto-attach: -2
execute_one_variant:FAIL:skel_attach unexpected error: -2 (errno 2)
#98 legacy_printk:FAIL
All error logs:
execute_one_variant:PASS:skel_open 0 nsec
execute_one_variant:PASS:my_pid_map_update 0 nsec
libbpf: failed to determine tracepoint 'raw_syscalls/sys_enter' perf event ID: No such file or directory
libbpf: prog 'handle_legacy': failed to create tracepoint 'raw_syscalls/sys_enter' perf event: No such file or directory
libbpf: prog 'handle_legacy': failed to auto-attach: -2
execute_one_variant:FAIL:skel_attach unexpected error: -2 (errno 2)
test_legacy_printk:FAIL:legacy_case unexpected error: -2 (errno 2)
execute_one_variant:PASS:skel_open 0 nsec
libbpf: failed to determine tracepoint 'raw_syscalls/sys_enter' perf event ID: No such file or directory
libbpf: prog 'handle_modern': failed to create tracepoint 'raw_syscalls/sys_enter' perf event: No such file or directory
libbpf: prog 'handle_modern': failed to auto-attach: -2
execute_one_variant:FAIL:skel_attach unexpected error: -2 (errno 2)
#98 legacy_printk:FAIL
Summary: 1/0 PASSED, 0 SKIPPED, 1 FAILED
when I run separately it passes:
[root@qemu bpf]# ./test_progs -n 98
#98 legacy_printk:OK
Summary: 1/0 PASSED, 0 SKIPPED, 0 FAILED
it seems that the open_netns/close_netns does not work properly,
and screw up access to tracefs for following tests
if I comment out all the umounts in setns_by_fd, it does not fail
jirka
> ---
> .../selftests/bpf/prog_tests/empty_skb.c | 146 ++++++++++++++++++
> tools/testing/selftests/bpf/progs/empty_skb.c | 37 +++++
> 2 files changed, 183 insertions(+)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/empty_skb.c
> create mode 100644 tools/testing/selftests/bpf/progs/empty_skb.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/empty_skb.c b/tools/testing/selftests/bpf/prog_tests/empty_skb.c
> new file mode 100644
> index 000000000000..32dd731e9070
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/empty_skb.c
> @@ -0,0 +1,146 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <test_progs.h>
> +#include <network_helpers.h>
> +#include <net/if.h>
> +#include "empty_skb.skel.h"
> +
> +#define SYS(cmd) ({ \
> + if (!ASSERT_OK(system(cmd), (cmd))) \
> + goto out; \
> +})
> +
> +void test_empty_skb(void)
> +{
> + LIBBPF_OPTS(bpf_test_run_opts, tattr);
> + struct empty_skb *bpf_obj = NULL;
> + struct nstoken *tok = NULL;
> + struct bpf_program *prog;
> + char eth_hlen_pp[15];
> + char eth_hlen[14];
> + int veth_ifindex;
> + int ipip_ifindex;
> + int err;
> + int i;
> +
> + struct {
> + const char *msg;
> + const void *data_in;
> + __u32 data_size_in;
> + int *ifindex;
> + int err;
> + int ret;
> + bool success_on_tc;
> + } tests[] = {
> + /* Empty packets are always rejected. */
> +
> + {
> + /* BPF_PROG_RUN ETH_HLEN size check */
> + .msg = "veth empty ingress packet",
> + .data_in = NULL,
> + .data_size_in = 0,
> + .ifindex = &veth_ifindex,
> + .err = -EINVAL,
> + },
> + {
> + /* BPF_PROG_RUN ETH_HLEN size check */
> + .msg = "ipip empty ingress packet",
> + .data_in = NULL,
> + .data_size_in = 0,
> + .ifindex = &ipip_ifindex,
> + .err = -EINVAL,
> + },
> +
> + /* ETH_HLEN-sized packets:
> + * - can not be redirected at LWT_XMIT
> + * - can be redirected at TC to non-tunneling dest
> + */
> +
> + {
> + /* __bpf_redirect_common */
> + .msg = "veth ETH_HLEN packet ingress",
> + .data_in = eth_hlen,
> + .data_size_in = sizeof(eth_hlen),
> + .ifindex = &veth_ifindex,
> + .ret = -ERANGE,
> + .success_on_tc = true,
> + },
> + {
> + /* __bpf_redirect_no_mac
> + *
> + * lwt: skb->len=0 <= skb_network_offset=0
> + * tc: skb->len=14 <= skb_network_offset=14
> + */
> + .msg = "ipip ETH_HLEN packet ingress",
> + .data_in = eth_hlen,
> + .data_size_in = sizeof(eth_hlen),
> + .ifindex = &ipip_ifindex,
> + .ret = -ERANGE,
> + },
> +
> + /* ETH_HLEN+1-sized packet should be redirected. */
> +
> + {
> + .msg = "veth ETH_HLEN+1 packet ingress",
> + .data_in = eth_hlen_pp,
> + .data_size_in = sizeof(eth_hlen_pp),
> + .ifindex = &veth_ifindex,
> + },
> + {
> + .msg = "ipip ETH_HLEN+1 packet ingress",
> + .data_in = eth_hlen_pp,
> + .data_size_in = sizeof(eth_hlen_pp),
> + .ifindex = &ipip_ifindex,
> + },
> + };
> +
> + SYS("ip netns add empty_skb");
> + tok = open_netns("empty_skb");
> + SYS("ip link add veth0 type veth peer veth1");
> + SYS("ip link set dev veth0 up");
> + SYS("ip link set dev veth1 up");
> + SYS("ip addr add 10.0.0.1/8 dev veth0");
> + SYS("ip addr add 10.0.0.2/8 dev veth1");
> + veth_ifindex = if_nametoindex("veth0");
> +
> + SYS("ip link add ipip0 type ipip local 10.0.0.1 remote 10.0.0.2");
> + SYS("ip link set ipip0 up");
> + SYS("ip addr add 192.168.1.1/16 dev ipip0");
> + ipip_ifindex = if_nametoindex("ipip0");
> +
> + bpf_obj = empty_skb__open_and_load();
> + if (!ASSERT_OK_PTR(bpf_obj, "open skeleton"))
> + goto out;
> +
> + for (i = 0; i < ARRAY_SIZE(tests); i++) {
> + bpf_object__for_each_program(prog, bpf_obj->obj) {
> + char buf[128];
> + bool at_tc = !strncmp(bpf_program__section_name(prog), "tc", 2);
> +
> + tattr.data_in = tests[i].data_in;
> + tattr.data_size_in = tests[i].data_size_in;
> +
> + tattr.data_size_out = 0;
> + bpf_obj->bss->ifindex = *tests[i].ifindex;
> + bpf_obj->bss->ret = 0;
> + err = bpf_prog_test_run_opts(bpf_program__fd(prog), &tattr);
> + sprintf(buf, "err: %s [%s]", tests[i].msg, bpf_program__name(prog));
> +
> + if (at_tc && tests[i].success_on_tc)
> + ASSERT_GE(err, 0, buf);
> + else
> + ASSERT_EQ(err, tests[i].err, buf);
> + sprintf(buf, "ret: %s [%s]", tests[i].msg, bpf_program__name(prog));
> + if (at_tc && tests[i].success_on_tc)
> + ASSERT_GE(bpf_obj->bss->ret, 0, buf);
> + else
> + ASSERT_EQ(bpf_obj->bss->ret, tests[i].ret, buf);
> + }
> + }
> +
> +out:
> + if (bpf_obj)
> + empty_skb__destroy(bpf_obj);
> + if (tok)
> + close_netns(tok);
> + system("ip netns del empty_skb");
> +}
> diff --git a/tools/testing/selftests/bpf/progs/empty_skb.c b/tools/testing/selftests/bpf/progs/empty_skb.c
> new file mode 100644
> index 000000000000..4b0cd6753251
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/empty_skb.c
> @@ -0,0 +1,37 @@
> +// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause
> +#include <linux/bpf.h>
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_endian.h>
> +
> +char _license[] SEC("license") = "GPL";
> +
> +int ifindex;
> +int ret;
> +
> +SEC("lwt_xmit")
> +int redirect_ingress(struct __sk_buff *skb)
> +{
> + ret = bpf_clone_redirect(skb, ifindex, BPF_F_INGRESS);
> + return 0;
> +}
> +
> +SEC("lwt_xmit")
> +int redirect_egress(struct __sk_buff *skb)
> +{
> + ret = bpf_clone_redirect(skb, ifindex, 0);
> + return 0;
> +}
> +
> +SEC("tc")
> +int tc_redirect_ingress(struct __sk_buff *skb)
> +{
> + ret = bpf_clone_redirect(skb, ifindex, BPF_F_INGRESS);
> + return 0;
> +}
> +
> +SEC("tc")
> +int tc_redirect_egress(struct __sk_buff *skb)
> +{
> + ret = bpf_clone_redirect(skb, ifindex, 0);
> + return 0;
> +}
> --
> 2.38.1.584.g0f3c55d4c2-goog
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next v2 2/2] selftests/bpf: Make sure zero-len skbs aren't redirectable
2022-11-23 12:23 ` Jiri Olsa
@ 2022-11-23 16:18 ` Yonghong Song
2022-11-23 17:17 ` sdf
0 siblings, 1 reply; 11+ messages in thread
From: Yonghong Song @ 2022-11-23 16:18 UTC (permalink / raw)
To: Jiri Olsa, Stanislav Fomichev
Cc: bpf, ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, haoluo
On 11/23/22 4:23 AM, Jiri Olsa wrote:
> On Mon, Nov 21, 2022 at 10:03:40AM -0800, Stanislav Fomichev wrote:
>> LWT_XMIT to test L3 case, TC to test L2 case.
>>
>> v2:
>> - s/veth_ifindex/ipip_ifindex/ in two places (Martin)
>> - add comment about which condition triggers the rejection (Martin)
>>
>> Signed-off-by: Stanislav Fomichev <sdf@google.com>
>
> hi,
> I'm getting selftest fails and it looks like it's because of this test:
>
> [root@qemu bpf]# ./test_progs -n 62,98
> #62 empty_skb:OK
> execute_one_variant:PASS:skel_open 0 nsec
> execute_one_variant:PASS:my_pid_map_update 0 nsec
> libbpf: failed to determine tracepoint 'raw_syscalls/sys_enter' perf event ID: No such file or directory
> libbpf: prog 'handle_legacy': failed to create tracepoint 'raw_syscalls/sys_enter' perf event: No such file or directory
> libbpf: prog 'handle_legacy': failed to auto-attach: -2
> execute_one_variant:FAIL:skel_attach unexpected error: -2 (errno 2)
> test_legacy_printk:FAIL:legacy_case unexpected error: -2 (errno 2)
> execute_one_variant:PASS:skel_open 0 nsec
> libbpf: failed to determine tracepoint 'raw_syscalls/sys_enter' perf event ID: No such file or directory
> libbpf: prog 'handle_modern': failed to create tracepoint 'raw_syscalls/sys_enter' perf event: No such file or directory
> libbpf: prog 'handle_modern': failed to auto-attach: -2
> execute_one_variant:FAIL:skel_attach unexpected error: -2 (errno 2)
> #98 legacy_printk:FAIL
>
> All error logs:
> execute_one_variant:PASS:skel_open 0 nsec
> execute_one_variant:PASS:my_pid_map_update 0 nsec
> libbpf: failed to determine tracepoint 'raw_syscalls/sys_enter' perf event ID: No such file or directory
> libbpf: prog 'handle_legacy': failed to create tracepoint 'raw_syscalls/sys_enter' perf event: No such file or directory
> libbpf: prog 'handle_legacy': failed to auto-attach: -2
> execute_one_variant:FAIL:skel_attach unexpected error: -2 (errno 2)
> test_legacy_printk:FAIL:legacy_case unexpected error: -2 (errno 2)
> execute_one_variant:PASS:skel_open 0 nsec
> libbpf: failed to determine tracepoint 'raw_syscalls/sys_enter' perf event ID: No such file or directory
> libbpf: prog 'handle_modern': failed to create tracepoint 'raw_syscalls/sys_enter' perf event: No such file or directory
> libbpf: prog 'handle_modern': failed to auto-attach: -2
> execute_one_variant:FAIL:skel_attach unexpected error: -2 (errno 2)
> #98 legacy_printk:FAIL
> Summary: 1/0 PASSED, 0 SKIPPED, 1 FAILED
>
> when I run separately it passes:
>
> [root@qemu bpf]# ./test_progs -n 98
> #98 legacy_printk:OK
> Summary: 1/0 PASSED, 0 SKIPPED, 0 FAILED
>
>
> it seems that the open_netns/close_netns does not work properly,
> and screw up access to tracefs for following tests
>
> if I comment out all the umounts in setns_by_fd, it does not fail
Agreed with the above observations.
With the current bpf-next, I can easily hit the above perf event ID issue.
But if I backout the following two patches:
68f8e3d4b916531ea3bb8b83e35138cf78f2fce5 selftests/bpf: Make sure
zero-len skbs aren't redirectable
114039b342014680911c35bd6b72624180fd669a bpf: Move skb->len == 0 checks
into __bpf_redirect
and run a few times with './test_progs -j' and I didn't hit any issues.
>
> jirka
>
>
>> ---
>> .../selftests/bpf/prog_tests/empty_skb.c | 146 ++++++++++++++++++
>> tools/testing/selftests/bpf/progs/empty_skb.c | 37 +++++
>> 2 files changed, 183 insertions(+)
>> create mode 100644 tools/testing/selftests/bpf/prog_tests/empty_skb.c
>> create mode 100644 tools/testing/selftests/bpf/progs/empty_skb.c
>>
>> diff --git a/tools/testing/selftests/bpf/prog_tests/empty_skb.c b/tools/testing/selftests/bpf/prog_tests/empty_skb.c
>> new file mode 100644
>> index 000000000000..32dd731e9070
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/prog_tests/empty_skb.c
>> @@ -0,0 +1,146 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +#include <test_progs.h>
>> +#include <network_helpers.h>
>> +#include <net/if.h>
>> +#include "empty_skb.skel.h"
>> +
>> +#define SYS(cmd) ({ \
>> + if (!ASSERT_OK(system(cmd), (cmd))) \
>> + goto out; \
>> +})
>> +
>> +void test_empty_skb(void)
>> +{
>> + LIBBPF_OPTS(bpf_test_run_opts, tattr);
>> + struct empty_skb *bpf_obj = NULL;
>> + struct nstoken *tok = NULL;
>> + struct bpf_program *prog;
>> + char eth_hlen_pp[15];
>> + char eth_hlen[14];
>> + int veth_ifindex;
>> + int ipip_ifindex;
>> + int err;
>> + int i;
>> +
>> + struct {
>> + const char *msg;
>> + const void *data_in;
>> + __u32 data_size_in;
>> + int *ifindex;
>> + int err;
>> + int ret;
>> + bool success_on_tc;
>> + } tests[] = {
>> + /* Empty packets are always rejected. */
>> +
>> + {
>> + /* BPF_PROG_RUN ETH_HLEN size check */
>> + .msg = "veth empty ingress packet",
>> + .data_in = NULL,
>> + .data_size_in = 0,
>> + .ifindex = &veth_ifindex,
>> + .err = -EINVAL,
>> + },
>> + {
>> + /* BPF_PROG_RUN ETH_HLEN size check */
>> + .msg = "ipip empty ingress packet",
>> + .data_in = NULL,
>> + .data_size_in = 0,
>> + .ifindex = &ipip_ifindex,
>> + .err = -EINVAL,
>> + },
>> +
>> + /* ETH_HLEN-sized packets:
>> + * - can not be redirected at LWT_XMIT
>> + * - can be redirected at TC to non-tunneling dest
>> + */
>> +
>> + {
>> + /* __bpf_redirect_common */
>> + .msg = "veth ETH_HLEN packet ingress",
>> + .data_in = eth_hlen,
>> + .data_size_in = sizeof(eth_hlen),
>> + .ifindex = &veth_ifindex,
>> + .ret = -ERANGE,
>> + .success_on_tc = true,
>> + },
>> + {
>> + /* __bpf_redirect_no_mac
>> + *
>> + * lwt: skb->len=0 <= skb_network_offset=0
>> + * tc: skb->len=14 <= skb_network_offset=14
>> + */
>> + .msg = "ipip ETH_HLEN packet ingress",
>> + .data_in = eth_hlen,
>> + .data_size_in = sizeof(eth_hlen),
>> + .ifindex = &ipip_ifindex,
>> + .ret = -ERANGE,
>> + },
>> +
>> + /* ETH_HLEN+1-sized packet should be redirected. */
>> +
>> + {
>> + .msg = "veth ETH_HLEN+1 packet ingress",
>> + .data_in = eth_hlen_pp,
>> + .data_size_in = sizeof(eth_hlen_pp),
>> + .ifindex = &veth_ifindex,
>> + },
>> + {
>> + .msg = "ipip ETH_HLEN+1 packet ingress",
>> + .data_in = eth_hlen_pp,
>> + .data_size_in = sizeof(eth_hlen_pp),
>> + .ifindex = &ipip_ifindex,
>> + },
>> + };
>> +
>> + SYS("ip netns add empty_skb");
>> + tok = open_netns("empty_skb");
>> + SYS("ip link add veth0 type veth peer veth1");
>> + SYS("ip link set dev veth0 up");
>> + SYS("ip link set dev veth1 up");
>> + SYS("ip addr add 10.0.0.1/8 dev veth0");
>> + SYS("ip addr add 10.0.0.2/8 dev veth1");
>> + veth_ifindex = if_nametoindex("veth0");
>> +
>> + SYS("ip link add ipip0 type ipip local 10.0.0.1 remote 10.0.0.2");
>> + SYS("ip link set ipip0 up");
>> + SYS("ip addr add 192.168.1.1/16 dev ipip0");
>> + ipip_ifindex = if_nametoindex("ipip0");
>> +
>> + bpf_obj = empty_skb__open_and_load();
>> + if (!ASSERT_OK_PTR(bpf_obj, "open skeleton"))
>> + goto out;
>> +
>> + for (i = 0; i < ARRAY_SIZE(tests); i++) {
>> + bpf_object__for_each_program(prog, bpf_obj->obj) {
>> + char buf[128];
>> + bool at_tc = !strncmp(bpf_program__section_name(prog), "tc", 2);
>> +
>> + tattr.data_in = tests[i].data_in;
>> + tattr.data_size_in = tests[i].data_size_in;
>> +
>> + tattr.data_size_out = 0;
>> + bpf_obj->bss->ifindex = *tests[i].ifindex;
>> + bpf_obj->bss->ret = 0;
>> + err = bpf_prog_test_run_opts(bpf_program__fd(prog), &tattr);
>> + sprintf(buf, "err: %s [%s]", tests[i].msg, bpf_program__name(prog));
>> +
>> + if (at_tc && tests[i].success_on_tc)
>> + ASSERT_GE(err, 0, buf);
>> + else
>> + ASSERT_EQ(err, tests[i].err, buf);
>> + sprintf(buf, "ret: %s [%s]", tests[i].msg, bpf_program__name(prog));
>> + if (at_tc && tests[i].success_on_tc)
>> + ASSERT_GE(bpf_obj->bss->ret, 0, buf);
>> + else
>> + ASSERT_EQ(bpf_obj->bss->ret, tests[i].ret, buf);
>> + }
>> + }
>> +
>> +out:
>> + if (bpf_obj)
>> + empty_skb__destroy(bpf_obj);
>> + if (tok)
>> + close_netns(tok);
>> + system("ip netns del empty_skb");
>> +}
>> diff --git a/tools/testing/selftests/bpf/progs/empty_skb.c b/tools/testing/selftests/bpf/progs/empty_skb.c
>> new file mode 100644
>> index 000000000000..4b0cd6753251
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/progs/empty_skb.c
>> @@ -0,0 +1,37 @@
>> +// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause
>> +#include <linux/bpf.h>
>> +#include <bpf/bpf_helpers.h>
>> +#include <bpf/bpf_endian.h>
>> +
>> +char _license[] SEC("license") = "GPL";
>> +
>> +int ifindex;
>> +int ret;
>> +
>> +SEC("lwt_xmit")
>> +int redirect_ingress(struct __sk_buff *skb)
>> +{
>> + ret = bpf_clone_redirect(skb, ifindex, BPF_F_INGRESS);
>> + return 0;
>> +}
>> +
>> +SEC("lwt_xmit")
>> +int redirect_egress(struct __sk_buff *skb)
>> +{
>> + ret = bpf_clone_redirect(skb, ifindex, 0);
>> + return 0;
>> +}
>> +
>> +SEC("tc")
>> +int tc_redirect_ingress(struct __sk_buff *skb)
>> +{
>> + ret = bpf_clone_redirect(skb, ifindex, BPF_F_INGRESS);
>> + return 0;
>> +}
>> +
>> +SEC("tc")
>> +int tc_redirect_egress(struct __sk_buff *skb)
>> +{
>> + ret = bpf_clone_redirect(skb, ifindex, 0);
>> + return 0;
>> +}
>> --
>> 2.38.1.584.g0f3c55d4c2-goog
>>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next v2 2/2] selftests/bpf: Make sure zero-len skbs aren't redirectable
2022-11-23 16:18 ` Yonghong Song
@ 2022-11-23 17:17 ` sdf
2022-11-23 19:07 ` Yonghong Song
0 siblings, 1 reply; 11+ messages in thread
From: sdf @ 2022-11-23 17:17 UTC (permalink / raw)
To: Yonghong Song
Cc: Jiri Olsa, bpf, ast, daniel, andrii, martin.lau, song, yhs,
john.fastabend, kpsingh, haoluo
On 11/23, Yonghong Song wrote:
> On 11/23/22 4:23 AM, Jiri Olsa wrote:
> > On Mon, Nov 21, 2022 at 10:03:40AM -0800, Stanislav Fomichev wrote:
> > > LWT_XMIT to test L3 case, TC to test L2 case.
> > >
> > > v2:
> > > - s/veth_ifindex/ipip_ifindex/ in two places (Martin)
> > > - add comment about which condition triggers the rejection (Martin)
> > >
> > > Signed-off-by: Stanislav Fomichev <sdf@google.com>
> >
> > hi,
> > I'm getting selftest fails and it looks like it's because of this test:
> >
> > [root@qemu bpf]# ./test_progs -n 62,98
> > #62 empty_skb:OK
> > execute_one_variant:PASS:skel_open 0 nsec
> > execute_one_variant:PASS:my_pid_map_update 0 nsec
> > libbpf: failed to determine tracepoint 'raw_syscalls/sys_enter' perf
> event ID: No such file or directory
> > libbpf: prog 'handle_legacy': failed to create
> tracepoint 'raw_syscalls/sys_enter' perf event: No such file or directory
> > libbpf: prog 'handle_legacy': failed to auto-attach: -2
> > execute_one_variant:FAIL:skel_attach unexpected error: -2 (errno 2)
> > test_legacy_printk:FAIL:legacy_case unexpected error: -2 (errno 2)
> > execute_one_variant:PASS:skel_open 0 nsec
> > libbpf: failed to determine tracepoint 'raw_syscalls/sys_enter' perf
> event ID: No such file or directory
> > libbpf: prog 'handle_modern': failed to create
> tracepoint 'raw_syscalls/sys_enter' perf event: No such file or directory
> > libbpf: prog 'handle_modern': failed to auto-attach: -2
> > execute_one_variant:FAIL:skel_attach unexpected error: -2 (errno 2)
> > #98 legacy_printk:FAIL
> >
> > All error logs:
> > execute_one_variant:PASS:skel_open 0 nsec
> > execute_one_variant:PASS:my_pid_map_update 0 nsec
> > libbpf: failed to determine tracepoint 'raw_syscalls/sys_enter' perf
> event ID: No such file or directory
> > libbpf: prog 'handle_legacy': failed to create
> tracepoint 'raw_syscalls/sys_enter' perf event: No such file or directory
> > libbpf: prog 'handle_legacy': failed to auto-attach: -2
> > execute_one_variant:FAIL:skel_attach unexpected error: -2 (errno 2)
> > test_legacy_printk:FAIL:legacy_case unexpected error: -2 (errno 2)
> > execute_one_variant:PASS:skel_open 0 nsec
> > libbpf: failed to determine tracepoint 'raw_syscalls/sys_enter' perf
> event ID: No such file or directory
> > libbpf: prog 'handle_modern': failed to create
> tracepoint 'raw_syscalls/sys_enter' perf event: No such file or directory
> > libbpf: prog 'handle_modern': failed to auto-attach: -2
> > execute_one_variant:FAIL:skel_attach unexpected error: -2 (errno 2)
> > #98 legacy_printk:FAIL
> > Summary: 1/0 PASSED, 0 SKIPPED, 1 FAILED
> >
> > when I run separately it passes:
> >
> > [root@qemu bpf]# ./test_progs -n 98
> > #98 legacy_printk:OK
> > Summary: 1/0 PASSED, 0 SKIPPED, 0 FAILED
> >
> >
> > it seems that the open_netns/close_netns does not work properly,
> > and screw up access to tracefs for following tests
> >
> > if I comment out all the umounts in setns_by_fd, it does not fail
> Agreed with the above observations.
> With the current bpf-next, I can easily hit the above perf event ID issue.
> But if I backout the following two patches:
> 68f8e3d4b916531ea3bb8b83e35138cf78f2fce5 selftests/bpf: Make sure zero-len
> skbs aren't redirectable
> 114039b342014680911c35bd6b72624180fd669a bpf: Move skb->len == 0 checks
> into
> __bpf_redirect
> and run a few times with './test_progs -j' and I didn't hit any issues.
My guess would be that we need to remount debugfs in setns_by_fd?
diff --git a/tools/testing/selftests/bpf/network_helpers.c
b/tools/testing/selftests/bpf/network_helpers.c
index bec15558fd93..1f37adff7632 100644
--- a/tools/testing/selftests/bpf/network_helpers.c
+++ b/tools/testing/selftests/bpf/network_helpers.c
@@ -426,6 +426,10 @@ static int setns_by_fd(int nsfd)
if (!ASSERT_OK(err, "mount /sys/fs/bpf"))
return err;
+ err = mount("debugfs", "/sys/kernel/debug", "debugfs", 0, NULL);
+ if (!ASSERT_OK(err, "mount /sys/kernel/debug"))
+ return err;
+
return 0;
}
> >
> > jirka
> >
> >
> > > ---
> > > .../selftests/bpf/prog_tests/empty_skb.c | 146
> ++++++++++++++++++
> > > tools/testing/selftests/bpf/progs/empty_skb.c | 37 +++++
> > > 2 files changed, 183 insertions(+)
> > > create mode 100644
> tools/testing/selftests/bpf/prog_tests/empty_skb.c
> > > create mode 100644 tools/testing/selftests/bpf/progs/empty_skb.c
> > >
> > > diff --git a/tools/testing/selftests/bpf/prog_tests/empty_skb.c
> b/tools/testing/selftests/bpf/prog_tests/empty_skb.c
> > > new file mode 100644
> > > index 000000000000..32dd731e9070
> > > --- /dev/null
> > > +++ b/tools/testing/selftests/bpf/prog_tests/empty_skb.c
> > > @@ -0,0 +1,146 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +#include <test_progs.h>
> > > +#include <network_helpers.h>
> > > +#include <net/if.h>
> > > +#include "empty_skb.skel.h"
> > > +
> > > +#define SYS(cmd) ({ \
> > > + if (!ASSERT_OK(system(cmd), (cmd))) \
> > > + goto out; \
> > > +})
> > > +
> > > +void test_empty_skb(void)
> > > +{
> > > + LIBBPF_OPTS(bpf_test_run_opts, tattr);
> > > + struct empty_skb *bpf_obj = NULL;
> > > + struct nstoken *tok = NULL;
> > > + struct bpf_program *prog;
> > > + char eth_hlen_pp[15];
> > > + char eth_hlen[14];
> > > + int veth_ifindex;
> > > + int ipip_ifindex;
> > > + int err;
> > > + int i;
> > > +
> > > + struct {
> > > + const char *msg;
> > > + const void *data_in;
> > > + __u32 data_size_in;
> > > + int *ifindex;
> > > + int err;
> > > + int ret;
> > > + bool success_on_tc;
> > > + } tests[] = {
> > > + /* Empty packets are always rejected. */
> > > +
> > > + {
> > > + /* BPF_PROG_RUN ETH_HLEN size check */
> > > + .msg = "veth empty ingress packet",
> > > + .data_in = NULL,
> > > + .data_size_in = 0,
> > > + .ifindex = &veth_ifindex,
> > > + .err = -EINVAL,
> > > + },
> > > + {
> > > + /* BPF_PROG_RUN ETH_HLEN size check */
> > > + .msg = "ipip empty ingress packet",
> > > + .data_in = NULL,
> > > + .data_size_in = 0,
> > > + .ifindex = &ipip_ifindex,
> > > + .err = -EINVAL,
> > > + },
> > > +
> > > + /* ETH_HLEN-sized packets:
> > > + * - can not be redirected at LWT_XMIT
> > > + * - can be redirected at TC to non-tunneling dest
> > > + */
> > > +
> > > + {
> > > + /* __bpf_redirect_common */
> > > + .msg = "veth ETH_HLEN packet ingress",
> > > + .data_in = eth_hlen,
> > > + .data_size_in = sizeof(eth_hlen),
> > > + .ifindex = &veth_ifindex,
> > > + .ret = -ERANGE,
> > > + .success_on_tc = true,
> > > + },
> > > + {
> > > + /* __bpf_redirect_no_mac
> > > + *
> > > + * lwt: skb->len=0 <= skb_network_offset=0
> > > + * tc: skb->len=14 <= skb_network_offset=14
> > > + */
> > > + .msg = "ipip ETH_HLEN packet ingress",
> > > + .data_in = eth_hlen,
> > > + .data_size_in = sizeof(eth_hlen),
> > > + .ifindex = &ipip_ifindex,
> > > + .ret = -ERANGE,
> > > + },
> > > +
> > > + /* ETH_HLEN+1-sized packet should be redirected. */
> > > +
> > > + {
> > > + .msg = "veth ETH_HLEN+1 packet ingress",
> > > + .data_in = eth_hlen_pp,
> > > + .data_size_in = sizeof(eth_hlen_pp),
> > > + .ifindex = &veth_ifindex,
> > > + },
> > > + {
> > > + .msg = "ipip ETH_HLEN+1 packet ingress",
> > > + .data_in = eth_hlen_pp,
> > > + .data_size_in = sizeof(eth_hlen_pp),
> > > + .ifindex = &ipip_ifindex,
> > > + },
> > > + };
> > > +
> > > + SYS("ip netns add empty_skb");
> > > + tok = open_netns("empty_skb");
> > > + SYS("ip link add veth0 type veth peer veth1");
> > > + SYS("ip link set dev veth0 up");
> > > + SYS("ip link set dev veth1 up");
> > > + SYS("ip addr add 10.0.0.1/8 dev veth0");
> > > + SYS("ip addr add 10.0.0.2/8 dev veth1");
> > > + veth_ifindex = if_nametoindex("veth0");
> > > +
> > > + SYS("ip link add ipip0 type ipip local 10.0.0.1 remote 10.0.0.2");
> > > + SYS("ip link set ipip0 up");
> > > + SYS("ip addr add 192.168.1.1/16 dev ipip0");
> > > + ipip_ifindex = if_nametoindex("ipip0");
> > > +
> > > + bpf_obj = empty_skb__open_and_load();
> > > + if (!ASSERT_OK_PTR(bpf_obj, "open skeleton"))
> > > + goto out;
> > > +
> > > + for (i = 0; i < ARRAY_SIZE(tests); i++) {
> > > + bpf_object__for_each_program(prog, bpf_obj->obj) {
> > > + char buf[128];
> > > + bool at_tc = !strncmp(bpf_program__section_name(prog), "tc", 2);
> > > +
> > > + tattr.data_in = tests[i].data_in;
> > > + tattr.data_size_in = tests[i].data_size_in;
> > > +
> > > + tattr.data_size_out = 0;
> > > + bpf_obj->bss->ifindex = *tests[i].ifindex;
> > > + bpf_obj->bss->ret = 0;
> > > + err = bpf_prog_test_run_opts(bpf_program__fd(prog), &tattr);
> > > + sprintf(buf, "err: %s [%s]", tests[i].msg,
> bpf_program__name(prog));
> > > +
> > > + if (at_tc && tests[i].success_on_tc)
> > > + ASSERT_GE(err, 0, buf);
> > > + else
> > > + ASSERT_EQ(err, tests[i].err, buf);
> > > + sprintf(buf, "ret: %s [%s]", tests[i].msg,
> bpf_program__name(prog));
> > > + if (at_tc && tests[i].success_on_tc)
> > > + ASSERT_GE(bpf_obj->bss->ret, 0, buf);
> > > + else
> > > + ASSERT_EQ(bpf_obj->bss->ret, tests[i].ret, buf);
> > > + }
> > > + }
> > > +
> > > +out:
> > > + if (bpf_obj)
> > > + empty_skb__destroy(bpf_obj);
> > > + if (tok)
> > > + close_netns(tok);
> > > + system("ip netns del empty_skb");
> > > +}
> > > diff --git a/tools/testing/selftests/bpf/progs/empty_skb.c
> b/tools/testing/selftests/bpf/progs/empty_skb.c
> > > new file mode 100644
> > > index 000000000000..4b0cd6753251
> > > --- /dev/null
> > > +++ b/tools/testing/selftests/bpf/progs/empty_skb.c
> > > @@ -0,0 +1,37 @@
> > > +// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause
> > > +#include <linux/bpf.h>
> > > +#include <bpf/bpf_helpers.h>
> > > +#include <bpf/bpf_endian.h>
> > > +
> > > +char _license[] SEC("license") = "GPL";
> > > +
> > > +int ifindex;
> > > +int ret;
> > > +
> > > +SEC("lwt_xmit")
> > > +int redirect_ingress(struct __sk_buff *skb)
> > > +{
> > > + ret = bpf_clone_redirect(skb, ifindex, BPF_F_INGRESS);
> > > + return 0;
> > > +}
> > > +
> > > +SEC("lwt_xmit")
> > > +int redirect_egress(struct __sk_buff *skb)
> > > +{
> > > + ret = bpf_clone_redirect(skb, ifindex, 0);
> > > + return 0;
> > > +}
> > > +
> > > +SEC("tc")
> > > +int tc_redirect_ingress(struct __sk_buff *skb)
> > > +{
> > > + ret = bpf_clone_redirect(skb, ifindex, BPF_F_INGRESS);
> > > + return 0;
> > > +}
> > > +
> > > +SEC("tc")
> > > +int tc_redirect_egress(struct __sk_buff *skb)
> > > +{
> > > + ret = bpf_clone_redirect(skb, ifindex, 0);
> > > + return 0;
> > > +}
> > > --
> > > 2.38.1.584.g0f3c55d4c2-goog
> > >
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next v2 2/2] selftests/bpf: Make sure zero-len skbs aren't redirectable
2022-11-23 17:17 ` sdf
@ 2022-11-23 19:07 ` Yonghong Song
2022-11-23 19:54 ` Stanislav Fomichev
2022-11-23 21:20 ` Jiri Olsa
0 siblings, 2 replies; 11+ messages in thread
From: Yonghong Song @ 2022-11-23 19:07 UTC (permalink / raw)
To: sdf
Cc: Jiri Olsa, bpf, ast, daniel, andrii, martin.lau, song, yhs,
john.fastabend, kpsingh, haoluo
On 11/23/22 9:17 AM, sdf@google.com wrote:
> On 11/23, Yonghong Song wrote:
>
>
>> On 11/23/22 4:23 AM, Jiri Olsa wrote:
>> > On Mon, Nov 21, 2022 at 10:03:40AM -0800, Stanislav Fomichev wrote:
>> > > LWT_XMIT to test L3 case, TC to test L2 case.
>> > >
>> > > v2:
>> > > - s/veth_ifindex/ipip_ifindex/ in two places (Martin)
>> > > - add comment about which condition triggers the rejection (Martin)
>> > >
>> > > Signed-off-by: Stanislav Fomichev <sdf@google.com>
>> >
>> > hi,
>> > I'm getting selftest fails and it looks like it's because of this test:
>> >
>> > [root@qemu bpf]# ./test_progs -n 62,98
>> > #62 empty_skb:OK
>> > execute_one_variant:PASS:skel_open 0 nsec
>> > execute_one_variant:PASS:my_pid_map_update 0 nsec
>> > libbpf: failed to determine tracepoint 'raw_syscalls/sys_enter'
>> perf event ID: No such file or directory
>> > libbpf: prog 'handle_legacy': failed to create tracepoint
>> 'raw_syscalls/sys_enter' perf event: No such file or directory
>> > libbpf: prog 'handle_legacy': failed to auto-attach: -2
>> > execute_one_variant:FAIL:skel_attach unexpected error: -2 (errno 2)
>> > test_legacy_printk:FAIL:legacy_case unexpected error: -2 (errno 2)
>> > execute_one_variant:PASS:skel_open 0 nsec
>> > libbpf: failed to determine tracepoint 'raw_syscalls/sys_enter'
>> perf event ID: No such file or directory
>> > libbpf: prog 'handle_modern': failed to create tracepoint
>> 'raw_syscalls/sys_enter' perf event: No such file or directory
>> > libbpf: prog 'handle_modern': failed to auto-attach: -2
>> > execute_one_variant:FAIL:skel_attach unexpected error: -2 (errno 2)
>> > #98 legacy_printk:FAIL
>> >
>> > All error logs:
>> > execute_one_variant:PASS:skel_open 0 nsec
>> > execute_one_variant:PASS:my_pid_map_update 0 nsec
>> > libbpf: failed to determine tracepoint 'raw_syscalls/sys_enter'
>> perf event ID: No such file or directory
>> > libbpf: prog 'handle_legacy': failed to create tracepoint
>> 'raw_syscalls/sys_enter' perf event: No such file or directory
>> > libbpf: prog 'handle_legacy': failed to auto-attach: -2
>> > execute_one_variant:FAIL:skel_attach unexpected error: -2 (errno 2)
>> > test_legacy_printk:FAIL:legacy_case unexpected error: -2 (errno 2)
>> > execute_one_variant:PASS:skel_open 0 nsec
>> > libbpf: failed to determine tracepoint 'raw_syscalls/sys_enter'
>> perf event ID: No such file or directory
>> > libbpf: prog 'handle_modern': failed to create tracepoint
>> 'raw_syscalls/sys_enter' perf event: No such file or directory
>> > libbpf: prog 'handle_modern': failed to auto-attach: -2
>> > execute_one_variant:FAIL:skel_attach unexpected error: -2 (errno 2)
>> > #98 legacy_printk:FAIL
>> > Summary: 1/0 PASSED, 0 SKIPPED, 1 FAILED
>> >
>> > when I run separately it passes:
>> >
>> > [root@qemu bpf]# ./test_progs -n 98
>> > #98 legacy_printk:OK
>> > Summary: 1/0 PASSED, 0 SKIPPED, 0 FAILED
>> >
>> >
>> > it seems that the open_netns/close_netns does not work properly,
>> > and screw up access to tracefs for following tests
>> >
>> > if I comment out all the umounts in setns_by_fd, it does not fail
>
>> Agreed with the above observations.
>> With the current bpf-next, I can easily hit the above perf event ID
>> issue.
>
>> But if I backout the following two patches:
>> 68f8e3d4b916531ea3bb8b83e35138cf78f2fce5 selftests/bpf: Make sure
>> zero-len
>> skbs aren't redirectable
>> 114039b342014680911c35bd6b72624180fd669a bpf: Move skb->len == 0
>> checks into
>> __bpf_redirect
>
>
>> and run a few times with './test_progs -j' and I didn't hit any issues.
>
> My guess would be that we need to remount debugfs in setns_by_fd?
>
> diff --git a/tools/testing/selftests/bpf/network_helpers.c
> b/tools/testing/selftests/bpf/network_helpers.c
> index bec15558fd93..1f37adff7632 100644
> --- a/tools/testing/selftests/bpf/network_helpers.c
> +++ b/tools/testing/selftests/bpf/network_helpers.c
> @@ -426,6 +426,10 @@ static int setns_by_fd(int nsfd)
> if (!ASSERT_OK(err, "mount /sys/fs/bpf"))
> return err;
>
> + err = mount("debugfs", "/sys/kernel/debug", "debugfs", 0, NULL);
> + if (!ASSERT_OK(err, "mount /sys/kernel/debug"))
> + return err;
> +
> return 0;
> }
Ya, this does fix the problem. Could you craft a patch for this?
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next v2 2/2] selftests/bpf: Make sure zero-len skbs aren't redirectable
2022-11-23 19:07 ` Yonghong Song
@ 2022-11-23 19:54 ` Stanislav Fomichev
2022-11-23 21:20 ` Jiri Olsa
1 sibling, 0 replies; 11+ messages in thread
From: Stanislav Fomichev @ 2022-11-23 19:54 UTC (permalink / raw)
To: Yonghong Song
Cc: Jiri Olsa, bpf, ast, daniel, andrii, martin.lau, song, yhs,
john.fastabend, kpsingh, haoluo
On Wed, Nov 23, 2022 at 11:07 AM Yonghong Song <yhs@meta.com> wrote:
>
>
>
> On 11/23/22 9:17 AM, sdf@google.com wrote:
> > On 11/23, Yonghong Song wrote:
> >
> >
> >> On 11/23/22 4:23 AM, Jiri Olsa wrote:
> >> > On Mon, Nov 21, 2022 at 10:03:40AM -0800, Stanislav Fomichev wrote:
> >> > > LWT_XMIT to test L3 case, TC to test L2 case.
> >> > >
> >> > > v2:
> >> > > - s/veth_ifindex/ipip_ifindex/ in two places (Martin)
> >> > > - add comment about which condition triggers the rejection (Martin)
> >> > >
> >> > > Signed-off-by: Stanislav Fomichev <sdf@google.com>
> >> >
> >> > hi,
> >> > I'm getting selftest fails and it looks like it's because of this test:
> >> >
> >> > [root@qemu bpf]# ./test_progs -n 62,98
> >> > #62 empty_skb:OK
> >> > execute_one_variant:PASS:skel_open 0 nsec
> >> > execute_one_variant:PASS:my_pid_map_update 0 nsec
> >> > libbpf: failed to determine tracepoint 'raw_syscalls/sys_enter'
> >> perf event ID: No such file or directory
> >> > libbpf: prog 'handle_legacy': failed to create tracepoint
> >> 'raw_syscalls/sys_enter' perf event: No such file or directory
> >> > libbpf: prog 'handle_legacy': failed to auto-attach: -2
> >> > execute_one_variant:FAIL:skel_attach unexpected error: -2 (errno 2)
> >> > test_legacy_printk:FAIL:legacy_case unexpected error: -2 (errno 2)
> >> > execute_one_variant:PASS:skel_open 0 nsec
> >> > libbpf: failed to determine tracepoint 'raw_syscalls/sys_enter'
> >> perf event ID: No such file or directory
> >> > libbpf: prog 'handle_modern': failed to create tracepoint
> >> 'raw_syscalls/sys_enter' perf event: No such file or directory
> >> > libbpf: prog 'handle_modern': failed to auto-attach: -2
> >> > execute_one_variant:FAIL:skel_attach unexpected error: -2 (errno 2)
> >> > #98 legacy_printk:FAIL
> >> >
> >> > All error logs:
> >> > execute_one_variant:PASS:skel_open 0 nsec
> >> > execute_one_variant:PASS:my_pid_map_update 0 nsec
> >> > libbpf: failed to determine tracepoint 'raw_syscalls/sys_enter'
> >> perf event ID: No such file or directory
> >> > libbpf: prog 'handle_legacy': failed to create tracepoint
> >> 'raw_syscalls/sys_enter' perf event: No such file or directory
> >> > libbpf: prog 'handle_legacy': failed to auto-attach: -2
> >> > execute_one_variant:FAIL:skel_attach unexpected error: -2 (errno 2)
> >> > test_legacy_printk:FAIL:legacy_case unexpected error: -2 (errno 2)
> >> > execute_one_variant:PASS:skel_open 0 nsec
> >> > libbpf: failed to determine tracepoint 'raw_syscalls/sys_enter'
> >> perf event ID: No such file or directory
> >> > libbpf: prog 'handle_modern': failed to create tracepoint
> >> 'raw_syscalls/sys_enter' perf event: No such file or directory
> >> > libbpf: prog 'handle_modern': failed to auto-attach: -2
> >> > execute_one_variant:FAIL:skel_attach unexpected error: -2 (errno 2)
> >> > #98 legacy_printk:FAIL
> >> > Summary: 1/0 PASSED, 0 SKIPPED, 1 FAILED
> >> >
> >> > when I run separately it passes:
> >> >
> >> > [root@qemu bpf]# ./test_progs -n 98
> >> > #98 legacy_printk:OK
> >> > Summary: 1/0 PASSED, 0 SKIPPED, 0 FAILED
> >> >
> >> >
> >> > it seems that the open_netns/close_netns does not work properly,
> >> > and screw up access to tracefs for following tests
> >> >
> >> > if I comment out all the umounts in setns_by_fd, it does not fail
> >
> >> Agreed with the above observations.
> >> With the current bpf-next, I can easily hit the above perf event ID
> >> issue.
> >
> >> But if I backout the following two patches:
> >> 68f8e3d4b916531ea3bb8b83e35138cf78f2fce5 selftests/bpf: Make sure
> >> zero-len
> >> skbs aren't redirectable
> >> 114039b342014680911c35bd6b72624180fd669a bpf: Move skb->len == 0
> >> checks into
> >> __bpf_redirect
> >
> >
> >> and run a few times with './test_progs -j' and I didn't hit any issues.
> >
> > My guess would be that we need to remount debugfs in setns_by_fd?
> >
> > diff --git a/tools/testing/selftests/bpf/network_helpers.c
> > b/tools/testing/selftests/bpf/network_helpers.c
> > index bec15558fd93..1f37adff7632 100644
> > --- a/tools/testing/selftests/bpf/network_helpers.c
> > +++ b/tools/testing/selftests/bpf/network_helpers.c
> > @@ -426,6 +426,10 @@ static int setns_by_fd(int nsfd)
> > if (!ASSERT_OK(err, "mount /sys/fs/bpf"))
> > return err;
> >
> > + err = mount("debugfs", "/sys/kernel/debug", "debugfs", 0, NULL);
> > + if (!ASSERT_OK(err, "mount /sys/kernel/debug"))
> > + return err;
> > +
> > return 0;
> > }
>
> Ya, this does fix the problem. Could you craft a patch for this?
Sure, give me a second..
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next v2 2/2] selftests/bpf: Make sure zero-len skbs aren't redirectable
2022-11-23 19:07 ` Yonghong Song
2022-11-23 19:54 ` Stanislav Fomichev
@ 2022-11-23 21:20 ` Jiri Olsa
1 sibling, 0 replies; 11+ messages in thread
From: Jiri Olsa @ 2022-11-23 21:20 UTC (permalink / raw)
To: Yonghong Song
Cc: sdf, Jiri Olsa, bpf, ast, daniel, andrii, martin.lau, song, yhs,
john.fastabend, kpsingh, haoluo
On Wed, Nov 23, 2022 at 11:07:18AM -0800, Yonghong Song wrote:
SNIP
> > > > if I comment out all the umounts in setns_by_fd, it does not fail
> >
> > > Agreed with the above observations.
> > > With the current bpf-next, I can easily hit the above perf event ID
> > > issue.
> >
> > > But if I backout the following two patches:
> > > 68f8e3d4b916531ea3bb8b83e35138cf78f2fce5 selftests/bpf: Make sure
> > > zero-len
> > > skbs aren't redirectable
> > > 114039b342014680911c35bd6b72624180fd669a bpf: Move skb->len == 0
> > > checks into
> > > __bpf_redirect
> >
> >
> > > and run a few times with './test_progs -j' and I didn't hit any issues.
> >
> > My guess would be that we need to remount debugfs in setns_by_fd?
> >
> > diff --git a/tools/testing/selftests/bpf/network_helpers.c
> > b/tools/testing/selftests/bpf/network_helpers.c
> > index bec15558fd93..1f37adff7632 100644
> > --- a/tools/testing/selftests/bpf/network_helpers.c
> > +++ b/tools/testing/selftests/bpf/network_helpers.c
> > @@ -426,6 +426,10 @@ static int setns_by_fd(int nsfd)
> > if (!ASSERT_OK(err, "mount /sys/fs/bpf"))
> > return err;
> >
> > + err = mount("debugfs", "/sys/kernel/debug", "debugfs", 0, NULL);
> > + if (!ASSERT_OK(err, "mount /sys/kernel/debug"))
> > + return err;
> > +
> > return 0;
> > }
>
> Ya, this does fix the problem. Could you craft a patch for this?
same here ;-) thanks
jirka
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next v2 2/2] selftests/bpf: Make sure zero-len skbs aren't redirectable
2022-11-21 18:03 ` [PATCH bpf-next v2 2/2] selftests/bpf: Make sure zero-len skbs aren't redirectable Stanislav Fomichev
2022-11-23 12:23 ` Jiri Olsa
@ 2023-09-08 16:54 ` Daniel Borkmann
2023-09-08 17:17 ` Stanislav Fomichev
1 sibling, 1 reply; 11+ messages in thread
From: Daniel Borkmann @ 2023-09-08 16:54 UTC (permalink / raw)
To: Stanislav Fomichev, bpf; +Cc: ast, martin.lau, netdev
Hi Stan,
Do you have some cycles to look into the below?
On 11/21/22 7:03 PM, Stanislav Fomichev wrote:
> LWT_XMIT to test L3 case, TC to test L2 case.
>
> v2:
> - s/veth_ifindex/ipip_ifindex/ in two places (Martin)
> - add comment about which condition triggers the rejection (Martin)
>
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
[...]
> + /* ETH_HLEN+1-sized packet should be redirected. */
> +
> + {
> + .msg = "veth ETH_HLEN+1 packet ingress",
> + .data_in = eth_hlen_pp,
> + .data_size_in = sizeof(eth_hlen_pp),
> + .ifindex = &veth_ifindex,
> + },
[...]
This one is now failing in BPF CI on net/net-next ff after the veth driver changed
it's drop error code in [0] from NETDEV_TX_OK (0) to NET_XMIT_DROP (1) :
test_empty_skb:FAIL:ret: veth ETH_HLEN+1 packet ingress [redirect_egress] unexpected ret: veth ETH_HLEN+1 packet ingress [redirect_egress]: actual 1 != expected 0
test_empty_skb:PASS:err: veth ETH_HLEN+1 packet ingress [tc_redirect_ingress] 0 nsec
test_empty_skb:PASS:ret: veth ETH_HLEN+1 packet ingress [tc_redirect_ingress] 0 nsec
test_empty_skb:PASS:err: veth ETH_HLEN+1 packet ingress [tc_redirect_egress] 0 nsec
test_empty_skb:PASS:ret: veth ETH_HLEN+1 packet ingress [tc_redirect_egress] 0 nsec
test_empty_skb:PASS:err: ipip ETH_HLEN+1 packet ingress [redirect_ingress] 0 nsec
test_empty_skb:PASS:ret: ipip ETH_HLEN+1 packet ingress [redirect_ingress] 0 nsec
test_empty_skb:PASS:err: ipip ETH_HLEN+1 packet ingress [redirect_egress] 0 nsec
test_empty_skb:PASS:ret: ipip ETH_HLEN+1 packet ingress [redirect_egress] 0 nsec
test_empty_skb:PASS:err: ipip ETH_HLEN+1 packet ingress [tc_redirect_ingress] 0 nsec
test_empty_skb:PASS:ret: ipip ETH_HLEN+1 packet ingress [tc_redirect_ingress] 0 nsec
test_empty_skb:PASS:err: ipip ETH_HLEN+1 packet ingress [tc_redirect_egress] 0 nsec
test_empty_skb:PASS:ret: ipip ETH_HLEN+1 packet ingress [tc_redirect_egress] 0 nsec
close_netns:PASS:setns 0 nsec
#71 empty_skb:FAIL
The test was testing bpf_clone_redirect which is still okay, just that for the
xmit sides it propagates the error code now into ret and hence the assert fails.
Perhaps we would need to tweak the test case to test for 0 or 1 ... 0 in case
bpf_clone_redirect pushes to ingress, 1 in case it pushes to egress and reaches
veth..
Thanks,
Daniel
[0] https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=151e887d8ff97e2e42110ffa1fb1e6a2128fb364
> + };
> +
> + SYS("ip netns add empty_skb");
> + tok = open_netns("empty_skb");
> + SYS("ip link add veth0 type veth peer veth1");
> + SYS("ip link set dev veth0 up");
> + SYS("ip link set dev veth1 up");
> + SYS("ip addr add 10.0.0.1/8 dev veth0");
> + SYS("ip addr add 10.0.0.2/8 dev veth1");
> + veth_ifindex = if_nametoindex("veth0");
> +
> + SYS("ip link add ipip0 type ipip local 10.0.0.1 remote 10.0.0.2");
> + SYS("ip link set ipip0 up");
> + SYS("ip addr add 192.168.1.1/16 dev ipip0");
> + ipip_ifindex = if_nametoindex("ipip0");
> +
> + bpf_obj = empty_skb__open_and_load();
> + if (!ASSERT_OK_PTR(bpf_obj, "open skeleton"))
> + goto out;
> +
> + for (i = 0; i < ARRAY_SIZE(tests); i++) {
> + bpf_object__for_each_program(prog, bpf_obj->obj) {
> + char buf[128];
> + bool at_tc = !strncmp(bpf_program__section_name(prog), "tc", 2);
> +
> + tattr.data_in = tests[i].data_in;
> + tattr.data_size_in = tests[i].data_size_in;
> +
> + tattr.data_size_out = 0;
> + bpf_obj->bss->ifindex = *tests[i].ifindex;
> + bpf_obj->bss->ret = 0;
> + err = bpf_prog_test_run_opts(bpf_program__fd(prog), &tattr);
> + sprintf(buf, "err: %s [%s]", tests[i].msg, bpf_program__name(prog));
> +
> + if (at_tc && tests[i].success_on_tc)
> + ASSERT_GE(err, 0, buf);
> + else
> + ASSERT_EQ(err, tests[i].err, buf);
> + sprintf(buf, "ret: %s [%s]", tests[i].msg, bpf_program__name(prog));
> + if (at_tc && tests[i].success_on_tc)
> + ASSERT_GE(bpf_obj->bss->ret, 0, buf);
> + else
> + ASSERT_EQ(bpf_obj->bss->ret, tests[i].ret, buf);
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next v2 2/2] selftests/bpf: Make sure zero-len skbs aren't redirectable
2023-09-08 16:54 ` Daniel Borkmann
@ 2023-09-08 17:17 ` Stanislav Fomichev
0 siblings, 0 replies; 11+ messages in thread
From: Stanislav Fomichev @ 2023-09-08 17:17 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: bpf, ast, martin.lau, netdev
On Fri, Sep 8, 2023 at 9:54 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> Hi Stan,
>
> Do you have some cycles to look into the below?
Sure, I'll take a look!
> On 11/21/22 7:03 PM, Stanislav Fomichev wrote:
> > LWT_XMIT to test L3 case, TC to test L2 case.
> >
> > v2:
> > - s/veth_ifindex/ipip_ifindex/ in two places (Martin)
> > - add comment about which condition triggers the rejection (Martin)
> >
> > Signed-off-by: Stanislav Fomichev <sdf@google.com>
> [...]
> > + /* ETH_HLEN+1-sized packet should be redirected. */
> > +
> > + {
> > + .msg = "veth ETH_HLEN+1 packet ingress",
> > + .data_in = eth_hlen_pp,
> > + .data_size_in = sizeof(eth_hlen_pp),
> > + .ifindex = &veth_ifindex,
> > + },
> [...]
>
> This one is now failing in BPF CI on net/net-next ff after the veth driver changed
> it's drop error code in [0] from NETDEV_TX_OK (0) to NET_XMIT_DROP (1) :
>
> test_empty_skb:FAIL:ret: veth ETH_HLEN+1 packet ingress [redirect_egress] unexpected ret: veth ETH_HLEN+1 packet ingress [redirect_egress]: actual 1 != expected 0
> test_empty_skb:PASS:err: veth ETH_HLEN+1 packet ingress [tc_redirect_ingress] 0 nsec
> test_empty_skb:PASS:ret: veth ETH_HLEN+1 packet ingress [tc_redirect_ingress] 0 nsec
> test_empty_skb:PASS:err: veth ETH_HLEN+1 packet ingress [tc_redirect_egress] 0 nsec
> test_empty_skb:PASS:ret: veth ETH_HLEN+1 packet ingress [tc_redirect_egress] 0 nsec
> test_empty_skb:PASS:err: ipip ETH_HLEN+1 packet ingress [redirect_ingress] 0 nsec
> test_empty_skb:PASS:ret: ipip ETH_HLEN+1 packet ingress [redirect_ingress] 0 nsec
> test_empty_skb:PASS:err: ipip ETH_HLEN+1 packet ingress [redirect_egress] 0 nsec
> test_empty_skb:PASS:ret: ipip ETH_HLEN+1 packet ingress [redirect_egress] 0 nsec
> test_empty_skb:PASS:err: ipip ETH_HLEN+1 packet ingress [tc_redirect_ingress] 0 nsec
> test_empty_skb:PASS:ret: ipip ETH_HLEN+1 packet ingress [tc_redirect_ingress] 0 nsec
> test_empty_skb:PASS:err: ipip ETH_HLEN+1 packet ingress [tc_redirect_egress] 0 nsec
> test_empty_skb:PASS:ret: ipip ETH_HLEN+1 packet ingress [tc_redirect_egress] 0 nsec
> close_netns:PASS:setns 0 nsec
> #71 empty_skb:FAIL
>
> The test was testing bpf_clone_redirect which is still okay, just that for the
> xmit sides it propagates the error code now into ret and hence the assert fails.
> Perhaps we would need to tweak the test case to test for 0 or 1 ... 0 in case
> bpf_clone_redirect pushes to ingress, 1 in case it pushes to egress and reaches
> veth..
>
> Thanks,
> Daniel
>
> [0] https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=151e887d8ff97e2e42110ffa1fb1e6a2128fb364
>
> > + };
> > +
> > + SYS("ip netns add empty_skb");
> > + tok = open_netns("empty_skb");
> > + SYS("ip link add veth0 type veth peer veth1");
> > + SYS("ip link set dev veth0 up");
> > + SYS("ip link set dev veth1 up");
> > + SYS("ip addr add 10.0.0.1/8 dev veth0");
> > + SYS("ip addr add 10.0.0.2/8 dev veth1");
> > + veth_ifindex = if_nametoindex("veth0");
> > +
> > + SYS("ip link add ipip0 type ipip local 10.0.0.1 remote 10.0.0.2");
> > + SYS("ip link set ipip0 up");
> > + SYS("ip addr add 192.168.1.1/16 dev ipip0");
> > + ipip_ifindex = if_nametoindex("ipip0");
> > +
> > + bpf_obj = empty_skb__open_and_load();
> > + if (!ASSERT_OK_PTR(bpf_obj, "open skeleton"))
> > + goto out;
> > +
> > + for (i = 0; i < ARRAY_SIZE(tests); i++) {
> > + bpf_object__for_each_program(prog, bpf_obj->obj) {
> > + char buf[128];
> > + bool at_tc = !strncmp(bpf_program__section_name(prog), "tc", 2);
> > +
> > + tattr.data_in = tests[i].data_in;
> > + tattr.data_size_in = tests[i].data_size_in;
> > +
> > + tattr.data_size_out = 0;
> > + bpf_obj->bss->ifindex = *tests[i].ifindex;
> > + bpf_obj->bss->ret = 0;
> > + err = bpf_prog_test_run_opts(bpf_program__fd(prog), &tattr);
> > + sprintf(buf, "err: %s [%s]", tests[i].msg, bpf_program__name(prog));
> > +
> > + if (at_tc && tests[i].success_on_tc)
> > + ASSERT_GE(err, 0, buf);
> > + else
> > + ASSERT_EQ(err, tests[i].err, buf);
> > + sprintf(buf, "ret: %s [%s]", tests[i].msg, bpf_program__name(prog));
> > + if (at_tc && tests[i].success_on_tc)
> > + ASSERT_GE(bpf_obj->bss->ret, 0, buf);
> > + else
> > + ASSERT_EQ(bpf_obj->bss->ret, tests[i].ret, buf);
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-09-08 17:17 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-21 18:03 [PATCH bpf-next v2 1/2] bpf: Move skb->len == 0 checks into __bpf_redirect Stanislav Fomichev
2022-11-21 18:03 ` [PATCH bpf-next v2 2/2] selftests/bpf: Make sure zero-len skbs aren't redirectable Stanislav Fomichev
2022-11-23 12:23 ` Jiri Olsa
2022-11-23 16:18 ` Yonghong Song
2022-11-23 17:17 ` sdf
2022-11-23 19:07 ` Yonghong Song
2022-11-23 19:54 ` Stanislav Fomichev
2022-11-23 21:20 ` Jiri Olsa
2023-09-08 16:54 ` Daniel Borkmann
2023-09-08 17:17 ` Stanislav Fomichev
2022-11-21 21:00 ` [PATCH bpf-next v2 1/2] bpf: Move skb->len == 0 checks into __bpf_redirect patchwork-bot+netdevbpf
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.