BPF List
 help / color / mirror / Atom feed
* sys_enter tracepoint ctx structure
@ 2023-04-20 13:50 Yauheni Kaliuta
       [not found] ` <CAADnVQ+JdPGV95Y30PskgdOomU2K0UXsoCydgqaJfJ5j4S8BtQ@mail.gmail.com>
  0 siblings, 1 reply; 10+ messages in thread
From: Yauheni Kaliuta @ 2023-04-20 13:50 UTC (permalink / raw)
  To: Yonghong Song; +Cc: bpf, asavkov, vmalik

Hi!

Should perf_call_bpf_enter/exit (kernel/trace/trace_syscalls.c)
use struct trace_event_raw_sys_enter/exit instead of locally
crafted struct syscall_tp_t nowadays? test_progs's vmlinux test
expects it as the context.

Or at least use struct trace_entry instead of struct pt_regs?

I have a problem with one RT patch with extends trace_entry.

-- 
WBR,
Yauheni Kaliuta


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Fwd: sys_enter tracepoint ctx structure
       [not found] ` <CAADnVQ+JdPGV95Y30PskgdOomU2K0UXsoCydgqaJfJ5j4S8BtQ@mail.gmail.com>
@ 2023-04-20 16:02   ` Alexei Starovoitov
  2023-04-20 20:37   ` Yauheni Kaliuta
  1 sibling, 0 replies; 10+ messages in thread
From: Alexei Starovoitov @ 2023-04-20 16:02 UTC (permalink / raw)
  To: Yauheni Kaliuta, Yonghong Song, bpf, Artem Savkov, Viktor Malik

On Thu, Apr 20, 2023 at 6:57 AM Yauheni Kaliuta <ykaliuta@redhat.com> wrote:
>
> Hi!
>
> Should perf_call_bpf_enter/exit (kernel/trace/trace_syscalls.c)
> use struct trace_event_raw_sys_enter/exit instead of locally
> crafted struct syscall_tp_t nowadays?


No. It needs syscall_tp_t.

> test_progs's vmlinux test
> expects it as the context.


what do you mean? Pls share a code pointer?

>
>
> Or at least use struct trace_entry instead of struct pt_regs?


no. It needs a pointer to pt_regs.
See all of the pe_* flavor of helpers.

>
>
> I have a problem with one RT patch with extends trace_entry.


Just extend it. It shouldn't matter.
I'm likely missing something.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: sys_enter tracepoint ctx structure
       [not found] ` <CAADnVQ+JdPGV95Y30PskgdOomU2K0UXsoCydgqaJfJ5j4S8BtQ@mail.gmail.com>
  2023-04-20 16:02   ` Fwd: " Alexei Starovoitov
@ 2023-04-20 20:37   ` Yauheni Kaliuta
  2023-04-20 20:54     ` Alexei Starovoitov
  1 sibling, 1 reply; 10+ messages in thread
From: Yauheni Kaliuta @ 2023-04-20 20:37 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Yonghong Song, bpf, Artem Savkov, Viktor Malik, jmarchan

Hi, Alexei!

>>>>> On Thu, 20 Apr 2023 08:59:09 -0700, Alexei Starovoitov  wrote:

 > On Thu, Apr 20, 2023 at 6:57 AM Yauheni Kaliuta <ykaliuta@redhat.com> wrote:
 >> Hi!
 >> 
 >> Should perf_call_bpf_enter/exit (kernel/trace/trace_syscalls.c)
 >> use struct trace_event_raw_sys_enter/exit instead of locally
 >> crafted struct syscall_tp_t nowadays?


 > No. It needs syscall_tp_t.

 > test_progs's vmlinux test
 >> expects it as the context.
 >> 

 > what do you mean? Pls share a code pointer?

https://github.com/torvalds/linux/blob/master/tools/testing/selftests/bpf/progs/test_vmlinux.c#L19

SEC("tp/syscalls/sys_enter_nanosleep")
int handle__tp(struct trace_event_raw_sys_enter *args)

