* [PATCH bpf] selftests/bpf: Relax time_tai test for equal timestamps in tai_forward
@ 2023-12-12 18:29 YiFei Zhu
2023-12-12 21:39 ` Yonghong Song
2023-12-13 0:00 ` patchwork-bot+netdevbpf
0 siblings, 2 replies; 6+ messages in thread
From: YiFei Zhu @ 2023-12-12 18:29 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Stanislav Fomichev,
Martin KaFai Lau, Andrii Nakryiko, Song Liu, Kurt Kanzenbach
We're observing test flakiness on an arm64 platform which might not
have timestamps as precise as x86. The test log looks like:
test_time_tai:PASS:tai_open 0 nsec
test_time_tai:PASS:test_run 0 nsec
test_time_tai:PASS:tai_ts1 0 nsec
test_time_tai:PASS:tai_ts2 0 nsec
test_time_tai:FAIL:tai_forward unexpected tai_forward: actual 1702348135471494160 <= expected 1702348135471494160
test_time_tai:PASS:tai_gettime 0 nsec
test_time_tai:PASS:tai_future_ts1 0 nsec
test_time_tai:PASS:tai_future_ts2 0 nsec
test_time_tai:PASS:tai_range_ts1 0 nsec
test_time_tai:PASS:tai_range_ts2 0 nsec
#199 time_tai:FAIL
This patch changes ASSERT_GT to ASSERT_GE in the tai_forward assertion
so that equal timestamps are permitted.
Fixes: 64e15820b987 ("selftests/bpf: Add BPF-helper test for CLOCK_TAI access")
Signed-off-by: YiFei Zhu <zhuyifei@google.com>
---
tools/testing/selftests/bpf/prog_tests/time_tai.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/time_tai.c b/tools/testing/selftests/bpf/prog_tests/time_tai.c
index a31119823666..f45af1b0ef2c 100644
--- a/tools/testing/selftests/bpf/prog_tests/time_tai.c
+++ b/tools/testing/selftests/bpf/prog_tests/time_tai.c
@@ -56,7 +56,7 @@ void test_time_tai(void)
ASSERT_NEQ(ts2, 0, "tai_ts2");
/* TAI is moving forward only */
- ASSERT_GT(ts2, ts1, "tai_forward");
+ ASSERT_GE(ts2, ts1, "tai_forward");
/* Check for future */
ret = clock_gettime(CLOCK_TAI, &now_tai);
--
2.43.0.472.g3155946c3a-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH bpf] selftests/bpf: Relax time_tai test for equal timestamps in tai_forward
2023-12-12 18:29 [PATCH bpf] selftests/bpf: Relax time_tai test for equal timestamps in tai_forward YiFei Zhu
@ 2023-12-12 21:39 ` Yonghong Song
2023-12-12 21:52 ` YiFei Zhu
2023-12-13 0:00 ` patchwork-bot+netdevbpf
1 sibling, 1 reply; 6+ messages in thread
From: Yonghong Song @ 2023-12-12 21:39 UTC (permalink / raw)
To: YiFei Zhu, bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Stanislav Fomichev,
Martin KaFai Lau, Andrii Nakryiko, Song Liu, Kurt Kanzenbach
On 12/12/23 10:29 AM, YiFei Zhu wrote:
> We're observing test flakiness on an arm64 platform which might not
> have timestamps as precise as x86. The test log looks like:
>
> test_time_tai:PASS:tai_open 0 nsec
> test_time_tai:PASS:test_run 0 nsec
> test_time_tai:PASS:tai_ts1 0 nsec
> test_time_tai:PASS:tai_ts2 0 nsec
> test_time_tai:FAIL:tai_forward unexpected tai_forward: actual 1702348135471494160 <= expected 1702348135471494160
> test_time_tai:PASS:tai_gettime 0 nsec
> test_time_tai:PASS:tai_future_ts1 0 nsec
> test_time_tai:PASS:tai_future_ts2 0 nsec
> test_time_tai:PASS:tai_range_ts1 0 nsec
> test_time_tai:PASS:tai_range_ts2 0 nsec
> #199 time_tai:FAIL
>
> This patch changes ASSERT_GT to ASSERT_GE in the tai_forward assertion
> so that equal timestamps are permitted.
>
> Fixes: 64e15820b987 ("selftests/bpf: Add BPF-helper test for CLOCK_TAI access")
> Signed-off-by: YiFei Zhu <zhuyifei@google.com>
> ---
> tools/testing/selftests/bpf/prog_tests/time_tai.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/time_tai.c b/tools/testing/selftests/bpf/prog_tests/time_tai.c
> index a31119823666..f45af1b0ef2c 100644
> --- a/tools/testing/selftests/bpf/prog_tests/time_tai.c
> +++ b/tools/testing/selftests/bpf/prog_tests/time_tai.c
> @@ -56,7 +56,7 @@ void test_time_tai(void)
> ASSERT_NEQ(ts2, 0, "tai_ts2");
>
> /* TAI is moving forward only */
> - ASSERT_GT(ts2, ts1, "tai_forward");
> + ASSERT_GE(ts2, ts1, "tai_forward");
Can we guard the new change with arm64 specific macro?
>
> /* Check for future */
> ret = clock_gettime(CLOCK_TAI, &now_tai);
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH bpf] selftests/bpf: Relax time_tai test for equal timestamps in tai_forward
2023-12-12 21:39 ` Yonghong Song
@ 2023-12-12 21:52 ` YiFei Zhu
2023-12-13 0:01 ` Andrii Nakryiko
2023-12-19 9:57 ` Kurt Kanzenbach
0 siblings, 2 replies; 6+ messages in thread
From: YiFei Zhu @ 2023-12-12 21:52 UTC (permalink / raw)
To: Yonghong Song
Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Stanislav Fomichev,
Martin KaFai Lau, Andrii Nakryiko, Song Liu, Kurt Kanzenbach
On Tue, Dec 12, 2023 at 1:39 PM Yonghong Song <yonghong.song@linux.dev> wrote:
>
>
> On 12/12/23 10:29 AM, YiFei Zhu wrote:
> > We're observing test flakiness on an arm64 platform which might not
> > have timestamps as precise as x86. The test log looks like:
> >
> > test_time_tai:PASS:tai_open 0 nsec
> > test_time_tai:PASS:test_run 0 nsec
> > test_time_tai:PASS:tai_ts1 0 nsec
> > test_time_tai:PASS:tai_ts2 0 nsec
> > test_time_tai:FAIL:tai_forward unexpected tai_forward: actual 1702348135471494160 <= expected 1702348135471494160
> > test_time_tai:PASS:tai_gettime 0 nsec
> > test_time_tai:PASS:tai_future_ts1 0 nsec
> > test_time_tai:PASS:tai_future_ts2 0 nsec
> > test_time_tai:PASS:tai_range_ts1 0 nsec
> > test_time_tai:PASS:tai_range_ts2 0 nsec
> > #199 time_tai:FAIL
> >
> > This patch changes ASSERT_GT to ASSERT_GE in the tai_forward assertion
> > so that equal timestamps are permitted.
> >
> > Fixes: 64e15820b987 ("selftests/bpf: Add BPF-helper test for CLOCK_TAI access")
> > Signed-off-by: YiFei Zhu <zhuyifei@google.com>
> > ---
> > tools/testing/selftests/bpf/prog_tests/time_tai.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/time_tai.c b/tools/testing/selftests/bpf/prog_tests/time_tai.c
> > index a31119823666..f45af1b0ef2c 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/time_tai.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/time_tai.c
> > @@ -56,7 +56,7 @@ void test_time_tai(void)
> > ASSERT_NEQ(ts2, 0, "tai_ts2");
> >
> > /* TAI is moving forward only */
> > - ASSERT_GT(ts2, ts1, "tai_forward");
> > + ASSERT_GE(ts2, ts1, "tai_forward");
>
> Can we guard the new change with arm64 specific macro?
Problem with this is that I'm not sure what other architectures could
be affected. AFAICT from the test, what it cares about is that time is
moving forwards rather than going backwards, so I thought GE is good
enough for what it's testing for.
>
> >
> > /* Check for future */
> > ret = clock_gettime(CLOCK_TAI, &now_tai);
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH bpf] selftests/bpf: Relax time_tai test for equal timestamps in tai_forward
2023-12-12 21:52 ` YiFei Zhu
@ 2023-12-13 0:01 ` Andrii Nakryiko
2023-12-19 9:57 ` Kurt Kanzenbach
1 sibling, 0 replies; 6+ messages in thread
From: Andrii Nakryiko @ 2023-12-13 0:01 UTC (permalink / raw)
To: YiFei Zhu
Cc: Yonghong Song, bpf, Alexei Starovoitov, Daniel Borkmann,
Stanislav Fomichev, Martin KaFai Lau, Andrii Nakryiko, Song Liu,
Kurt Kanzenbach
On Tue, Dec 12, 2023 at 1:53 PM YiFei Zhu <zhuyifei@google.com> wrote:
>
> On Tue, Dec 12, 2023 at 1:39 PM Yonghong Song <yonghong.song@linux.dev> wrote:
> >
> >
> > On 12/12/23 10:29 AM, YiFei Zhu wrote:
> > > We're observing test flakiness on an arm64 platform which might not
> > > have timestamps as precise as x86. The test log looks like:
> > >
> > > test_time_tai:PASS:tai_open 0 nsec
> > > test_time_tai:PASS:test_run 0 nsec
> > > test_time_tai:PASS:tai_ts1 0 nsec
> > > test_time_tai:PASS:tai_ts2 0 nsec
> > > test_time_tai:FAIL:tai_forward unexpected tai_forward: actual 1702348135471494160 <= expected 1702348135471494160
> > > test_time_tai:PASS:tai_gettime 0 nsec
> > > test_time_tai:PASS:tai_future_ts1 0 nsec
> > > test_time_tai:PASS:tai_future_ts2 0 nsec
> > > test_time_tai:PASS:tai_range_ts1 0 nsec
> > > test_time_tai:PASS:tai_range_ts2 0 nsec
> > > #199 time_tai:FAIL
> > >
> > > This patch changes ASSERT_GT to ASSERT_GE in the tai_forward assertion
> > > so that equal timestamps are permitted.
> > >
> > > Fixes: 64e15820b987 ("selftests/bpf: Add BPF-helper test for CLOCK_TAI access")
> > > Signed-off-by: YiFei Zhu <zhuyifei@google.com>
> > > ---
> > > tools/testing/selftests/bpf/prog_tests/time_tai.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/tools/testing/selftests/bpf/prog_tests/time_tai.c b/tools/testing/selftests/bpf/prog_tests/time_tai.c
> > > index a31119823666..f45af1b0ef2c 100644
> > > --- a/tools/testing/selftests/bpf/prog_tests/time_tai.c
> > > +++ b/tools/testing/selftests/bpf/prog_tests/time_tai.c
> > > @@ -56,7 +56,7 @@ void test_time_tai(void)
> > > ASSERT_NEQ(ts2, 0, "tai_ts2");
> > >
> > > /* TAI is moving forward only */
> > > - ASSERT_GT(ts2, ts1, "tai_forward");
> > > + ASSERT_GE(ts2, ts1, "tai_forward");
> >
> > Can we guard the new change with arm64 specific macro?
>
> Problem with this is that I'm not sure what other architectures could
> be affected. AFAICT from the test, what it cares about is that time is
> moving forwards rather than going backwards, so I thought GE is good
> enough for what it's testing for.
>
Agreed. I think having architecture-specific GE vs GT here will just
add more complexity than necessary without providing any added safety.
So I applied the patch to bpf-next as is, thanks.
> >
> > >
> > > /* Check for future */
> > > ret = clock_gettime(CLOCK_TAI, &now_tai);
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH bpf] selftests/bpf: Relax time_tai test for equal timestamps in tai_forward
2023-12-12 21:52 ` YiFei Zhu
2023-12-13 0:01 ` Andrii Nakryiko
@ 2023-12-19 9:57 ` Kurt Kanzenbach
1 sibling, 0 replies; 6+ messages in thread
From: Kurt Kanzenbach @ 2023-12-19 9:57 UTC (permalink / raw)
To: YiFei Zhu, Yonghong Song
Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Stanislav Fomichev,
Martin KaFai Lau, Andrii Nakryiko, Song Liu
[-- Attachment #1: Type: text/plain, Size: 968 bytes --]
On Tue Dec 12 2023, YiFei Zhu wrote:
>> > diff --git a/tools/testing/selftests/bpf/prog_tests/time_tai.c b/tools/testing/selftests/bpf/prog_tests/time_tai.c
>> > index a31119823666..f45af1b0ef2c 100644
>> > --- a/tools/testing/selftests/bpf/prog_tests/time_tai.c
>> > +++ b/tools/testing/selftests/bpf/prog_tests/time_tai.c
>> > @@ -56,7 +56,7 @@ void test_time_tai(void)
>> > ASSERT_NEQ(ts2, 0, "tai_ts2");
>> >
>> > /* TAI is moving forward only */
>> > - ASSERT_GT(ts2, ts1, "tai_forward");
>> > + ASSERT_GE(ts2, ts1, "tai_forward");
>>
>> Can we guard the new change with arm64 specific macro?
>
> Problem with this is that I'm not sure what other architectures could
> be affected. AFAICT from the test, what it cares about is that time is
> moving forwards rather than going backwards, so I thought GE is good
> enough for what it's testing for.
Yes, exactly. The time must not go backwards. Checking for GE is
fine. Thanks for fixing this.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf] selftests/bpf: Relax time_tai test for equal timestamps in tai_forward
2023-12-12 18:29 [PATCH bpf] selftests/bpf: Relax time_tai test for equal timestamps in tai_forward YiFei Zhu
2023-12-12 21:39 ` Yonghong Song
@ 2023-12-13 0:00 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-12-13 0:00 UTC (permalink / raw)
To: YiFei Zhu; +Cc: bpf, ast, daniel, sdf, martin.lau, andrii, songliubraving, kurt
Hello:
This patch was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <andrii@kernel.org>:
On Tue, 12 Dec 2023 18:29:11 +0000 you wrote:
> We're observing test flakiness on an arm64 platform which might not
> have timestamps as precise as x86. The test log looks like:
>
> test_time_tai:PASS:tai_open 0 nsec
> test_time_tai:PASS:test_run 0 nsec
> test_time_tai:PASS:tai_ts1 0 nsec
> test_time_tai:PASS:tai_ts2 0 nsec
> test_time_tai:FAIL:tai_forward unexpected tai_forward: actual 1702348135471494160 <= expected 1702348135471494160
> test_time_tai:PASS:tai_gettime 0 nsec
> test_time_tai:PASS:tai_future_ts1 0 nsec
> test_time_tai:PASS:tai_future_ts2 0 nsec
> test_time_tai:PASS:tai_range_ts1 0 nsec
> test_time_tai:PASS:tai_range_ts2 0 nsec
> #199 time_tai:FAIL
>
> [...]
Here is the summary with links:
- [bpf] selftests/bpf: Relax time_tai test for equal timestamps in tai_forward
https://git.kernel.org/bpf/bpf-next/c/e1ba7f64b192
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] 6+ messages in thread
end of thread, other threads:[~2023-12-19 9:57 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-12 18:29 [PATCH bpf] selftests/bpf: Relax time_tai test for equal timestamps in tai_forward YiFei Zhu
2023-12-12 21:39 ` Yonghong Song
2023-12-12 21:52 ` YiFei Zhu
2023-12-13 0:01 ` Andrii Nakryiko
2023-12-19 9:57 ` Kurt Kanzenbach
2023-12-13 0:00 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox