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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAEf4BzZ7Srd2k5a_t6JKW9_=cUQVqvxXhd+4rvbpMHKRJAQbiw@mail.gmail.com>
Date:   Thu, 24 Sep 2020 13:22:02 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     "Pujari, Bimmy" <bimmy.pujari@...el.com>
Cc:     bpf <bpf@...r.kernel.org>, Networking <netdev@...r.kernel.org>,
        mchehab@...nel.org, Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Martin Lau <kafai@...com>,
        Maciej Żenczykowski <maze@...gle.com>,
        "Nikravesh, Ashkan" <ashkan.nikravesh@...el.com>
Subject: Re: [PATCH bpf-next v3 2/2] selftests/bpf: Verifying real time helper function

On Wed, Sep 23, 2020 at 7:26 PM <bimmy.pujari@...el.com> wrote:
>
> From: Bimmy Pujari <bimmy.pujari@...el.com>
>
> 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 <bimmy.pujari@...el.com>
> ---

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
>

[...]