So, should it use different structure then? syscall_tp_t to be
declared publicly?

 >> 
 >> Or at least use struct trace_entry instead of struct pt_regs?
 >> 

 > no. It needs a pointer to pt_regs.
 > See all of the pe_* flavor of helpers.


 >> 
 >> I have a problem with one RT patch with extends trace_entry.
 >> 

 > Just extend it. It shouldn't matter.
 > I'm likely missing something.

Or me. Let's figure out :)

-- 
WBR,
Yauheni Kaliuta


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: sys_enter tracepoint ctx structure
  2023-04-20 20:37   ` Yauheni Kaliuta
@ 2023-04-20 20:54     ` Alexei Starovoitov
  2023-04-20 21:40       ` Yauheni Kaliuta
  0 siblings, 1 reply; 10+ messages in thread
From: Alexei Starovoitov @ 2023-04-20 20:54 UTC (permalink / raw)
  To: Yauheni Kaliuta
  Cc: Yonghong Song, bpf, Artem Savkov, Viktor Malik, Jerome Marchand

On Thu, Apr 20, 2023 at 1:37 PM Yauheni Kaliuta <ykaliuta@redhat.com> wrote:
>
> Hi, Alexei!
>
> >>>>> On Thu, 20 Apr 2023 08:59:09 -0700, Alexei Starovoitov  wrote:
>
>  > On Thu, Apr 20, 2023 at 6:57 AM Yauheni Kaliuta <ykaliuta@redhat.com> wrote:
>  >> Hi!
>  >>
>  >> Should perf_call_bpf_enter/exit (kernel/trace/trace_syscalls.c)
>  >> use struct trace_event_raw_sys_enter/exit instead of locally
>  >> crafted struct syscall_tp_t nowadays?
>
>
>  > No. It needs syscall_tp_t.
>
>  > test_progs's vmlinux test
>  >> expects it as the context.
>  >>
>
>  > what do you mean? Pls share a code pointer?
>
> https://github.com/torvalds/linux/blob/master/tools/testing/selftests/bpf/progs/test_vmlinux.c#L19
>
> SEC("tp/syscalls/sys_enter_nanosleep")
> int handle__tp(struct trace_event_raw_sys_enter *args)

I see. That bit is correct and that's what bpftrace is doing
when attaching to syscalls.
What do you see in your patched RT kernel when you do:
cat /sys/kernel/debug/tracing/events/syscalls/sys_enter_nanosleep/format
?
Depending on the answer we might need to fix
the kernel side that has to use struct trace_entry
in syscall_tp_t instead of plain long long.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: sys_enter tracepoint ctx structure
  2023-04-20 20:54     ` Alexei Starovoitov
@ 2023-04-20 21:40       ` Yauheni Kaliuta
  2023-04-20 23:12         ` Alexei Starovoitov
  0 siblings, 1 reply; 10+ messages in thread
From: Yauheni Kaliuta @ 2023-04-20 21:40 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Yonghong Song, bpf, Artem Savkov, Viktor Malik, Jerome Marchand

Hi, Alexei!

>>>>> On Thu, 20 Apr 2023 13:54:26 -0700, Alexei Starovoitov  wrote:
 > On Thu, Apr 20, 2023 at 1:37 PM Yauheni Kaliuta <ykaliuta@redhat.com> wrote:
 >> >>>>> On Thu, 20 Apr 2023 08:59:09 -0700, Alexei Starovoitov  wrote:
 >> >>
 >> >> Should perf_call_bpf_enter/exit (kernel/trace/trace_syscalls.c)
 >> >> use struct trace_event_raw_sys_enter/exit instead of locally
 >> >> crafted struct syscall_tp_t nowadays?
 >> 
 >> 
 >> > No. It needs syscall_tp_t.
 >> 
 >> > test_progs's vmlinux test
 >> >> expects it as the context.
 >> >>
 >> 
 >> > what do you mean? Pls share a code pointer?
 >> 
 >> https://github.com/torvalds/linux/blob/master/tools/testing/selftests/bpf/progs/test_vmlinux.c#L19
 >> 
 >> SEC("tp/syscalls/sys_enter_nanosleep")
 >> int handle__tp(struct trace_event_raw_sys_enter *args)

 > I see. That bit is correct and that's what bpftrace is doing
 > when attaching to syscalls.
 > What do you see in your patched RT kernel when you do:
 > cat /sys/kernel/debug/tracing/events/syscalls/sys_enter_nanosleep/format
 > ?
 > Depending on the answer we might need to fix
 > the kernel side that has to use struct trace_entry
 > in syscall_tp_t instead of plain long long.

 # cat /sys/kernel/debug/tracing/events/syscalls/sys_enter_nanosleep/format
