Re: [dpdk-dev] [PATCH v8 03/12] bpf: allow self-xor operation

[Ananyev, Konstantin]

> When doing BPF filter program conversion, a common way
> to zero a register in single instruction is:
>      xor r7,r7
> The BPF validator would not allow this because the value of
> r7 was undefined. But after this operation it always zero.
> 
> Signed-off-by: Stephen Hemminger <step...@networkplumber.org>
> ---
>  lib/bpf/bpf_validate.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/bpf/bpf_validate.c b/lib/bpf/bpf_validate.c
> index 7b1291b382e9..7647a7454dc2 100644
> --- a/lib/bpf/bpf_validate.c
> +++ b/lib/bpf/bpf_validate.c
> @@ -661,8 +661,12 @@ eval_alu(struct bpf_verifier *bvf, const struct 
> ebpf_insn *ins)
> 
>       op = BPF_OP(ins->code);
> 
> -     err = eval_defined((op != EBPF_MOV) ? rd : NULL,
> -                     (op != BPF_NEG) ? &rs : NULL);
> +     /* Allow self-xor as way to zero register */
> +     if (op == BPF_XOR && ins->src_reg == ins->dst_reg)
> +             err = NULL;
> +     else
> +             err = eval_defined((op != EBPF_MOV) ? rd : NULL,
> +                                (op != BPF_NEG) ? &rs : NULL);

Two things:
- We probably need to check that this is instruction with source register (not 
imm value).
- rd value is not evaluated to zero, while it probably should
  (will help evaluator to better predict further values) 

So might be better to do something like:

/* Allow self-xor as way to zero register */
        if (op == BPF_XOR && BPF_SRC(ins->code) == BPF_X &&
                        ins->src_reg == ins->dst_reg) {
                eval_fill_imm(&rs, UINT64_MAX, 0);
                eval_fill_imm(rd, UINT64_MAX, 0);
        }

        err = eval_defined((op != EBPF_MOV) ? rd : NULL,
                           (op != BPF_NEG) ? &rs : NULL);
        if (err != NULL)
                return err;

...

Another thing - shouldn't that patch be treated like a fix (cc to stable, etc.)?

>       if (err != NULL)
>               return err;
> 
> --
> 2.30.2