Re: [PATCH] bpf,x64: pad NOPs to make images converge more easily

On Fri, Dec 11, 2020 at 1:13 PM Daniel Borkmann [off-list ref] wrote:

quoted

quoted

+                       }
  emit_jmp:
                         if (is_imm8(jmp_offset)) {
+                               if (jmp_padding)
+                                       cnt += emit_nops(&prog, INSN_SZ_DIFF - 2);

Could you describe all possible numbers of bytes in padding?
Is it 0, 2, 4 ?
Would be good to add warn_on_once to make sure the number
of nops is expected.

quoted

quoted

  struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
  {
         struct bpf_binary_header *header = NULL;

@@ -1981,6 +1997,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
         struct jit_context ctx = {};
         bool tmp_blinded = false;
         bool extra_pass = false;
+       bool padding = prog->padded;

can this ever be true on assignment? I.e., can the program be jitted twice?

Yes, progs can be passed into the JIT twice, see also jit_subprogs(). In one of
the earlier patches it would still potentially change the image size a second
time which would break subprogs aka bpf2bpf calls.

Right. I think memorized padded flag shouldn't be in sticky bits
of the prog structure.
It's only needed between the last pass and extra pass for bpf2bpf calls.
I think it would be cleaner to keep it in struct x64_jit_data *jit_data.

As others have said the selftests are must have.
Especially for bpf2bpf calls where one subprog is padded.

Thanks!