On 1/20/25 7:29 AM, Daniel Borkmann wrote:
On 1/18/25 8:20 PM, Yonghong Song wrote:
Since 'may_goto 0' insns are actually no-op, let us remove them.
Otherwise, verifier will generate code like
/* r10 - 8 stores the implicit loop count */
r11 = *(u64 *)(r10 -8)
if r11 == 0x0 goto pc+2
r11 -= 1
*(u64 *)(r10 -8) = r11
which is the pure overhead.
The following code patterns (from the previous commit) are also
handled:
may_goto 2
may_goto 1
may_goto 0
With this commit, the above three 'may_goto' insns are all
eliminated.
Signed-off-by: Yonghong Song <yonghong.song@xxxxxxxxx>
---
kernel/bpf/verifier.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 963dfda81c06..784547aa40a8 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -20187,20 +20187,25 @@ static const struct bpf_insn NOP =
BPF_JMP_IMM(BPF_JA, 0, 0, 0);
static int opt_remove_nops(struct bpf_verifier_env *env)
{
+ const struct bpf_insn may_goto_0 = BPF_RAW_INSN(BPF_JMP |
BPF_JCOND, 0, 0, 0, 0);
const struct bpf_insn ja = NOP;
struct bpf_insn *insn = env->prog->insnsi;
int insn_cnt = env->prog->len;
+ bool is_may_goto_0, is_ja;
int i, err;
for (i = 0; i < insn_cnt; i++) {
- if (memcmp(&insn[i], &ja, sizeof(ja)))
+ is_may_goto_0 = !memcmp(&insn[i], &may_goto_0,
sizeof(may_goto_0));
+ is_ja = !memcmp(&insn[i], &ja, sizeof(ja));
+
+ if (!is_may_goto_0 && !is_ja)
continue;
Why the extra may_goto_0 stack var?
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 245f1f3f1aec..16ba26295ec7 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -20185,16 +20185,19 @@ static int opt_remove_dead_code(struct
bpf_verifier_env *env)
}
static const struct bpf_insn NOP = BPF_JMP_IMM(BPF_JA, 0, 0, 0);
+static const struct bpf_insn MAY_GOTO_0 = BPF_RAW_INSN(BPF_JMP |
BPF_JCOND, 0, 0, 0, 0);
This actually is what I did initially. I changed to use the stack var because
NOP is used in other functions too while MAY_GOTO_0 is only used in
opt_remove_nops(). Certainly, using MAY_GOTO_0 as static variable works too.
static int opt_remove_nops(struct bpf_verifier_env *env)
{
- const struct bpf_insn ja = NOP;
struct bpf_insn *insn = env->prog->insnsi;
int insn_cnt = env->prog->len;
+ bool is_ja, is_may_goto_0;
int i, err;
for (i = 0; i < insn_cnt; i++) {
- if (memcmp(&insn[i], &ja, sizeof(ja)))
+ is_may_goto_0 = !memcmp(&insn[i], &MAY_GOTO_0,
sizeof(MAY_GOTO_0));
+ is_ja = !memcmp(&insn[i], &NOP, sizeof(NOP));
+ if (!is_may_goto_0 && !is_ja)
continue;
err = verifier_remove_insns(env, i, 1);
if (err)
return err;
insn_cnt--;
- i--;
+ i -= (is_may_goto_0 && i > 0) ? 2 : 1;
Maybe add a comment for this logic?
Thanks Alexei for adding comments before merging!
Thanks,
Daniel
