Re: [PATCH bpf-next v5 3/5] bpf: Inline calls to bpf_loop when callback is known

[Song Liu <song@xxxxxxxxxx>]

On Sat, Jun 11, 2022 at 4:42 AM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote:
>

[...]

> Signed-off-by: Eduard Zingerman <eddyz87@xxxxxxxxx>
> ---

[...]

>
> +static struct bpf_insn_aux_data *cur_aux(struct bpf_verifier_env *env)
> +{
> +       return &env->insn_aux_data[env->insn_idx];
> +}
> +
> +static void update_loop_inline_state(struct bpf_verifier_env *env, u32 subprogno)
> +{
> +       struct bpf_loop_inline_state *state = &cur_aux(env)->loop_inline_state;
> +       struct bpf_reg_state *regs = cur_regs(env);
> +       struct bpf_reg_state *flags_reg = &regs[BPF_REG_4];
> +       int flags_is_zero =
> +               register_is_const(flags_reg) && flags_reg->var_off.value == 0;

How about we add helper

static bool flag_is_zero(struct bpf_verifier_env *env, int regno)
{
        struct bpf_loop_inline_state *state = &cur_aux(env)->loop_inline_state;
        struct bpf_reg_state *regs = cur_regs(env);
        struct bpf_reg_state *flags_reg = &regs[regno];

        return register_is_const(flags_reg) && flags_reg->var_off.value == 0;
}

and ...

> +
> +       if (!state->initialized) {
> +               state->initialized = 1;
> +               state->fit_for_inline = flags_is_zero;

       state->fit_for_inline = flag_is_zero(env, BPF_REG_4);

and ...

> +               state->callback_subprogno = subprogno;
> +               return;
> +       }
> +
> +       if (!state->fit_for_inline)
> +               return;
> +
> +       state->fit_for_inline =
> +               flags_is_zero &&

flag_is_zero(env, BPF_REG_4);

This would avoid calculating flag_is_zero for !state->fix_for_inline case.


> +               state->callback_subprogno == subprogno;
> +}
> +
>  static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn,

[...]

>  static void free_states(struct bpf_verifier_env *env)
>  {
>         struct bpf_verifier_state_list *sl, *sln;
> @@ -15030,6 +15188,9 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr)
>         if (ret == 0)
>                 ret = check_max_stack_depth(env);
>
> +       if (ret == 0)
> +               optimize_bpf_loop(env);
> +
>         /* instruction rewrites happen after this point */

nit: I guess optimize_bpf_loop() is part of "instruction rewrite", so
this comment
should be above optimize_bpf_loop()?

>         if (is_priv) {
>                 if (ret == 0)
> --
> 2.25.1
>