* [PATCH bpf-next] bpf: Minor improvements for bpf_cmp.
@ 2024-01-12 22:01 Alexei Starovoitov
2024-01-13 1:24 ` Yonghong Song
2024-01-15 17:10 ` patchwork-bot+netdevbpf
0 siblings, 2 replies; 3+ messages in thread
From: Alexei Starovoitov @ 2024-01-12 22:01 UTC (permalink / raw)
To: bpf; +Cc: daniel, andrii, martin.lau, kernel-team
From: Alexei Starovoitov <ast@kernel.org>
Few minor improvements for bpf_cmp() macro:
. reduce number of args in __bpf_cmp()
. rename NOFLIP to UNLIKELY
. add a comment about 64-bit truncation in "i" constraint
. use "ri" constraint for sizeof(rhs) <= 4
. improve error message for bpf_cmp_likely()
Before:
progs/iters_task_vma.c:31:7: error: variable 'ret' is uninitialized when used here [-Werror,-Wuninitialized]
31 | if (bpf_cmp_likely(seen, <==, 1000))
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../bpf/bpf_experimental.h:325:3: note: expanded from macro 'bpf_cmp_likely'
325 | ret;
| ^~~
progs/iters_task_vma.c:31:7: note: variable 'ret' is declared here
../bpf/bpf_experimental.h:310:3: note: expanded from macro 'bpf_cmp_likely'
310 | bool ret;
| ^
After:
progs/iters_task_vma.c:31:7: error: invalid operand for instruction
31 | if (bpf_cmp_likely(seen, <==, 1000))
| ^
../bpf/bpf_experimental.h:324:17: note: expanded from macro 'bpf_cmp_likely'
324 | asm volatile("r0 " #OP " invalid compare");
| ^
<inline asm>:1:5: note: instantiated into assembly here
1 | r0 <== invalid compare
| ^
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
.../testing/selftests/bpf/bpf_experimental.h | 21 +++++++++++--------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/tools/testing/selftests/bpf/bpf_experimental.h b/tools/testing/selftests/bpf/bpf_experimental.h
index f44875f8b367..0d749006d107 100644
--- a/tools/testing/selftests/bpf/bpf_experimental.h
+++ b/tools/testing/selftests/bpf/bpf_experimental.h
@@ -260,11 +260,11 @@ extern void bpf_throw(u64 cookie) __ksym;
#define __is_signed_type(type) (((type)(-1)) < (type)1)
-#define __bpf_cmp(LHS, OP, SIGN, PRED, RHS, DEFAULT) \
+#define __bpf_cmp(LHS, OP, PRED, RHS, DEFAULT) \
({ \
__label__ l_true; \
bool ret = DEFAULT; \
- asm volatile goto("if %[lhs] " SIGN #OP " %[rhs] goto %l[l_true]" \
+ asm volatile goto("if %[lhs] " OP " %[rhs] goto %l[l_true]" \
:: [lhs] "r"((short)LHS), [rhs] PRED (RHS) :: l_true); \
ret = !DEFAULT; \
l_true: \
@@ -276,7 +276,7 @@ l_true: \
* __lhs OP __rhs below will catch the mistake.
* Be aware that we check only __lhs to figure out the sign of compare.
*/
-#define _bpf_cmp(LHS, OP, RHS, NOFLIP) \
+#define _bpf_cmp(LHS, OP, RHS, UNLIKELY) \
({ \
typeof(LHS) __lhs = (LHS); \
typeof(RHS) __rhs = (RHS); \
@@ -285,14 +285,17 @@ l_true: \
(void)(__lhs OP __rhs); \
if (__cmp_cannot_be_signed(OP) || !__is_signed_type(typeof(__lhs))) { \
if (sizeof(__rhs) == 8) \
- ret = __bpf_cmp(__lhs, OP, "", "r", __rhs, NOFLIP); \
+ /* "i" will truncate 64-bit constant into s32, \
+ * so we have to use extra register via "r". \
+ */ \
+ ret = __bpf_cmp(__lhs, #OP, "r", __rhs, UNLIKELY); \
else \
- ret = __bpf_cmp(__lhs, OP, "", "i", __rhs, NOFLIP); \
+ ret = __bpf_cmp(__lhs, #OP, "ri", __rhs, UNLIKELY); \
} else { \
if (sizeof(__rhs) == 8) \
- ret = __bpf_cmp(__lhs, OP, "s", "r", __rhs, NOFLIP); \
+ ret = __bpf_cmp(__lhs, "s"#OP, "r", __rhs, UNLIKELY); \
else \
- ret = __bpf_cmp(__lhs, OP, "s", "i", __rhs, NOFLIP); \
+ ret = __bpf_cmp(__lhs, "s"#OP, "ri", __rhs, UNLIKELY); \
} \
ret; \
})
@@ -304,7 +307,7 @@ l_true: \
#ifndef bpf_cmp_likely
#define bpf_cmp_likely(LHS, OP, RHS) \
({ \
- bool ret; \
+ bool ret = 0; \
if (__builtin_strcmp(#OP, "==") == 0) \
ret = _bpf_cmp(LHS, !=, RHS, false); \
else if (__builtin_strcmp(#OP, "!=") == 0) \
@@ -318,7 +321,7 @@ l_true: \
else if (__builtin_strcmp(#OP, ">=") == 0) \
ret = _bpf_cmp(LHS, <, RHS, false); \
else \
- (void) "bug"; \
+ asm volatile("r0 " #OP " invalid compare"); \
ret; \
})
#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH bpf-next] bpf: Minor improvements for bpf_cmp.
2024-01-12 22:01 [PATCH bpf-next] bpf: Minor improvements for bpf_cmp Alexei Starovoitov
@ 2024-01-13 1:24 ` Yonghong Song
2024-01-15 17:10 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: Yonghong Song @ 2024-01-13 1:24 UTC (permalink / raw)
To: Alexei Starovoitov, bpf; +Cc: daniel, andrii, martin.lau, kernel-team
On 1/12/24 2:01 PM, Alexei Starovoitov wrote:
> From: Alexei Starovoitov <ast@kernel.org>
>
> Few minor improvements for bpf_cmp() macro:
> . reduce number of args in __bpf_cmp()
> . rename NOFLIP to UNLIKELY
> . add a comment about 64-bit truncation in "i" constraint
> . use "ri" constraint for sizeof(rhs) <= 4
> . improve error message for bpf_cmp_likely()
>
> Before:
> progs/iters_task_vma.c:31:7: error: variable 'ret' is uninitialized when used here [-Werror,-Wuninitialized]
> 31 | if (bpf_cmp_likely(seen, <==, 1000))
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ../bpf/bpf_experimental.h:325:3: note: expanded from macro 'bpf_cmp_likely'
> 325 | ret;
> | ^~~
> progs/iters_task_vma.c:31:7: note: variable 'ret' is declared here
> ../bpf/bpf_experimental.h:310:3: note: expanded from macro 'bpf_cmp_likely'
> 310 | bool ret;
> | ^
>
> After:
> progs/iters_task_vma.c:31:7: error: invalid operand for instruction
> 31 | if (bpf_cmp_likely(seen, <==, 1000))
> | ^
> ../bpf/bpf_experimental.h:324:17: note: expanded from macro 'bpf_cmp_likely'
> 324 | asm volatile("r0 " #OP " invalid compare");
> | ^
> <inline asm>:1:5: note: instantiated into assembly here
> 1 | r0 <== invalid compare
> | ^
>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Yonghong Song <yonghong.song@linux.dev>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH bpf-next] bpf: Minor improvements for bpf_cmp.
2024-01-12 22:01 [PATCH bpf-next] bpf: Minor improvements for bpf_cmp Alexei Starovoitov
2024-01-13 1:24 ` Yonghong Song
@ 2024-01-15 17:10 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-01-15 17:10 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: bpf, daniel, andrii, martin.lau, kernel-team
Hello:
This patch was applied to bpf/bpf-next.git (master)
by Daniel Borkmann <daniel@iogearbox.net>:
On Fri, 12 Jan 2024 14:01:34 -0800 you wrote:
> From: Alexei Starovoitov <ast@kernel.org>
>
> Few minor improvements for bpf_cmp() macro:
> . reduce number of args in __bpf_cmp()
> . rename NOFLIP to UNLIKELY
> . add a comment about 64-bit truncation in "i" constraint
> . use "ri" constraint for sizeof(rhs) <= 4
> . improve error message for bpf_cmp_likely()
>
> [...]
Here is the summary with links:
- [bpf-next] bpf: Minor improvements for bpf_cmp.
https://git.kernel.org/bpf/bpf-next/c/7f0aa0dd07e4
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-01-15 17:10 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-12 22:01 [PATCH bpf-next] bpf: Minor improvements for bpf_cmp Alexei Starovoitov
2024-01-13 1:24 ` Yonghong Song
2024-01-15 17:10 ` patchwork-bot+netdevbpf
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).