Re: [PATCH bpf-next] selftests/bpf: retry bpf_map_update_elem() when E2BIG is returned

[Matt Bobrowski <mattbobrowski@google.com>]

On Wed, Nov 12, 2025 at 05:00:50PM -0800, Song Liu wrote:
> On Wed, Nov 12, 2025 at 12:32 AM Matt Bobrowski
> <mattbobrowski@google.com> wrote:
> >
> > Executing the test_maps binary on platforms with extremely high core
> > counts may cause intermittent assertion failures in
> > test_update_delete() (called via test_map_parallel()). This can occur
> > because bpf_map_update_elem() under some circumstances (specifically
> > in this case while performing bpf_map_update_elem() with BPF_NOEXIST
> > on a BPF_MAP_TYPE_HASH with its map_flags set to BPF_F_NO_PREALLOC)
> > can return an E2BIG error code i.e.
> >
> > error -7 7
> > tools/testing/selftests/bpf/test_maps.c:#: void test_update_delete(unsigned int, void *): Assertion `err == 0' failed.
> > tools/testing/selftests/bpf/test_maps.c:#: void
> > __run_parallel(unsigned int, void (*)(unsigned int, void *), void *): Assertion `status == 0' failed.
> >
> > As it turns out, is_map_full() which is called from alloc_htab_elem()
> > can take on a conservative approach when htab->use_percpu_counter is
> > true (which is the case here because the percpu_counter is used when a
> > BPF_MAP_TYPE_HASH is created with its map_flags set to
> > BPF_F_NO_PREALLOC). This conservative approach approach prioritizes
> 
> s/approach approach/approach
> 
> AFAICT checkpatch.pl also warns double "approach", as well as line exceed
> 75 character above.

ACK. Will respin with nits addressed at some point today.

> > preventing over-allocation and potential issues that could arise from
> > possibly exceeding htab->map.max_entries in highly concurrent
> > environments, even if it means slightly under-utilizing the htab map's
> > capacity.
> >
> > Given that bpf_map_update_elem() from test_update_delete() can return
> > E2BIG, update can_retry() such that it also accounts for the E2BIG
> > error code (specifically only when running with map_flags being set to
> > BPF_F_NO_PREALLOC). The retry loop will allow the global count
> > belonging to the percpu_counter to become synchronized and better
> > reflect the current htab map's capacity.
> >
> > Signed-off-by: Matt Bobrowski <mattbobrowski@google.com>
> 
> Other than the nitpick above, this looks good to me.
> 
> Acked-by: Song Liu <song@kernel.org>
> 
> 
> > ---
> >  tools/testing/selftests/bpf/test_maps.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c
> > index 3fae9ce46ca9..ccc5acd55ff9 100644
> > --- a/tools/testing/selftests/bpf/test_maps.c
> > +++ b/tools/testing/selftests/bpf/test_maps.c
> > @@ -1399,7 +1399,8 @@ static void test_map_stress(void)
> >  static bool can_retry(int err)
> >  {
> >         return (err == EAGAIN || err == EBUSY ||
> > -               (err == ENOMEM && map_opts.map_flags == BPF_F_NO_PREALLOC));
> > +               ((err == ENOMEM || err == E2BIG) &&
> > +                map_opts.map_flags == BPF_F_NO_PREALLOC));
> >  }
> >
> >  int map_update_retriable(int map_fd, const void *key, const void *value, int flags, int attempts,
> > --
> > 2.51.2.1041.gc1ab5b90ca-goog
> >