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 <[email protected]> Sent: Thursday, September 24, 2020 1:22 PM To: Pujari, Bimmy <[email protected]> Cc: bpf <[email protected]>; Networking <[email protected]>; [email protected]; Alexei Starovoitov <[email protected]>; Daniel Borkmann <[email protected]>; Martin Lau <[email protected]>; Maciej Żenczykowski <[email protected]>; Nikravesh, Ashkan <[email protected]> Subject: Re: [PATCH bpf-next v3 2/2] selftests/bpf: Verifying real time helper function On Wed, Sep 23, 2020 at 7:26 PM <[email protected]> wrote: > > From: Bimmy Pujari <[email protected]> > > 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 <[email protected]> > --- 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 > [...]
