Message ID | 20221202162907.26721-1-danieltimlee@gmail.com (mailing list archive) |
---|---|
State | Changes Requested |
Delegated to: | BPF |
Headers | show |
Series | samples: bpf: fix broken behavior of tracex2 write_size count | expand |
On Fri, Dec 2, 2022 at 8:29 AM Daniel T. Lee <danieltimlee@gmail.com> wrote: > > Currently, there is a problem with tracex2, as it doesn't print the > histogram properly and the results are misleading. (all results report > as 0) > > The problem is caused by a change in arguments of the function to which > the kprobe connects. This tracex2 bpf program uses kprobe (attached > to __x64_sys_write) to figure out the size of the write system call. In > order to achieve this, the third argument 'count' must be intact. > > The following is a prototype of the sys_write variant. (checked with > pfunct) > > ~/git/linux$ pfunct -P fs/read_write.o | grep sys_write > ssize_t ksys_write(unsigned int fd, const char * buf, size_t count); > long int __x64_sys_write(const struct pt_regs * regs); > ... cross compile with s390x ... > long int __s390_sys_write(struct pt_regs * regs); > > Since the __x64_sys_write (or s390x also) doesn't have the proper > argument, changing the kprobe event to ksys_write will fix the problem. > > Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com> > --- > samples/bpf/tracex2_kern.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/samples/bpf/tracex2_kern.c b/samples/bpf/tracex2_kern.c > index 93e0b7680b4f..fc65c589e87f 100644 > --- a/samples/bpf/tracex2_kern.c > +++ b/samples/bpf/tracex2_kern.c > @@ -78,7 +78,7 @@ struct { > __uint(max_entries, 1024); > } my_hist_map SEC(".maps"); > > -SEC("kprobe/" SYSCALL(sys_write)) > +SEC("kprobe/ksys_write") > int bpf_prog3(struct pt_regs *ctx) > { > long write_size = PT_REGS_PARM3(ctx); use SEC("ksyscall/write") int BPF_KSYSCALL(bpf_prog3, unsigned int fd, const char *buf, size_t count) instead? And maybe let's update other samples to use SEC("ksyscall") and BPF_KSYSCALL() macro as well? > -- > 2.34.1 >
On Sat, Dec 3, 2022 at 9:34 AM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Fri, Dec 2, 2022 at 8:29 AM Daniel T. Lee <danieltimlee@gmail.com> wrote: > > > > Currently, there is a problem with tracex2, as it doesn't print the > > histogram properly and the results are misleading. (all results report > > as 0) > > > > The problem is caused by a change in arguments of the function to which > > the kprobe connects. This tracex2 bpf program uses kprobe (attached > > to __x64_sys_write) to figure out the size of the write system call. In > > order to achieve this, the third argument 'count' must be intact. > > > > The following is a prototype of the sys_write variant. (checked with > > pfunct) > > > > ~/git/linux$ pfunct -P fs/read_write.o | grep sys_write > > ssize_t ksys_write(unsigned int fd, const char * buf, size_t count); > > long int __x64_sys_write(const struct pt_regs * regs); > > ... cross compile with s390x ... > > long int __s390_sys_write(struct pt_regs * regs); > > > > Since the __x64_sys_write (or s390x also) doesn't have the proper > > argument, changing the kprobe event to ksys_write will fix the problem. > > > > Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com> > > --- > > samples/bpf/tracex2_kern.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/samples/bpf/tracex2_kern.c b/samples/bpf/tracex2_kern.c > > index 93e0b7680b4f..fc65c589e87f 100644 > > --- a/samples/bpf/tracex2_kern.c > > +++ b/samples/bpf/tracex2_kern.c > > @@ -78,7 +78,7 @@ struct { > > __uint(max_entries, 1024); > > } my_hist_map SEC(".maps"); > > > > -SEC("kprobe/" SYSCALL(sys_write)) > > +SEC("kprobe/ksys_write") > > int bpf_prog3(struct pt_regs *ctx) > > { > > long write_size = PT_REGS_PARM3(ctx); > > > use > > SEC("ksyscall/write") > int BPF_KSYSCALL(bpf_prog3, unsigned int fd, const char *buf, size_t count) > > instead? > > And maybe let's update other samples to use SEC("ksyscall") and > BPF_KSYSCALL() macro as well? > > Thanks for the review! I'll check with the new BPF_KSYSCALL and try to fix others as well! > > -- > > 2.34.1 > >
diff --git a/samples/bpf/tracex2_kern.c b/samples/bpf/tracex2_kern.c index 93e0b7680b4f..fc65c589e87f 100644 --- a/samples/bpf/tracex2_kern.c +++ b/samples/bpf/tracex2_kern.c @@ -78,7 +78,7 @@ struct { __uint(max_entries, 1024); } my_hist_map SEC(".maps"); -SEC("kprobe/" SYSCALL(sys_write)) +SEC("kprobe/ksys_write") int bpf_prog3(struct pt_regs *ctx) { long write_size = PT_REGS_PARM3(ctx);
Currently, there is a problem with tracex2, as it doesn't print the histogram properly and the results are misleading. (all results report as 0) The problem is caused by a change in arguments of the function to which the kprobe connects. This tracex2 bpf program uses kprobe (attached to __x64_sys_write) to figure out the size of the write system call. In order to achieve this, the third argument 'count' must be intact. The following is a prototype of the sys_write variant. (checked with pfunct) ~/git/linux$ pfunct -P fs/read_write.o | grep sys_write ssize_t ksys_write(unsigned int fd, const char * buf, size_t count); long int __x64_sys_write(const struct pt_regs * regs); ... cross compile with s390x ... long int __s390_sys_write(struct pt_regs * regs); Since the __x64_sys_write (or s390x also) doesn't have the proper argument, changing the kprobe event to ksys_write will fix the problem. Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com> --- samples/bpf/tracex2_kern.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)