On 16/6/26 05:29, Alexei Starovoitov wrote:
> On Mon Jun 15, 2026 at 8:26 AM PDT, Leon Hwang wrote:
[...]
>> + ASSERT_EQ(ld_imm64_xlated[0].code, ld_imm64_raw[0].code, "ld_imm64
>> opcode");
>> + ASSERT_TRUE(ld_imm64_xlated[0].dst_reg == ld_imm64_raw[0].dst_reg,
>> "ld_imm64 dst_reg");
>> + /*
>> + * The xlated instruction has the map ID in imm and the offset
>> + * in the next instruction's imm. The raw instruction just has
>> + * the offset in its imm.
>> + */
>> + ASSERT_EQ(ld_imm64_xlated[1].imm, ld_imm64_to_u64(ld_imm64_raw),
>> "ld_imm64 off");
>> +
>> + mov64_percpu_reg = BPF_MOV64_PERCPU_REG(ld_imm64_raw[0].dst_reg,
>> ld_imm64_raw[0].dst_reg);
>> + ASSERT_MEMEQ(&insns[idx + 2], &mov64_percpu_reg, insn_sz,
>> "mov64_percpu_reg");
>
> If the point of the test was to check that
> percpu_array_map_direct_value_meta()
> computes 'off' correctly then it failed to achieve that goal.
It was to check the 'off' and the mov64_percpu_reg insn.
To avoid relying on the insns loaded from ELF obj, use raw insns to
check the 'off' with a percpu_array map:
struct bpf_insn raw_insns[] = {
BPF_LD_MAP_VALUE(BPF_REG_1, 0, 0),
BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1, 0),
BPF_EXIT_INSN(),
};
skel = test_global_percpu_data__open_and_load();
exp_off = offsetof(struct test_global_percpu_data__percpu,
struct_data.nums[6]);
raw_insns[0].imm = bpf_map__fd(skel->maps.percpu);
raw_insns[1].imm = exp_off;
prog_fd = bpf_prog_load(...);
err = get_xlated_program(prog_fd, &insns, &cnt);
idx = find_percpu_ld_imm64(insns, cnt);
ASSERT_EQ(insns[idx + 1].imm, exp_off, "exp_off");
Would this achieve that goal?
> It checks that map ID is correct which is a pointless test.
> Either make it a real test or drop this patch.
>
No map ID check here.
Thanks,
Leon