On 12/22/19 10:18 PM, Andrii Nakryiko wrote: > Clang patch [0] enables emitting relocatable generic ALU/ALU64 instructions > (i.e, shifts and arithmetic operations), as well as generic load/store > instructions. The former ones are already supported by libbpf as is. This > patch adds further support for load/store instructions. Relocatable field > offset is encoded in BPF instruction's 16-bit offset section and are adjusted > by libbpf based on target kernel BTF. > > These Clang changes and corresponding libbpf changes allow for more succinct > generated BPF code by encoding relocatable field reads as a single > LD/ST/LDX/STX instruction. It also enables relocatable access to BPF context. > Previously, if context struct (e.g., __sk_buff) was accessed with CO-RE > relocations (e.g., due to preserve_access_index attribute), it would be > rejected by BPF verifier due to modified context pointer dereference. With > Clang patch, such context accesses are both relocatable and have a fixed > offset from the point of view of BPF verifier. > > [0] https://reviews.llvm.org/D71790 > > Signed-off-by: Andrii Nakryiko <andriin@xxxxxx> > --- > tools/lib/bpf/libbpf.c | 32 +++++++++++++++++++++++++++++--- > 1 file changed, 29 insertions(+), 3 deletions(-) > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > index 9576a90c5a1c..2dbc2204a02c 100644 > --- a/tools/lib/bpf/libbpf.c > +++ b/tools/lib/bpf/libbpf.c > @@ -18,6 +18,7 @@ > #include <stdarg.h> > #include <libgen.h> > #include <inttypes.h> > +#include <limits.h> > #include <string.h> > #include <unistd.h> > #include <endian.h> > @@ -3810,11 +3811,13 @@ static int bpf_core_reloc_insn(struct bpf_program *prog, > insn = &prog->insns[insn_idx]; > class = BPF_CLASS(insn->code); > > - if (class == BPF_ALU || class == BPF_ALU64) { > + switch (class) { > + case BPF_ALU: > + case BPF_ALU64: > if (BPF_SRC(insn->code) != BPF_K) > return -EINVAL; > if (!failed && validate && insn->imm != orig_val) { > - pr_warn("prog '%s': unexpected insn #%d value: got %u, exp %u -> %u\n", > + pr_warn("prog '%s': unexpected insn #%d (ALU/ALU64) value: got %u, exp %u -> %u\n", > bpf_program__title(prog, false), insn_idx, > insn->imm, orig_val, new_val); > return -EINVAL; > @@ -3824,7 +3827,30 @@ static int bpf_core_reloc_insn(struct bpf_program *prog, > pr_debug("prog '%s': patched insn #%d (ALU/ALU64)%s imm %u -> %u\n", > bpf_program__title(prog, false), insn_idx, > failed ? " w/ failed reloc" : "", orig_val, new_val); > - } else { > + break; > + case BPF_LD: Maybe we should remove BPF_LD here? BPF_LD is used for ld_imm64, ld_abs and ld_ind, where the insn->off = 0 and not really used. > + case BPF_LDX: > + case BPF_ST: > + case BPF_STX: > + if (!failed && validate && insn->off != orig_val) { > + pr_warn("prog '%s': unexpected insn #%d (LD/LDX/ST/STX) value: got %u, exp %u -> %u\n", > + bpf_program__title(prog, false), insn_idx, > + insn->off, orig_val, new_val); > + return -EINVAL; > + } > + if (new_val > SHRT_MAX) { > + pr_warn("prog '%s': insn #%d (LD/LDX/ST/STX) value too big: %u\n", > + bpf_program__title(prog, false), insn_idx, > + new_val); > + return -ERANGE; > + } > + orig_val = insn->off; > + insn->off = new_val; > + pr_debug("prog '%s': patched insn #%d (LD/LDX/ST/STX)%s off %u -> %u\n", > + bpf_program__title(prog, false), insn_idx, > + failed ? " w/ failed reloc" : "", orig_val, new_val); > + break; > + default: > pr_warn("prog '%s': trying to relocate unrecognized insn #%d, code:%x, src:%x, dst:%x, off:%x, imm:%x\n", > bpf_program__title(prog, false), > insn_idx, insn->code, insn->src_reg, insn->dst_reg, >