RE: [PATCH bpf-next v3 2/2] selftests/bpf: Verifying real time helper function

I agree with you. I will write a separate new test. In the meanwhile I think it should be okay to prepare and send just one patch.

Thanks
Bimmy

-----Original Message-----
From: Andrii Nakryiko <redacted> 
Sent: Thursday, September 24, 2020 1:22 PM
To: Pujari, Bimmy <redacted>
Cc: bpf <redacted>; Networking <redacted>; mchehab@kernel.org; Alexei Starovoitov <ast@kernel.org>; Daniel Borkmann <daniel@iogearbox.net>; Martin Lau <redacted>; Maciej Żenczykowski <redacted>; Nikravesh, Ashkan <redacted>
Subject: Re: [PATCH bpf-next v3 2/2] selftests/bpf: Verifying real time helper function

On Wed, Sep 23, 2020 at 7:26 PM [off-list ref] wrote:

From: Bimmy Pujari <redacted>

Test xdping measures RTT from xdp using monotonic time helper.
Extending xdping test to use real time helper function in order to 
verify this helper function.

Signed-off-by: Bimmy Pujari <redacted>
---

This is exactly the use of REALTIME clock that I was arguing against, and yet you are actually creating an example of how to use it for such case. CLOCK_REALTIME should not be used to measuring time elapsed (not within the same machine, at least), there are strictly better alternatives.

So if you want to write a test for a new helper (assuming everyone else thinks it's a good idea), then do just that - write a separate minimal test that tests just your new functionality. Don't couple it with a massive XDP program. And also don't create unnecessarily almost
400 lines of code churn.

 .../testing/selftests/bpf/progs/xdping_kern.c | 183 +----------------  
.../testing/selftests/bpf/progs/xdping_kern.h | 193 ++++++++++++++++++
 .../bpf/progs/xdping_realtime_kern.c          |   4 +
 tools/testing/selftests/bpf/test_xdping.sh    |  44 +++-
 4 files changed, 235 insertions(+), 189 deletions(-)  create mode 
100644 tools/testing/selftests/bpf/progs/xdping_kern.h
 create mode 100644 
tools/testing/selftests/bpf/progs/xdping_realtime_kern.c

[...]