* [PATCH bpf-next] selftests/bpf: Preserve errno in test_progs CHECK macros
@ 2019-12-20 0:05 Andrey Ignatov
2019-12-20 18:09 ` Andrii Nakryiko
2019-12-21 0:09 ` Alexei Starovoitov
0 siblings, 2 replies; 3+ messages in thread
From: Andrey Ignatov @ 2019-12-20 0:05 UTC (permalink / raw)
To: bpf; +Cc: Andrey Ignatov, ast, daniel, andriin, kernel-team
It's follow-up for discussion [1]
CHECK and CHECK_FAIL macros in test_progs.h can affect errno in some
circumstances, e.g. if some code accidentally closes stdout. It makes
checking errno in patterns like this unreliable:
if (CHECK(!bpf_prog_attach_xattr(...), "tag", "msg"))
goto err;
CHECK_FAIL(errno != ENOENT);
, since by CHECK_FAIL time errno could be affected not only by
bpf_prog_attach_xattr but by CHECK as well.
Fix it by saving and restoring errno in the macros. There is no "Fixes"
tag since no problems were discovered yet and it's rather precaution.
test_progs was run with this change and no difference was identified.
[1] https://lore.kernel.org/bpf/20191219210907.GD16266@rdna-mbp.dhcp.thefacebook.com/
Signed-off-by: Andrey Ignatov <rdna@fb.com>
---
tools/testing/selftests/bpf/test_progs.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h
index 8477df835979..de1fdaa4e7b4 100644
--- a/tools/testing/selftests/bpf/test_progs.h
+++ b/tools/testing/selftests/bpf/test_progs.h
@@ -100,6 +100,7 @@ extern struct ipv6_packet pkt_v6;
#define _CHECK(condition, tag, duration, format...) ({ \
int __ret = !!(condition); \
+ int __save_errno = errno; \
if (__ret) { \
test__fail(); \
printf("%s:FAIL:%s ", __func__, tag); \
@@ -108,15 +109,18 @@ extern struct ipv6_packet pkt_v6;
printf("%s:PASS:%s %d nsec\n", \
__func__, tag, duration); \
} \
+ errno = __save_errno; \
__ret; \
})
#define CHECK_FAIL(condition) ({ \
int __ret = !!(condition); \
+ int __save_errno = errno; \
if (__ret) { \
test__fail(); \
printf("%s:FAIL:%d\n", __func__, __LINE__); \
} \
+ errno = __save_errno; \
__ret; \
})
--
2.17.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Preserve errno in test_progs CHECK macros
2019-12-20 0:05 [PATCH bpf-next] selftests/bpf: Preserve errno in test_progs CHECK macros Andrey Ignatov
@ 2019-12-20 18:09 ` Andrii Nakryiko
2019-12-21 0:09 ` Alexei Starovoitov
1 sibling, 0 replies; 3+ messages in thread
From: Andrii Nakryiko @ 2019-12-20 18:09 UTC (permalink / raw)
To: Andrey Ignatov
Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Kernel Team
On Thu, Dec 19, 2019 at 4:06 PM Andrey Ignatov <rdna@fb.com> wrote:
>
> It's follow-up for discussion [1]
>
> CHECK and CHECK_FAIL macros in test_progs.h can affect errno in some
> circumstances, e.g. if some code accidentally closes stdout. It makes
> checking errno in patterns like this unreliable:
>
> if (CHECK(!bpf_prog_attach_xattr(...), "tag", "msg"))
> goto err;
> CHECK_FAIL(errno != ENOENT);
>
> , since by CHECK_FAIL time errno could be affected not only by
> bpf_prog_attach_xattr but by CHECK as well.
>
> Fix it by saving and restoring errno in the macros. There is no "Fixes"
> tag since no problems were discovered yet and it's rather precaution.
>
> test_progs was run with this change and no difference was identified.
>
> [1] https://lore.kernel.org/bpf/20191219210907.GD16266@rdna-mbp.dhcp.thefacebook.com/
>
> Signed-off-by: Andrey Ignatov <rdna@fb.com>
> ---
LGTM. Doesn't hurt for sure.
Acked-by: Andrii Nakryiko <andriin@fb.com>
> tools/testing/selftests/bpf/test_progs.h | 4 ++++
> 1 file changed, 4 insertions(+)
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Preserve errno in test_progs CHECK macros
2019-12-20 0:05 [PATCH bpf-next] selftests/bpf: Preserve errno in test_progs CHECK macros Andrey Ignatov
2019-12-20 18:09 ` Andrii Nakryiko
@ 2019-12-21 0:09 ` Alexei Starovoitov
1 sibling, 0 replies; 3+ messages in thread
From: Alexei Starovoitov @ 2019-12-21 0:09 UTC (permalink / raw)
To: Andrey Ignatov
Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Kernel Team
On Thu, Dec 19, 2019 at 4:05 PM Andrey Ignatov <rdna@fb.com> wrote:
>
> It's follow-up for discussion [1]
>
> CHECK and CHECK_FAIL macros in test_progs.h can affect errno in some
> circumstances, e.g. if some code accidentally closes stdout. It makes
> checking errno in patterns like this unreliable:
>
> if (CHECK(!bpf_prog_attach_xattr(...), "tag", "msg"))
> goto err;
> CHECK_FAIL(errno != ENOENT);
>
> , since by CHECK_FAIL time errno could be affected not only by
> bpf_prog_attach_xattr but by CHECK as well.
>
> Fix it by saving and restoring errno in the macros. There is no "Fixes"
> tag since no problems were discovered yet and it's rather precaution.
>
> test_progs was run with this change and no difference was identified.
>
> [1] https://lore.kernel.org/bpf/20191219210907.GD16266@rdna-mbp.dhcp.thefacebook.com/
>
> Signed-off-by: Andrey Ignatov <rdna@fb.com>
Applied. Thanks
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-12-21 0:09 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-20 0:05 [PATCH bpf-next] selftests/bpf: Preserve errno in test_progs CHECK macros Andrey Ignatov
2019-12-20 18:09 ` Andrii Nakryiko
2019-12-21 0:09 ` Alexei Starovoitov
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).