[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAEf4BzYSzOnJ9_76RFu7e6o_Q2JEen+F-GZbzaW86yh5xnM3Qw@mail.gmail.com>
Date: Wed, 3 Aug 2022 10:29:53 -0700
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Maciej Fijalkowski <maciej.fijalkowski@...el.com>
Cc: Kurt Kanzenbach <kurt@...utronix.de>,
Alexei Starovoitov <alexei.starovoitov@...il.com>,
Jesper Dangaard Brouer <jbrouer@...hat.com>,
Thomas Gleixner <tglx@...utronix.de>,
Jesper Dangaard Brouer <brouer@...hat.com>,
Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Andrii Nakryiko <andrii@...nel.org>,
Martin KaFai Lau <kafai@...com>,
Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
John Fastabend <john.fastabend@...il.com>,
KP Singh <kpsingh@...nel.org>,
Joanne Koong <joannelkoong@...il.com>,
Jiri Olsa <jolsa@...nel.org>,
Dave Marchevsky <davemarchevsky@...com>,
Lorenzo Bianconi <lorenzo@...nel.org>,
Geliang Tang <geliang.tang@...e.com>,
Jakub Sitnicki <jakub@...udflare.com>,
Network Development <netdev@...r.kernel.org>,
bpf <bpf@...r.kernel.org>
Subject: Re: [PATCH bpf-next] bpf: Add BPF-helper for accessing CLOCK_TAI
On Wed, Aug 3, 2022 at 2:29 AM Maciej Fijalkowski
<maciej.fijalkowski@...el.com> wrote:
>
> On Wed, Aug 03, 2022 at 08:29:29AM +0200, Kurt Kanzenbach wrote:
> > On Tue Aug 02 2022, Alexei Starovoitov wrote:
> > > On Tue, Aug 2, 2022 at 12:06 AM Kurt Kanzenbach <kurt@...utronix.de> wrote:
> > >>
> > >> Hi Alexei,
> > >>
> > >> On Tue Jun 07 2022, Alexei Starovoitov wrote:
> > >> > Anyway I guess new helper bpf_ktime_get_tai_ns() is ok, since
> > >> > it's so trivial, but selftest is necessary.
> > >>
> > >> So, I did write a selftest [1] for testing bpf_ktime_get_tai_ns() and
> > >> verifying that the access to the clock works. It uses AF_XDP sockets and
> > >> timestamps the incoming packets. The timestamps are then validated in
> > >> user space.
> > >>
> > >> Since AF_XDP related code is migrating from libbpf to libxdp, I'm
> > >> wondering if that sample fits into the kernel's selftests or not. What
> > >> kind of selftest are you looking for?
> > >
> > > Please use selftests/bpf framework.
> > > There are plenty of networking tests in there.
> > > bpf_ktime_get_tai_ns() doesn't have to rely on af_xdp.
> >
> > OK.
> >
> > > It can be skb based.
>
> FWIW there is xskxceiver and libbpf's xsk part in selftests/bpf framework,
> so your initial work should be fine in there. Personally I found both
> (AF_XDP and SKB one, below) tests valuable.
test_progs is always tested on each patch/patch set. xskxceiver can
potentially break without anyone noticing for a while. So having
something like this in test_progs is much better.
>
> Later on, if we add a support to xskxceiver for loading external BPF progs
> then your sample would just become another test case in there.
>
> >
> > Something like this?
> >
> > +++ b/tools/testing/selftests/bpf/prog_tests/check_tai.c
> > @@ -0,0 +1,57 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Copyright (C) 2022 Linutronix GmbH */
> > +
> > +#include <test_progs.h>
> > +#include <network_helpers.h>
> > +
> > +#include <time.h>
> > +#include <stdint.h>
> > +
> > +#define TAI_THRESHOLD 1000000000ULL /* 1s */
> > +#define NSEC_PER_SEC 1000000000ULL
> > +
> > +static __u64 ts_to_ns(const struct timespec *ts)
> > +{
> > + return ts->tv_sec * NSEC_PER_SEC + ts->tv_nsec;
> > +}
> > +
> > +void test_tai(void)
> > +{
> > + struct __sk_buff skb = {
> > + .tstamp = 0,
> > + .hwtstamp = 0,
> > + };
> > + LIBBPF_OPTS(bpf_test_run_opts, topts,
> > + .data_in = &pkt_v4,
> > + .data_size_in = sizeof(pkt_v4),
> > + .ctx_in = &skb,
> > + .ctx_size_in = sizeof(skb),
> > + .ctx_out = &skb,
> > + .ctx_size_out = sizeof(skb),
> > + );
> > + struct timespec now_tai;
> > + struct bpf_object *obj;
> > + int ret, prog_fd;
> > +
> > + ret = bpf_prog_test_load("./test_tai.o",
> > + BPF_PROG_TYPE_SCHED_CLS, &obj, &prog_fd);
> > + if (!ASSERT_OK(ret, "load"))
> > + return;
> > + ret = bpf_prog_test_run_opts(prog_fd, &topts);
> > + ASSERT_OK(ret, "test_run");
> > +
> > + /* TAI != 0 */
> > + ASSERT_NEQ(skb.tstamp, 0, "tai_ts0_0");
> > + ASSERT_NEQ(skb.hwtstamp, 0, "tai_ts0_1");
> > +
> > + /* TAI is moving forward only */
> > + ASSERT_GT(skb.hwtstamp, skb.tstamp, "tai_forward");
> > +
> > + /* Check for reasoneable range */
> > + ret = clock_gettime(CLOCK_TAI, &now_tai);
> > + ASSERT_EQ(ret, 0, "tai_gettime");
> > + ASSERT_TRUE((ts_to_ns(&now_tai) - skb.hwtstamp) < TAI_THRESHOLD,
> > + "tai_range");
> > +
> > + bpf_object__close(obj);
> > +}
> > diff --git a/tools/testing/selftests/bpf/progs/test_tai.c b/tools/testing/selftests/bpf/progs/test_tai.c
> > new file mode 100644
> > index 000000000000..34ac4175e29d
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/progs/test_tai.c
> > @@ -0,0 +1,17 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Copyright (C) 2022 Linutronix GmbH */
> > +
> > +#include <linux/bpf.h>
> > +#include <bpf/bpf_helpers.h>
> > +
> > +char _license[] SEC("license") = "GPL";
> > +
> > +SEC("tc")
> > +int save_tai(struct __sk_buff *skb)
> > +{
> > + /* Save TAI timestamps */
> > + skb->tstamp = bpf_ktime_get_tai_ns();
> > + skb->hwtstamp = bpf_ktime_get_tai_ns();
> > +
> > + return 0;
> > +}
>
>
Powered by blists - more mailing lists