On Wed, Apr 3, 2024 at 10:41 AM Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx> wrote: > > On Mon, Apr 1, 2024 at 7:13 PM Andrii Nakryiko <andrii@xxxxxxxxxx> wrote: > > > > Add a new BPF instruction for resolving per-CPU memory addresses. > > > > New instruction is a special form of BPF_ALU64 | BPF_MOV | BPF_DW, with > > insns->off set to BPF_ADDR_PERCPU (== -1). It resolves provided per-CPU offset > > to an absolute address where per-CPU data resides for "this" CPU. > > Fixed the typo s/BPF_DW/BPF_X/ while applying. Oops, thanks! > > I'm still a bit worried that there is no BUILD_BUG_ON BUILD_BUG_ON is problematic, because those gen_lookup callbacks are compiled on 32-bit arches just fine (just not used). I'm totally fine adding extra checks as a follow up. Let's just agree on some consistent approach her? See [0] for the verifier check I mentioned. I can a) add #ifdef guards around all current gen_lookup callbacks to only build them on 64-bit arches or b) add runtime checks inside get_lookup to return -EOPNOTSUPP if architecture isn't 64-bit. I don't particularly care which one, I care that it's done for all callbacks if we are doing this. Please let me know your preference. [0] https://github.com/torvalds/linux/blob/master/kernel/bpf/verifier.c#L19942-L19946 > for 32-bit archs while both array and htab perpu gen_lookup > assume sizeof(void*) == 8 in few places: > htab: > *insn++ = BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_0, 0); > > array: > *insn++ = BPF_ALU64_IMM(BPF_LSH, BPF_REG_0, 3); > *insn++ = BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_0, 0); > Worth following up or I'm overthinking this? I don't know, maybe? :) See above.
