On Mon, Jan 3, 2022 at 10:09 AM Grant Seltzer Richman
<grantseltzer@xxxxxxxxx> wrote:
>
> On Mon, Dec 27, 2021 at 7:25 AM Hengqi Chen <hengqi.chen@xxxxxxxxx> wrote:
> >
> >
> >
> > On 2021/12/26 4:37 AM, grantseltzer wrote:
> > > From: Grant Seltzer <grantseltzer@xxxxxxxxx>
> > >
> > > This adds documentation for:
> > >
> > > - bpf_map_delete_batch()
> > > - bpf_map_lookup_batch()
> > > - bpf_map_lookup_and_delete_batch()
> > > - bpf_map_update_batch()
> > >
> > > Signed-off-by: Grant Seltzer <grantseltzer@xxxxxxxxx>
> > > ---
> > > tools/lib/bpf/bpf.c | 4 +-
> > > tools/lib/bpf/bpf.h | 112 +++++++++++++++++++++++++++++++++++++++++++-
> > > 2 files changed, 112 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> > > index 9b64eed2b003..25f3d6f85fe5 100644
> > > --- a/tools/lib/bpf/bpf.c
> > > +++ b/tools/lib/bpf/bpf.c
> > > @@ -691,7 +691,7 @@ static int bpf_map_batch_common(int cmd, int fd, void *in_batch,
> > > return libbpf_err_errno(ret);
> > > }
> > >
> > > -int bpf_map_delete_batch(int fd, void *keys, __u32 *count,
> > > +int bpf_map_delete_batch(int fd, const void *keys, __u32 *count,
> >
> > Maybe you should drop these const qualifier changes.
>
> Can you help me understand the benefit of using the const qualifier in
> this context? I added it at Andrii's suggestion without proper
> understanding. I understand that it will properly convey that the keys
> or values aren't output parameters like in other batch operation
> functions, I don't think it would change where the underlying data is
> stored, just the pointer variable.
>
> Is it worth it to have seperate 'common' functions for the sake of
> having a const qualifier?
> >
> > All batch operations use `bpf_map_batch_common`, which has the following signature:
> >
> > static int bpf_map_batch_common(int cmd, int fd, void *in_batch,
> > void *out_batch, void *keys, void *values,
> > __u32 *count,
> > const struct bpf_map_batch_opts *opts)
> >
> > Adding these const qualifiers causes the following error:
> >
> > bpf.c:698:15: error: passing argument 5 of ‘bpf_map_batch_common’ discards
> > ‘const’ qualifier from pointer target type [-Werror=discarded-qualifiers]
we can either forcefully cast to (void *) internally when calling
bpf_map_batch_common(), or just make bpf_map_batch_common() take const
void *. It's a bit misleading, but either way it just gets converted
to u64.
I think it's more important to have public API reflect the guarantees
correctly, so if public API specifies that something is `const void *`
that means that no one is overwriting the contents of that memory.
> >
> > > const struct bpf_map_batch_opts *opts)
> > > {
> > > return bpf_map_batch_common(BPF_MAP_DELETE_BATCH, fd, NULL,
> > > @@ -715,7 +715,7 @@ int bpf_map_lookup_and_delete_batch(int fd, void *in_batch, void *out_batch,
> > > count, opts);
> > > }
> > >
[...]
