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.