name: sys_enter_nanosleep
ID: 374
format:
        field:unsigned short common_type;       offset:0;       size:2; signed:0;
        field:unsigned char common_flags;       offset:2;       size:1; signed:0;
        field:unsigned char common_preempt_count;       offset:3;       size:1; signed:0;
        field:int common_pid;   offset:4;       size:4; signed:1;
        field:unsigned char common_preempt_lazy_count;  offset:8;       size:1; signed:0;

        field:int __syscall_nr; offset:12;      size:4; signed:1;
        field:struct __kernel_timespec * rqtp;  offset:16;      size:8; signed:0;
        field:struct __kernel_timespec * rmtp;  offset:24;      size:8; signed:0;

print fmt: "rqtp: 0x%08lx, rmtp: 0x%08lx", ((unsigned long)(REC->rqtp)), ((unsigned long)(REC->rmtp))



-- 
WBR,
Yauheni Kaliuta


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: sys_enter tracepoint ctx structure
  2023-04-20 21:40       ` Yauheni Kaliuta
@ 2023-04-20 23:12         ` Alexei Starovoitov
  2023-04-21 11:17           ` Yauheni Kaliuta
  0 siblings, 1 reply; 10+ messages in thread
From: Alexei Starovoitov @ 2023-04-20 23:12 UTC (permalink / raw)
  To: Yauheni Kaliuta
  Cc: Yonghong Song, bpf, Artem Savkov, Viktor Malik, Jerome Marchand

On Thu, Apr 20, 2023 at 2:40 PM Yauheni Kaliuta <ykaliuta@redhat.com> wrote:
>
> Hi, Alexei!
>
> >>>>> On Thu, 20 Apr 2023 13:54:26 -0700, Alexei Starovoitov  wrote:
>  > On Thu, Apr 20, 2023 at 1:37 PM Yauheni Kaliuta <ykaliuta@redhat.com> wrote:
>  >> >>>>> On Thu, 20 Apr 2023 08:59:09 -0700, Alexei Starovoitov  wrote:
>  >> >>
>  >> >> Should perf_call_bpf_enter/exit (kernel/trace/trace_syscalls.c)
>  >> >> use struct trace_event_raw_sys_enter/exit instead of locally
>  >> >> crafted struct syscall_tp_t nowadays?
>  >>
>  >>
>  >> > No. It needs syscall_tp_t.
>  >>
>  >> > test_progs's vmlinux test
>  >> >> expects it as the context.
>  >> >>
>  >>
>  >> > what do you mean? Pls share a code pointer?
>  >>
>  >> https://github.com/torvalds/linux/blob/master/tools/testing/selftests/bpf/progs/test_vmlinux.c#L19
>  >>
>  >> SEC("tp/syscalls/sys_enter_nanosleep")
>  >> int handle__tp(struct trace_event_raw_sys_enter *args)
>
>  > I see. That bit is correct and that's what bpftrace is doing
>  > when attaching to syscalls.
>  > What do you see in your patched RT kernel when you do:
>  > cat /sys/kernel/debug/tracing/events/syscalls/sys_enter_nanosleep/format
>  > ?
>  > Depending on the answer we might need to fix
>  > the kernel side that has to use struct trace_entry
>  > in syscall_tp_t instead of plain long long.
>
>  # cat /sys/kernel/debug/tracing/events/syscalls/sys_enter_nanosleep/format
> name: sys_enter_nanosleep
> ID: 374
> format:
>         field:unsigned short common_type;       offset:0;       size:2; signed:0;
>         field:unsigned char common_flags;       offset:2;       size:1; signed:0;
>         field:unsigned char common_preempt_count;       offset:3;       size:1; signed:0;
>         field:int common_pid;   offset:4;       size:4; signed:1;
>         field:unsigned char common_preempt_lazy_count;  offset:8;       size:1; signed:0;
>
>         field:int __syscall_nr; offset:12;      size:4; signed:1;
>         field:struct __kernel_timespec * rqtp;  offset:16;      size:8; signed:0;
>         field:struct __kernel_timespec * rmtp;  offset:24;      size:8; signed:0;
>
> print fmt: "rqtp: 0x%08lx, rmtp: 0x%08lx", ((unsigned long)(REC->rqtp)), ((unsigned long)(REC->rmtp))


Lol.
Jiri even fixed the issue with this format in bpftrace 3 years ago:
https://github.com/iovisor/bpftrace/commit/a2e3d5dbc03ceb49b776cf5602d31896158844a7

Let's fix the kernel side too. Something like this should do it:

diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index 942ddbdace4a..7aa1f4299486 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -555,7 +555,7 @@ static int perf_call_bpf_enter(struct
trace_event_call *call, struct pt_regs *re
                               struct syscall_trace_enter *rec)
 {
        struct syscall_tp_t {
-               unsigned long long regs;
+               struct trace_entry ent;
                unsigned long syscall_nr;
                unsigned long args[SYSCALL_DEFINE_MAXARGS];
        } param;
@@ -657,7 +657,7 @@ static int perf_call_bpf_exit(struct
trace_event_call *call, struct pt_regs *reg
                              struct syscall_trace_exit *rec)
 {
        struct syscall_tp_t {
-               unsigned long long regs;
+               struct trace_entry ent;


pls add build_bug_on that sizeof(ent) >= sizeof(void*).

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: sys_enter tracepoint ctx structure
  2023-04-20 23:12         ` Alexei Starovoitov
