Re: [PATCH bpf-next v4 2/8] bpf: Introduce global percpu data

["Alexei Starovoitov" <alexei.starovoitov@gmail.com>]

On Sun Apr 19, 2026 at 10:24 PM PDT, Leon Hwang wrote:
>
> int xdp_prog(struct xdp_md * ctx):
> ; cnt++;
>    0: (18) r6 = map[id:28][0]+0
>    2: (bf) r6 = &(void __percpu *)(r6)

well. that insn was inserted by the verifier and it shows up in xlated.
That was expected.
The point about 'bogus xlated' was about offset translation.
map_direct_value_meta() should recover proper insns[i + 1].imm = off;
In your example it's zero, so not an interesting test.


>    3: (61) r1 = *(u32 *)(r6 +0)
> ; cnt++;
>    4: (07) r1 += 1
> ; cnt++;
>    5: (63) *(u32 *)(r6 +0) = r1
> ; __u32 cpu = bpf_get_smp_processor_id();
>    6: (b7) r0 = -1280774092
>    7: (bf) r0 = &(void __percpu *)(r0)
>    8: (61) r0 = *(u32 *)(r0 +0)
> ; bpf_printk("cpu: %u, cnt: %u\n", cpu, cnt);
>    9: (61) r4 = *(u32 *)(r6 +0)
>   10: (18) r1 = map[id:30][0]+0
>   12: (b7) r2 = 18
>   13: (bf) r3 = r0
>   14: (85) call bpf_trace_printk#-129408
> ; return XDP_PASS;
>   15: (b7) r0 = 2
>   16: (95) exit
>
> The difference between these xlated insns is "r6 = &(void __percpu *)(r6)".
> This insn is for ".percpu", not for ".data".
>
>>>>
>>>> Ah, let me dive deeper.
>>>>
>>>
>>> As for the above changes, let me explain them using diff snippet.
>>>
>>> @@ -5808,6 +5808,8 @@ int bpf_map_direct_read(struct bpf_map *map, int
>>> off, int size, u64 *val,
>>>         u64 addr;
>>>         int err;
>>>
>>> +       if (map->map_type == BPF_MAP_TYPE_PERCPU_ARRAY)
>>> +               return -EINVAL;
>>>         err = map->ops->map_direct_value_addr(map, &addr, off);
>>>         if (err)
>>>                 return err;
>>>
>>> It is to guard percpu_array map against const_reg_xfer(). Instead of
>>> updating const_reg_xfer(), better to update bpf_map_direct_read(). WDYT?
>>
>> yeah and move map_type != BPF_MAP_TYPE_INSN_ARRAY check
>> into bpf_map_direct_read() as well.
>> To cleanup const_reg_xfer() a bit.
>
> Before sending the next revision, just confirm the change:
>
> 1. Move "map->map_type == BPF_MAP_TYPE_INSN_ARRAY" from const_reg_xfer()
>    to bpf_map_direct_read().
> 2. Keep "map->map_type != BPF_MAP_TYPE_INSN_ARRAY" in
>    check_mem_access(), because we should not propagate the error from
>    bpf_map_direct_read() for insn_array and percpu_array.
>
> Thanks,
> Leon
>
> ---
>
> --- a/kernel/bpf/const_fold.c
> +++ b/kernel/bpf/const_fold.c
> @@ -174,7 +181,6 @@ static void const_reg_xfer(struct bpf_verifier_env *env, struct const_arg_info *
>                 u64 val = 0;
>
>                 if (!bpf_map_is_rdonly(map) || !map->ops->map_direct_value_addr ||
> -                   map->map_type == BPF_MAP_TYPE_INSN_ARRAY ||

yes

>                     off < 0 || off + size > map->value_size ||
>                     bpf_map_direct_read(map, off, size, &val, is_ldsx)) {
>                         *dst = unknown;
>
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -5816,6 +5816,8 @@ int bpf_map_direct_read(struct bpf_map *map, int off, int size, u64 *val,
>         u64 addr;
>         int err;
>
> +       if (map->map_type == BPF_MAP_TYPE_INSN_ARRAY || map->map_type == BPF_MAP_TYPE_PERCPU_ARRAY)
> +               return -EINVAL;

yes

>         err = map->ops->map_direct_value_addr(map, &addr, off);
>         if (err)
>                 return err;
> @@ -6370,7 +6372,8 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
>                         if (tnum_is_const(reg->var_off) &&
>                             bpf_map_is_rdonly(map) &&
>                             map->ops->map_direct_value_addr &&
> -                           map->map_type != BPF_MAP_TYPE_INSN_ARRAY) {
> +                           map->map_type != BPF_MAP_TYPE_INSN_ARRAY &&
> +                           map->map_type != BPF_MAP_TYPE_PERCPU_ARRAY) {

why add BPF_MAP_TYPE_PERCPU_ARRAY here?

>                                 int map_off = off + reg->var_off.value;
>                                 u64 val = 0;