On Wed, Jul 5, 2023 at 10:42 PM Anton Protopopov <aspsk@xxxxxxxxxxxxx> wrote:
>
> On Wed, Jul 05, 2023 at 06:26:25PM -0700, Alexei Starovoitov wrote:
> > On Wed, Jul 5, 2023 at 9:00 AM Anton Protopopov <aspsk@xxxxxxxxxxxxx> wrote:
> > >
> > > Previous commits populated the ->elem_count per-cpu pointer for hash maps.
> > > Check that this pointer is non-NULL in an existing map.
> > >
> > > Signed-off-by: Anton Protopopov <aspsk@xxxxxxxxxxxxx>
> > > ---
> > > tools/testing/selftests/bpf/progs/map_ptr_kern.c | 3 +++
> > > 1 file changed, 3 insertions(+)
> > >
> > > diff --git a/tools/testing/selftests/bpf/progs/map_ptr_kern.c b/tools/testing/selftests/bpf/progs/map_ptr_kern.c
> > > index db388f593d0a..d6e234a37ccb 100644
> > > --- a/tools/testing/selftests/bpf/progs/map_ptr_kern.c
> > > +++ b/tools/testing/selftests/bpf/progs/map_ptr_kern.c
> > > @@ -33,6 +33,7 @@ struct bpf_map {
> > > __u32 value_size;
> > > __u32 max_entries;
> > > __u32 id;
> > > + __s64 *elem_count;
> > > } __attribute__((preserve_access_index));
> > >
> > > static inline int check_bpf_map_fields(struct bpf_map *map, __u32 key_size,
> > > @@ -111,6 +112,8 @@ static inline int check_hash(void)
> > >
> > > VERIFY(check_default_noinline(&hash->map, map));
> > >
> > > + VERIFY(map->elem_count != NULL);
> > > +
> >
> > imo that's worse than no test.
> > Just use kfunc here and get the real count?
>
> Then, as I mentioned in the previous version, I will have to teach kfuncs to
> recognize const_ptr_to_map args just for the sake of this selftest, while we
> already testing all functionality in the new selftest for test_maps. So I would
> just omit this one. Or am I missing something?
Don't you want to do:
val = bpf_map_lookup_elem(map, ...);
cnt = bpf_map_sum_elem_count(map);
and that's the main use case ?
So teaching the verifier to understand that const_ptr_to_map matches
BTF 'struct bpf_map *' is essential ?