@ 2023-04-21 11:17           ` Yauheni Kaliuta
  2023-04-21 16:02             ` Alexei Starovoitov
  0 siblings, 1 reply; 10+ messages in thread
From: Yauheni Kaliuta @ 2023-04-21 11:17 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Yonghong Song, bpf, Artem Savkov, Viktor Malik, Jerome Marchand

Hi, Alexei!

>>>>> On Thu, 20 Apr 2023 16:12:49 -0700, Alexei Starovoitov  wrote:
 > On Thu, Apr 20, 2023 at 2:40 PM Yauheni Kaliuta <ykaliuta@redhat.com> wrote:
 >> >>>>> On Thu, 20 Apr 2023 13:54:26 -0700, Alexei Starovoitov  wrote:
 >> > On Thu, Apr 20, 2023 at 1:37 PM Yauheni Kaliuta <ykaliuta@redhat.com> wrote:
 >> >> >>>>> On Thu, 20 Apr 2023 08:59:09 -0700, Alexei Starovoitov  wrote:
 >> >> >>
 >> >> >> Should perf_call_bpf_enter/exit (kernel/trace/trace_syscalls.c)
 >> >> >> use struct trace_event_raw_sys_enter/exit instead of locally
 >> >> >> crafted struct syscall_tp_t nowadays?
 >> >>
 >> >>
 >> >> > No. It needs syscall_tp_t.
 >> >>
 >> >> > test_progs's vmlinux test
 >> >> >> expects it as the context.
 >> >> >>
 >> >>
 >> >> > what do you mean? Pls share a code pointer?
 >> >>
 >> >> https://github.com/torvalds/linux/blob/master/tools/testing/selftests/bpf/progs/test_vmlinux.c#L19
 >> >>
 >> >> SEC("tp/syscalls/sys_enter_nanosleep")
 >> >> int handle__tp(struct trace_event_raw_sys_enter *args)
 >> 
 >> > I see. That bit is correct and that's what bpftrace is doing
 >> > when attaching to syscalls.
 >> > What do you see in your patched RT kernel when you do:
 >> > cat /sys/kernel/debug/tracing/events/syscalls/sys_enter_nanosleep/format
 >> > ?
 >> > Depending on the answer we might need to fix
 >> > the kernel side that has to use struct trace_entry
 >> > in syscall_tp_t instead of plain long long.
 >> 
 >> # cat /sys/kernel/debug/tracing/events/syscalls/sys_enter_nanosleep/format
 >> name: sys_enter_nanosleep
 >> ID: 374
 >> format:
 >> field:unsigned short common_type;       offset:0;       size:2; signed:0;
 >> field:unsigned char common_flags;       offset:2;       size:1; signed:0;
 >> field:unsigned char common_preempt_count;       offset:3;       size:1; signed:0;
 >> field:int common_pid;   offset:4;       size:4; signed:1;
 >> field:unsigned char common_preempt_lazy_count;  offset:8;       size:1; signed:0;
 >> 
 >> field:int __syscall_nr; offset:12;      size:4; signed:1;
 >> field:struct __kernel_timespec * rqtp;  offset:16;      size:8; signed:0;
 >> field:struct __kernel_timespec * rmtp;  offset:24;      size:8; signed:0;
 >> 
 >> print fmt: "rqtp: 0x%08lx, rmtp: 0x%08lx", ((unsigned long)(REC->rqtp)), ((unsigned long)(REC->rmtp))


 > Lol.
 > Jiri even fixed the issue with this format in bpftrace 3 years ago:
 > https://github.com/iovisor/bpftrace/commit/a2e3d5dbc03ceb49b776cf5602d31896158844a7

