On 5/13/20 12:42 PM, Quentin Monnet wrote:
2020-05-13 09:58 UTC+0200 ~ Daniel Borkmann <daniel@xxxxxxxxxxxxx>
In Cilium we've recently switched to make use of bpf_jiffies64() for
parts of our tc and XDP datapath since bpf_ktime_get_ns() is more
expensive and high-precision is not needed for our timeouts we have
anyway. Our agent has a probe manager which picks up the json of
bpftool's feature probe and we also use the macro output in our C
programs e.g. to have workarounds when helpers are not available on
older kernels.
Extend the kernel config info dump to also include the kernel's
CONFIG_HZ, and rework the probe_kernel_image_config() for allowing a
macro dump such that CONFIG_HZ can be propagated to BPF C code as a
simple define if available via config. Latter allows to have _compile-
time_ resolution of jiffies <-> sec conversion in our code since all
are propagated as known constants.
Given we cannot generally assume availability of kconfig everywhere,
we also have a kernel hz probe [0] as a fallback. Potentially, bpftool
could have an integrated probe fallback as well, although to derive it,
we might need to place it under 'bpftool feature probe full' or similar
given it would slow down the probing process overall. Yet 'full' doesn't
fit either for us since we don't want to pollute the kernel log with
warning messages from bpf_probe_write_user() and bpf_trace_printk() on
agent startup; I've left it out for the time being.
[0] https://github.com/cilium/cilium/blob/master/bpf/cilium-probe-kernel-hz.c
Signed-off-by: Daniel Borkmann <daniel@xxxxxxxxxxxxx>
Cc: Martin KaFai Lau <kafai@xxxxxx>
Looks good to me, thanks!
I think at the time the "bpftool feature probe" was added we didn't
settle on a particular format for dumping the CONFIG_* as part as the C
macro output, but other than that I can see no specific reason why not
to have them, so we could even list them all and avoid the macro_dump
bool. But I'm fine either way, other CONFIG_* can still be added to C
macro output at a later time if someone needs them anyway.
Right, I initially thought about listing them all, but then my thinking
was that we should really only dump those CONFIG_* as defines that actually
have a real-world use case in a BPF prog somewhere. This also helps to
better understand what is useful and why and avoids unrelated noise e.g.
in the bpf_features.h dump we have in Cilium as part of the agent bootstrap.
Regarding a fallback for the jiffies, not sure what would be best. I
agree with you for the "full" keyword, so we would need another word I
suppose. But adding new keyword for fallbacks for probing features not
directly related to BPF might be going a bit beyond bpftool's scope? I
don't know. Anyway, for the current patch:
Agree, had the same thought.
Reviewed-by: Quentin Monnet <quentin@xxxxxxxxxxxxx>
Thanks!
Daniel
