Re: [PATCH bpf-next 2/2] selftests/bpf: Test bpf_for_each_map_elem on BPF_MAP_TYPE_HASH_OF_MAPS

[David Vernet]

On Mon, Jun 05, 2023 at 03:07:21PM -0700, Alexei Starovoitov wrote:
> On Mon, Jun 5, 2023 at 1:05 PM David Vernet <void@xxxxxxxxxxxxx> wrote:
> >
> > +
> > +static __u64 set_invoked(struct bpf_map *map, __u64 *key, __u64 *val, struct callback_ctx *ctx)
> > +{
> > +       struct bpf_map *inner_map;
> > +
> > +       ctx->invoked = true;
> > +       inner_map = bpf_map_lookup_elem(map, key);
> > +       if (!inner_map) {
> > +               ctx->failed = true;
> > +               return 1;
> > +       }
> 
> This doesn't look right. 'val' is not 'u64 *'.
> It probably should be 'struct bpf_map *',
> so lookup shouldn't be necessary.
> 
> map_set_for_each_callback_args() just sets it to PTR_TO_MAP_VALUE.
> It probably should be CONST_PTR_TO_MAP.
> Also for normal hash map for_each_elem 'val' is writeable.
> It shouldn't be in this case.
> So to cleanly support iterating hash_of_maps patch 1 needs more work.

Thanks for the review and pointing out these issues. This all makes
sense, I'll incorporate it into v2.