Extract the common part as probe_func_comm, which will be used in
both libbpf_probe_bpf_{helper, kfunc}
Signed-off-by: Tao Chen <chen.dylane@gmail.com>
---
tools/lib/bpf/libbpf_probes.c | 38 ++++++++++++++++++++++++-----------
1 file changed, 26 insertions(+), 12 deletions(-)
diff --git a/tools/lib/bpf/libbpf_probes.c b/tools/lib/bpf/libbpf_probes.c
index 9dfbe7750f56..b73345977b4e 100644
--- a/tools/lib/bpf/libbpf_probes.c
+++ b/tools/lib/bpf/libbpf_probes.c
@@ -413,22 +413,20 @@ int libbpf_probe_bpf_map_type(enum bpf_map_type map_type, const void *opts)
return libbpf_err(ret);
}
-int libbpf_probe_bpf_helper(enum bpf_prog_type prog_type, enum bpf_func_id helper_id,
- const void *opts)
+static int probe_func_comm(enum bpf_prog_type prog_type, struct bpf_insn insn,
+ char *accepted_msgs, size_t msgs_size)
{
struct bpf_insn insns[] = {
- BPF_EMIT_CALL((__u32)helper_id),
+ BPF_EXIT_INSN(),
BPF_EXIT_INSN(),
};
const size_t insn_cnt = ARRAY_SIZE(insns);
- char buf[4096];
- int ret;
+ int err;
- if (opts)
- return libbpf_err(-EINVAL);
+ insns[0] = insn;
/* we can't successfully load all prog types to check for BPF helper
- * support, so bail out with -EOPNOTSUPP error
+ * and kfunc support, so bail out with -EOPNOTSUPP error
*/
switch (prog_type) {
case BPF_PROG_TYPE_TRACING:
@@ -440,10 +438,26 @@ int libbpf_probe_bpf_helper(enum bpf_prog_type prog_type, enum bpf_func_id helpe
break;
}
- buf[0] = '\0';
- ret = probe_prog_load(prog_type, insns, insn_cnt, buf, sizeof(buf));
- if (ret < 0)
- return libbpf_err(ret);
+ accepted_msgs[0] = '\0';
+ err = probe_prog_load(prog_type, insns, insn_cnt, accepted_msgs, msgs_size);
+ if (err < 0)
+ return libbpf_err(err);
+
+ return 0;
+}
+
+int libbpf_probe_bpf_helper(enum bpf_prog_type prog_type, enum bpf_func_id helper_id,
+ const void *opts)
+{
+ char buf[4096];
+ int ret;
+
+ if (opts)
+ return libbpf_err(-EINVAL);
+
+ ret = probe_func_comm(prog_type, BPF_EMIT_CALL((__u32)helper_id), buf, sizeof(buf));
+ if (ret)
+ return ret;
/* If BPF verifier doesn't recognize BPF helper ID (enum bpf_func_id)
* at all, it will emit something like "invalid func unknown#181".
--
2.43.0On Fri, Jan 24, 2025 at 6:44 AM Tao Chen <chen.dylane@gmail.com> wrote:
>
> Extract the common part as probe_func_comm, which will be used in
> both libbpf_probe_bpf_{helper, kfunc}
>
> Signed-off-by: Tao Chen <chen.dylane@gmail.com>
> ---
> tools/lib/bpf/libbpf_probes.c | 38 ++++++++++++++++++++++++-----------
> 1 file changed, 26 insertions(+), 12 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf_probes.c b/tools/lib/bpf/libbpf_probes.c
> index 9dfbe7750f56..b73345977b4e 100644
> --- a/tools/lib/bpf/libbpf_probes.c
> +++ b/tools/lib/bpf/libbpf_probes.c
> @@ -413,22 +413,20 @@ int libbpf_probe_bpf_map_type(enum bpf_map_type map_type, const void *opts)
> return libbpf_err(ret);
> }
>
> -int libbpf_probe_bpf_helper(enum bpf_prog_type prog_type, enum bpf_func_id helper_id,
> - const void *opts)
> +static int probe_func_comm(enum bpf_prog_type prog_type, struct bpf_insn insn,
> + char *accepted_msgs, size_t msgs_size)
> {
> struct bpf_insn insns[] = {
> - BPF_EMIT_CALL((__u32)helper_id),
> + BPF_EXIT_INSN(),
> BPF_EXIT_INSN(),
> };
> const size_t insn_cnt = ARRAY_SIZE(insns);
> - char buf[4096];
> - int ret;
> + int err;
>
> - if (opts)
> - return libbpf_err(-EINVAL);
> + insns[0] = insn;
>
> /* we can't successfully load all prog types to check for BPF helper
> - * support, so bail out with -EOPNOTSUPP error
> + * and kfunc support, so bail out with -EOPNOTSUPP error
> */
> switch (prog_type) {
> case BPF_PROG_TYPE_TRACING:
there isn't much logic that you will extract here besides this check
whether program type can even be successfully loaded, so I wouldn't
extract probe_func_comm(), but rather extract just the check:
static bool can_probe_prog_type(enum bpf_prog_type prog_type)
{
/* we can't successfully load all prog types to check for BPF
helper/kfunc
* support, so check this early and bail
*/
switch (prog_type) {
...: return false
default:
return true;
}
And just check that can_probe_prog_type() inside
libbpf_probe_bpf_helper and libbpf_probe_bpf_kfunc
pw-bot: cr
> @@ -440,10 +438,26 @@ int libbpf_probe_bpf_helper(enum bpf_prog_type prog_type, enum bpf_func_id helpe
> break;
> }
>
> - buf[0] = '\0';
> - ret = probe_prog_load(prog_type, insns, insn_cnt, buf, sizeof(buf));
> - if (ret < 0)
> - return libbpf_err(ret);
> + accepted_msgs[0] = '\0';
> + err = probe_prog_load(prog_type, insns, insn_cnt, accepted_msgs, msgs_size);
> + if (err < 0)
> + return libbpf_err(err);
> +
> + return 0;
> +}
> +
> +int libbpf_probe_bpf_helper(enum bpf_prog_type prog_type, enum bpf_func_id helper_id,
> + const void *opts)
> +{
> + char buf[4096];
> + int ret;
> +
> + if (opts)
> + return libbpf_err(-EINVAL);
> +
> + ret = probe_func_comm(prog_type, BPF_EMIT_CALL((__u32)helper_id), buf, sizeof(buf));
> + if (ret)
> + return ret;
>
> /* If BPF verifier doesn't recognize BPF helper ID (enum bpf_func_id)
> * at all, it will emit something like "invalid func unknown#181".
> --
> 2.43.0
>
在 2025/1/25 02:44, Andrii Nakryiko 写道:
> On Fri, Jan 24, 2025 at 6:44 AM Tao Chen <chen.dylane@gmail.com> wrote:
>>
>> Extract the common part as probe_func_comm, which will be used in
>> both libbpf_probe_bpf_{helper, kfunc}
>>
>> Signed-off-by: Tao Chen <chen.dylane@gmail.com>
>> ---
>> tools/lib/bpf/libbpf_probes.c | 38 ++++++++++++++++++++++++-----------
>> 1 file changed, 26 insertions(+), 12 deletions(-)
>>
>> diff --git a/tools/lib/bpf/libbpf_probes.c b/tools/lib/bpf/libbpf_probes.c
>> index 9dfbe7750f56..b73345977b4e 100644
>> --- a/tools/lib/bpf/libbpf_probes.c
>> +++ b/tools/lib/bpf/libbpf_probes.c
>> @@ -413,22 +413,20 @@ int libbpf_probe_bpf_map_type(enum bpf_map_type map_type, const void *opts)
>> return libbpf_err(ret);
>> }
>>
>> -int libbpf_probe_bpf_helper(enum bpf_prog_type prog_type, enum bpf_func_id helper_id,
>> - const void *opts)
>> +static int probe_func_comm(enum bpf_prog_type prog_type, struct bpf_insn insn,
>> + char *accepted_msgs, size_t msgs_size)
>> {
>> struct bpf_insn insns[] = {
>> - BPF_EMIT_CALL((__u32)helper_id),
>> + BPF_EXIT_INSN(),
>> BPF_EXIT_INSN(),
>> };
>> const size_t insn_cnt = ARRAY_SIZE(insns);
>> - char buf[4096];
>> - int ret;
>> + int err;
>>
>> - if (opts)
>> - return libbpf_err(-EINVAL);
>> + insns[0] = insn;
>>
>> /* we can't successfully load all prog types to check for BPF helper
>> - * support, so bail out with -EOPNOTSUPP error
>> + * and kfunc support, so bail out with -EOPNOTSUPP error
>> */
>> switch (prog_type) {
>> case BPF_PROG_TYPE_TRACING:
>
> there isn't much logic that you will extract here besides this check
> whether program type can even be successfully loaded, so I wouldn't
> extract probe_func_comm(), but rather extract just the check:
>
> static bool can_probe_prog_type(enum bpf_prog_type prog_type)
> {
> /* we can't successfully load all prog types to check for BPF
> helper/kfunc
> * support, so check this early and bail
> */
> switch (prog_type) {
> ...: return false
> default:
> return true;
> }
>
>
> And just check that can_probe_prog_type() inside
> libbpf_probe_bpf_helper and libbpf_probe_bpf_kfunc
>
> pw-bot: cr
>
Hi Andrii,
Thank you for your review, jiri also suggested putting the insn part
back into libbpf_bpf_probe_{helper, kfunc}, so I'll make the
modifications as you suggested in v4.
>> @@ -440,10 +438,26 @@ int libbpf_probe_bpf_helper(enum bpf_prog_type prog_type, enum bpf_func_id helpe
>> break;
>> }
>>
>> - buf[0] = '\0';
>> - ret = probe_prog_load(prog_type, insns, insn_cnt, buf, sizeof(buf));
>> - if (ret < 0)
>> - return libbpf_err(ret);
>> + accepted_msgs[0] = '\0';
>> + err = probe_prog_load(prog_type, insns, insn_cnt, accepted_msgs, msgs_size);
>> + if (err < 0)
>> + return libbpf_err(err);
>> +
>> + return 0;
>> +}
>> +
>> +int libbpf_probe_bpf_helper(enum bpf_prog_type prog_type, enum bpf_func_id helper_id,
>> + const void *opts)
>> +{
>> + char buf[4096];
>> + int ret;
>> +
>> + if (opts)
>> + return libbpf_err(-EINVAL);
>> +
>> + ret = probe_func_comm(prog_type, BPF_EMIT_CALL((__u32)helper_id), buf, sizeof(buf));
>> + if (ret)
>> + return ret;
>>
>> /* If BPF verifier doesn't recognize BPF helper ID (enum bpf_func_id)
>> * at all, it will emit something like "invalid func unknown#181".
>> --
>> 2.43.0
>>
--
Best Regards
Dylane Chen
On Fri, Jan 24, 2025 at 10:44:09PM +0800, Tao Chen wrote:
> Extract the common part as probe_func_comm, which will be used in
> both libbpf_probe_bpf_{helper, kfunc}
>
> Signed-off-by: Tao Chen <chen.dylane@gmail.com>
> ---
> tools/lib/bpf/libbpf_probes.c | 38 ++++++++++++++++++++++++-----------
> 1 file changed, 26 insertions(+), 12 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf_probes.c b/tools/lib/bpf/libbpf_probes.c
> index 9dfbe7750f56..b73345977b4e 100644
> --- a/tools/lib/bpf/libbpf_probes.c
> +++ b/tools/lib/bpf/libbpf_probes.c
> @@ -413,22 +413,20 @@ int libbpf_probe_bpf_map_type(enum bpf_map_type map_type, const void *opts)
> return libbpf_err(ret);
> }
>
> -int libbpf_probe_bpf_helper(enum bpf_prog_type prog_type, enum bpf_func_id helper_id,
> - const void *opts)
> +static int probe_func_comm(enum bpf_prog_type prog_type, struct bpf_insn insn,
> + char *accepted_msgs, size_t msgs_size)
> {
> struct bpf_insn insns[] = {
> - BPF_EMIT_CALL((__u32)helper_id),
> + BPF_EXIT_INSN(),
> BPF_EXIT_INSN(),
I'd just keep above in libbpf_probe_bpf_helper and pass insns to probe_func_comm,
seems easier
jirka
> };
> const size_t insn_cnt = ARRAY_SIZE(insns);
> - char buf[4096];
> - int ret;
> + int err;
>
> - if (opts)
> - return libbpf_err(-EINVAL);
> + insns[0] = insn;
>
> /* we can't successfully load all prog types to check for BPF helper
> - * support, so bail out with -EOPNOTSUPP error
> + * and kfunc support, so bail out with -EOPNOTSUPP error
> */
> switch (prog_type) {
> case BPF_PROG_TYPE_TRACING:
> @@ -440,10 +438,26 @@ int libbpf_probe_bpf_helper(enum bpf_prog_type prog_type, enum bpf_func_id helpe
> break;
> }
>
> - buf[0] = '\0';
> - ret = probe_prog_load(prog_type, insns, insn_cnt, buf, sizeof(buf));
> - if (ret < 0)
> - return libbpf_err(ret);
> + accepted_msgs[0] = '\0';
> + err = probe_prog_load(prog_type, insns, insn_cnt, accepted_msgs, msgs_size);
> + if (err < 0)
> + return libbpf_err(err);
> +
> + return 0;
> +}
> +
> +int libbpf_probe_bpf_helper(enum bpf_prog_type prog_type, enum bpf_func_id helper_id,
> + const void *opts)
> +{
> + char buf[4096];
> + int ret;
> +
> + if (opts)
> + return libbpf_err(-EINVAL);
> +
> + ret = probe_func_comm(prog_type, BPF_EMIT_CALL((__u32)helper_id), buf, sizeof(buf));
> + if (ret)
> + return ret;
>
> /* If BPF verifier doesn't recognize BPF helper ID (enum bpf_func_id)
> * at all, it will emit something like "invalid func unknown#181".
> --
> 2.43.0
>
在 2025/1/25 00:26, Jiri Olsa 写道:
> On Fri, Jan 24, 2025 at 10:44:09PM +0800, Tao Chen wrote:
>> Extract the common part as probe_func_comm, which will be used in
>> both libbpf_probe_bpf_{helper, kfunc}
>>
>> Signed-off-by: Tao Chen <chen.dylane@gmail.com>
>> ---
>> tools/lib/bpf/libbpf_probes.c | 38 ++++++++++++++++++++++++-----------
>> 1 file changed, 26 insertions(+), 12 deletions(-)
>>
>> diff --git a/tools/lib/bpf/libbpf_probes.c b/tools/lib/bpf/libbpf_probes.c
>> index 9dfbe7750f56..b73345977b4e 100644
>> --- a/tools/lib/bpf/libbpf_probes.c
>> +++ b/tools/lib/bpf/libbpf_probes.c
>> @@ -413,22 +413,20 @@ int libbpf_probe_bpf_map_type(enum bpf_map_type map_type, const void *opts)
>> return libbpf_err(ret);
>> }
>>
>> -int libbpf_probe_bpf_helper(enum bpf_prog_type prog_type, enum bpf_func_id helper_id,
>> - const void *opts)
>> +static int probe_func_comm(enum bpf_prog_type prog_type, struct bpf_insn insn,
>> + char *accepted_msgs, size_t msgs_size)
>> {
>> struct bpf_insn insns[] = {
>> - BPF_EMIT_CALL((__u32)helper_id),
>> + BPF_EXIT_INSN(),
>> BPF_EXIT_INSN(),
>
> I'd just keep above in libbpf_probe_bpf_helper and pass insns to probe_func_comm,
> seems easier
>
> jirka
>
Hi jiri,
Thank you for your review, your suggestion seems better, i will
send it in v4.
>> };
>> const size_t insn_cnt = ARRAY_SIZE(insns);
>> - char buf[4096];
>> - int ret;
>> + int err;
>>
>> - if (opts)
>> - return libbpf_err(-EINVAL);
>> + insns[0] = insn;
>>
>> /* we can't successfully load all prog types to check for BPF helper
>> - * support, so bail out with -EOPNOTSUPP error
>> + * and kfunc support, so bail out with -EOPNOTSUPP error
>> */
>> switch (prog_type) {
>> case BPF_PROG_TYPE_TRACING:
>> @@ -440,10 +438,26 @@ int libbpf_probe_bpf_helper(enum bpf_prog_type prog_type, enum bpf_func_id helpe
>> break;
>> }
>>
>> - buf[0] = '\0';
>> - ret = probe_prog_load(prog_type, insns, insn_cnt, buf, sizeof(buf));
>> - if (ret < 0)
>> - return libbpf_err(ret);
>> + accepted_msgs[0] = '\0';
>> + err = probe_prog_load(prog_type, insns, insn_cnt, accepted_msgs, msgs_size);
>> + if (err < 0)
>> + return libbpf_err(err);
>> +
>> + return 0;
>> +}
>> +
>> +int libbpf_probe_bpf_helper(enum bpf_prog_type prog_type, enum bpf_func_id helper_id,
>> + const void *opts)
>> +{
>> + char buf[4096];
>> + int ret;
>> +
>> + if (opts)
>> + return libbpf_err(-EINVAL);
>> +
>> + ret = probe_func_comm(prog_type, BPF_EMIT_CALL((__u32)helper_id), buf, sizeof(buf));
>> + if (ret)
>> + return ret;
>>
>> /* If BPF verifier doesn't recognize BPF helper ID (enum bpf_func_id)
>> * at all, it will emit something like "invalid func unknown#181".
>> --
>> 2.43.0
>>
--
Best Regards
Dylane Chen
© 2016 - 2026 Red Hat, Inc.