* [PATCH bpf 0/4] Various fixes for sockmap and reuseport tests
@ 2020-01-23 16:59 Lorenz Bauer
2020-01-23 16:59 ` [PATCH bpf 1/4] selftests: bpf: use a temporary file in test_sockmap Lorenz Bauer
` (4 more replies)
0 siblings, 5 replies; 26+ messages in thread
From: Lorenz Bauer @ 2020-01-23 16:59 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Yonghong Song, Andrii Nakryiko
Cc: Lorenz Bauer, netdev, bpf
I noticed some rough edges while running the sockmap and reuseport tests in
a virtme environment. It only has a single CPU, and the root filesystem is
mounted read-only.
The cause for patch #2 is either the VM being slow because it only has a
single CPU. Patches #3-4 helped me narrow down the problem, so they are
probably useful for others as well.
Lorenz Bauer (4):
selftests: bpf: use a temporary file in test_sockmap
selftests: bpf: ignore RST packets for reuseport tests
selftests: bpf: make reuseport test output more legible
selftests: bpf: reset global state between reuseport test runs
.../bpf/prog_tests/select_reuseport.c | 44 ++++++++++++++++---
.../bpf/progs/test_select_reuseport_kern.c | 6 +++
tools/testing/selftests/bpf/test_sockmap.c | 15 +++----
3 files changed, 50 insertions(+), 15 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCH bpf 1/4] selftests: bpf: use a temporary file in test_sockmap 2020-01-23 16:59 [PATCH bpf 0/4] Various fixes for sockmap and reuseport tests Lorenz Bauer @ 2020-01-23 16:59 ` Lorenz Bauer 2020-01-23 18:34 ` Martin Lau ` (2 more replies) 2020-01-23 16:59 ` [PATCH bpf 2/4] selftests: bpf: ignore RST packets for reuseport tests Lorenz Bauer ` (3 subsequent siblings) 4 siblings, 3 replies; 26+ messages in thread From: Lorenz Bauer @ 2020-01-23 16:59 UTC (permalink / raw) To: Shuah Khan, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko Cc: Lorenz Bauer, linux-kselftest, netdev, bpf, linux-kernel Use a proper temporary file for sendpage tests. This means that running the tests doesn't clutter the working directory, and allows running the test on read-only filesystems. Signed-off-by: Lorenz Bauer <lmb@cloudflare.com> --- tools/testing/selftests/bpf/test_sockmap.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/tools/testing/selftests/bpf/test_sockmap.c b/tools/testing/selftests/bpf/test_sockmap.c index 4a851513c842..779e11da979c 100644 --- a/tools/testing/selftests/bpf/test_sockmap.c +++ b/tools/testing/selftests/bpf/test_sockmap.c @@ -331,7 +331,7 @@ static int msg_loop_sendpage(int fd, int iov_length, int cnt, FILE *file; int i, fp; - file = fopen(".sendpage_tst.tmp", "w+"); + file = tmpfile(); if (!file) { perror("create file for sendpage"); return 1; @@ -340,13 +340,8 @@ static int msg_loop_sendpage(int fd, int iov_length, int cnt, fwrite(&k, sizeof(char), 1, file); fflush(file); fseek(file, 0, SEEK_SET); - fclose(file); - fp = open(".sendpage_tst.tmp", O_RDONLY); - if (fp < 0) { - perror("reopen file for sendpage"); - return 1; - } + fp = fileno(file); clock_gettime(CLOCK_MONOTONIC, &s->start); for (i = 0; i < cnt; i++) { @@ -354,11 +349,11 @@ static int msg_loop_sendpage(int fd, int iov_length, int cnt, if (!drop && sent < 0) { perror("send loop error"); - close(fp); + fclose(file); return sent; } else if (drop && sent >= 0) { printf("sendpage loop error expected: %i\n", sent); - close(fp); + fclose(file); return -EIO; } @@ -366,7 +361,7 @@ static int msg_loop_sendpage(int fd, int iov_length, int cnt, s->bytes_sent += sent; } clock_gettime(CLOCK_MONOTONIC, &s->end); - close(fp); + fclose(file); return 0; } -- 2.20.1 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH bpf 1/4] selftests: bpf: use a temporary file in test_sockmap 2020-01-23 16:59 ` [PATCH bpf 1/4] selftests: bpf: use a temporary file in test_sockmap Lorenz Bauer @ 2020-01-23 18:34 ` Martin Lau 2020-01-24 19:50 ` John Fastabend 2020-01-24 9:20 ` Jakub Sitnicki 2020-01-24 19:43 ` John Fastabend 2 siblings, 1 reply; 26+ messages in thread From: Martin Lau @ 2020-01-23 18:34 UTC (permalink / raw) To: Lorenz Bauer, John Fastabend Cc: Shuah Khan, Alexei Starovoitov, Daniel Borkmann, Song Liu, Yonghong Song, Andrii Nakryiko, linux-kselftest, netdev, bpf, linux-kernel On Thu, Jan 23, 2020 at 04:59:30PM +0000, Lorenz Bauer wrote: > Use a proper temporary file for sendpage tests. This means that running > the tests doesn't clutter the working directory, and allows running the > test on read-only filesystems. > > Signed-off-by: Lorenz Bauer <lmb@cloudflare.com> > --- > tools/testing/selftests/bpf/test_sockmap.c | 15 +++++---------- > 1 file changed, 5 insertions(+), 10 deletions(-) > > diff --git a/tools/testing/selftests/bpf/test_sockmap.c b/tools/testing/selftests/bpf/test_sockmap.c > index 4a851513c842..779e11da979c 100644 > --- a/tools/testing/selftests/bpf/test_sockmap.c > +++ b/tools/testing/selftests/bpf/test_sockmap.c > @@ -331,7 +331,7 @@ static int msg_loop_sendpage(int fd, int iov_length, int cnt, > FILE *file; > int i, fp; > > - file = fopen(".sendpage_tst.tmp", "w+"); > + file = tmpfile(); > if (!file) { > perror("create file for sendpage"); > return 1; > @@ -340,13 +340,8 @@ static int msg_loop_sendpage(int fd, int iov_length, int cnt, > fwrite(&k, sizeof(char), 1, file); > fflush(file); > fseek(file, 0, SEEK_SET); > - fclose(file); > > - fp = open(".sendpage_tst.tmp", O_RDONLY); > - if (fp < 0) { > - perror("reopen file for sendpage"); > - return 1; > - } > + fp = fileno(file); It may be better to keep fp == -1 check here. It is not clear to me the original intention of reopen. I would defer to John for comment. > > clock_gettime(CLOCK_MONOTONIC, &s->start); > for (i = 0; i < cnt; i++) { ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH bpf 1/4] selftests: bpf: use a temporary file in test_sockmap 2020-01-23 18:34 ` Martin Lau @ 2020-01-24 19:50 ` John Fastabend 0 siblings, 0 replies; 26+ messages in thread From: John Fastabend @ 2020-01-24 19:50 UTC (permalink / raw) To: Martin Lau, Lorenz Bauer, John Fastabend Cc: Shuah Khan, Alexei Starovoitov, Daniel Borkmann, Song Liu, Yonghong Song, Andrii Nakryiko, linux-kselftest, netdev, bpf, linux-kernel Martin Lau wrote: > On Thu, Jan 23, 2020 at 04:59:30PM +0000, Lorenz Bauer wrote: > > Use a proper temporary file for sendpage tests. This means that running > > the tests doesn't clutter the working directory, and allows running the > > test on read-only filesystems. > > > > Signed-off-by: Lorenz Bauer <lmb@cloudflare.com> > > --- > > tools/testing/selftests/bpf/test_sockmap.c | 15 +++++---------- > > 1 file changed, 5 insertions(+), 10 deletions(-) > > > > diff --git a/tools/testing/selftests/bpf/test_sockmap.c b/tools/testing/selftests/bpf/test_sockmap.c > > index 4a851513c842..779e11da979c 100644 > > --- a/tools/testing/selftests/bpf/test_sockmap.c > > +++ b/tools/testing/selftests/bpf/test_sockmap.c > > @@ -331,7 +331,7 @@ static int msg_loop_sendpage(int fd, int iov_length, int cnt, > > FILE *file; > > int i, fp; > > > > - file = fopen(".sendpage_tst.tmp", "w+"); > > + file = tmpfile(); > > if (!file) { > > perror("create file for sendpage"); > > return 1; > > @@ -340,13 +340,8 @@ static int msg_loop_sendpage(int fd, int iov_length, int cnt, > > fwrite(&k, sizeof(char), 1, file); > > fflush(file); > > fseek(file, 0, SEEK_SET); > > - fclose(file); > > > > - fp = open(".sendpage_tst.tmp", O_RDONLY); > > - if (fp < 0) { > > - perror("reopen file for sendpage"); > > - return 1; > > - } > > + fp = fileno(file); > It may be better to keep fp == -1 check here. > It is not clear to me the original intention of reopen. > I would defer to John for comment. > Seeing fileno shouldn't fail seems OK to me. > > > > clock_gettime(CLOCK_MONOTONIC, &s->start); > > for (i = 0; i < cnt; i++) { ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH bpf 1/4] selftests: bpf: use a temporary file in test_sockmap 2020-01-23 16:59 ` [PATCH bpf 1/4] selftests: bpf: use a temporary file in test_sockmap Lorenz Bauer 2020-01-23 18:34 ` Martin Lau @ 2020-01-24 9:20 ` Jakub Sitnicki 2020-01-24 19:43 ` John Fastabend 2 siblings, 0 replies; 26+ messages in thread From: Jakub Sitnicki @ 2020-01-24 9:20 UTC (permalink / raw) To: Lorenz Bauer Cc: Shuah Khan, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko, linux-kselftest, netdev, bpf, linux-kernel On Thu, Jan 23, 2020 at 05:59 PM CET, Lorenz Bauer wrote: > Use a proper temporary file for sendpage tests. This means that running > the tests doesn't clutter the working directory, and allows running the > test on read-only filesystems. > > Signed-off-by: Lorenz Bauer <lmb@cloudflare.com> > --- > tools/testing/selftests/bpf/test_sockmap.c | 15 +++++---------- > 1 file changed, 5 insertions(+), 10 deletions(-) > > diff --git a/tools/testing/selftests/bpf/test_sockmap.c b/tools/testing/selftests/bpf/test_sockmap.c > index 4a851513c842..779e11da979c 100644 > --- a/tools/testing/selftests/bpf/test_sockmap.c > +++ b/tools/testing/selftests/bpf/test_sockmap.c > @@ -331,7 +331,7 @@ static int msg_loop_sendpage(int fd, int iov_length, int cnt, > FILE *file; > int i, fp; > > - file = fopen(".sendpage_tst.tmp", "w+"); > + file = tmpfile(); > if (!file) { > perror("create file for sendpage"); > return 1; memfd_create() would be an alternative that doesn't clutter anything. [...] ^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [PATCH bpf 1/4] selftests: bpf: use a temporary file in test_sockmap 2020-01-23 16:59 ` [PATCH bpf 1/4] selftests: bpf: use a temporary file in test_sockmap Lorenz Bauer 2020-01-23 18:34 ` Martin Lau 2020-01-24 9:20 ` Jakub Sitnicki @ 2020-01-24 19:43 ` John Fastabend 2 siblings, 0 replies; 26+ messages in thread From: John Fastabend @ 2020-01-24 19:43 UTC (permalink / raw) To: Lorenz Bauer, Shuah Khan, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko Cc: Lorenz Bauer, linux-kselftest, netdev, bpf, linux-kernel Lorenz Bauer wrote: > Use a proper temporary file for sendpage tests. This means that running > the tests doesn't clutter the working directory, and allows running the > test on read-only filesystems. > > Signed-off-by: Lorenz Bauer <lmb@cloudflare.com> > --- > tools/testing/selftests/bpf/test_sockmap.c | 15 +++++---------- > 1 file changed, 5 insertions(+), 10 deletions(-) > Agree nice bit of cleanup. We should merge this in bpf-next though considering its late in the bpf tree. Acked-by: John Fastabend <john.fastabend@gmail.com> ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH bpf 2/4] selftests: bpf: ignore RST packets for reuseport tests 2020-01-23 16:59 [PATCH bpf 0/4] Various fixes for sockmap and reuseport tests Lorenz Bauer 2020-01-23 16:59 ` [PATCH bpf 1/4] selftests: bpf: use a temporary file in test_sockmap Lorenz Bauer @ 2020-01-23 16:59 ` Lorenz Bauer 2020-01-23 17:16 ` Martin Lau ` (2 more replies) 2020-01-23 16:59 ` [PATCH bpf 3/4] selftests: bpf: make reuseport test output more legible Lorenz Bauer ` (2 subsequent siblings) 4 siblings, 3 replies; 26+ messages in thread From: Lorenz Bauer @ 2020-01-23 16:59 UTC (permalink / raw) To: Shuah Khan, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko Cc: Lorenz Bauer, linux-kselftest, netdev, bpf, linux-kernel The reuseport tests currently suffer from a race condition: RST packets count towards DROP_ERR_SKB_DATA, since they don't contain a valid struct cmd. Tests will spuriously fail depending on whether check_results is called before or after the RST is processed. Exit the BPF program early if FIN is set. Signed-off-by: Lorenz Bauer <lmb@cloudflare.com> --- .../selftests/bpf/progs/test_select_reuseport_kern.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tools/testing/selftests/bpf/progs/test_select_reuseport_kern.c b/tools/testing/selftests/bpf/progs/test_select_reuseport_kern.c index d69a1f2bbbfd..26e77dcc7e91 100644 --- a/tools/testing/selftests/bpf/progs/test_select_reuseport_kern.c +++ b/tools/testing/selftests/bpf/progs/test_select_reuseport_kern.c @@ -113,6 +113,12 @@ int _select_by_skb_data(struct sk_reuseport_md *reuse_md) data_check.skb_ports[0] = th->source; data_check.skb_ports[1] = th->dest; + if (th->fin) + /* The connection is being torn down at the end of a + * test. It can't contain a cmd, so return early. + */ + return SK_PASS; + if ((th->doff << 2) + sizeof(*cmd) > data_check.len) GOTO_DONE(DROP_ERR_SKB_DATA); if (bpf_skb_load_bytes(reuse_md, th->doff << 2, &cmd_copy, -- 2.20.1 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH bpf 2/4] selftests: bpf: ignore RST packets for reuseport tests 2020-01-23 16:59 ` [PATCH bpf 2/4] selftests: bpf: ignore RST packets for reuseport tests Lorenz Bauer @ 2020-01-23 17:16 ` Martin Lau 2020-01-23 17:18 ` Lorenz Bauer 2020-01-23 21:53 ` Martin Lau 2020-01-24 19:45 ` John Fastabend 2 siblings, 1 reply; 26+ messages in thread From: Martin Lau @ 2020-01-23 17:16 UTC (permalink / raw) To: Lorenz Bauer Cc: Shuah Khan, Alexei Starovoitov, Daniel Borkmann, Song Liu, Yonghong Song, Andrii Nakryiko, linux-kselftest, netdev, bpf, linux-kernel On Thu, Jan 23, 2020 at 04:59:31PM +0000, Lorenz Bauer wrote: > The reuseport tests currently suffer from a race condition: RST > packets count towards DROP_ERR_SKB_DATA, since they don't contain > a valid struct cmd. Tests will spuriously fail depending on whether > check_results is called before or after the RST is processed. > > Exit the BPF program early if FIN is set. Make sense. Is it a RST or FIN? The earlier commit message said RST. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH bpf 2/4] selftests: bpf: ignore RST packets for reuseport tests 2020-01-23 17:16 ` Martin Lau @ 2020-01-23 17:18 ` Lorenz Bauer 0 siblings, 0 replies; 26+ messages in thread From: Lorenz Bauer @ 2020-01-23 17:18 UTC (permalink / raw) To: Martin Lau Cc: Shuah Khan, Alexei Starovoitov, Daniel Borkmann, Song Liu, Yonghong Song, Andrii Nakryiko, linux-kselftest, netdev, bpf, linux-kernel On Thu, 23 Jan 2020 at 17:16, Martin Lau <kafai@fb.com> wrote: > > On Thu, Jan 23, 2020 at 04:59:31PM +0000, Lorenz Bauer wrote: > > The reuseport tests currently suffer from a race condition: RST > > packets count towards DROP_ERR_SKB_DATA, since they don't contain > > a valid struct cmd. Tests will spuriously fail depending on whether > > check_results is called before or after the RST is processed. > > > > Exit the BPF program early if FIN is set. > Make sense. > Is it a RST or FIN? The earlier commit message said RST. FIN, sorry. I'll update in a follow up. -- Lorenz Bauer | Systems Engineer 6th Floor, County Hall/The Riverside Building, SE1 7PB, UK www.cloudflare.com ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH bpf 2/4] selftests: bpf: ignore RST packets for reuseport tests 2020-01-23 16:59 ` [PATCH bpf 2/4] selftests: bpf: ignore RST packets for reuseport tests Lorenz Bauer 2020-01-23 17:16 ` Martin Lau @ 2020-01-23 21:53 ` Martin Lau 2020-01-24 9:00 ` Lorenz Bauer 2020-01-24 19:45 ` John Fastabend 2 siblings, 1 reply; 26+ messages in thread From: Martin Lau @ 2020-01-23 21:53 UTC (permalink / raw) To: Lorenz Bauer Cc: Shuah Khan, Alexei Starovoitov, Daniel Borkmann, Song Liu, Yonghong Song, Andrii Nakryiko, linux-kselftest, netdev, bpf, linux-kernel On Thu, Jan 23, 2020 at 04:59:31PM +0000, Lorenz Bauer wrote: > The reuseport tests currently suffer from a race condition: RST > packets count towards DROP_ERR_SKB_DATA, since they don't contain > a valid struct cmd. Tests will spuriously fail depending on whether > check_results is called before or after the RST is processed. > > Exit the BPF program early if FIN is set. btw, it needs a Fixes tag. Patch 4 and Patch 1 also need a Fixes tag. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH bpf 2/4] selftests: bpf: ignore RST packets for reuseport tests 2020-01-23 21:53 ` Martin Lau @ 2020-01-24 9:00 ` Lorenz Bauer 2020-01-24 9:32 ` Daniel Borkmann 0 siblings, 1 reply; 26+ messages in thread From: Lorenz Bauer @ 2020-01-24 9:00 UTC (permalink / raw) To: Martin Lau Cc: Shuah Khan, Alexei Starovoitov, Daniel Borkmann, Song Liu, Yonghong Song, Andrii Nakryiko, linux-kselftest, netdev, bpf, linux-kernel On Thu, 23 Jan 2020 at 21:54, Martin Lau <kafai@fb.com> wrote: > > btw, it needs a Fixes tag. > > Patch 4 and Patch 1 also need a Fixes tag. This makes me wonder, should these go via bpf or bpf-next? Do I have to split the series then? -- Lorenz Bauer | Systems Engineer 6th Floor, County Hall/The Riverside Building, SE1 7PB, UK www.cloudflare.com ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH bpf 2/4] selftests: bpf: ignore RST packets for reuseport tests 2020-01-24 9:00 ` Lorenz Bauer @ 2020-01-24 9:32 ` Daniel Borkmann 0 siblings, 0 replies; 26+ messages in thread From: Daniel Borkmann @ 2020-01-24 9:32 UTC (permalink / raw) To: Lorenz Bauer, Martin Lau Cc: Shuah Khan, Alexei Starovoitov, Song Liu, Yonghong Song, Andrii Nakryiko, linux-kselftest, netdev, bpf, linux-kernel On 1/24/20 10:00 AM, Lorenz Bauer wrote: > On Thu, 23 Jan 2020 at 21:54, Martin Lau <kafai@fb.com> wrote: >> >> btw, it needs a Fixes tag. >> >> Patch 4 and Patch 1 also need a Fixes tag. > > This makes me wonder, should these go via bpf or bpf-next? Do I have > to split the series then? Lets do all of these for bpf-next since timing is very close before v5.5 release. If needed, we can later have them picked up for 5.5 stable. Thanks, Daniel ^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [PATCH bpf 2/4] selftests: bpf: ignore RST packets for reuseport tests 2020-01-23 16:59 ` [PATCH bpf 2/4] selftests: bpf: ignore RST packets for reuseport tests Lorenz Bauer 2020-01-23 17:16 ` Martin Lau 2020-01-23 21:53 ` Martin Lau @ 2020-01-24 19:45 ` John Fastabend 2 siblings, 0 replies; 26+ messages in thread From: John Fastabend @ 2020-01-24 19:45 UTC (permalink / raw) To: Lorenz Bauer, Shuah Khan, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko Cc: Lorenz Bauer, linux-kselftest, netdev, bpf, linux-kernel Lorenz Bauer wrote: > The reuseport tests currently suffer from a race condition: RST > packets count towards DROP_ERR_SKB_DATA, since they don't contain > a valid struct cmd. Tests will spuriously fail depending on whether > check_results is called before or after the RST is processed. > > Exit the BPF program early if FIN is set. > > Signed-off-by: Lorenz Bauer <lmb@cloudflare.com> > --- > .../selftests/bpf/progs/test_select_reuseport_kern.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/tools/testing/selftests/bpf/progs/test_select_reuseport_kern.c b/tools/testing/selftests/bpf/progs/test_select_reuseport_kern.c > index d69a1f2bbbfd..26e77dcc7e91 100644 > --- a/tools/testing/selftests/bpf/progs/test_select_reuseport_kern.c > +++ b/tools/testing/selftests/bpf/progs/test_select_reuseport_kern.c > @@ -113,6 +113,12 @@ int _select_by_skb_data(struct sk_reuseport_md *reuse_md) > data_check.skb_ports[0] = th->source; > data_check.skb_ports[1] = th->dest; > > + if (th->fin) > + /* The connection is being torn down at the end of a > + * test. It can't contain a cmd, so return early. > + */ > + return SK_PASS; > + > if ((th->doff << 2) + sizeof(*cmd) > data_check.len) > GOTO_DONE(DROP_ERR_SKB_DATA); > if (bpf_skb_load_bytes(reuse_md, th->doff << 2, &cmd_copy, > -- > 2.20.1 > Acked-by: John Fastabend <john.fastabend@gmail.com> ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH bpf 3/4] selftests: bpf: make reuseport test output more legible 2020-01-23 16:59 [PATCH bpf 0/4] Various fixes for sockmap and reuseport tests Lorenz Bauer 2020-01-23 16:59 ` [PATCH bpf 1/4] selftests: bpf: use a temporary file in test_sockmap Lorenz Bauer 2020-01-23 16:59 ` [PATCH bpf 2/4] selftests: bpf: ignore RST packets for reuseport tests Lorenz Bauer @ 2020-01-23 16:59 ` Lorenz Bauer 2020-01-23 17:26 ` Martin Lau 2020-01-24 19:46 ` John Fastabend 2020-01-23 16:59 ` [PATCH bpf 4/4] selftests: bpf: reset global state between reuseport test runs Lorenz Bauer 2020-01-24 11:27 ` [PATCH bpf-next v2 0/4] Various fixes for sockmap and reuseport tests Lorenz Bauer 4 siblings, 2 replies; 26+ messages in thread From: Lorenz Bauer @ 2020-01-23 16:59 UTC (permalink / raw) To: Shuah Khan, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko Cc: Lorenz Bauer, linux-kselftest, netdev, bpf, linux-kernel Include the name of the mismatching result in human readable format when reporting an error. The new output looks like the following: unexpected result result: [1, 0, 0, 0, 0, 0] expected: [0, 0, 0, 0, 0, 0] mismatch on DROP_ERR_INNER_MAP (bpf_prog_linum:153) check_results:FAIL:382 Signed-off-by: Lorenz Bauer <lmb@cloudflare.com> --- .../bpf/prog_tests/select_reuseport.c | 30 ++++++++++++++++--- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/tools/testing/selftests/bpf/prog_tests/select_reuseport.c b/tools/testing/selftests/bpf/prog_tests/select_reuseport.c index 2c37ae7dc214..09a536af139a 100644 --- a/tools/testing/selftests/bpf/prog_tests/select_reuseport.c +++ b/tools/testing/selftests/bpf/prog_tests/select_reuseport.c @@ -316,6 +316,28 @@ static void check_data(int type, sa_family_t family, const struct cmd *cmd, expected.len, result.len, get_linum()); } +static const char *result_to_str(enum result res) +{ + switch (res) { + case DROP_ERR_INNER_MAP: + return "DROP_ERR_INNER_MAP"; + case DROP_ERR_SKB_DATA: + return "DROP_ERR_SKB_DATA"; + case DROP_ERR_SK_SELECT_REUSEPORT: + return "DROP_ERR_SK_SELECT_REUSEPORT"; + case DROP_MISC: + return "DROP_MISC"; + case PASS: + return "PASS"; + case PASS_ERR_SK_SELECT_REUSEPORT: + return "PASS_ERR_SK_SELECT_REUSEPORT"; + case NR_RESULTS: + return "NR_RESULTS"; + default: + return "UNKNOWN"; + } +} + static void check_results(void) { __u32 results[NR_RESULTS]; @@ -351,10 +373,10 @@ static void check_results(void) printf(", %u", expected_results[i]); printf("]\n"); - RET_IF(expected_results[broken] != results[broken], - "unexpected result", - "expected_results[%u] != results[%u] bpf_prog_linum:%ld\n", - broken, broken, get_linum()); + printf("mismatch on %s (bpf_prog_linum:%ld)\n", result_to_str(broken), + get_linum()); + + CHECK_FAIL(true); } static int send_data(int type, sa_family_t family, void *data, size_t len, -- 2.20.1 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH bpf 3/4] selftests: bpf: make reuseport test output more legible 2020-01-23 16:59 ` [PATCH bpf 3/4] selftests: bpf: make reuseport test output more legible Lorenz Bauer @ 2020-01-23 17:26 ` Martin Lau 2020-01-24 19:46 ` John Fastabend 1 sibling, 0 replies; 26+ messages in thread From: Martin Lau @ 2020-01-23 17:26 UTC (permalink / raw) To: Lorenz Bauer Cc: Shuah Khan, Alexei Starovoitov, Daniel Borkmann, Song Liu, Yonghong Song, Andrii Nakryiko, linux-kselftest, netdev, bpf, linux-kernel On Thu, Jan 23, 2020 at 04:59:32PM +0000, Lorenz Bauer wrote: > Include the name of the mismatching result in human readable format > when reporting an error. The new output looks like the following: > > unexpected result > result: [1, 0, 0, 0, 0, 0] > expected: [0, 0, 0, 0, 0, 0] > mismatch on DROP_ERR_INNER_MAP (bpf_prog_linum:153) > check_results:FAIL:382 > > Signed-off-by: Lorenz Bauer <lmb@cloudflare.com> > --- > .../bpf/prog_tests/select_reuseport.c | 30 ++++++++++++++++--- > 1 file changed, 26 insertions(+), 4 deletions(-) > > diff --git a/tools/testing/selftests/bpf/prog_tests/select_reuseport.c b/tools/testing/selftests/bpf/prog_tests/select_reuseport.c > index 2c37ae7dc214..09a536af139a 100644 > --- a/tools/testing/selftests/bpf/prog_tests/select_reuseport.c > +++ b/tools/testing/selftests/bpf/prog_tests/select_reuseport.c > @@ -316,6 +316,28 @@ static void check_data(int type, sa_family_t family, const struct cmd *cmd, > expected.len, result.len, get_linum()); > } > > +static const char *result_to_str(enum result res) > +{ > + switch (res) { > + case DROP_ERR_INNER_MAP: > + return "DROP_ERR_INNER_MAP"; > + case DROP_ERR_SKB_DATA: > + return "DROP_ERR_SKB_DATA"; > + case DROP_ERR_SK_SELECT_REUSEPORT: > + return "DROP_ERR_SK_SELECT_REUSEPORT"; > + case DROP_MISC: > + return "DROP_MISC"; > + case PASS: > + return "PASS"; > + case PASS_ERR_SK_SELECT_REUSEPORT: > + return "PASS_ERR_SK_SELECT_REUSEPORT"; > + case NR_RESULTS: This should return "UNKNOWN" also. > + return "NR_RESULTS"; > + default: > + return "UNKNOWN"; > + } > +} > + ^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [PATCH bpf 3/4] selftests: bpf: make reuseport test output more legible 2020-01-23 16:59 ` [PATCH bpf 3/4] selftests: bpf: make reuseport test output more legible Lorenz Bauer 2020-01-23 17:26 ` Martin Lau @ 2020-01-24 19:46 ` John Fastabend 1 sibling, 0 replies; 26+ messages in thread From: John Fastabend @ 2020-01-24 19:46 UTC (permalink / raw) To: Lorenz Bauer, Shuah Khan, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko Cc: Lorenz Bauer, linux-kselftest, netdev, bpf, linux-kernel Lorenz Bauer wrote: > Include the name of the mismatching result in human readable format > when reporting an error. The new output looks like the following: > > unexpected result > result: [1, 0, 0, 0, 0, 0] > expected: [0, 0, 0, 0, 0, 0] > mismatch on DROP_ERR_INNER_MAP (bpf_prog_linum:153) > check_results:FAIL:382 > > Signed-off-by: Lorenz Bauer <lmb@cloudflare.com> > --- > .../bpf/prog_tests/select_reuseport.c | 30 ++++++++++++++++--- > 1 file changed, 26 insertions(+), 4 deletions(-) > > diff --git a/tools/testing/selftests/bpf/prog_tests/select_reuseport.c b/tools/testing/selftests/bpf/prog_tests/select_reuseport.c > index 2c37ae7dc214..09a536af139a 100644 > --- a/tools/testing/selftests/bpf/prog_tests/select_reuseport.c > +++ b/tools/testing/selftests/bpf/prog_tests/select_reuseport.c > @@ -316,6 +316,28 @@ static void check_data(int type, sa_family_t family, const struct cmd *cmd, > expected.len, result.len, get_linum()); > } > > +static const char *result_to_str(enum result res) > +{ > + switch (res) { > + case DROP_ERR_INNER_MAP: > + return "DROP_ERR_INNER_MAP"; > + case DROP_ERR_SKB_DATA: > + return "DROP_ERR_SKB_DATA"; > + case DROP_ERR_SK_SELECT_REUSEPORT: > + return "DROP_ERR_SK_SELECT_REUSEPORT"; > + case DROP_MISC: > + return "DROP_MISC"; > + case PASS: > + return "PASS"; > + case PASS_ERR_SK_SELECT_REUSEPORT: > + return "PASS_ERR_SK_SELECT_REUSEPORT"; > + case NR_RESULTS: > + return "NR_RESULTS"; > + default: > + return "UNKNOWN"; > + } > +} > + > static void check_results(void) > { > __u32 results[NR_RESULTS]; > @@ -351,10 +373,10 @@ static void check_results(void) > printf(", %u", expected_results[i]); > printf("]\n"); > > - RET_IF(expected_results[broken] != results[broken], > - "unexpected result", > - "expected_results[%u] != results[%u] bpf_prog_linum:%ld\n", > - broken, broken, get_linum()); > + printf("mismatch on %s (bpf_prog_linum:%ld)\n", result_to_str(broken), > + get_linum()); > + > + CHECK_FAIL(true); > } > > static int send_data(int type, sa_family_t family, void *data, size_t len, > -- > 2.20.1 > Acked-by: John Fastabend <john.fastabend@gmail.com> ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH bpf 4/4] selftests: bpf: reset global state between reuseport test runs 2020-01-23 16:59 [PATCH bpf 0/4] Various fixes for sockmap and reuseport tests Lorenz Bauer ` (2 preceding siblings ...) 2020-01-23 16:59 ` [PATCH bpf 3/4] selftests: bpf: make reuseport test output more legible Lorenz Bauer @ 2020-01-23 16:59 ` Lorenz Bauer 2020-01-23 17:56 ` Martin Lau 2020-01-24 11:27 ` [PATCH bpf-next v2 0/4] Various fixes for sockmap and reuseport tests Lorenz Bauer 4 siblings, 1 reply; 26+ messages in thread From: Lorenz Bauer @ 2020-01-23 16:59 UTC (permalink / raw) To: Shuah Khan, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko Cc: Lorenz Bauer, linux-kselftest, netdev, bpf, linux-kernel Currently, there is a lot of false positives if a single reuseport test fails. This is because expected_results and the result map are not cleared. Zero both after individual test runs, which fixes the mentioned false positives. Signed-off-by: Lorenz Bauer <lmb@cloudflare.com> --- .../selftests/bpf/prog_tests/select_reuseport.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/bpf/prog_tests/select_reuseport.c b/tools/testing/selftests/bpf/prog_tests/select_reuseport.c index 09a536af139a..0bab8b1ca1c3 100644 --- a/tools/testing/selftests/bpf/prog_tests/select_reuseport.c +++ b/tools/testing/selftests/bpf/prog_tests/select_reuseport.c @@ -699,7 +699,19 @@ static void setup_per_test(int type, sa_family_t family, bool inany, static void cleanup_per_test(bool no_inner_map) { - int i, err; + int i, err, zero = 0; + + memset(expected_results, 0, sizeof(expected_results)); + + for (i = 0; i < NR_RESULTS; i++) { + err = bpf_map_update_elem(result_map, &i, &zero, BPF_ANY); + RET_IF(err, "reset elem in result_map", + "i:%u err:%d errno:%d\n", i, err, errno); + } + + err = bpf_map_update_elem(linum_map, &zero, &zero, BPF_ANY); + RET_IF(err, "reset line number in linum_map", "err:%d errno:%d\n", + err, errno); for (i = 0; i < REUSEPORT_ARRAY_SIZE; i++) close(sk_fds[i]); -- 2.20.1 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH bpf 4/4] selftests: bpf: reset global state between reuseport test runs 2020-01-23 16:59 ` [PATCH bpf 4/4] selftests: bpf: reset global state between reuseport test runs Lorenz Bauer @ 2020-01-23 17:56 ` Martin Lau 0 siblings, 0 replies; 26+ messages in thread From: Martin Lau @ 2020-01-23 17:56 UTC (permalink / raw) To: Lorenz Bauer Cc: Shuah Khan, Alexei Starovoitov, Daniel Borkmann, Song Liu, Yonghong Song, Andrii Nakryiko, linux-kselftest, netdev, bpf, linux-kernel On Thu, Jan 23, 2020 at 04:59:33PM +0000, Lorenz Bauer wrote: > Currently, there is a lot of false positives if a single reuseport test > fails. This is because expected_results and the result map are not cleared. Ah, right. An earlier test failure has ripple effect on the following tests. I notice another embarrassing typo. Can you also make this change in this fix? -static enum result expected_results[NR_RESULTS]; +static __u32 expected_results[NR_RESULTS]; > > Zero both after individual test runs, which fixes the mentioned false > positives. Thanks for the fix! Acked-by: Martin KaFai Lau <kafai@fb.com> ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH bpf-next v2 0/4] Various fixes for sockmap and reuseport tests 2020-01-23 16:59 [PATCH bpf 0/4] Various fixes for sockmap and reuseport tests Lorenz Bauer ` (3 preceding siblings ...) 2020-01-23 16:59 ` [PATCH bpf 4/4] selftests: bpf: reset global state between reuseport test runs Lorenz Bauer @ 2020-01-24 11:27 ` Lorenz Bauer 2020-01-24 11:27 ` [PATCH bpf-next v2 1/4] selftests: bpf: use a temporary file in test_sockmap Lorenz Bauer ` (5 more replies) 4 siblings, 6 replies; 26+ messages in thread From: Lorenz Bauer @ 2020-01-24 11:27 UTC (permalink / raw) To: Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko Cc: Lorenz Bauer, netdev, bpf I've fixed the commit messages, added Fixes tags and am submitting to bpf-next instead of the bpf tree. There is still the question whether patch #1 needs to preserve O_RDONLY, which John can hopefully answer. Lorenz Bauer (4): selftests: bpf: use a temporary file in test_sockmap selftests: bpf: ignore FIN packets for reuseport tests selftests: bpf: make reuseport test output more legible selftests: bpf: reset global state between reuseport test runs .../bpf/prog_tests/select_reuseport.c | 44 ++++++++++++++++--- .../bpf/progs/test_select_reuseport_kern.c | 6 +++ tools/testing/selftests/bpf/test_sockmap.c | 15 +++---- 3 files changed, 49 insertions(+), 16 deletions(-) -- 2.20.1 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH bpf-next v2 1/4] selftests: bpf: use a temporary file in test_sockmap 2020-01-24 11:27 ` [PATCH bpf-next v2 0/4] Various fixes for sockmap and reuseport tests Lorenz Bauer @ 2020-01-24 11:27 ` Lorenz Bauer 2020-01-24 11:27 ` [PATCH bpf-next v2 2/4] selftests: bpf: ignore FIN packets for reuseport tests Lorenz Bauer ` (4 subsequent siblings) 5 siblings, 0 replies; 26+ messages in thread From: Lorenz Bauer @ 2020-01-24 11:27 UTC (permalink / raw) To: Shuah Khan, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko, John Fastabend Cc: Lorenz Bauer, linux-kselftest, netdev, bpf, linux-kernel Use a proper temporary file for sendpage tests. This means that running the tests doesn't clutter the working directory, and allows running the test on read-only filesystems. Signed-off-by: Lorenz Bauer <lmb@cloudflare.com> Fixes: 16962b2404ac ("bpf: sockmap, add selftests") --- tools/testing/selftests/bpf/test_sockmap.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/tools/testing/selftests/bpf/test_sockmap.c b/tools/testing/selftests/bpf/test_sockmap.c index 4a851513c842..779e11da979c 100644 --- a/tools/testing/selftests/bpf/test_sockmap.c +++ b/tools/testing/selftests/bpf/test_sockmap.c @@ -331,7 +331,7 @@ static int msg_loop_sendpage(int fd, int iov_length, int cnt, FILE *file; int i, fp; - file = fopen(".sendpage_tst.tmp", "w+"); + file = tmpfile(); if (!file) { perror("create file for sendpage"); return 1; @@ -340,13 +340,8 @@ static int msg_loop_sendpage(int fd, int iov_length, int cnt, fwrite(&k, sizeof(char), 1, file); fflush(file); fseek(file, 0, SEEK_SET); - fclose(file); - fp = open(".sendpage_tst.tmp", O_RDONLY); - if (fp < 0) { - perror("reopen file for sendpage"); - return 1; - } + fp = fileno(file); clock_gettime(CLOCK_MONOTONIC, &s->start); for (i = 0; i < cnt; i++) { @@ -354,11 +349,11 @@ static int msg_loop_sendpage(int fd, int iov_length, int cnt, if (!drop && sent < 0) { perror("send loop error"); - close(fp); + fclose(file); return sent; } else if (drop && sent >= 0) { printf("sendpage loop error expected: %i\n", sent); - close(fp); + fclose(file); return -EIO; } @@ -366,7 +361,7 @@ static int msg_loop_sendpage(int fd, int iov_length, int cnt, s->bytes_sent += sent; } clock_gettime(CLOCK_MONOTONIC, &s->end); - close(fp); + fclose(file); return 0; } -- 2.20.1 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH bpf-next v2 2/4] selftests: bpf: ignore FIN packets for reuseport tests 2020-01-24 11:27 ` [PATCH bpf-next v2 0/4] Various fixes for sockmap and reuseport tests Lorenz Bauer 2020-01-24 11:27 ` [PATCH bpf-next v2 1/4] selftests: bpf: use a temporary file in test_sockmap Lorenz Bauer @ 2020-01-24 11:27 ` Lorenz Bauer 2020-01-24 11:27 ` [PATCH bpf-next v2 3/4] selftests: bpf: make reuseport test output more legible Lorenz Bauer ` (3 subsequent siblings) 5 siblings, 0 replies; 26+ messages in thread From: Lorenz Bauer @ 2020-01-24 11:27 UTC (permalink / raw) To: Shuah Khan, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko Cc: Lorenz Bauer, linux-kselftest, netdev, bpf, linux-kernel The reuseport tests currently suffer from a race condition: FIN packets count towards DROP_ERR_SKB_DATA, since they don't contain a valid struct cmd. Tests will spuriously fail depending on whether check_results is called before or after the FIN is processed. Exit the BPF program early if FIN is set. Signed-off-by: Lorenz Bauer <lmb@cloudflare.com> Fixes: 91134d849a0e ("bpf: Test BPF_PROG_TYPE_SK_REUSEPORT") --- .../selftests/bpf/progs/test_select_reuseport_kern.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tools/testing/selftests/bpf/progs/test_select_reuseport_kern.c b/tools/testing/selftests/bpf/progs/test_select_reuseport_kern.c index d69a1f2bbbfd..26e77dcc7e91 100644 --- a/tools/testing/selftests/bpf/progs/test_select_reuseport_kern.c +++ b/tools/testing/selftests/bpf/progs/test_select_reuseport_kern.c @@ -113,6 +113,12 @@ int _select_by_skb_data(struct sk_reuseport_md *reuse_md) data_check.skb_ports[0] = th->source; data_check.skb_ports[1] = th->dest; + if (th->fin) + /* The connection is being torn down at the end of a + * test. It can't contain a cmd, so return early. + */ + return SK_PASS; + if ((th->doff << 2) + sizeof(*cmd) > data_check.len) GOTO_DONE(DROP_ERR_SKB_DATA); if (bpf_skb_load_bytes(reuse_md, th->doff << 2, &cmd_copy, -- 2.20.1 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH bpf-next v2 3/4] selftests: bpf: make reuseport test output more legible 2020-01-24 11:27 ` [PATCH bpf-next v2 0/4] Various fixes for sockmap and reuseport tests Lorenz Bauer 2020-01-24 11:27 ` [PATCH bpf-next v2 1/4] selftests: bpf: use a temporary file in test_sockmap Lorenz Bauer 2020-01-24 11:27 ` [PATCH bpf-next v2 2/4] selftests: bpf: ignore FIN packets for reuseport tests Lorenz Bauer @ 2020-01-24 11:27 ` Lorenz Bauer 2020-01-24 11:27 ` [PATCH bpf-next v2 4/4] selftests: bpf: reset global state between reuseport test runs Lorenz Bauer ` (2 subsequent siblings) 5 siblings, 0 replies; 26+ messages in thread From: Lorenz Bauer @ 2020-01-24 11:27 UTC (permalink / raw) To: Shuah Khan, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko Cc: Lorenz Bauer, linux-kselftest, netdev, bpf, linux-kernel Include the name of the mismatching result in human readable format when reporting an error. The new output looks like the following: unexpected result result: [1, 0, 0, 0, 0, 0] expected: [0, 0, 0, 0, 0, 0] mismatch on DROP_ERR_INNER_MAP (bpf_prog_linum:153) check_results:FAIL:382 Signed-off-by: Lorenz Bauer <lmb@cloudflare.com> --- .../bpf/prog_tests/select_reuseport.c | 28 ++++++++++++++++--- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/tools/testing/selftests/bpf/prog_tests/select_reuseport.c b/tools/testing/selftests/bpf/prog_tests/select_reuseport.c index 2c37ae7dc214..e7e56929751c 100644 --- a/tools/testing/selftests/bpf/prog_tests/select_reuseport.c +++ b/tools/testing/selftests/bpf/prog_tests/select_reuseport.c @@ -316,6 +316,26 @@ static void check_data(int type, sa_family_t family, const struct cmd *cmd, expected.len, result.len, get_linum()); } +static const char *result_to_str(enum result res) +{ + switch (res) { + case DROP_ERR_INNER_MAP: + return "DROP_ERR_INNER_MAP"; + case DROP_ERR_SKB_DATA: + return "DROP_ERR_SKB_DATA"; + case DROP_ERR_SK_SELECT_REUSEPORT: + return "DROP_ERR_SK_SELECT_REUSEPORT"; + case DROP_MISC: + return "DROP_MISC"; + case PASS: + return "PASS"; + case PASS_ERR_SK_SELECT_REUSEPORT: + return "PASS_ERR_SK_SELECT_REUSEPORT"; + default: + return "UNKNOWN"; + } +} + static void check_results(void) { __u32 results[NR_RESULTS]; @@ -351,10 +371,10 @@ static void check_results(void) printf(", %u", expected_results[i]); printf("]\n"); - RET_IF(expected_results[broken] != results[broken], - "unexpected result", - "expected_results[%u] != results[%u] bpf_prog_linum:%ld\n", - broken, broken, get_linum()); + printf("mismatch on %s (bpf_prog_linum:%ld)\n", result_to_str(broken), + get_linum()); + + CHECK_FAIL(true); } static int send_data(int type, sa_family_t family, void *data, size_t len, -- 2.20.1 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH bpf-next v2 4/4] selftests: bpf: reset global state between reuseport test runs 2020-01-24 11:27 ` [PATCH bpf-next v2 0/4] Various fixes for sockmap and reuseport tests Lorenz Bauer ` (2 preceding siblings ...) 2020-01-24 11:27 ` [PATCH bpf-next v2 3/4] selftests: bpf: make reuseport test output more legible Lorenz Bauer @ 2020-01-24 11:27 ` Lorenz Bauer 2020-01-24 12:45 ` [PATCH bpf-next v2 0/4] Various fixes for sockmap and reuseport tests Jakub Sitnicki 2020-01-24 15:45 ` Martin Lau 5 siblings, 0 replies; 26+ messages in thread From: Lorenz Bauer @ 2020-01-24 11:27 UTC (permalink / raw) To: Shuah Khan, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko Cc: Lorenz Bauer, linux-kselftest, netdev, bpf, linux-kernel Currently, there is a lot of false positives if a single reuseport test fails. This is because expected_results and the result map are not cleared. Zero both after individual test runs, which fixes the mentioned false positives. Signed-off-by: Lorenz Bauer <lmb@cloudflare.com> Fixes: 91134d849a0e ("bpf: Test BPF_PROG_TYPE_SK_REUSEPORT") --- .../selftests/bpf/prog_tests/select_reuseport.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/bpf/prog_tests/select_reuseport.c b/tools/testing/selftests/bpf/prog_tests/select_reuseport.c index e7e56929751c..098bcae5f827 100644 --- a/tools/testing/selftests/bpf/prog_tests/select_reuseport.c +++ b/tools/testing/selftests/bpf/prog_tests/select_reuseport.c @@ -33,7 +33,7 @@ #define REUSEPORT_ARRAY_SIZE 32 static int result_map, tmp_index_ovr_map, linum_map, data_check_map; -static enum result expected_results[NR_RESULTS]; +static __u32 expected_results[NR_RESULTS]; static int sk_fds[REUSEPORT_ARRAY_SIZE]; static int reuseport_array = -1, outer_map = -1; static int select_by_skb_data_prog; @@ -697,7 +697,19 @@ static void setup_per_test(int type, sa_family_t family, bool inany, static void cleanup_per_test(bool no_inner_map) { - int i, err; + int i, err, zero = 0; + + memset(expected_results, 0, sizeof(expected_results)); + + for (i = 0; i < NR_RESULTS; i++) { + err = bpf_map_update_elem(result_map, &i, &zero, BPF_ANY); + RET_IF(err, "reset elem in result_map", + "i:%u err:%d errno:%d\n", i, err, errno); + } + + err = bpf_map_update_elem(linum_map, &zero, &zero, BPF_ANY); + RET_IF(err, "reset line number in linum_map", "err:%d errno:%d\n", + err, errno); for (i = 0; i < REUSEPORT_ARRAY_SIZE; i++) close(sk_fds[i]); -- 2.20.1 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH bpf-next v2 0/4] Various fixes for sockmap and reuseport tests 2020-01-24 11:27 ` [PATCH bpf-next v2 0/4] Various fixes for sockmap and reuseport tests Lorenz Bauer ` (3 preceding siblings ...) 2020-01-24 11:27 ` [PATCH bpf-next v2 4/4] selftests: bpf: reset global state between reuseport test runs Lorenz Bauer @ 2020-01-24 12:45 ` Jakub Sitnicki 2020-01-24 15:45 ` Martin Lau 5 siblings, 0 replies; 26+ messages in thread From: Jakub Sitnicki @ 2020-01-24 12:45 UTC (permalink / raw) To: Lorenz Bauer Cc: Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko, netdev, bpf On Fri, Jan 24, 2020 at 12:27 PM CET, Lorenz Bauer wrote: > I've fixed the commit messages, added Fixes tags and am submitting to bpf-next instead > of the bpf tree. > > There is still the question whether patch #1 needs to preserve O_RDONLY, which John > can hopefully answer. > > Lorenz Bauer (4): > selftests: bpf: use a temporary file in test_sockmap > selftests: bpf: ignore FIN packets for reuseport tests > selftests: bpf: make reuseport test output more legible > selftests: bpf: reset global state between reuseport test runs > > .../bpf/prog_tests/select_reuseport.c | 44 ++++++++++++++++--- > .../bpf/progs/test_select_reuseport_kern.c | 6 +++ > tools/testing/selftests/bpf/test_sockmap.c | 15 +++---- > 3 files changed, 49 insertions(+), 16 deletions(-) For the series: Reviewed-by: Jakub Sitnicki <jakub@cloudflare.com> ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH bpf-next v2 0/4] Various fixes for sockmap and reuseport tests 2020-01-24 11:27 ` [PATCH bpf-next v2 0/4] Various fixes for sockmap and reuseport tests Lorenz Bauer ` (4 preceding siblings ...) 2020-01-24 12:45 ` [PATCH bpf-next v2 0/4] Various fixes for sockmap and reuseport tests Jakub Sitnicki @ 2020-01-24 15:45 ` Martin Lau 2020-01-24 21:14 ` Daniel Borkmann 5 siblings, 1 reply; 26+ messages in thread From: Martin Lau @ 2020-01-24 15:45 UTC (permalink / raw) To: Lorenz Bauer Cc: Alexei Starovoitov, Daniel Borkmann, Song Liu, Yonghong Song, Andrii Nakryiko, netdev, bpf On Fri, Jan 24, 2020 at 11:27:50AM +0000, Lorenz Bauer wrote: > I've fixed the commit messages, added Fixes tags and am submitting to bpf-next instead > of the bpf tree. Acked-by: Martin KaFai Lau <kafai@fb.com> ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH bpf-next v2 0/4] Various fixes for sockmap and reuseport tests 2020-01-24 15:45 ` Martin Lau @ 2020-01-24 21:14 ` Daniel Borkmann 0 siblings, 0 replies; 26+ messages in thread From: Daniel Borkmann @ 2020-01-24 21:14 UTC (permalink / raw) To: Martin Lau, Lorenz Bauer Cc: Alexei Starovoitov, Song Liu, Yonghong Song, Andrii Nakryiko, netdev, bpf On 1/24/20 4:45 PM, Martin Lau wrote: > On Fri, Jan 24, 2020 at 11:27:50AM +0000, Lorenz Bauer wrote: >> I've fixed the commit messages, added Fixes tags and am submitting to bpf-next instead >> of the bpf tree. > Acked-by: Martin KaFai Lau <kafai@fb.com> Series applied (and also added John's ACKs he just sent for the v1 set), thanks! ^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2020-01-24 21:15 UTC | newest] Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-01-23 16:59 [PATCH bpf 0/4] Various fixes for sockmap and reuseport tests Lorenz Bauer 2020-01-23 16:59 ` [PATCH bpf 1/4] selftests: bpf: use a temporary file in test_sockmap Lorenz Bauer 2020-01-23 18:34 ` Martin Lau 2020-01-24 19:50 ` John Fastabend 2020-01-24 9:20 ` Jakub Sitnicki 2020-01-24 19:43 ` John Fastabend 2020-01-23 16:59 ` [PATCH bpf 2/4] selftests: bpf: ignore RST packets for reuseport tests Lorenz Bauer 2020-01-23 17:16 ` Martin Lau 2020-01-23 17:18 ` Lorenz Bauer 2020-01-23 21:53 ` Martin Lau 2020-01-24 9:00 ` Lorenz Bauer 2020-01-24 9:32 ` Daniel Borkmann 2020-01-24 19:45 ` John Fastabend 2020-01-23 16:59 ` [PATCH bpf 3/4] selftests: bpf: make reuseport test output more legible Lorenz Bauer 2020-01-23 17:26 ` Martin Lau 2020-01-24 19:46 ` John Fastabend 2020-01-23 16:59 ` [PATCH bpf 4/4] selftests: bpf: reset global state between reuseport test runs Lorenz Bauer 2020-01-23 17:56 ` Martin Lau 2020-01-24 11:27 ` [PATCH bpf-next v2 0/4] Various fixes for sockmap and reuseport tests Lorenz Bauer 2020-01-24 11:27 ` [PATCH bpf-next v2 1/4] selftests: bpf: use a temporary file in test_sockmap Lorenz Bauer 2020-01-24 11:27 ` [PATCH bpf-next v2 2/4] selftests: bpf: ignore FIN packets for reuseport tests Lorenz Bauer 2020-01-24 11:27 ` [PATCH bpf-next v2 3/4] selftests: bpf: make reuseport test output more legible Lorenz Bauer 2020-01-24 11:27 ` [PATCH bpf-next v2 4/4] selftests: bpf: reset global state between reuseport test runs Lorenz Bauer 2020-01-24 12:45 ` [PATCH bpf-next v2 0/4] Various fixes for sockmap and reuseport tests Jakub Sitnicki 2020-01-24 15:45 ` Martin Lau 2020-01-24 21:14 ` Daniel Borkmann
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).