2022-05-30 00:47:43

by Daniel Xu

[permalink] [raw]
Subject: [PATCH bpf-next 0/2] Add PROG_TEST_RUN support to BPF_PROG_TYPE_KPROBE

This patchset adds PROG_TEST_RUN support to BPF_PROG_TYPE_KPROBE progs.
On top of being generally useful for unit testing kprobe progs, this
feature more specifically helps solve a relability problem with bpftrace
BEGIN and END probes.

BEGIN and END probes are run exactly once at the beginning and end of a
bpftrace tracing session, respectively. bpftrace currently implements
the probes by creating two dummy functions and attaching the BEGIN and
END progs, if defined, to those functions and calling the dummy
functions as appropriate. This works pretty well most of the time except
for when distros strip symbols from bpftrace. Every now and then this
happens and users get confused. Having PROG_TEST_RUN support will help
solve this issue by allowing us to directly trigger uprobes from
userspace.

Admittedly, this is a pretty specific problem and could probably be
solved other ways. However, PROG_TEST_RUN also makes unit testing more
convenient, especially as users start building more complex tracing
applications. So I see this as killing two birds with one stone.

Daniel Xu (2):
bpf, test_run: Add PROG_TEST_RUN support to kprobe
Add PROG_TEST_RUN selftest for BPF_PROG_TYPE_KPROBE

include/linux/bpf.h | 10 ++++
kernel/trace/bpf_trace.c | 1 +
net/bpf/test_run.c | 36 ++++++++++++
.../selftests/bpf/prog_tests/kprobe_ctx.c | 57 +++++++++++++++++++
.../testing/selftests/bpf/progs/kprobe_ctx.c | 33 +++++++++++
5 files changed, 137 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/kprobe_ctx.c
create mode 100644 tools/testing/selftests/bpf/progs/kprobe_ctx.c

--
2.36.1



2022-05-30 07:51:28

by Daniel Xu

[permalink] [raw]
Subject: [PATCH bpf-next 1/2] bpf, test_run: Add PROG_TEST_RUN support to kprobe

This commit adds PROG_TEST_RUN support to BPF_PROG_TYPE_KPROBE progs. On
top of being generally useful for unit testing kprobe progs, this commit
more specifically helps solve a relability problem with bpftrace BEGIN
and END probes.

BEGIN and END probes are run exactly once at the beginning and end of a
bpftrace tracing session, respectively. bpftrace currently implements
the probes by creating two dummy functions and attaching the BEGIN and
END progs, if defined, to those functions and calling the dummy
functions as appropriate. This works pretty well most of the time except
for when distros strip symbols from bpftrace. Every now and then this
happens and users get confused. Having PROG_TEST_RUN support will help
solve this issue by allowing us to directly trigger uprobes from
userspace.

Admittedly, this is a pretty specific problem and could probably be
solved other ways. However, PROG_TEST_RUN also makes unit testing more
convenient, especially as users start building more complex tracing
applications. So I see this as killing two birds with one stone.

