In the case of keeping the system running, the preferred method for
tracing the kernel is dynamic tracing (kprobe), but the drawback of
this method is that events are lost, especially when tracing packages
in the network stack.
Livepatching provides a potential solution, which is to reimplement the
function you want to replace and insert a static tracepoint.
In such a way, custom stable static tracepoints can be expanded without
rebooting the system.
Signed-off-by: Jianlin Lv <[email protected]>
---
kernel/tracepoint.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
index f23144af5743..8d1507dd0724 100644
--- a/kernel/tracepoint.c
+++ b/kernel/tracepoint.c
@@ -571,8 +571,8 @@ static void for_each_tracepoint_range(
bool trace_module_has_bad_taint(struct module *mod)
{
return mod->taints & ~((1 << TAINT_OOT_MODULE) | (1 << TAINT_CRAP) |
- (1 << TAINT_UNSIGNED_MODULE) |
- (1 << TAINT_TEST));
+ (1 << TAINT_UNSIGNED_MODULE) | (1 << TAINT_TEST) |
+ (1 << TAINT_LIVEPATCH));
}
static BLOCKING_NOTIFIER_HEAD(tracepoint_notify_list);
--
2.25.1
On Wed, 2 Nov 2022 16:02:36 +0000
Jianlin Lv <[email protected]> wrote:
> In the case of keeping the system running, the preferred method for
> tracing the kernel is dynamic tracing (kprobe), but the drawback of
> this method is that events are lost, especially when tracing packages
> in the network stack.
I'm not against this change, but the above is where I'm a bit confused. How
are events more likely to be lost with kprobes over a static event?
-- Steve
On Wed, 2 Nov 2022 16:02:36 +0000
Jianlin Lv <[email protected]> wrote:
> In the case of keeping the system running, the preferred method for
> tracing the kernel is dynamic tracing (kprobe), but the drawback of
> this method is that events are lost, especially when tracing packages
> in the network stack.
>
> Livepatching provides a potential solution, which is to reimplement the
> function you want to replace and insert a static tracepoint.
> In such a way, custom stable static tracepoints can be expanded without
> rebooting the system.
Well that's definitely one way to implement dynamic trace events! :-D
-- Steve
>
> Signed-off-by: Jianlin Lv <[email protected]>
> ---
> kernel/tracepoint.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
> index f23144af5743..8d1507dd0724 100644
> --- a/kernel/tracepoint.c
> +++ b/kernel/tracepoint.c
> @@ -571,8 +571,8 @@ static void for_each_tracepoint_range(
> bool trace_module_has_bad_taint(struct module *mod)
> {
> return mod->taints & ~((1 << TAINT_OOT_MODULE) | (1 << TAINT_CRAP) |
> - (1 << TAINT_UNSIGNED_MODULE) |
> - (1 << TAINT_TEST));
> + (1 << TAINT_UNSIGNED_MODULE) | (1 << TAINT_TEST) |
> + (1 << TAINT_LIVEPATCH));
> }
>
> static BLOCKING_NOTIFIER_HEAD(tracepoint_notify_list);
On Tue, Nov 15, 2022 at 1:22 AM Steven Rostedt <[email protected]> wrote:
>
> On Wed, 2 Nov 2022 16:02:36 +0000
> Jianlin Lv <[email protected]> wrote:
>
> > In the case of keeping the system running, the preferred method for
> > tracing the kernel is dynamic tracing (kprobe), but the drawback of
> > this method is that events are lost, especially when tracing packages
> > in the network stack.
>
> I'm not against this change, but the above is where I'm a bit confused. How
> are events more likely to be lost with kprobes over a static event?
We have encountered a case of kprobes missing event, detailed
information can refer to the following link:
https://github.com/iovisor/bcc/issues/4198
Replacing kprobe with ’bpf + raw tracepoint‘, no missing events occur.
> -- Steve
On Tue, 15 Nov 2022 10:38:34 +0800
Jianlin Lv <[email protected]> wrote:
> On Tue, Nov 15, 2022 at 1:22 AM Steven Rostedt <[email protected]> wrote:
> >
> > On Wed, 2 Nov 2022 16:02:36 +0000
> > Jianlin Lv <[email protected]> wrote:
> >
> > > In the case of keeping the system running, the preferred method for
> > > tracing the kernel is dynamic tracing (kprobe), but the drawback of
> > > this method is that events are lost, especially when tracing packages
> > > in the network stack.
> >
> > I'm not against this change, but the above is where I'm a bit confused. How
> > are events more likely to be lost with kprobes over a static event?
>
> We have encountered a case of kprobes missing event, detailed
> information can refer to the following link:
> https://github.com/iovisor/bcc/issues/4198
>
> Replacing kprobe with ’bpf + raw tracepoint‘, no missing events occur.
>
Masami,
What's the reason that kprobes are not re-entrant when using ftrace?
-- Steve
On Mon, 14 Nov 2022 22:02:16 -0500
Steven Rostedt <[email protected]> wrote:
> On Tue, 15 Nov 2022 10:38:34 +0800
> Jianlin Lv <[email protected]> wrote:
>
> > On Tue, Nov 15, 2022 at 1:22 AM Steven Rostedt <[email protected]> wrote:
> > >
> > > On Wed, 2 Nov 2022 16:02:36 +0000
> > > Jianlin Lv <[email protected]> wrote:
> > >
> > > > In the case of keeping the system running, the preferred method for
> > > > tracing the kernel is dynamic tracing (kprobe), but the drawback of
> > > > this method is that events are lost, especially when tracing packages
> > > > in the network stack.
> > >
> > > I'm not against this change, but the above is where I'm a bit confused. How
> > > are events more likely to be lost with kprobes over a static event?
> >
> > We have encountered a case of kprobes missing event, detailed
> > information can refer to the following link:
> > https://github.com/iovisor/bcc/issues/4198
> >
> > Replacing kprobe with ’bpf + raw tracepoint‘, no missing events occur.
> >
>
> Masami,
>
> What's the reason that kprobes are not re-entrant when using ftrace?
I think we had discussed this issue when I drop the irq_disable() from
kprobe ftrace handler on x86, see commit a19b2e3d7839 ("kprobes/x86:
Remove IRQ disabling from ftrace-based/optimized kprobes").
Anyway, kprobes itself is not re-entrant (and no need to be re-entrant
when using int3) because it uses a per-cpu variable to memorize the
current running kprobes while processing the int3 handling and the
singlestep (trap) handling so that it can go back to the correct track
safely. It also has a single-stage "backup" (see save_previous_kprobe())
for unexpectedly re-entrant kprobes (e.g. call a probed function from
kprobe user handler.)
Thus the kprobe user doesn't need to write a re-entrant handler code.
Since kprobes on function entry is transparently changed to the ftrace,
we have to keep this limitation on the kprobes on ftrace.
BTW, now the kprobe_ftrace_handler() uses ftrace_test_recursion_trylock()
to avoid ftrace recursion, is that OK for this case?
Thank you,
--
Masami Hiramatsu (Google) <[email protected]>
On Wed, 16 Nov 2022 00:07:07 +0900
Masami Hiramatsu (Google) <[email protected]> wrote:
> BTW, now the kprobe_ftrace_handler() uses ftrace_test_recursion_trylock()
> to avoid ftrace recursion, is that OK for this case?
Note, the ftrace_test_recursion_trylock() only prevents "same context"
recursion. That is, it will not let normal context recurse into normal
context, or interrupt context recurse into interrupt context.
It has the logic of breaking up into 4 levels:
1. normal
2. softirq
3. irq
4. NMI
It allows the high levels to recurse into lower levels
(e.g. irq context into normal context)
Thus, the code within the ftrace_test_recursion_trylock() must itself be
re-entrant to handle being called from different contexts.
-- Steve
On Tue, Nov 15, 2022 at 11:17 PM Steven Rostedt <[email protected]> wrote:
>
> On Wed, 16 Nov 2022 00:07:07 +0900
> Masami Hiramatsu (Google) <[email protected]> wrote:
>
> > BTW, now the kprobe_ftrace_handler() uses ftrace_test_recursion_trylock()
> > to avoid ftrace recursion, is that OK for this case?
>
> Note, the ftrace_test_recursion_trylock() only prevents "same context"
> recursion. That is, it will not let normal context recurse into normal
> context, or interrupt context recurse into interrupt context.
>
> It has the logic of breaking up into 4 levels:
>
> 1. normal
> 2. softirq
> 3. irq
> 4. NMI
>
> It allows the high levels to recurse into lower levels
> (e.g. irq context into normal context)
>
> Thus, the code within the ftrace_test_recursion_trylock() must itself be
> re-entrant to handle being called from different contexts.
>
> -- Steve
hi, Steve
Any other comments for code changes?
Is it possible for this patch to be merged?
Regards,
Jianlin
On Fri, 23 Dec 2022 12:52:18 +0800
Jianlin Lv <[email protected]> wrote:
> hi, Steve
> Any other comments for code changes?
> Is it possible for this patch to be merged?
Ah, I had it marked as waiting for a reply. But I think we got side
tracked on the discussion.
Anyway, this is a trivial patch, I think I can get it in during -rc1.
-- Steve
On Fri, 23 Dec 2022 00:08:08 -0500
Steven Rostedt <[email protected]> wrote:
> On Fri, 23 Dec 2022 12:52:18 +0800
> Jianlin Lv <[email protected]> wrote:
>
> > hi, Steve
> > Any other comments for code changes?
> > Is it possible for this patch to be merged?
>
> Ah, I had it marked as waiting for a reply. But I think we got side
> tracked on the discussion.
>
> Anyway, this is a trivial patch, I think I can get it in during -rc1.
>
And it appears that due to the Christmas holidays, I dropped the patch.
I'm adding it to the queue now.
-- Steve