retis-org / retis

Tracing packets in the Linux networking stack & friends
https://retis.readthedocs.io/en/stable/
100 stars 14 forks source link

Add kretprobe support #81

Closed amorenoz closed 1 year ago

amorenoz commented 1 year ago

After having investigated fexit support, we found them not fit for our design since freplace is not supported in fexit probes [1].

Therefore, the approach to support return values is to bind kprobes and kretprobes: A small program is inserted in the kprobe and used to store the context in a map. This context is the retrieved by the kretprobe before executing the hooks.

Fixes #36

[1] https://elixir.bootlin.com/linux/v6.2-rc8/source/kernel/bpf/verifier.c#L16586

amorenoz commented 1 year ago
* The current implementation makes dynamic hooks to run on both the kprobe and the kretprobe, so they run twice on the same symbol and we see duplicate events.

The second point is not easy because not running dynamic hooks would mean loosing information like the tracking one on kretprobes, but at the same time running hooks twice doesn't seem a great solution. As we shouldn't have lots of example where we have both a kprobe and a kretprobe on the same target, we might want to just report the event is coming from the end of a target so we can differentiate events.

But if we define two probes with different types on the same target (like we can with tracepoints and kprobes today), duplicate events are expected right? Currently you can distinguish between kprobe and raw tracepoint because they are printed slightly different but it's a duplicate event from all other points of view (typically as tp and functions have strong correlation in practice). If we want to reliably distinguish them we could add the probe type in the event section. WDYT?

atenart commented 1 year ago

But if we define two probes with different types on the same target (like we can with tracepoints and kprobes today), duplicate events are expected right?

Fair point. It depends on of if you see kretprobes as a way to get the returned value of kprobes, or if they are an independent probe type. The later also makes sense.

Currently you can distinguish between kprobe and raw tracepoint because they are printed slightly different but it's a duplicate event from all other points of view (typically as tp and functions have strong correlation in practice). If we want to reliably distinguish them we could add the probe type in the event section. WDYT?

Yes, I think that would be nice and resolve my concerns.