Hehe :)

 > Let's fix the kernel side too. Something like this should do it:

 > diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
 > index 942ddbdace4a..7aa1f4299486 100644
 > --- a/kernel/trace/trace_syscalls.c
 > +++ b/kernel/trace/trace_syscalls.c
 > @@ -555,7 +555,7 @@ static int perf_call_bpf_enter(struct
 > trace_event_call *call, struct pt_regs *re
 >                                struct syscall_trace_enter *rec)
 >  {
 >         struct syscall_tp_t {
 > -               unsigned long long regs;
 > +               struct trace_entry ent;
 >                 unsigned long syscall_nr;
 >                 unsigned long args[SYSCALL_DEFINE_MAXARGS];
 >         } param;
 > @@ -657,7 +657,7 @@ static int perf_call_bpf_exit(struct
 > trace_event_call *call, struct pt_regs *reg
 >                               struct syscall_trace_exit *rec)
 >  {
 >         struct syscall_tp_t {
 > -               unsigned long long regs;
 > +               struct trace_entry ent;


 > pls add build_bug_on that sizeof(ent) >= sizeof(void*).

Ok. Should the line *(struct pt_regs **)&param = regs; be commented somehow?

-- 
WBR,
Yauheni Kaliuta


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: sys_enter tracepoint ctx structure
  2023-04-21 11:17           ` Yauheni Kaliuta
@ 2023-04-21 16:02             ` Alexei Starovoitov
  2023-04-23  9:04               ` Yauheni Kaliuta
  0 siblings, 1 reply; 10+ messages in thread
From: Alexei Starovoitov @ 2023-04-21 16:02 UTC (permalink / raw)
  To: Yauheni Kaliuta
  Cc: Yonghong Song, bpf, Artem Savkov, Viktor Malik, Jerome Marchand

On Fri, Apr 21, 2023 at 4:17 AM Yauheni Kaliuta <ykaliuta@redhat.com> wrote:
>
> Hi, Alexei!
>
> >>>>> On Thu, 20 Apr 2023 16:12:49 -0700, Alexei Starovoitov  wrote:
>  > On Thu, Apr 20, 2023 at 2:40 PM Yauheni Kaliuta <ykaliuta@redhat.com> wrote:
>  >> >>>>> On Thu, 20 Apr 2023 13:54:26 -0700, Alexei Starovoitov  wrote:
>  >> > On Thu, Apr 20, 2023 at 1:37 PM Yauheni Kaliuta <ykaliuta@redhat.com> wrote:
>  >> >> >>>>> On Thu, 20 Apr 2023 08:59:09 -0700, Alexei Starovoitov  wrote:
>  >> >> >>
>  >> >> >> Should perf_call_bpf_enter/exit (kernel/trace/trace_syscalls.c)
>  >> >> >> use struct trace_event_raw_sys_enter/exit instead of locally
>  >> >> >> crafted struct syscall_tp_t nowadays?
>  >> >>
>  >> >>
>  >> >> > No. It needs syscall_tp_t.
>  >> >>
>  >> >> > test_progs's vmlinux test
>  >> >> >> expects it as the context.
>  >> >> >>
>  >> >>
>  >> >> > what do you mean? Pls share a code pointer?
>  >> >>
>  >> >> https://github.com/torvalds/linux/blob/master/tools/testing/selftests/bpf/progs/test_vmlinux.c#L19
>  >> >>
>  >> >> SEC("tp/syscalls/sys_enter_nanosleep")
>  >> >> int handle__tp(struct trace_event_raw_sys_enter *args)
>  >>
>  >> > I see. That bit is correct and that's what bpftrace is doing
>  >> > when attaching to syscalls.
>  >> > What do you see in your patched RT kernel when you do:
>  >> > cat /sys/kernel/debug/tracing/events/syscalls/sys_enter_nanosleep/format
>  >> > ?
>  >> > Depending on the answer we might need to fix
>  >> > the kernel side that has to use struct trace_entry
>  >> > in syscall_tp_t instead of plain long long.
>  >>
>  >> # cat /sys/kernel/debug/tracing/events/syscalls/sys_enter_nanosleep/format
>  >> name: sys_enter_nanosleep
>  >> ID: 374
>  >> format:
>  >> field:unsigned short common_type;       offset:0;       size:2; signed:0;
>  >> field:unsigned char common_flags;       offset:2;       size:1; signed:0;
>  >> field:unsigned char common_preempt_count;       offset:3;       size:1; signed:0;
>  >> field:int common_pid;   offset:4;       size:4; signed:1;
>  >> field:unsigned char common_preempt_lazy_count;  offset:8;       size:1; signed:0;
>  >>
>  >> field:int __syscall_nr; offset:12;      size:4; signed:1;
>  >> field:struct __kernel_timespec * rqtp;  offset:16;      size:8; signed:0;
>  >> field:struct __kernel_timespec * rmtp;  offset:24;      size:8; signed:0;
>  >>
>  >> print fmt: "rqtp: 0x%08lx, rmtp: 0x%08lx", ((unsigned long)(REC->rqtp)), ((unsigned long)(REC->rmtp))
>
>
>  > Lol.
>  > Jiri even fixed the issue with this format in bpftrace 3 years ago:
>  > https://github.com/iovisor/bpftrace/commit/a2e3d5dbc03ceb49b776cf5602d31896158844a7
>
> Hehe :)
>
>  > Let's fix the kernel side too. Something like this should do it:
>
>  > diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
>  > index 942ddbdace4a..7aa1f4299486 100644
>  > --- a/kernel/trace/trace_syscalls.c
>  > +++ b/kernel/trace/trace_syscalls.c
>  > @@ -555,7 +555,7 @@ static int perf_call_bpf_enter(struct
>  > trace_event_call *call, struct pt_regs *re
>  >                                struct syscall_trace_enter *rec)
>  >  {
>  >         struct syscall_tp_t {
>  > -               unsigned long long regs;
>  > +               struct trace_entry ent;
>  >                 unsigned long syscall_nr;
>  >                 unsigned long args[SYSCALL_DEFINE_MAXARGS];
>  >         } param;
>  > @@ -657,7 +657,7 @@ static int perf_call_bpf_exit(struct
>  > trace_event_call *call, struct pt_regs *reg
>  >                               struct syscall_trace_exit *rec)
>  >  {
>  >         struct syscall_tp_t {
>  > -               unsigned long long regs;
>  > +               struct trace_entry ent;
>
>
>  > pls add build_bug_on that sizeof(ent) >= sizeof(void*).
>
> Ok. Should the line *(struct pt_regs **)&param = regs; be commented somehow?

commented out? No. It's mandatory.
And the reason for build_bug_on existence... to make sure that there
is enough space there.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: sys_enter tracepoint ctx structure
  2023-04-21 16:02             ` Alexei Starovoitov
@ 2023-04-23  9:04               ` Yauheni Kaliuta
  2023-04-23 16:15                 ` Alexei Starovoitov
  0 siblings, 1 reply; 10+ messages in thread
From: Yauheni Kaliuta @ 2023-04-23  9:04 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Yonghong Song, bpf, Artem Savkov, Viktor Malik, Jerome Marchand

Hi, Alexei!

>>>>> On Fri, 21 Apr 2023 09:02:34 -0700, Alexei Starovoitov  wrote:
 > On Fri, Apr 21, 2023 at 4:17 AM Yauheni Kaliuta <ykaliuta@redhat.com> wrote:
 >> >>>>> On Thu, 20 Apr 2023 16:12:49 -0700, Alexei Starovoitov  wrote:
 >> > On Thu, Apr 20, 2023 at 2:40 PM Yauheni Kaliuta <ykaliuta@redhat.com> wrote:
 >> >> >>>>> On Thu, 20 Apr 2023 13:54:26 -0700, Alexei Starovoitov  wrote:
 >> >> > On Thu, Apr 20, 2023 at 1:37 PM Yauheni Kaliuta <ykaliuta@redhat.com> wrote:
 >> >> >> >>>>> On Thu, 20 Apr 2023 08:59:09 -0700, Alexei Starovoitov  wrote:

[...]

 >> > Let's fix the kernel side too. Something like this should do it:
 >> 
 >> > diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
 >> > index 942ddbdace4a..7aa1f4299486 100644
 >> > --- a/kernel/trace/trace_syscalls.c
 >> > +++ b/kernel/trace/trace_syscalls.c
 >> > @@ -555,7 +555,7 @@ static int perf_call_bpf_enter(struct
 >> > trace_event_call *call, struct pt_regs *re
 >> >                                struct syscall_trace_enter *rec)
 >> >  {
 >> >         struct syscall_tp_t {
 >> > -               unsigned long long regs;
 >> > +               struct trace_entry ent;
 >> >                 unsigned long syscall_nr;
 >> >                 unsigned long args[SYSCALL_DEFINE_MAXARGS];
 >> >         } param;
 >> > @@ -657,7 +657,7 @@ static int perf_call_bpf_exit(struct
 >> > trace_event_call *call, struct pt_regs *reg
 >> >                               struct syscall_trace_exit *rec)
 >> >  {
 >> >         struct syscall_tp_t {
 >> > -               unsigned long long regs;
 >> > +               struct trace_entry ent;
 >> 
 >> 
 >> > pls add build_bug_on that sizeof(ent) >= sizeof(void*).
 >> 
 >> Ok. Should the line *(struct pt_regs **)&param = regs; be commented somehow?

 > commented out?

No, no :)

 > No. It's mandatory.
 > And the reason for build_bug_on existence... to make sure that there
 > is enough space there.

Yes, it's clear for sure.

It can be not obvious why basically 'ent' is inited with
'regs'. Before it was called 'regs' at least.

-- 
WBR,
Yauheni Kaliuta


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: sys_enter tracepoint ctx structure
  2023-04-23  9:04               ` Yauheni Kaliuta
@ 2023-04-23 16:15                 ` Alexei Starovoitov
  0 siblings, 0 replies; 10+ messages in thread
From: Alexei Starovoitov @ 2023-04-23 16:15 UTC (permalink / raw)
  To: Yauheni Kaliuta
  Cc: Yonghong Song, bpf, Artem Savkov, Viktor Malik, Jerome Marchand

On Sun, Apr 23, 2023 at 2:04 AM Yauheni Kaliuta <ykaliuta@redhat.com> wrote:
>
> Hi, Alexei!
>
> >>>>> On Fri, 21 Apr 2023 09:02:34 -0700, Alexei Starovoitov  wrote:
>  > On Fri, Apr 21, 2023 at 4:17 AM Yauheni Kaliuta <ykaliuta@redhat.com> wrote:
>  >> >>>>> On Thu, 20 Apr 2023 16:12:49 -0700, Alexei Starovoitov  wrote:
>  >> > On Thu, Apr 20, 2023 at 2:40 PM Yauheni Kaliuta <ykaliuta@redhat.com> wrote:
>  >> >> >>>>> On Thu, 20 Apr 2023 13:54:26 -0700, Alexei Starovoitov  wrote:
>  >> >> > On Thu, Apr 20, 2023 at 1:37 PM Yauheni Kaliuta <ykaliuta@redhat.com> wrote:
>  >> >> >> >>>>> On Thu, 20 Apr 2023 08:59:09 -0700, Alexei Starovoitov  wrote:
>
> [...]
>
>  >> > Let's fix the kernel side too. Something like this should do it:
>  >>
>  >> > diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
>  >> > index 942ddbdace4a..7aa1f4299486 100644
>  >> > --- a/kernel/trace/trace_syscalls.c
>  >> > +++ b/kernel/trace/trace_syscalls.c
>  >> > @@ -555,7 +555,7 @@ static int perf_call_bpf_enter(struct
>  >> > trace_event_call *call, struct pt_regs *re
>  >> >                                struct syscall_trace_enter *rec)
>  >> >  {
>  >> >         struct syscall_tp_t {
>  >> > -               unsigned long long regs;
>  >> > +               struct trace_entry ent;
>  >> >                 unsigned long syscall_nr;
>  >> >                 unsigned long args[SYSCALL_DEFINE_MAXARGS];
>  >> >         } param;
>  >> > @@ -657,7 +657,7 @@ static int perf_call_bpf_exit(struct
>  >> > trace_event_call *call, struct pt_regs *reg
>  >> >                               struct syscall_trace_exit *rec)
>  >> >  {
>  >> >         struct syscall_tp_t {
>  >> > -               unsigned long long regs;
>  >> > +               struct trace_entry ent;
>  >>
>  >>
>  >> > pls add build_bug_on that sizeof(ent) >= sizeof(void*).
>  >>
>  >> Ok. Should the line *(struct pt_regs **)&param = regs; be commented somehow?
>
>  > commented out?
>
> No, no :)
>
>  > No. It's mandatory.
>  > And the reason for build_bug_on existence... to make sure that there
>  > is enough space there.
>
> Yes, it's clear for sure.
>
> It can be not obvious why basically 'ent' is inited with
> 'regs'. Before it was called 'regs' at least.

Got it :) Yeah. A comment describing the intent would be nice.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-04-23 16:15 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-20 13:50 sys_enter tracepoint ctx structure Yauheni Kaliuta
     [not found] ` <CAADnVQ+JdPGV95Y30PskgdOomU2K0UXsoCydgqaJfJ5j4S8BtQ@mail.gmail.com>
2023-04-20 16:02   ` Fwd: " Alexei Starovoitov
2023-04-20 20:37   ` Yauheni Kaliuta
2023-04-20 20:54     ` Alexei Starovoitov
2023-04-20 21:40       ` Yauheni Kaliuta
2023-04-20 23:12         ` Alexei Starovoitov
2023-04-21 11:17           ` Yauheni Kaliuta
2023-04-21 16:02             ` Alexei Starovoitov
2023-04-23  9:04               ` Yauheni Kaliuta
2023-04-23 16:15                 ` Alexei Starovoitov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox