On Tue, Jul 28, 2020 at 10:57 AM Maciej Żenczykowski [off-list ref] wrote:

On Mon, Jul 27, 2020 at 10:01 PM Andrii Nakryiko [off-list ref] wrote:

quoted

On Mon, Jul 27, 2020 at 4:35 PM [off-list ref] wrote:

quoted

From: Ashkan Nikravesh <redacted>

The existing bpf helper functions to get timestamp return the time
elapsed since system boot. This timestamp is not particularly useful
where epoch timestamp is required or more than one server is involved
and time sync is required. Instead, you want to use CLOCK_REALTIME,
which provides epoch timestamp.
Hence add bfp_ktime_get_real_ns() based around CLOCK_REALTIME.

This doesn't seem like a good idea.

I disagree.

quoted

With time-since-boot it's very
easy to translate timestamp into a real time on the host.

While it's easy to do, it's annoying, because you need to hardcode the offset into the bpf program
(which has security implications) or use another bpf array lookup/read (perf implications).

There is no array lookup/read if you are using a global variable for this.

quoted

Having
get_real_ns() variant might just encourage people to assume precise
wall-clock timestamps that can be compared between within or even
across different hosts.

In some environments they *can*.  Within a datacenter it is pretty common
to have extremely tightly synchronized clocks across thousands of machines.

In some, yes, which also means that in some other they can't. So I'm
still worried about misuses of REALCLOCK, within (internal daemons
within the company) our outside (BCC tools and alike) of data centers.
Especially if people will start using it to measure elapsed time
between events. I'd rather not have to explain over and over again
that REALCLOCK is not for measuring passage of time.

quoted

REALCLOCK can jump around, you can get
duplicate timestamps, timestamps can go back in time, etc.

That's only true if you allow it to.  On a server machine, once it's booted

Right, but BPF is used so widely that it will be true at least in some cases.

up and time is synchronized, it no longer jumps around, only the frequency
is slowly adjusted to keep things in sync.

quoted

It's just not a good way to measure time.

It doesn't change the fact that some packets/protocols include real world timestamps.
We already have the since-boot time fetchers, so this doesn't prevent you from
using that - if that is what you want.

Also one thing to remember is that a lot of time you only need ~1s precision.
In which case 'real' can simply be a lot easier to deal with.

Again, I'm worried about ensuing confusion by users trying to pick
which variant to use. I'm not worried about those who understand the
difference and can pick the right one for their needs, I'm worried
about those that don't even give it a second thought. Given it's
rather simple to convert boot time into wall-clock time, it doesn't
seem like a necessary addition.

quoted

Also, you mention the need for time sync. It's an extremely hard thing
to have synchronized time between two different physical hosts, as
anyone that has dealt with distributed systems will attest. Having
this helper will just create a dangerous illusion that it is possible
and will just cause more problems down the road for people.

It is possible.  But yes.  It is very very hard.
You can read up on Google TrueTime as an example real world implementation.

Thank you, I did, though quite a while ago. Notice, I didn't say it's
impossible, right? ;) But then Google TrueTime provides a range of
time within confidence intervals, not a single seemingly-deterministic
timestamp. And it needs custom hardware, so it's not realistic to
expect to have it everywhere :)

quoted

quoted

Signed-off-by: Ashkan Nikravesh <redacted>
Signed-off-by: Bimmy Pujari <redacted>
---
 drivers/media/rc/bpf-lirc.c    |  2 ++
 include/linux/bpf.h            |  1 +
 include/uapi/linux/bpf.h       |  7 +++++++
 kernel/bpf/core.c              |  1 +
 kernel/bpf/helpers.c           | 14 ++++++++++++++
 kernel/trace/bpf_trace.c       |  2 ++
 tools/include/uapi/linux/bpf.h |  7 +++++++
 7 files changed, 34 insertions(+)

[...]

Maciej Żenczykowski, Kernel Networking Developer @ Google