| Message ID | 20200123165934.9584-2-lmb@cloudflare.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [bpf,1/4] selftests: bpf: use a temporary file in test_sockmap | expand |
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++) {
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. [...]
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>
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++) {
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; }
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(-)