On Mon, Jan 20, 2025 at 7:29 AM Daniel Borkmann <daniel@xxxxxxxxxxxxx> 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);
>
> 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?
Adjusted both while applying.
Thanks!
