| Message ID | 20230405193354.1956209-1-zhuyifei@google.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 5af607a861d43ffff830fc1890033e579ec44799 |
| Delegated to: | BPF |
| Headers | show |
| Series | [v2,bpf] selftests/bpf: Wait for receive in cg_storage_multi test | expand |
On 4/5/23 12:33 PM, YiFei Zhu wrote: > In some cases the loopback latency might be large enough, causing > the assertion on invocations to be run before ingress prog getting > executed. The assertion would fail and the test would flake. > > This can be reliably reproduced by arbitrarily increasing the > loopback latency (thanks to [1]): > tc qdisc add dev lo root handle 1: htb default 12 > tc class add dev lo parent 1:1 classid 1:12 htb rate 20kbps ceil 20kbps > tc qdisc add dev lo parent 1:12 netem delay 100ms > > Fix this by waiting on the receive end, instead of instantly > returning to the assert. The call to read() will wait for the > default SO_RCVTIMEO timeout of 3 seconds provided by > start_server(). > > [1] https://gist.github.com/kstevens715/4598301 > > Reported-by: Martin KaFai Lau <martin.lau@linux.dev> > Link: https://lore.kernel.org/bpf/9c5c8b7e-1d89-a3af-5400-14fde81f4429@linux.dev/ > Fixes: 3573f384014f ("selftests/bpf: Test CGROUP_STORAGE behavior on shared egress + ingress") > Signed-off-by: YiFei Zhu <zhuyifei@google.com> > --- > v1 -> v2: > - Changed from a call to poll() to a call to read() (Martin) Thanks for the fix. Added "Acked-by: Stanislav Fomichev <sdf@google.com>" and applied.
Hello: This patch was applied to bpf/bpf-next.git (master) by Martin KaFai Lau <martin.lau@kernel.org>: On Wed, 5 Apr 2023 19:33:54 +0000 you wrote: > In some cases the loopback latency might be large enough, causing > the assertion on invocations to be run before ingress prog getting > executed. The assertion would fail and the test would flake. > > This can be reliably reproduced by arbitrarily increasing the > loopback latency (thanks to [1]): > tc qdisc add dev lo root handle 1: htb default 12 > tc class add dev lo parent 1:1 classid 1:12 htb rate 20kbps ceil 20kbps > tc qdisc add dev lo parent 1:12 netem delay 100ms > > [...] Here is the summary with links: - [v2,bpf] selftests/bpf: Wait for receive in cg_storage_multi test https://git.kernel.org/bpf/bpf-next/c/5af607a861d4 You are awesome, thank you!
diff --git a/tools/testing/selftests/bpf/prog_tests/cg_storage_multi.c b/tools/testing/selftests/bpf/prog_tests/cg_storage_multi.c index 621c57222191..63ee892bc757 100644 --- a/tools/testing/selftests/bpf/prog_tests/cg_storage_multi.c +++ b/tools/testing/selftests/bpf/prog_tests/cg_storage_multi.c @@ -56,8 +56,9 @@ static bool assert_storage_noexist(struct bpf_map *map, const void *key) static bool connect_send(const char *cgroup_path) { - bool res = true; int server_fd = -1, client_fd = -1; + char message[] = "message"; + bool res = true; if (join_cgroup(cgroup_path)) goto out_clean; @@ -70,7 +71,10 @@ static bool connect_send(const char *cgroup_path) if (client_fd < 0) goto out_clean; - if (send(client_fd, "message", strlen("message"), 0) < 0) + if (send(client_fd, &message, sizeof(message), 0) < 0) + goto out_clean; + + if (read(server_fd, &message, sizeof(message)) < 0) goto out_clean; res = false;
In some cases the loopback latency might be large enough, causing the assertion on invocations to be run before ingress prog getting executed. The assertion would fail and the test would flake. This can be reliably reproduced by arbitrarily increasing the loopback latency (thanks to [1]): tc qdisc add dev lo root handle 1: htb default 12 tc class add dev lo parent 1:1 classid 1:12 htb rate 20kbps ceil 20kbps tc qdisc add dev lo parent 1:12 netem delay 100ms Fix this by waiting on the receive end, instead of instantly returning to the assert. The call to read() will wait for the default SO_RCVTIMEO timeout of 3 seconds provided by start_server(). [1] https://gist.github.com/kstevens715/4598301 Reported-by: Martin KaFai Lau <martin.lau@linux.dev> Link: https://lore.kernel.org/bpf/9c5c8b7e-1d89-a3af-5400-14fde81f4429@linux.dev/ Fixes: 3573f384014f ("selftests/bpf: Test CGROUP_STORAGE behavior on shared egress + ingress") Signed-off-by: YiFei Zhu <zhuyifei@google.com> --- v1 -> v2: - Changed from a call to poll() to a call to read() (Martin) tools/testing/selftests/bpf/prog_tests/cg_storage_multi.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)