Signed-off-by: Daniel Xu <[email protected]>
---
include/linux/bpf.h | 10 ++++++++++
kernel/trace/bpf_trace.c | 1 +
net/bpf/test_run.c | 36 ++++++++++++++++++++++++++++++++++++
3 files changed, 47 insertions(+)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 2b914a56a2c5..dec3082ee158 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1751,6 +1751,9 @@ int bpf_prog_test_run_raw_tp(struct bpf_prog *prog,
int bpf_prog_test_run_sk_lookup(struct bpf_prog *prog,
const union bpf_attr *kattr,
union bpf_attr __user *uattr);
+int bpf_prog_test_run_kprobe(struct bpf_prog *prog,
+ const union bpf_attr *kattr,
+ union bpf_attr __user *uattr);
bool btf_ctx_access(int off, int size, enum bpf_access_type type,
const struct bpf_prog *prog,
struct bpf_insn_access_aux *info);
@@ -1998,6 +2001,13 @@ static inline int bpf_prog_test_run_sk_lookup(struct bpf_prog *prog,
return -ENOTSUPP;
}

+static inline int bpf_prog_test_run_kprobe(struct bpf_prog *prog,
+ const union bpf_attr *kattr,
+ union bpf_attr __user *uattr)
+{
+ return -ENOTSUPP;
+}
+
static inline void bpf_map_put(struct bpf_map *map)
{
}
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 10b157a6d73e..b452e84b9ba4 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -1363,6 +1363,7 @@ const struct bpf_verifier_ops kprobe_verifier_ops = {
};

const struct bpf_prog_ops kprobe_prog_ops = {
+ .test_run = bpf_prog_test_run_kprobe,
};

BPF_CALL_5(bpf_perf_event_output_tp, void *, tp_buff, struct bpf_map *, map,
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index 56f059b3c242..0b6fc17ce901 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -1622,6 +1622,42 @@ int bpf_prog_test_run_syscall(struct bpf_prog *prog,
return err;
}

+int bpf_prog_test_run_kprobe(struct bpf_prog *prog,
+ const union bpf_attr *kattr,
+ union bpf_attr __user *uattr)
+{
+ void __user *ctx_in = u64_to_user_ptr(kattr->test.ctx_in);
+ __u32 ctx_size_in = kattr->test.ctx_size_in;
+ u32 repeat = kattr->test.repeat;
+ struct pt_regs *ctx = NULL;
+ u32 retval, duration;
+ int err = 0;
+
+ if (kattr->test.data_in || kattr->test.data_out ||
+ kattr->test.ctx_out || kattr->test.flags ||
+ kattr->test.cpu || kattr->test.batch_size)
+ return -EINVAL;
+
+ if (ctx_size_in != sizeof(struct pt_regs))
+ return -EINVAL;
+
+ ctx = memdup_user(ctx_in, ctx_size_in);
+ if (IS_ERR(ctx))
+ return PTR_ERR(ctx);
+
+ err = bpf_test_run(prog, ctx, repeat, &retval, &duration, false);
+ if (err)
+ goto out;
+
+ if (copy_to_user(&uattr->test.retval, &retval, sizeof(retval)) ||
+ copy_to_user(&uattr->test.duration, &duration, sizeof(duration))) {
+ err = -EFAULT;
+ }
+out:
+ kfree(ctx);
+ return err;
+}
+
static const struct btf_kfunc_id_set bpf_prog_test_kfunc_set = {
.owner = THIS_MODULE,
.check_set = &test_sk_check_kfunc_ids,
--
2.36.1


2022-05-30 10:01:43

by Song Liu

[permalink] [raw]
Subject: Re: [PATCH bpf-next 0/2] Add PROG_TEST_RUN support to BPF_PROG_TYPE_KPROBE

On Sun, May 29, 2022 at 3:06 PM Daniel Xu <[email protected]> wrote:
>
> This patchset adds PROG_TEST_RUN support to BPF_PROG_TYPE_KPROBE progs.
> On top of being generally useful for unit testing kprobe progs, this
> feature more specifically helps solve a relability problem with bpftrace
> BEGIN and END probes.
>
> BEGIN and END probes are run exactly once at the beginning and end of a
> bpftrace tracing session, respectively. bpftrace currently implements
> the probes by creating two dummy functions and attaching the BEGIN and
> END progs, if defined, to those functions and calling the dummy
> functions as appropriate. This works pretty well most of the time except
> for when distros strip symbols from bpftrace. Every now and then this
> happens and users get confused. Having PROG_TEST_RUN support will help
> solve this issue by allowing us to directly trigger uprobes from
> userspace.
>
> Admittedly, this is a pretty specific problem and could probably be
> solved other ways. However, PROG_TEST_RUN also makes unit testing more
> convenient, especially as users start building more complex tracing
> applications. So I see this as killing two birds with one stone.

We have BPF_PROG_RUN which is an alias of BPF_PROG_TEST_RUN. I guess
that's a better name for the BEGIN/END use case.

Have you checked out bpf_prog_test_run_raw_tp()? AFAICT, it works as good as
kprobe for this use case.

Thanks,
Song

2022-06-01 13:55:54

by Daniel Xu

[permalink] [raw]
Subject: Re: [PATCH bpf-next 0/2] Add PROG_TEST_RUN support to BPF_PROG_TYPE_KPROBE

Hi Song,

On Sun, May 29, 2022 at 11:00:48PM -0700, Song Liu wrote:
> On Sun, May 29, 2022 at 3:06 PM Daniel Xu <[email protected]> wrote:
> >
> > This patchset adds PROG_TEST_RUN support to BPF_PROG_TYPE_KPROBE progs.
> > On top of being generally useful for unit testing kprobe progs, this
> > feature more specifically helps solve a relability problem with bpftrace
> > BEGIN and END probes.
> >
> > BEGIN and END probes are run exactly once at the beginning and end of a
> > bpftrace tracing session, respectively. bpftrace currently implements
> > the probes by creating two dummy functions and attaching the BEGIN and
> > END progs, if defined, to those functions and calling the dummy
> > functions as appropriate. This works pretty well most of the time except
> > for when distros strip symbols from bpftrace. Every now and then this
> > happens and users get confused. Having PROG_TEST_RUN support will help
> > solve this issue by allowing us to directly trigger uprobes from
> > userspace.
> >
> > Admittedly, this is a pretty specific problem and could probably be
> > solved other ways. However, PROG_TEST_RUN also makes unit testing more
> > convenient, especially as users start building more complex tracing
> > applications. So I see this as killing two birds with one stone.
>
> We have BPF_PROG_RUN which is an alias of BPF_PROG_TEST_RUN. I guess
> that's a better name for the BEGIN/END use case.

Right, sorry. Was getting names mixed up.

>
> Have you checked out bpf_prog_test_run_raw_tp()? AFAICT, it works as good as
> kprobe for this use case.

I just took a look -- I think it'll work for BEGIN/END use case. But
also like I mentioned, BPF_PROG_RUN/BPF_PROG_TEST_RUN support for
kprobes is probably still useful. For example if kprobe accesses 13th
register. I suppose the raw_tp 12 arg limit could be lifted but it might
be tricky to capture that logic in the absence of something like `struct
pt_regs` to check the ctx_size_in against.

Thanks,
Daniel

2022-06-01 19:13:45

by Song Liu

[permalink] [raw]
Subject: Re: [PATCH bpf-next 1/2] bpf, test_run: Add PROG_TEST_RUN support to kprobe

On Sun, May 29, 2022 at 3:06 PM Daniel Xu <[email protected]> wrote:
>
> This commit adds PROG_TEST_RUN support to BPF_PROG_TYPE_KPROBE progs. On
> top of being generally useful for unit testing kprobe progs, this commit
> more specifically helps solve a relability problem with bpftrace BEGIN
> and END probes.
>
> BEGIN and END probes are run exactly once at the beginning and end of a
> bpftrace tracing session, respectively. bpftrace currently implements
> the probes by creating two dummy functions and attaching the BEGIN and
> END progs, if defined, to those functions and calling the dummy
> functions as appropriate. This works pretty well most of the time except
> for when distros strip symbols from bpftrace. Every now and then this
> happens and users get confused. Having PROG_TEST_RUN support will help
> solve this issue by allowing us to directly trigger uprobes from
> userspace.
>
> Admittedly, this is a pretty specific problem and could probably be
> solved other ways. However, PROG_TEST_RUN also makes unit testing more
> convenient, especially as users start building more complex tracing
> applications. So I see this as killing two birds with one stone.
>
> Signed-off-by: Daniel Xu <[email protected]>
> ---
> include/linux/bpf.h | 10 ++++++++++
> kernel/trace/bpf_trace.c | 1 +
> net/bpf/test_run.c | 36 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 47 insertions(+)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 2b914a56a2c5..dec3082ee158 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1751,6 +1751,9 @@ int bpf_prog_test_run_raw_tp(struct bpf_prog *prog,
> int bpf_prog_test_run_sk_lookup(struct bpf_prog *prog,
> const union bpf_attr *kattr,
> union bpf_attr __user *uattr);
> +int bpf_prog_test_run_kprobe(struct bpf_prog *prog,
> + const union bpf_attr *kattr,
> + union bpf_attr __user *uattr);
> bool btf_ctx_access(int off, int size, enum bpf_access_type type,
> const struct bpf_prog *prog,
> struct bpf_insn_access_aux *info);
> @@ -1998,6 +2001,13 @@ static inline int bpf_prog_test_run_sk_lookup(struct bpf_prog *prog,
> return -ENOTSUPP;
> }
>
> +static inline int bpf_prog_test_run_kprobe(struct bpf_prog *prog,
> + const union bpf_attr *kattr,
> + union bpf_attr __user *uattr)
> +{
> + return -ENOTSUPP;
> +}

As the kernel test bot reported, this is not enough to cover all
different configs. We can
follow the pattern with bpf_prog_test_run_tracing().

Otherwise, this looks good to me.

Song

> +
> static inline void bpf_map_put(struct bpf_map *map)
> {
> }
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 10b157a6d73e..b452e84b9ba4 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -1363,6 +1363,7 @@ const struct bpf_verifier_ops kprobe_verifier_ops = {
> };
>
> const struct bpf_prog_ops kprobe_prog_ops = {
> + .test_run = bpf_prog_test_run_kprobe,
> };
>
> BPF_CALL_5(bpf_perf_event_output_tp, void *, tp_buff, struct bpf_map *, map,
> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index 56f059b3c242..0b6fc17ce901 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -1622,6 +1622,42 @@ int bpf_prog_test_run_syscall(struct bpf_prog *prog,
> return err;
> }
>
> +int bpf_prog_test_run_kprobe(struct bpf_prog *prog,
> + const union bpf_attr *kattr,
> + union bpf_attr __user *uattr)
> +{
> + void __user *ctx_in = u64_to_user_ptr(kattr->test.ctx_in);
> + __u32 ctx_size_in = kattr->test.ctx_size_in;
> + u32 repeat = kattr->test.repeat;
> + struct pt_regs *ctx = NULL;
> + u32 retval, duration;
> + int err = 0;
> +
> + if (kattr->test.data_in || kattr->test.data_out ||
> + kattr->test.ctx_out || kattr->test.flags ||
> + kattr->test.cpu || kattr->test.batch_size)
> + return -EINVAL;
> +
> + if (ctx_size_in != sizeof(struct pt_regs))
> + return -EINVAL;
> +
> + ctx = memdup_user(ctx_in, ctx_size_in);
> + if (IS_ERR(ctx))
> + return PTR_ERR(ctx);
> +
> + err = bpf_test_run(prog, ctx, repeat, &retval, &duration, false);
> + if (err)
> + goto out;
> +
> + if (copy_to_user(&uattr->test.retval, &retval, sizeof(retval)) ||
> + copy_to_user(&uattr->test.duration, &duration, sizeof(duration))) {
> + err = -EFAULT;
> + }
> +out:
> + kfree(ctx);
> + return err;
> +}
> +
> static const struct btf_kfunc_id_set bpf_prog_test_kfunc_set = {
> .owner = THIS_MODULE,
> .check_set = &test_sk_check_kfunc_ids,
> --
> 2.36.1
>

2022-06-01 19:20:45

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH bpf-next 1/2] bpf, test_run: Add PROG_TEST_RUN support to kprobe

Hi Daniel,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on bpf-next/master]

url: https://github.com/intel-lab-lkp/linux/commits/Daniel-Xu/Add-PROG_TEST_RUN-support-to-BPF_PROG_TYPE_KPROBE/20220530-060742
base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: s390-randconfig-r002-20220531 (https://download.01.org/0day-ci/archive/20220601/[email protected]/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project c825abd6b0198fb088d9752f556a70705bc99dfd)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install s390 cross compiling tool for clang build
# apt-get install binutils-s390x-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/a547a4c795103fd002d3bbb5ee4d7141113716c0
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Daniel-Xu/Add-PROG_TEST_RUN-support-to-BPF_PROG_TYPE_KPROBE/20220530-060742
git checkout a547a4c795103fd002d3bbb5ee4d7141113716c0
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

>> /opt/cross/gcc-11.3.0-nolibc/s390x-linux/bin/s390x-linux-ld: kernel/trace/bpf_trace.o:kernel/trace/bpf_trace.c:1365: undefined reference to `bpf_prog_test_run_kprobe'

--
0-DAY CI Kernel Test Service
https://01.org/lkp

2022-06-01 20:12:42

by Song Liu

[permalink] [raw]
Subject: Re: [PATCH bpf-next 0/2] Add PROG_TEST_RUN support to BPF_PROG_TYPE_KPROBE

On Mon, May 30, 2022 at 7:56 AM Daniel Xu <[email protected]> wrote:
>
> Hi Song,
>
> On Sun, May 29, 2022 at 11:00:48PM -0700, Song Liu wrote:
> > On Sun, May 29, 2022 at 3:06 PM Daniel Xu <[email protected]> wrote:
> > >
> > > This patchset adds PROG_TEST_RUN support to BPF_PROG_TYPE_KPROBE progs.
> > > On top of being generally useful for unit testing kprobe progs, this
> > > feature more specifically helps solve a relability problem with bpftrace
> > > BEGIN and END probes.
> > >
> > > BEGIN and END probes are run exactly once at the beginning and end of a
> > > bpftrace tracing session, respectively. bpftrace currently implements
> > > the probes by creating two dummy functions and attaching the BEGIN and
> > > END progs, if defined, to those functions and calling the dummy
> > > functions as appropriate. This works pretty well most of the time except
> > > for when distros strip symbols from bpftrace. Every now and then this
> > > happens and users get confused. Having PROG_TEST_RUN support will help
> > > solve this issue by allowing us to directly trigger uprobes from
> > > userspace.
> > >
> > > Admittedly, this is a pretty specific problem and could probably be
> > > solved other ways. However, PROG_TEST_RUN also makes unit testing more
> > > convenient, especially as users start building more complex tracing
> > > applications. So I see this as killing two birds with one stone.
> >
> > We have BPF_PROG_RUN which is an alias of BPF_PROG_TEST_RUN. I guess
> > that's a better name for the BEGIN/END use case.
>
> Right, sorry. Was getting names mixed up.
>
> >
> > Have you checked out bpf_prog_test_run_raw_tp()? AFAICT, it works as good as
> > kprobe for this use case.
>
> I just took a look -- I think it'll work for BEGIN/END use case. But
> also like I mentioned, BPF_PROG_RUN/BPF_PROG_TEST_RUN support for
> kprobes is probably still useful. For example if kprobe accesses 13th
> register. I suppose the raw_tp 12 arg limit could be lifted but it might
> be tricky to capture that logic in the absence of something like `struct
> pt_regs` to check the ctx_size_in against.

Agreed that unit tests support for kprobe programs is great.

Thanks,
Song

2022-06-01 20:21:16

by Alexei Starovoitov

[permalink] [raw]
Subject: Re: [PATCH bpf-next 1/2] bpf, test_run: Add PROG_TEST_RUN support to kprobe

On Sun, May 29, 2022 at 3:06 PM Daniel Xu <[email protected]> wrote:
>
> This commit adds PROG_TEST_RUN support to BPF_PROG_TYPE_KPROBE progs. On
> top of being generally useful for unit testing kprobe progs, this commit
> more specifically helps solve a relability problem with bpftrace BEGIN
> and END probes.
>
> BEGIN and END probes are run exactly once at the beginning and end of a
> bpftrace tracing session, respectively. bpftrace currently implements
> the probes by creating two dummy functions and attaching the BEGIN and
> END progs, if defined, to those functions and calling the dummy
> functions as appropriate. This works pretty well most of the time except
> for when distros strip symbols from bpftrace. Every now and then this
> happens and users get confused. Having PROG_TEST_RUN support will help
> solve this issue by allowing us to directly trigger uprobes from
> userspace.
>
> Admittedly, this is a pretty specific problem and could probably be
> solved other ways. However, PROG_TEST_RUN also makes unit testing more
> convenient, especially as users start building more complex tracing
> applications. So I see this as killing two birds with one stone.

bpftrace approach of uprobe-ing into BEGIN_trigger can
be solved with raw_tp prog.
"BEGIN" bpftrace's program doesn't have to be uprobe.
I can be raw_tp and prog_test_run command is
already implemented for raw_tp.

kprobe prog has pt_regs as arguments,
raw_tp has tracepoint args.
Both progs expect kernel addresses in args.
Passing 'struct pt_regs' filled with integers and non-kernel addresses
won't help to unit test bpf-kprobe programs.
There is little use in creating a dummy kprobe-bpf prog
that expects RDI to be 1 and RSI to be 2
(like selftest from patch 2 does) and running it.
We already have raw_tp with similar args and such
progs can be executed already.
Whether SEC() part says kprobe/ or raw_tp/ doesn't change
much in the prog itself.
More so raw_tp prog will work on all architectures,
whereas kprobe's pt_regs are arch specific.
So even if kprobe progs were runnable, bpftrace
should probably be using raw_tp to be arch independent.

2022-06-01 20:24:26

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH bpf-next 1/2] bpf, test_run: Add PROG_TEST_RUN support to kprobe

Hi Daniel,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on bpf-next/master]

url: https://github.com/intel-lab-lkp/linux/commits/Daniel-Xu/Add-PROG_TEST_RUN-support-to-BPF_PROG_TYPE_KPROBE/20220530-060742
base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: s390-randconfig-r026-20220531 (https://download.01.org/0day-ci/archive/20220531/[email protected]/config)
compiler: s390-linux-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/a547a4c795103fd002d3bbb5ee4d7141113716c0
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Daniel-Xu/Add-PROG_TEST_RUN-support-to-BPF_PROG_TYPE_KPROBE/20220530-060742
git checkout a547a4c795103fd002d3bbb5ee4d7141113716c0
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

>> s390-linux-ld: kernel/trace/bpf_trace.o:(.data.rel.ro+0x100): undefined reference to `bpf_prog_test_run_kprobe'
s390-linux-ld: drivers/dma/qcom/hidma.o: in function `hidma_probe':
hidma.c:(.text+0x313e): undefined reference to `devm_ioremap_resource'
s390-linux-ld: hidma.c:(.text+0x3192): undefined reference to `devm_ioremap_resource'

--
0-DAY CI Kernel Test Service
https://01.org/lkp

2022-06-02 16:53:52

by Daniel Xu

[permalink] [raw]
Subject: Re: [PATCH bpf-next 1/2] bpf, test_run: Add PROG_TEST_RUN support to kprobe

On Tue, May 31, 2022 at 11:07:31AM -0700, Alexei Starovoitov wrote:
> On Sun, May 29, 2022 at 3:06 PM Daniel Xu <[email protected]> wrote:
> >
> > This commit adds PROG_TEST_RUN support to BPF_PROG_TYPE_KPROBE progs. On
> > top of being generally useful for unit testing kprobe progs, this commit
> > more specifically helps solve a relability problem with bpftrace BEGIN
> > and END probes.
> >
> > BEGIN and END probes are run exactly once at the beginning and end of a
> > bpftrace tracing session, respectively. bpftrace currently implements
> > the probes by creating two dummy functions and attaching the BEGIN and
> > END progs, if defined, to those functions and calling the dummy
> > functions as appropriate. This works pretty well most of the time except
> > for when distros strip symbols from bpftrace. Every now and then this
> > happens and users get confused. Having PROG_TEST_RUN support will help
> > solve this issue by allowing us to directly trigger uprobes from
> > userspace.
> >
> > Admittedly, this is a pretty specific problem and could probably be
> > solved other ways. However, PROG_TEST_RUN also makes unit testing more
> > convenient, especially as users start building more complex tracing
> > applications. So I see this as killing two birds with one stone.
>
> bpftrace approach of uprobe-ing into BEGIN_trigger can
> be solved with raw_tp prog.
> "BEGIN" bpftrace's program doesn't have to be uprobe.
> I can be raw_tp and prog_test_run command is
> already implemented for raw_tp.
>
> kprobe prog has pt_regs as arguments,
> raw_tp has tracepoint args.
> Both progs expect kernel addresses in args.
> Passing 'struct pt_regs' filled with integers and non-kernel addresses
> won't help to unit test bpf-kprobe programs.
> There is little use in creating a dummy kprobe-bpf prog
> that expects RDI to be 1 and RSI to be 2
> (like selftest from patch 2 does) and running it.

Yeah that's a good point about the kprobe case. But AFAICT uprobes are
implemented using a kprobe prog in which case it would be both possible
and useful to insert userspace args and userspace addrspace addrs.

> We already have raw_tp with similar args and such
> progs can be executed already.
> Whether SEC() part says kprobe/ or raw_tp/ doesn't change
> much in the prog itself.

I suppose so, and I guess you could always bpf_program__set_type(..).

> More so raw_tp prog will work on all architectures,
> whereas kprobe's pt_regs are arch specific.
> So even if kprobe progs were runnable, bpftrace
> should probably be using raw_tp to be arch independent.

bpftrace has all the infra to pull arbitrary structs out of vmlinux BTF
already. It should be fairly simple to get the arch-specific struct
pt_regs size and construct a buffer of all 0s. And fall back to old
logic (that'll be necessary for a while) if kprobe BPF_PROG_RUN or
vmlinux BTF is missing.

That being said, I didn't realize that when I put up the patch, so
thanks for the hint. It sounds like it's probably simpler to just use
raw_tp then.

FWIW I still think this feature is useful, but since I probably won't
use it in bpftrace I'm fine with dropping the patchset. If anyone still
wants it in I'm also fine with continuing on it.

Thanks,
Daniel