* [PATCH bpf-next 1/2] libbpf: implement bpf_usdt_arg_size BPF function @ 2025-02-20 21:59 Ihor Solodrai 2025-02-20 21:59 ` [PATCH bpf-next 2/2] selftests/bpf: test bpf_usdt_arg_size() function Ihor Solodrai 2025-02-24 22:05 ` [PATCH bpf-next 1/2] libbpf: implement bpf_usdt_arg_size BPF function Andrii Nakryiko 0 siblings, 2 replies; 7+ messages in thread From: Ihor Solodrai @ 2025-02-20 21:59 UTC (permalink / raw) To: bpf; +Cc: andrii, ast, daniel, eddyz87, mykolal, kernel-team Information about USDT argument size is implicitly stored in __bpf_usdt_arg_spec, but currently it's not accessbile to BPF programs that use USDT. Implement bpf_sdt_arg_size() that returns the size of an USDT argument in bytes. Factor out __bpf_usdt_arg_spec() routine from bpf_usdt_arg(). It searches for arg_spec given ctx and arg_num. Suggested-by: Andrii Nakryiko <andrii@kernel.org> Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev> --- tools/lib/bpf/usdt.bpf.h | 59 ++++++++++++++++++++++++++++++++-------- 1 file changed, 47 insertions(+), 12 deletions(-) diff --git a/tools/lib/bpf/usdt.bpf.h b/tools/lib/bpf/usdt.bpf.h index b811f754939f..6041271c5e4e 100644 --- a/tools/lib/bpf/usdt.bpf.h +++ b/tools/lib/bpf/usdt.bpf.h @@ -108,19 +108,12 @@ int bpf_usdt_arg_cnt(struct pt_regs *ctx) return spec->arg_cnt; } -/* Fetch USDT argument #*arg_num* (zero-indexed) and put its value into *res. - * Returns 0 on success; negative error, otherwise. - * On error *res is guaranteed to be set to zero. - */ -__weak __hidden -int bpf_usdt_arg(struct pt_regs *ctx, __u64 arg_num, long *res) +/* Validate ctx and arg_num, if ok set arg_spec pointer */ +static __always_inline +int __bpf_usdt_arg_spec(struct pt_regs *ctx, __u64 arg_num, struct __bpf_usdt_arg_spec **arg_spec) { struct __bpf_usdt_spec *spec; - struct __bpf_usdt_arg_spec *arg_spec; - unsigned long val; - int err, spec_id; - - *res = 0; + int spec_id; spec_id = __bpf_usdt_spec_id(ctx); if (spec_id < 0) @@ -136,7 +129,49 @@ int bpf_usdt_arg(struct pt_regs *ctx, __u64 arg_num, long *res) if (arg_num >= spec->arg_cnt) return -ENOENT; - arg_spec = &spec->args[arg_num]; + *arg_spec = &spec->args[arg_num]; + + return 0; +} + +/* Returns the size in bytes of the #*arg_num* (zero-indexed) USDT argument. + * Returns negative error if argument is not found or arg_num is invalid. + */ +static __always_inline +int bpf_usdt_arg_size(struct pt_regs *ctx, __u64 arg_num) +{ + struct __bpf_usdt_arg_spec *arg_spec; + int err; + + err = __bpf_usdt_arg_spec(ctx, arg_num, &arg_spec); + if (err) + return err; + + /* arg_spec->arg_bitshift = 64 - arg_sz * 8 + * so: arg_sz = (64 - arg_spec->arg_bitshift) / 8 + * Do a bitshift instead of a division to avoid + * "unsupported signed division" error. + */ + return (64 - arg_spec->arg_bitshift) >> 3; +} + +/* Fetch USDT argument #*arg_num* (zero-indexed) and put its value into *res. + * Returns 0 on success; negative error, otherwise. + * On error *res is guaranteed to be set to zero. + */ +__weak __hidden +int bpf_usdt_arg(struct pt_regs *ctx, __u64 arg_num, long *res) +{ + struct __bpf_usdt_arg_spec *arg_spec; + unsigned long val; + int err; + + *res = 0; + + err = __bpf_usdt_arg_spec(ctx, arg_num, &arg_spec); + if (err) + return err; + switch (arg_spec->arg_type) { case BPF_USDT_ARG_CONST: /* Arg is just a constant ("-4@$-9" in USDT arg spec). -- 2.48.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH bpf-next 2/2] selftests/bpf: test bpf_usdt_arg_size() function 2025-02-20 21:59 [PATCH bpf-next 1/2] libbpf: implement bpf_usdt_arg_size BPF function Ihor Solodrai @ 2025-02-20 21:59 ` Ihor Solodrai 2025-02-24 22:05 ` [PATCH bpf-next 1/2] libbpf: implement bpf_usdt_arg_size BPF function Andrii Nakryiko 1 sibling, 0 replies; 7+ messages in thread From: Ihor Solodrai @ 2025-02-20 21:59 UTC (permalink / raw) To: bpf; +Cc: andrii, ast, daniel, eddyz87, mykolal, kernel-team Update usdt tests to also check for correct behavior of bpf_usdt_arg_size(). Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev> --- tools/testing/selftests/bpf/prog_tests/usdt.c | 11 ++++++++++- tools/testing/selftests/bpf/progs/test_usdt.c | 14 ++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/bpf/prog_tests/usdt.c b/tools/testing/selftests/bpf/prog_tests/usdt.c index 56ed1eb9b527..495d66414b57 100644 --- a/tools/testing/selftests/bpf/prog_tests/usdt.c +++ b/tools/testing/selftests/bpf/prog_tests/usdt.c @@ -45,7 +45,7 @@ static void subtest_basic_usdt(void) LIBBPF_OPTS(bpf_usdt_opts, opts); struct test_usdt *skel; struct test_usdt__bss *bss; - int err; + int err, i; skel = test_usdt__open_and_load(); if (!ASSERT_OK_PTR(skel, "skel_open")) @@ -75,6 +75,7 @@ static void subtest_basic_usdt(void) ASSERT_EQ(bss->usdt0_cookie, 0xcafedeadbeeffeed, "usdt0_cookie"); ASSERT_EQ(bss->usdt0_arg_cnt, 0, "usdt0_arg_cnt"); ASSERT_EQ(bss->usdt0_arg_ret, -ENOENT, "usdt0_arg_ret"); + ASSERT_EQ(bss->usdt0_arg_size, -ENOENT, "usdt0_arg_size"); /* auto-attached usdt3 gets default zero cookie value */ ASSERT_EQ(bss->usdt3_cookie, 0, "usdt3_cookie"); @@ -86,6 +87,9 @@ static void subtest_basic_usdt(void) ASSERT_EQ(bss->usdt3_args[0], 1, "usdt3_arg1"); ASSERT_EQ(bss->usdt3_args[1], 42, "usdt3_arg2"); ASSERT_EQ(bss->usdt3_args[2], (uintptr_t)&bla, "usdt3_arg3"); + ASSERT_EQ(bss->usdt3_arg_sizes[0], 4, "usdt3_arg1_size"); + ASSERT_EQ(bss->usdt3_arg_sizes[1], 8, "usdt3_arg2_size"); + ASSERT_EQ(bss->usdt3_arg_sizes[2], 8, "usdt3_arg3_size"); /* auto-attached usdt12 gets default zero cookie value */ ASSERT_EQ(bss->usdt12_cookie, 0, "usdt12_cookie"); @@ -104,6 +108,11 @@ static void subtest_basic_usdt(void) ASSERT_EQ(bss->usdt12_args[10], nums[idx], "usdt12_arg11"); ASSERT_EQ(bss->usdt12_args[11], t1.y, "usdt12_arg12"); + int usdt12_expected_arg_sizes[12] = { 4, 4, 8, 8, 4, 8, 8, 8, 4, 2, 2, 1 }; + + for (i = 0; i < 12; i++) + ASSERT_EQ(bss->usdt12_arg_sizes[i], usdt12_expected_arg_sizes[i], "usdt12_arg_size"); + /* trigger_func() is marked __always_inline, so USDT invocations will be * inlined in two different places, meaning that each USDT will have * at least 2 different places to be attached to. This verifies that diff --git a/tools/testing/selftests/bpf/progs/test_usdt.c b/tools/testing/selftests/bpf/progs/test_usdt.c index 505aab9a5234..096488f47fbc 100644 --- a/tools/testing/selftests/bpf/progs/test_usdt.c +++ b/tools/testing/selftests/bpf/progs/test_usdt.c @@ -11,6 +11,7 @@ int usdt0_called; u64 usdt0_cookie; int usdt0_arg_cnt; int usdt0_arg_ret; +int usdt0_arg_size; SEC("usdt") int usdt0(struct pt_regs *ctx) @@ -26,6 +27,7 @@ int usdt0(struct pt_regs *ctx) usdt0_arg_cnt = bpf_usdt_arg_cnt(ctx); /* should return -ENOENT for any arg_num */ usdt0_arg_ret = bpf_usdt_arg(ctx, bpf_get_prandom_u32(), &tmp); + usdt0_arg_size = bpf_usdt_arg_size(ctx, bpf_get_prandom_u32()); return 0; } @@ -34,6 +36,7 @@ u64 usdt3_cookie; int usdt3_arg_cnt; int usdt3_arg_rets[3]; u64 usdt3_args[3]; +int usdt3_arg_sizes[3]; SEC("usdt//proc/self/exe:test:usdt3") int usdt3(struct pt_regs *ctx) @@ -50,12 +53,15 @@ int usdt3(struct pt_regs *ctx) usdt3_arg_rets[0] = bpf_usdt_arg(ctx, 0, &tmp); usdt3_args[0] = (int)tmp; + usdt3_arg_sizes[0] = bpf_usdt_arg_size(ctx, 0); usdt3_arg_rets[1] = bpf_usdt_arg(ctx, 1, &tmp); usdt3_args[1] = (long)tmp; + usdt3_arg_sizes[1] = bpf_usdt_arg_size(ctx, 1); usdt3_arg_rets[2] = bpf_usdt_arg(ctx, 2, &tmp); usdt3_args[2] = (uintptr_t)tmp; + usdt3_arg_sizes[2] = bpf_usdt_arg_size(ctx, 2); return 0; } @@ -64,12 +70,15 @@ int usdt12_called; u64 usdt12_cookie; int usdt12_arg_cnt; u64 usdt12_args[12]; +int usdt12_arg_sizes[12]; SEC("usdt//proc/self/exe:test:usdt12") int BPF_USDT(usdt12, int a1, int a2, long a3, long a4, unsigned a5, long a6, __u64 a7, uintptr_t a8, int a9, short a10, short a11, signed char a12) { + int i; + if (my_pid != (bpf_get_current_pid_tgid() >> 32)) return 0; @@ -90,6 +99,11 @@ int BPF_USDT(usdt12, int a1, int a2, long a3, long a4, unsigned a5, usdt12_args[9] = a10; usdt12_args[10] = a11; usdt12_args[11] = a12; + + bpf_for(i, 0, 12) { + usdt12_arg_sizes[i] = bpf_usdt_arg_size(ctx, i); + } + return 0; } -- 2.48.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: implement bpf_usdt_arg_size BPF function 2025-02-20 21:59 [PATCH bpf-next 1/2] libbpf: implement bpf_usdt_arg_size BPF function Ihor Solodrai 2025-02-20 21:59 ` [PATCH bpf-next 2/2] selftests/bpf: test bpf_usdt_arg_size() function Ihor Solodrai @ 2025-02-24 22:05 ` Andrii Nakryiko 2025-02-24 22:21 ` Ihor Solodrai 2025-02-24 23:14 ` Ihor Solodrai 1 sibling, 2 replies; 7+ messages in thread From: Andrii Nakryiko @ 2025-02-24 22:05 UTC (permalink / raw) To: Ihor Solodrai; +Cc: bpf, andrii, ast, daniel, eddyz87, mykolal, kernel-team On Thu, Feb 20, 2025 at 1:59 PM Ihor Solodrai <ihor.solodrai@linux.dev> wrote: > > Information about USDT argument size is implicitly stored in > __bpf_usdt_arg_spec, but currently it's not accessbile to BPF programs > that use USDT. > > Implement bpf_sdt_arg_size() that returns the size of an USDT argument > in bytes. > > Factor out __bpf_usdt_arg_spec() routine from bpf_usdt_arg(). It > searches for arg_spec given ctx and arg_num. > > Suggested-by: Andrii Nakryiko <andrii@kernel.org> > Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev> > --- > tools/lib/bpf/usdt.bpf.h | 59 ++++++++++++++++++++++++++++++++-------- > 1 file changed, 47 insertions(+), 12 deletions(-) > > diff --git a/tools/lib/bpf/usdt.bpf.h b/tools/lib/bpf/usdt.bpf.h > index b811f754939f..6041271c5e4e 100644 > --- a/tools/lib/bpf/usdt.bpf.h > +++ b/tools/lib/bpf/usdt.bpf.h > @@ -108,19 +108,12 @@ int bpf_usdt_arg_cnt(struct pt_regs *ctx) > return spec->arg_cnt; > } > > -/* Fetch USDT argument #*arg_num* (zero-indexed) and put its value into *res. > - * Returns 0 on success; negative error, otherwise. > - * On error *res is guaranteed to be set to zero. > - */ > -__weak __hidden > -int bpf_usdt_arg(struct pt_regs *ctx, __u64 arg_num, long *res) > +/* Validate ctx and arg_num, if ok set arg_spec pointer */ > +static __always_inline > +int __bpf_usdt_arg_spec(struct pt_regs *ctx, __u64 arg_num, struct __bpf_usdt_arg_spec **arg_spec) > { > struct __bpf_usdt_spec *spec; > - struct __bpf_usdt_arg_spec *arg_spec; > - unsigned long val; > - int err, spec_id; > - > - *res = 0; > + int spec_id; > > spec_id = __bpf_usdt_spec_id(ctx); > if (spec_id < 0) > @@ -136,7 +129,49 @@ int bpf_usdt_arg(struct pt_regs *ctx, __u64 arg_num, long *res) > if (arg_num >= spec->arg_cnt) > return -ENOENT; > > - arg_spec = &spec->args[arg_num]; > + *arg_spec = &spec->args[arg_num]; > + > + return 0; > +} > + > +/* Returns the size in bytes of the #*arg_num* (zero-indexed) USDT argument. > + * Returns negative error if argument is not found or arg_num is invalid. > + */ > +static __always_inline > +int bpf_usdt_arg_size(struct pt_regs *ctx, __u64 arg_num) > +{ > + struct __bpf_usdt_arg_spec *arg_spec; > + int err; > + > + err = __bpf_usdt_arg_spec(ctx, arg_num, &arg_spec); let's not extract this into a separate function. I don't particularly like the out parameter, and no need to add another function that could be used by users. There isn't much duplication, I think it's fine if we just copy/paste few lines of code. pw-bot: cr > + if (err) > + return err; > + > + /* arg_spec->arg_bitshift = 64 - arg_sz * 8 > + * so: arg_sz = (64 - arg_spec->arg_bitshift) / 8 > + * Do a bitshift instead of a division to avoid > + * "unsupported signed division" error. > + */ > + return (64 - arg_spec->arg_bitshift) >> 3; arg_bitshift is stored as char (which could be signed), so that's why you were getting signed division, just cast to unsigned and keep division: return (64 - (unsigned)arg_spec->arg_bitshift) / 8; > +} > + > +/* Fetch USDT argument #*arg_num* (zero-indexed) and put its value into *res. > + * Returns 0 on success; negative error, otherwise. > + * On error *res is guaranteed to be set to zero. > + */ > +__weak __hidden > +int bpf_usdt_arg(struct pt_regs *ctx, __u64 arg_num, long *res) > +{ > + struct __bpf_usdt_arg_spec *arg_spec; > + unsigned long val; > + int err; > + > + *res = 0; > + > + err = __bpf_usdt_arg_spec(ctx, arg_num, &arg_spec); > + if (err) > + return err; > + > switch (arg_spec->arg_type) { > case BPF_USDT_ARG_CONST: > /* Arg is just a constant ("-4@$-9" in USDT arg spec). > -- > 2.48.1 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: implement bpf_usdt_arg_size BPF function 2025-02-24 22:05 ` [PATCH bpf-next 1/2] libbpf: implement bpf_usdt_arg_size BPF function Andrii Nakryiko @ 2025-02-24 22:21 ` Ihor Solodrai 2025-02-24 23:14 ` Ihor Solodrai 1 sibling, 0 replies; 7+ messages in thread From: Ihor Solodrai @ 2025-02-24 22:21 UTC (permalink / raw) To: Andrii Nakryiko; +Cc: bpf, andrii, ast, daniel, eddyz87, mykolal, kernel-team On 2/24/25 2:05 PM, Andrii Nakryiko wrote: > On Thu, Feb 20, 2025 at 1:59 PM Ihor Solodrai <ihor.solodrai@linux.dev> wrote: >> >> Information about USDT argument size is implicitly stored in >> __bpf_usdt_arg_spec, but currently it's not accessbile to BPF programs >> that use USDT. >> >> Implement bpf_sdt_arg_size() that returns the size of an USDT argument >> in bytes. >> >> Factor out __bpf_usdt_arg_spec() routine from bpf_usdt_arg(). It >> searches for arg_spec given ctx and arg_num. >> >> Suggested-by: Andrii Nakryiko <andrii@kernel.org> >> Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev> >> --- >> tools/lib/bpf/usdt.bpf.h | 59 ++++++++++++++++++++++++++++++++-------- >> 1 file changed, 47 insertions(+), 12 deletions(-) >> >> diff --git a/tools/lib/bpf/usdt.bpf.h b/tools/lib/bpf/usdt.bpf.h >> index b811f754939f..6041271c5e4e 100644 >> --- a/tools/lib/bpf/usdt.bpf.h >> +++ b/tools/lib/bpf/usdt.bpf.h >> @@ -108,19 +108,12 @@ int bpf_usdt_arg_cnt(struct pt_regs *ctx) >> return spec->arg_cnt; >> } >> >> -/* Fetch USDT argument #*arg_num* (zero-indexed) and put its value into *res. >> - * Returns 0 on success; negative error, otherwise. >> - * On error *res is guaranteed to be set to zero. >> - */ >> -__weak __hidden >> -int bpf_usdt_arg(struct pt_regs *ctx, __u64 arg_num, long *res) >> +/* Validate ctx and arg_num, if ok set arg_spec pointer */ >> +static __always_inline >> +int __bpf_usdt_arg_spec(struct pt_regs *ctx, __u64 arg_num, struct __bpf_usdt_arg_spec **arg_spec) >> { >> struct __bpf_usdt_spec *spec; >> - struct __bpf_usdt_arg_spec *arg_spec; >> - unsigned long val; >> - int err, spec_id; >> - >> - *res = 0; >> + int spec_id; >> >> spec_id = __bpf_usdt_spec_id(ctx); >> if (spec_id < 0) >> @@ -136,7 +129,49 @@ int bpf_usdt_arg(struct pt_regs *ctx, __u64 arg_num, long *res) >> if (arg_num >= spec->arg_cnt) >> return -ENOENT; >> >> - arg_spec = &spec->args[arg_num]; >> + *arg_spec = &spec->args[arg_num]; >> + >> + return 0; >> +} >> + >> +/* Returns the size in bytes of the #*arg_num* (zero-indexed) USDT argument. >> + * Returns negative error if argument is not found or arg_num is invalid. >> + */ >> +static __always_inline >> +int bpf_usdt_arg_size(struct pt_regs *ctx, __u64 arg_num) >> +{ >> + struct __bpf_usdt_arg_spec *arg_spec; >> + int err; >> + >> + err = __bpf_usdt_arg_spec(ctx, arg_num, &arg_spec); > > let's not extract this into a separate function. I don't particularly > like the out parameter, and no need to add another function that could > be used by users. There isn't much duplication, I think it's fine if > we just copy/paste few lines of code. Ok. I wasn't sure what is the tolerance level for copy-pasting in this context. > > pw-bot: cr > >> + if (err) >> + return err; >> + >> + /* arg_spec->arg_bitshift = 64 - arg_sz * 8 >> + * so: arg_sz = (64 - arg_spec->arg_bitshift) / 8 >> + * Do a bitshift instead of a division to avoid >> + * "unsupported signed division" error. >> + */ >> + return (64 - arg_spec->arg_bitshift) >> 3; > > arg_bitshift is stored as char (which could be signed), so that's why > you were getting signed division, just cast to unsigned and keep > division: > > return (64 - (unsigned)arg_spec->arg_bitshift) / 8; I haven't realized that. Thanks. > >> +} >> + >> +/* Fetch USDT argument #*arg_num* (zero-indexed) and put its value into *res. >> + * Returns 0 on success; negative error, otherwise. >> + * On error *res is guaranteed to be set to zero. >> + */ >> +__weak __hidden >> +int bpf_usdt_arg(struct pt_regs *ctx, __u64 arg_num, long *res) >> +{ >> + struct __bpf_usdt_arg_spec *arg_spec; >> + unsigned long val; >> + int err; >> + >> + *res = 0; >> + >> + err = __bpf_usdt_arg_spec(ctx, arg_num, &arg_spec); >> + if (err) >> + return err; >> + >> switch (arg_spec->arg_type) { >> case BPF_USDT_ARG_CONST: >> /* Arg is just a constant ("-4@$-9" in USDT arg spec). >> -- >> 2.48.1 >> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: implement bpf_usdt_arg_size BPF function 2025-02-24 22:05 ` [PATCH bpf-next 1/2] libbpf: implement bpf_usdt_arg_size BPF function Andrii Nakryiko 2025-02-24 22:21 ` Ihor Solodrai @ 2025-02-24 23:14 ` Ihor Solodrai 2025-02-24 23:25 ` Andrii Nakryiko 1 sibling, 1 reply; 7+ messages in thread From: Ihor Solodrai @ 2025-02-24 23:14 UTC (permalink / raw) To: Andrii Nakryiko; +Cc: bpf, andrii, ast, daniel, eddyz87, mykolal, kernel-team On 2/24/25 2:05 PM, Andrii Nakryiko wrote: > [...] > > arg_bitshift is stored as char (which could be signed), so that's why > you were getting signed division, just cast to unsigned and keep > division: > > return (64 - (unsigned)arg_spec->arg_bitshift) / 8; As it turns out, this doesn't work either. Presumably because (64 - (u8)x) is still a signed int. This works, however: return (unsigned char)(64 - arg_spec->arg_bitshift) / 8; > >> [...] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: implement bpf_usdt_arg_size BPF function 2025-02-24 23:14 ` Ihor Solodrai @ 2025-02-24 23:25 ` Andrii Nakryiko 2025-02-24 23:38 ` Ihor Solodrai 0 siblings, 1 reply; 7+ messages in thread From: Andrii Nakryiko @ 2025-02-24 23:25 UTC (permalink / raw) To: Ihor Solodrai; +Cc: bpf, andrii, ast, daniel, eddyz87, mykolal, kernel-team On Mon, Feb 24, 2025 at 3:15 PM Ihor Solodrai <ihor.solodrai@linux.dev> wrote: > > On 2/24/25 2:05 PM, Andrii Nakryiko wrote: > > > [...] > > > > arg_bitshift is stored as char (which could be signed), so that's why > > you were getting signed division, just cast to unsigned and keep > > division: > > > > return (64 - (unsigned)arg_spec->arg_bitshift) / 8; > > As it turns out, this doesn't work either. Presumably because > (64 - (u8)x) is still a signed int. hm... ok, surprising, but fine > > This works, however: > > return (unsigned char)(64 - arg_spec->arg_bitshift) / 8; > nit: just unsigned (int), BPF doesn't have single-byte division anyways, so it will be upconverted to 32-bit (or 64-bit for noalu32). So let's be bold and use 32-bits here ;) > > > >> [...] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: implement bpf_usdt_arg_size BPF function 2025-02-24 23:25 ` Andrii Nakryiko @ 2025-02-24 23:38 ` Ihor Solodrai 0 siblings, 0 replies; 7+ messages in thread From: Ihor Solodrai @ 2025-02-24 23:38 UTC (permalink / raw) To: Andrii Nakryiko; +Cc: bpf, andrii, ast, daniel, eddyz87, mykolal, kernel-team On 2/24/25 3:25 PM, Andrii Nakryiko wrote: > On Mon, Feb 24, 2025 at 3:15 PM Ihor Solodrai <ihor.solodrai@linux.dev> wrote: >> >> On 2/24/25 2:05 PM, Andrii Nakryiko wrote: >> >>> [...] >>> >>> arg_bitshift is stored as char (which could be signed), so that's why >>> you were getting signed division, just cast to unsigned and keep >>> division: >>> >>> return (64 - (unsigned)arg_spec->arg_bitshift) / 8; >> >> As it turns out, this doesn't work either. Presumably because >> (64 - (u8)x) is still a signed int. > > hm... ok, surprising, but fine > >> >> This works, however: >> >> return (unsigned char)(64 - arg_spec->arg_bitshift) / 8; >> > > > nit: just unsigned (int), BPF doesn't have single-byte division > anyways, so it will be upconverted to 32-bit (or 64-bit for noalu32). > So let's be bold and use 32-bits here ;) oh I specified a type because: $ ./scripts/checkpatch.pl /tmpfs/0001-libbpf-implement-bpf_usdt_arg_size-BPF-function.patch WARNING: Prefer 'unsigned int' to bare use of 'unsigned' #56: FILE: tools/lib/bpf/usdt.bpf.h:140: + return (64 - (unsigned)arg_spec->arg_bitshift) / 8; (unsigned int) it is then > >>> >>>> [...] ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-02-24 23:38 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-02-20 21:59 [PATCH bpf-next 1/2] libbpf: implement bpf_usdt_arg_size BPF function Ihor Solodrai 2025-02-20 21:59 ` [PATCH bpf-next 2/2] selftests/bpf: test bpf_usdt_arg_size() function Ihor Solodrai 2025-02-24 22:05 ` [PATCH bpf-next 1/2] libbpf: implement bpf_usdt_arg_size BPF function Andrii Nakryiko 2025-02-24 22:21 ` Ihor Solodrai 2025-02-24 23:14 ` Ihor Solodrai 2025-02-24 23:25 ` Andrii Nakryiko 2025-02-24 23:38 ` Ihor Solodrai
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.