Use bpf_ktime_get_boot_ns instead

[nicholasberlin]

bpf_ktime_get_ns does not include suspension time, but we would like hdr.ts to represent real world time. Switching the helper function achieves that. The helper was introduced in 5.8 which is earlier than our current support range, 5.10

Update: Add a new member variable instead and leave hdr.ts alone.

[haesbaert]

So does that mean we have to move all userland comparisons to use CLOCK_BOOTIME on clock_gettime(3)? I'm guessing yes. I've tested on RHEL8 and it works (4.18). I'd prefer if we could fallback to the old one and also tell userland which one is being used, I can do that in a later step though.

[nicholasberlin]

I'd prefer if we could fallback to the old one and also tell userland which one is being used, I can do that in a later step though.

How would you feel about adding a new member to the header instead?

struct ebpf_event_header {
    uint64_t ts;
    uint64_t ts_boot;
    uint64_t type;
} __attribute__((packed));

[nicholasberlin]

I spent a little time trying to figure out how to check at runtime for helper functions, but couldn't get there. Our current logic is around disabling probes based on features. Wasn't sure how to avoid loading ebpf code that called unsupported helper functions. Seems doable, but I gave up once I realized that the new helper function is within our support range, and has apparently been backported.

[haesbaert]

I'd prefer if we could fallback to the old one and also tell userland which one is being used, I can do that in a later step though.

How would you feel about adding a new member to the header instead?

struct ebpf_event_header {
    uint64_t ts;
    uint64_t ts_boot;
    uint64_t type;
} __attribute__((packed));

I don't think it belongs there, since it's a one time thing, we could stash it under stats, and userland is responsible for querying it once.

[haesbaert]

I spent a little time trying to figure out how to check at runtime for helper functions, but couldn't get there. Our current logic is around disabling probes based on features. Wasn't sure how to avoid loading ebpf code that called unsupported helper functions. Seems doable, but I gave up once I realized that the new helper function is within our support range, and has apparently been backported.

I think the way to do is to test for BPF_FUNC_KTIME_GET...., something like:

if (bpf_core_enum_value_exists(enum bpf_func_id, BPF_FUNC_ringbuf_reserve)) {
    /* use bpf_ringbuf_reserve() safely */
} else {
    /* fall back to using bpf_perf_event_output() */
}

from: https://nakryiko.com/posts/bpf-core-reference-guide/

I'm happy to work on this in the future

[nicholasberlin]

After reading this:

It's worth reiterating that such code is correctly detected by BPF verifier as a dead code, and so is never validated. This means that such code can use kernel and BPF functionality (e.g., new BPF helpers) that do not exist on the host kernel without worrying about BPF verification failures. E.g., if the first branch above were to use bpf_get_attach_cookie() helper for the BPF cookie feature, the program would be validated properly on older kernels that don't yet have that helper.

this PR was changed to add a new member to the header file. This way users of ts will not have to adjust. And the new member, ts_boot is set conditionally based on the helper function's existence.

[haesbaert]

me gusta!