* [PATCH bpf-next] selftests/bpf: fix also no-alu32 strobemeta selftest
@ 2021-11-01 23:01 Andrii Nakryiko
2021-11-01 23:17 ` Yonghong Song
0 siblings, 1 reply; 2+ messages in thread
From: Andrii Nakryiko @ 2021-11-01 23:01 UTC (permalink / raw)
To: bpf, ast, daniel; +Cc: andrii, kernel-team
Previous fix aded bpf_clamp_umax() helper use to re-validate boundaries.
While that works correctly, it introduces more branches, which blows up
past 1 million instructions in no-alu32 variant of strobemeta selftests.
Switching len variable from u32 to u64 also fixes the issue and reduces
the number of validated instructions, so use that instead. Fix this
patch and bpf_clamp_umax() removed, both alu32 and no-alu32 selftests
pass.
Fixes: 0133c20480b1 ("selftests/bpf: Fix strobemeta selftest regression")
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
tools/testing/selftests/bpf/progs/strobemeta.h | 15 ++-------------
1 file changed, 2 insertions(+), 13 deletions(-)
diff --git a/tools/testing/selftests/bpf/progs/strobemeta.h b/tools/testing/selftests/bpf/progs/strobemeta.h
index 3687ea755ab5..60c93aee2f4a 100644
--- a/tools/testing/selftests/bpf/progs/strobemeta.h
+++ b/tools/testing/selftests/bpf/progs/strobemeta.h
@@ -10,14 +10,6 @@
#include <linux/types.h>
#include <bpf/bpf_helpers.h>
-#define bpf_clamp_umax(VAR, UMAX) \
- asm volatile ( \
- "if %0 <= %[max] goto +1\n" \
- "%0 = %[max]\n" \
- : "+r"(VAR) \
- : [max]"i"(UMAX) \
- )
-
typedef uint32_t pid_t;
struct task_struct {};
@@ -366,7 +358,7 @@ static __always_inline uint64_t read_str_var(struct strobemeta_cfg *cfg,
void *payload)
{
void *location;
- uint32_t len;
+ uint64_t len;
data->str_lens[idx] = 0;
location = calc_location(&cfg->str_locs[idx], tls_base);
@@ -398,7 +390,7 @@ static __always_inline void *read_map_var(struct strobemeta_cfg *cfg,
struct strobe_map_descr* descr = &data->map_descrs[idx];
struct strobe_map_raw map;
void *location;
- uint32_t len;
+ uint64_t len;
int i;
descr->tag_len = 0; /* presume no tag is set */
@@ -421,7 +413,6 @@ static __always_inline void *read_map_var(struct strobemeta_cfg *cfg,
len = bpf_probe_read_user_str(payload, STROBE_MAX_STR_LEN, map.tag);
if (len <= STROBE_MAX_STR_LEN) {
- bpf_clamp_umax(len, STROBE_MAX_STR_LEN);
descr->tag_len = len;
payload += len;
}
@@ -439,7 +430,6 @@ static __always_inline void *read_map_var(struct strobemeta_cfg *cfg,
len = bpf_probe_read_user_str(payload, STROBE_MAX_STR_LEN,
map.entries[i].key);
if (len <= STROBE_MAX_STR_LEN) {
- bpf_clamp_umax(len, STROBE_MAX_STR_LEN);
descr->key_lens[i] = len;
payload += len;
}
@@ -447,7 +437,6 @@ static __always_inline void *read_map_var(struct strobemeta_cfg *cfg,
len = bpf_probe_read_user_str(payload, STROBE_MAX_STR_LEN,
map.entries[i].val);
if (len <= STROBE_MAX_STR_LEN) {
- bpf_clamp_umax(len, STROBE_MAX_STR_LEN);
descr->val_lens[i] = len;
payload += len;
}
--
2.30.2
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: fix also no-alu32 strobemeta selftest
2021-11-01 23:01 [PATCH bpf-next] selftests/bpf: fix also no-alu32 strobemeta selftest Andrii Nakryiko
@ 2021-11-01 23:17 ` Yonghong Song
0 siblings, 0 replies; 2+ messages in thread
From: Yonghong Song @ 2021-11-01 23:17 UTC (permalink / raw)
To: Andrii Nakryiko, bpf, ast, daniel; +Cc: kernel-team
On 11/1/21 4:01 PM, Andrii Nakryiko wrote:
> Previous fix aded bpf_clamp_umax() helper use to re-validate boundaries.
> While that works correctly, it introduces more branches, which blows up
> past 1 million instructions in no-alu32 variant of strobemeta selftests.
>
> Switching len variable from u32 to u64 also fixes the issue and reduces
> the number of validated instructions, so use that instead. Fix this
> patch and bpf_clamp_umax() removed, both alu32 and no-alu32 selftests
> pass.
>
> Fixes: 0133c20480b1 ("selftests/bpf: Fix strobemeta selftest regression")
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
This indeed is a better approach for workaround. It disabled offending
llvm optimization since "truncation" operation (64->32bit) is gone. BTW,
llvm patch https://reviews.llvm.org/D112938 is also landed in the
hope to fix the same issue to please the verifier.
Acked-by: Yonghong Song <yhs@fb.com>
> ---
> tools/testing/selftests/bpf/progs/strobemeta.h | 15 ++-------------
> 1 file changed, 2 insertions(+), 13 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/progs/strobemeta.h b/tools/testing/selftests/bpf/progs/strobemeta.h
> index 3687ea755ab5..60c93aee2f4a 100644
> --- a/tools/testing/selftests/bpf/progs/strobemeta.h
> +++ b/tools/testing/selftests/bpf/progs/strobemeta.h
> @@ -10,14 +10,6 @@
> #include <linux/types.h>
> #include <bpf/bpf_helpers.h>
>
> -#define bpf_clamp_umax(VAR, UMAX) \
> - asm volatile ( \
> - "if %0 <= %[max] goto +1\n" \
> - "%0 = %[max]\n" \
> - : "+r"(VAR) \
> - : [max]"i"(UMAX) \
> - )
> -
> typedef uint32_t pid_t;
> struct task_struct {};
>
> @@ -366,7 +358,7 @@ static __always_inline uint64_t read_str_var(struct strobemeta_cfg *cfg,
> void *payload)
> {
> void *location;
> - uint32_t len;
> + uint64_t len;
>
> data->str_lens[idx] = 0;
> location = calc_location(&cfg->str_locs[idx], tls_base);
> @@ -398,7 +390,7 @@ static __always_inline void *read_map_var(struct strobemeta_cfg *cfg,
> struct strobe_map_descr* descr = &data->map_descrs[idx];
> struct strobe_map_raw map;
> void *location;
> - uint32_t len;
> + uint64_t len;
> int i;
>
> descr->tag_len = 0; /* presume no tag is set */
> @@ -421,7 +413,6 @@ static __always_inline void *read_map_var(struct strobemeta_cfg *cfg,
>
> len = bpf_probe_read_user_str(payload, STROBE_MAX_STR_LEN, map.tag);
> if (len <= STROBE_MAX_STR_LEN) {
> - bpf_clamp_umax(len, STROBE_MAX_STR_LEN);
> descr->tag_len = len;
> payload += len;
> }
> @@ -439,7 +430,6 @@ static __always_inline void *read_map_var(struct strobemeta_cfg *cfg,
> len = bpf_probe_read_user_str(payload, STROBE_MAX_STR_LEN,
> map.entries[i].key);
> if (len <= STROBE_MAX_STR_LEN) {
> - bpf_clamp_umax(len, STROBE_MAX_STR_LEN);
> descr->key_lens[i] = len;
> payload += len;
> }
> @@ -447,7 +437,6 @@ static __always_inline void *read_map_var(struct strobemeta_cfg *cfg,
> len = bpf_probe_read_user_str(payload, STROBE_MAX_STR_LEN,
> map.entries[i].val);
> if (len <= STROBE_MAX_STR_LEN) {
> - bpf_clamp_umax(len, STROBE_MAX_STR_LEN);
> descr->val_lens[i] = len;
> payload += len;
> }
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-11-01 23:17 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-01 23:01 [PATCH bpf-next] selftests/bpf: fix also no-alu32 strobemeta selftest Andrii Nakryiko
2021-11-01 23:17 ` Yonghong Song
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).