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


Reply via email to