[bpf-next,v2] libbpf: Add documentation for bpf_map batch operations

Commit Message

Grant Seltzer Richman Dec. 25, 2021, 8:37 p.m. UTC

From: Grant Seltzer <[e-mail]>

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 <[e-mail]>
---
 tools/lib/bpf/bpf.c |   4 +-
 tools/lib/bpf/bpf.h | 112 +++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 112 insertions(+), 4 deletions(-)

Comments

Hengqi Chen Dec. 27, 2021, 12:25 p.m. UTC | #1

On 2021/12/26 4:37 AM, grantseltzer wrote:
> From: Grant Seltzer <[e-mail]>
> 
> 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 <[e-mail]>
> ---
>  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.

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]

>  			 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);
>  }
>  
> -int bpf_map_update_batch(int fd, void *keys, void *values, __u32 *count,
> +int bpf_map_update_batch(int fd, const void *keys, const void *values, __u32 *count,
>  			 const struct bpf_map_batch_opts *opts)
>  {
>  	return bpf_map_batch_common(BPF_MAP_UPDATE_BATCH, fd, NULL, NULL,
> diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
> index 00619f64a040..01011747f127 100644
> --- a/tools/lib/bpf/bpf.h
> +++ b/tools/lib/bpf/bpf.h
> @@ -254,20 +254,128 @@ struct bpf_map_batch_opts {
>  };
>  #define bpf_map_batch_opts__last_field flags
>  
> -LIBBPF_API int bpf_map_delete_batch(int fd, void *keys,
> +
> +/**
> + * @brief **bpf_map_delete_batch()** allows for batch deletion of multiple
> + * elements in a BPF map.
> + *
> + * @param fd BPF map file descriptor
> + * @param keys pointer to an array of *count* keys
> + * @param count number of elements in the map to sequentially delete
> + * @param opts options for configuring the way the batch deletion works
> + * @return 0, on success; negative error code, otherwise (errno is also set to
> + * the error code)
> + */
> +LIBBPF_API int bpf_map_delete_batch(int fd, const void *keys,
>  				    __u32 *count,
>  				    const struct bpf_map_batch_opts *opts);
> +
> +/**
> + * @brief **bpf_map_lookup_batch()** allows for batch lookup of BPF map elements.
> + *
> + * The parameter *in_batch* is the address of the first element in the batch to read.
> + * *out_batch* is an output parameter that should be passed as *in_batch* to subsequent
> + * calls to **bpf_map_lookup_batch()**. NULL can be passed for *in_batch* to indicate
> + * that the batched lookup starts from the beginning of the map.
> + *
> + * The *keys* and *values* are output parameters which must point to memory large enough to
> + * hold *count* items based on the key and value size of the map *map_fd*. The *keys*
> + * buffer must be of *key_size* * *count*. The *values* buffer must be of
> + * *value_size* * *count*.
> + *
> + * @param fd BPF map file descriptor
> + * @param in_batch address of the first element in batch to read, can pass NULL to
> + * indicate that the batched lookup starts from the beginning of the map.
> + * @param out_batch output parameter that should be passed to next call as *in_batch*
> + * @param keys pointer to an array large enough for *count* keys
> + * @param values pointer to an array large enough for *count* values
> + * @param count number of elements in the map to read in batch. If ENOENT is
> + * returned, count will be set as the number of elements that were read before
> + * running out of entries in the map
> + * @param opts options for configuring the way the batch lookup works
> + * @return 0, on success; negative error code, otherwise (errno is also set to
> + * the error code)
> + */
>  LIBBPF_API int bpf_map_lookup_batch(int fd, void *in_batch, void *out_batch,
>  				    void *keys, void *values, __u32 *count,
>  				    const struct bpf_map_batch_opts *opts);
> +
> +/**
> + * @brief **bpf_map_lookup_and_delete_batch()** allows for batch lookup and deletion
> + * of BPF map elements where each element is deleted after being retrieved.
> + *
> + * Note that *count* is an input and output parameter, where on output it
> + * represents how many elements were successfully deleted. Also note that if
> + * **EFAULT** is returned up to *count* elements may have been deleted without
> + * being returned via the *keys* and *values* output parameters. If **ENOENT**
> + * is returned then *count* will be set to the number of elements that were read
> + * before running out of entries in the map.
> + *
> + * @param fd BPF map file descriptor
> + * @param in_batch address of the first element in batch to read, can pass NULL to
> + * get address of the first element in *out_batch*
> + * @param out_batch output parameter that should be passed to next call as *in_batch*
> + * @param keys pointer to an array of *count* keys
> + * @param values pointer to an array large enough for *count* values
> + * @param count input and output parameter; on input it's the number of elements
> + * in the map to read and delete in batch; on output it represents number of elements
> + * that were successfully read and deleted
> + * If ENOENT is returned, count will be set as the number of elements that were
> + * read before running out of entries in the map
> + * @param opts options for configuring the way the batch lookup and delete works
> + * @return 0, on success; negative error code, otherwise (errno is also set to
> + * the error code)
> + */
>  LIBBPF_API int bpf_map_lookup_and_delete_batch(int fd, void *in_batch,
>  					void *out_batch, void *keys,
>  					void *values, __u32 *count,
>  					const struct bpf_map_batch_opts *opts);
> -LIBBPF_API int bpf_map_update_batch(int fd, void *keys, void *values,
> +
> +/**
> + * @brief **bpf_map_update_batch()** updates multiple elements in a map
> + * by specifying keys and their corresponding values.
> + *
> + * The *keys* and *values* parameters must point to memory large enough
> + * to hold *count* items based on the key and value size of the map.
> + *
> + * The *opts* parameter can be used to control how *bpf_map_update_batch()*
> + * should handle keys that either do or do not already exist in the map.
> + * In particular the *flags* parameter of *bpf_map_batch_opts* can be
> + * one of the following:
> + *
> + * Note that *count* is an input and output parameter, where on output it
> + * represents how many elements were successfully updated. Also note that if
> + * **EFAULT** then *count* should not be trusted to be correct.
> + *
> + * **BPF_ANY**
> + *     Create new elements or update existing.
> + *
> + * **BPF_NOEXIST**
> + *    Create new elements only if they do not exist.
> + *
> + * **BPF_EXIST**
> + *    Update existing elements.
> + *
> + * **BPF_F_LOCK**
> + *    Update spin_lock-ed map elements. This must be
> + *    specified if the map value contains a spinlock.
> + *
> + * @param fd BPF map file descriptor
> + * @param keys pointer to an array of *count* keys
> + * @param values pointer to an array of *count* values
> + * @param count input and output parameter; on input it's the number of elements
> + * in the map to update in batch; on output it represents the number of elements
> + * that were successfully updated. If EFAULT is returned, *count* should not
> + * be trusted to be correct.
> + * @param opts options for configuring the way the batch update works
> + * @return 0, on success; negative error code, otherwise (errno is also set to
> + * the error code)
> + */
> +LIBBPF_API int bpf_map_update_batch(int fd, const void *keys, const void *values,
>  				    __u32 *count,
>  				    const struct bpf_map_batch_opts *opts);
>  
> +
>  LIBBPF_API int bpf_obj_pin(int fd, const char *pathname);
>  LIBBPF_API int bpf_obj_get(const char *pathname);
>

Grant Seltzer Richman Jan. 3, 2022, 6:08 p.m. UTC | #2

On Mon, Dec 27, 2021 at 7:25 AM Hengqi Chen <[e-mail]> wrote:
>
>
>
> On 2021/12/26 4:37 AM, grantseltzer wrote:
> > From: Grant Seltzer <[e-mail]>
> >
> > 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 <[e-mail]>
> > ---
> >  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]
>
> >                        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);
> >  }
> >
> > -int bpf_map_update_batch(int fd, void *keys, void *values, __u32 *count,
> > +int bpf_map_update_batch(int fd, const void *keys, const void *values, __u32 *count,
> >                        const struct bpf_map_batch_opts *opts)
> >  {
> >       return bpf_map_batch_common(BPF_MAP_UPDATE_BATCH, fd, NULL, NULL,
> > diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
> > index 00619f64a040..01011747f127 100644
> > --- a/tools/lib/bpf/bpf.h
> > +++ b/tools/lib/bpf/bpf.h
> > @@ -254,20 +254,128 @@ struct bpf_map_batch_opts {
> >  };
> >  #define bpf_map_batch_opts__last_field flags
> >
> > -LIBBPF_API int bpf_map_delete_batch(int fd, void *keys,
> > +
> > +/**
> > + * @brief **bpf_map_delete_batch()** allows for batch deletion of multiple
> > + * elements in a BPF map.
> > + *
> > + * @param fd BPF map file descriptor
> > + * @param keys pointer to an array of *count* keys
> > + * @param count number of elements in the map to sequentially delete
> > + * @param opts options for configuring the way the batch deletion works
> > + * @return 0, on success; negative error code, otherwise (errno is also set to
> > + * the error code)
> > + */
> > +LIBBPF_API int bpf_map_delete_batch(int fd, const void *keys,
> >                                   __u32 *count,
> >                                   const struct bpf_map_batch_opts *opts);
> > +
> > +/**
> > + * @brief **bpf_map_lookup_batch()** allows for batch lookup of BPF map elements.
> > + *
> > + * The parameter *in_batch* is the address of the first element in the batch to read.
> > + * *out_batch* is an output parameter that should be passed as *in_batch* to subsequent
> > + * calls to **bpf_map_lookup_batch()**. NULL can be passed for *in_batch* to indicate
> > + * that the batched lookup starts from the beginning of the map.
> > + *
> > + * The *keys* and *values* are output parameters which must point to memory large enough to
> > + * hold *count* items based on the key and value size of the map *map_fd*. The *keys*
> > + * buffer must be of *key_size* * *count*. The *values* buffer must be of
> > + * *value_size* * *count*.
> > + *
> > + * @param fd BPF map file descriptor
> > + * @param in_batch address of the first element in batch to read, can pass NULL to
> > + * indicate that the batched lookup starts from the beginning of the map.
> > + * @param out_batch output parameter that should be passed to next call as *in_batch*
> > + * @param keys pointer to an array large enough for *count* keys
> > + * @param values pointer to an array large enough for *count* values
> > + * @param count number of elements in the map to read in batch. If ENOENT is
> > + * returned, count will be set as the number of elements that were read before
> > + * running out of entries in the map
> > + * @param opts options for configuring the way the batch lookup works
> > + * @return 0, on success; negative error code, otherwise (errno is also set to
> > + * the error code)
> > + */
> >  LIBBPF_API int bpf_map_lookup_batch(int fd, void *in_batch, void *out_batch,
> >                                   void *keys, void *values, __u32 *count,
> >                                   const struct bpf_map_batch_opts *opts);
> > +
> > +/**
> > + * @brief **bpf_map_lookup_and_delete_batch()** allows for batch lookup and deletion
> > + * of BPF map elements where each element is deleted after being retrieved.
> > + *
> > + * Note that *count* is an input and output parameter, where on output it
> > + * represents how many elements were successfully deleted. Also note that if
> > + * **EFAULT** is returned up to *count* elements may have been deleted without
> > + * being returned via the *keys* and *values* output parameters. If **ENOENT**
> > + * is returned then *count* will be set to the number of elements that were read
> > + * before running out of entries in the map.
> > + *
> > + * @param fd BPF map file descriptor
> > + * @param in_batch address of the first element in batch to read, can pass NULL to
> > + * get address of the first element in *out_batch*
> > + * @param out_batch output parameter that should be passed to next call as *in_batch*
> > + * @param keys pointer to an array of *count* keys
> > + * @param values pointer to an array large enough for *count* values
> > + * @param count input and output parameter; on input it's the number of elements
> > + * in the map to read and delete in batch; on output it represents number of elements
> > + * that were successfully read and deleted
> > + * If ENOENT is returned, count will be set as the number of elements that were
> > + * read before running out of entries in the map
> > + * @param opts options for configuring the way the batch lookup and delete works
> > + * @return 0, on success; negative error code, otherwise (errno is also set to
> > + * the error code)
> > + */
> >  LIBBPF_API int bpf_map_lookup_and_delete_batch(int fd, void *in_batch,
> >                                       void *out_batch, void *keys,
> >                                       void *values, __u32 *count,
> >                                       const struct bpf_map_batch_opts *opts);
> > -LIBBPF_API int bpf_map_update_batch(int fd, void *keys, void *values,
> > +
> > +/**
> > + * @brief **bpf_map_update_batch()** updates multiple elements in a map
> > + * by specifying keys and their corresponding values.
> > + *
> > + * The *keys* and *values* parameters must point to memory large enough
> > + * to hold *count* items based on the key and value size of the map.
> > + *
> > + * The *opts* parameter can be used to control how *bpf_map_update_batch()*
> > + * should handle keys that either do or do not already exist in the map.
> > + * In particular the *flags* parameter of *bpf_map_batch_opts* can be
> > + * one of the following:
> > + *
> > + * Note that *count* is an input and output parameter, where on output it
> > + * represents how many elements were successfully updated. Also note that if
> > + * **EFAULT** then *count* should not be trusted to be correct.
> > + *
> > + * **BPF_ANY**
> > + *     Create new elements or update existing.
> > + *
> > + * **BPF_NOEXIST**
> > + *    Create new elements only if they do not exist.
> > + *
> > + * **BPF_EXIST**
> > + *    Update existing elements.
> > + *
> > + * **BPF_F_LOCK**
> > + *    Update spin_lock-ed map elements. This must be
> > + *    specified if the map value contains a spinlock.
> > + *
> > + * @param fd BPF map file descriptor
> > + * @param keys pointer to an array of *count* keys
> > + * @param values pointer to an array of *count* values
> > + * @param count input and output parameter; on input it's the number of elements
> > + * in the map to update in batch; on output it represents the number of elements
> > + * that were successfully updated. If EFAULT is returned, *count* should not
> > + * be trusted to be correct.
> > + * @param opts options for configuring the way the batch update works
> > + * @return 0, on success; negative error code, otherwise (errno is also set to
> > + * the error code)
> > + */
> > +LIBBPF_API int bpf_map_update_batch(int fd, const void *keys, const void *values,
> >                                   __u32 *count,
> >                                   const struct bpf_map_batch_opts *opts);
> >
> > +
> >  LIBBPF_API int bpf_obj_pin(int fd, const char *pathname);
> >  LIBBPF_API int bpf_obj_get(const char *pathname);
> >

Andrii Nakryiko Jan. 6, 2022, 2:31 a.m. UTC | #3

On Mon, Jan 3, 2022 at 10:09 AM Grant Seltzer Richman
<[e-mail]> wrote:
>
> On Mon, Dec 27, 2021 at 7:25 AM Hengqi Chen <[e-mail]> wrote:
> >
> >
> >
> > On 2021/12/26 4:37 AM, grantseltzer wrote:
> > > From: Grant Seltzer <[e-mail]>
> > >
> > > 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 <[e-mail]>
> > > ---
> > >  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);
> > >  }
> > >

[...]

Andrii Nakryiko Jan. 6, 2022, 2:37 a.m. UTC | #4

On Sat, Dec 25, 2021 at 12:37 PM grantseltzer <[e-mail]> wrote:
>
> From: Grant Seltzer <[e-mail]>
>
> 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 <[e-mail]>
> ---
>  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,
>                          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);
>  }
>
> -int bpf_map_update_batch(int fd, void *keys, void *values, __u32 *count,
> +int bpf_map_update_batch(int fd, const void *keys, const void *values, __u32 *count,
>                          const struct bpf_map_batch_opts *opts)
>  {
>         return bpf_map_batch_common(BPF_MAP_UPDATE_BATCH, fd, NULL, NULL,
> diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
> index 00619f64a040..01011747f127 100644
> --- a/tools/lib/bpf/bpf.h
> +++ b/tools/lib/bpf/bpf.h
> @@ -254,20 +254,128 @@ struct bpf_map_batch_opts {
>  };
>  #define bpf_map_batch_opts__last_field flags
>
> -LIBBPF_API int bpf_map_delete_batch(int fd, void *keys,
> +
> +/**
> + * @brief **bpf_map_delete_batch()** allows for batch deletion of multiple
> + * elements in a BPF map.
> + *
> + * @param fd BPF map file descriptor
> + * @param keys pointer to an array of *count* keys
> + * @param count number of elements in the map to sequentially delete

it's important to mention that count is updated after
bpf_map_delete_batch() returns with the actual number of elements that
were deleted. Please double-check the other APIs as well whether there
are important points to mention. These batch APIs are one of the
trickiest ones in bpf() syscall, let's do a thorough job documenting
them so that users don't have to read kernel code each time they want
to use it

But other than that, great job! I've CC'ed Yonghong to take another
look, as he should know the semantics of batch APIs much better.
Yonghong, please take a look when you can. Thanks!


> + * @param opts options for configuring the way the batch deletion works
> + * @return 0, on success; negative error code, otherwise (errno is also set to
> + * the error code)
> + */
> +LIBBPF_API int bpf_map_delete_batch(int fd, const void *keys,
>                                     __u32 *count,
>                                     const struct bpf_map_batch_opts *opts);

[...]

Yonghong Song Jan. 6, 2022, 4:54 a.m. UTC | #5

On 1/5/22 6:37 PM, Andrii Nakryiko wrote:
> On Sat, Dec 25, 2021 at 12:37 PM grantseltzer <[e-mail]> wrote:
>>
>> From: Grant Seltzer <[e-mail]>
>>
>> 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 <[e-mail]>
>> ---
>>   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,
>>                           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);
>>   }
>>
>> -int bpf_map_update_batch(int fd, void *keys, void *values, __u32 *count,
>> +int bpf_map_update_batch(int fd, const void *keys, const void *values, __u32 *count,
>>                           const struct bpf_map_batch_opts *opts)
>>   {
>>          return bpf_map_batch_common(BPF_MAP_UPDATE_BATCH, fd, NULL, NULL,
>> diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
>> index 00619f64a040..01011747f127 100644
>> --- a/tools/lib/bpf/bpf.h
>> +++ b/tools/lib/bpf/bpf.h
>> @@ -254,20 +254,128 @@ struct bpf_map_batch_opts {
>>   };
>>   #define bpf_map_batch_opts__last_field flags
>>
>> -LIBBPF_API int bpf_map_delete_batch(int fd, void *keys,
>> +
>> +/**
>> + * @brief **bpf_map_delete_batch()** allows for batch deletion of multiple
>> + * elements in a BPF map.
>> + *
>> + * @param fd BPF map file descriptor
>> + * @param keys pointer to an array of *count* keys
>> + * @param count number of elements in the map to sequentially delete
> 
> it's important to mention that count is updated after
> bpf_map_delete_batch() returns with the actual number of elements that
> were deleted. Please double-check the other APIs as well whether there
> are important points to mention. These batch APIs are one of the
> trickiest ones in bpf() syscall, let's do a thorough job documenting
> them so that users don't have to read kernel code each time they want
> to use it
> 
> But other than that, great job! I've CC'ed Yonghong to take another
> look, as he should know the semantics of batch APIs much better.
> Yonghong, please take a look when you can. Thanks!

Will take a look later today.

> 
> 
>> + * @param opts options for configuring the way the batch deletion works
>> + * @return 0, on success; negative error code, otherwise (errno is also set to
>> + * the error code)
>> + */
>> +LIBBPF_API int bpf_map_delete_batch(int fd, const void *keys,
>>                                      __u32 *count,
>>                                      const struct bpf_map_batch_opts *opts);
> 
> [...]

Yonghong Song Jan. 6, 2022, 7:31 a.m. UTC | #6

On 12/25/21 12:37 PM, grantseltzer wrote:
> From: Grant Seltzer <[e-mail]>
> 
> 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 <[e-mail]>
> ---
>   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,
>   			 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);
>   }
>   
> -int bpf_map_update_batch(int fd, void *keys, void *values, __u32 *count,
> +int bpf_map_update_batch(int fd, const void *keys, const void *values, __u32 *count,
>   			 const struct bpf_map_batch_opts *opts)
>   {
>   	return bpf_map_batch_common(BPF_MAP_UPDATE_BATCH, fd, NULL, NULL,
> diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
> index 00619f64a040..01011747f127 100644
> --- a/tools/lib/bpf/bpf.h
> +++ b/tools/lib/bpf/bpf.h
> @@ -254,20 +254,128 @@ struct bpf_map_batch_opts {
>   };
>   #define bpf_map_batch_opts__last_field flags
>   
> -LIBBPF_API int bpf_map_delete_batch(int fd, void *keys,
> +
> +/**
> + * @brief **bpf_map_delete_batch()** allows for batch deletion of multiple
> + * elements in a BPF map.
> + *
> + * @param fd BPF map file descriptor
> + * @param keys pointer to an array of *count* keys
> + * @param count number of elements in the map to sequentially delete

Here, "count" is an input/output parameter. We can describe its output
semantics like below.

When on output, if an error returns, **count** represents the number of
deleted elements if the output **count** value is not equal to the
input **count** value and if the error code is not EFAULT.

> + * @param opts options for configuring the way the batch deletion works
> + * @return 0, on success; negative error code, otherwise (errno is also set to
> + * the error code)
> + */
> +LIBBPF_API int bpf_map_delete_batch(int fd, const void *keys,
>   				    __u32 *count,
>   				    const struct bpf_map_batch_opts *opts);
> +
> +/**
> + * @brief **bpf_map_lookup_batch()** allows for batch lookup of BPF map elements.
> + *
> + * The parameter *in_batch* is the address of the first element in the batch to read.
> + * *out_batch* is an output parameter that should be passed as *in_batch* to subsequent
> + * calls to **bpf_map_lookup_batch()**. NULL can be passed for *in_batch* to indicate
> + * that the batched lookup starts from the beginning of the map.
> + *
> + * The *keys* and *values* are output parameters which must point to memory large enough to
> + * hold *count* items based on the key and value size of the map *map_fd*. The *keys*
> + * buffer must be of *key_size* * *count*. The *values* buffer must be of
> + * *value_size* * *count*.
> + *
> + * @param fd BPF map file descriptor
> + * @param in_batch address of the first element in batch to read, can pass NULL to
> + * indicate that the batched lookup starts from the beginning of the map.
> + * @param out_batch output parameter that should be passed to next call as *in_batch*
> + * @param keys pointer to an array large enough for *count* keys
> + * @param values pointer to an array large enough for *count* values
> + * @param count number of elements in the map to read in batch. If ENOENT is
> + * returned, count will be set as the number of elements that were read before
> + * running out of entries in the map
> + * @param opts options for configuring the way the batch lookup works
> + * @return 0, on success; negative error code, otherwise (errno is also set to
> + * the error code)
> + */
>   LIBBPF_API int bpf_map_lookup_batch(int fd, void *in_batch, void *out_batch,
>   				    void *keys, void *values, __u32 *count,
>   				    const struct bpf_map_batch_opts *opts);
> +
> +/**
> + * @brief **bpf_map_lookup_and_delete_batch()** allows for batch lookup and deletion
> + * of BPF map elements where each element is deleted after being retrieved.
> + *
> + * Note that *count* is an input and output parameter, where on output it
> + * represents how many elements were successfully deleted. Also note that if
> + * **EFAULT** is returned up to *count* elements may have been deleted without

if an non-**EFAULT** error code is returned and if the output **count** 
value is not equal to the input **count** value, up to **count** 
elements may have been deleted.

This applies to the above bpf_map_lookup_batch as well.

> + * being returned via the *keys* and *values* output parameters. If **ENOENT**
> + * is returned then *count* will be set to the number of elements that were read
> + * before running out of entries in the map.
> + *
> + * @param fd BPF map file descriptor
> + * @param in_batch address of the first element in batch to read, can pass NULL to
> + * get address of the first element in *out_batch*
> + * @param out_batch output parameter that should be passed to next call as *in_batch*
> + * @param keys pointer to an array of *count* keys
> + * @param values pointer to an array large enough for *count* values
> + * @param count input and output parameter; on input it's the number of elements
> + * in the map to read and delete in batch; on output it represents number of elements
> + * that were successfully read and deleted
> + * If ENOENT is returned, count will be set as the number of elements that were
> + * read before running out of entries in the map
> + * @param opts options for configuring the way the batch lookup and delete works
> + * @return 0, on success; negative error code, otherwise (errno is also set to
> + * the error code)
> + */
>   LIBBPF_API int bpf_map_lookup_and_delete_batch(int fd, void *in_batch,
>   					void *out_batch, void *keys,
>   					void *values, __u32 *count,
>   					const struct bpf_map_batch_opts *opts);
> -LIBBPF_API int bpf_map_update_batch(int fd, void *keys, void *values,
> +
> +/**
> + * @brief **bpf_map_update_batch()** updates multiple elements in a map
> + * by specifying keys and their corresponding values.
> + *
> + * The *keys* and *values* parameters must point to memory large enough
> + * to hold *count* items based on the key and value size of the map.
> + *
> + * The *opts* parameter can be used to control how *bpf_map_update_batch()*
> + * should handle keys that either do or do not already exist in the map.
> + * In particular the *flags* parameter of *bpf_map_batch_opts* can be
> + * one of the following:
> + *
> + * Note that *count* is an input and output parameter, where on output it
> + * represents how many elements were successfully updated. Also note that if
> + * **EFAULT** then *count* should not be trusted to be correct.

The semantics of **count** here is similar to bpf_map_delete_batch() above.

When on output, if an error returns, **count** represents the number of
deleted elements if the output **count** value is not equal to the
input **count** value and if the error code is not EFAULT.

> + *
> + * **BPF_ANY**
> + *     Create new elements or update existing.
> + *
> + * **BPF_NOEXIST**
> + *    Create new elements only if they do not exist.
> + *
> + * **BPF_EXIST**
> + *    Update existing elements.
> + *
> + * **BPF_F_LOCK**
> + *    Update spin_lock-ed map elements. This must be
> + *    specified if the map value contains a spinlock.
> + *
> + * @param fd BPF map file descriptor
> + * @param keys pointer to an array of *count* keys
> + * @param values pointer to an array of *count* values
> + * @param count input and output parameter; on input it's the number of elements
> + * in the map to update in batch; on output it represents the number of elements
> + * that were successfully updated. If EFAULT is returned, *count* should not
> + * be trusted to be correct.
> + * @param opts options for configuring the way the batch update works
> + * @return 0, on success; negative error code, otherwise (errno is also set to
> + * the error code)
> + */
> +LIBBPF_API int bpf_map_update_batch(int fd, const void *keys, const void *values,
>   				    __u32 *count,
>   				    const struct bpf_map_batch_opts *opts);
>   
> +
>   LIBBPF_API int bpf_obj_pin(int fd, const char *pathname);
>   LIBBPF_API int bpf_obj_get(const char *pathname);
>

Patch

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,
 			 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);
 }
 
-int bpf_map_update_batch(int fd, void *keys, void *values, __u32 *count,
+int bpf_map_update_batch(int fd, const void *keys, const void *values, __u32 *count,
 			 const struct bpf_map_batch_opts *opts)
 {
 	return bpf_map_batch_common(BPF_MAP_UPDATE_BATCH, fd, NULL, NULL,
diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index 00619f64a040..01011747f127 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -254,20 +254,128 @@  struct bpf_map_batch_opts {
 };
 #define bpf_map_batch_opts__last_field flags
 
-LIBBPF_API int bpf_map_delete_batch(int fd, void *keys,
+
+/**
+ * @brief **bpf_map_delete_batch()** allows for batch deletion of multiple
+ * elements in a BPF map.
+ *
+ * @param fd BPF map file descriptor
+ * @param keys pointer to an array of *count* keys
+ * @param count number of elements in the map to sequentially delete
+ * @param opts options for configuring the way the batch deletion works
+ * @return 0, on success; negative error code, otherwise (errno is also set to
+ * the error code)
+ */
+LIBBPF_API int bpf_map_delete_batch(int fd, const void *keys,
 				    __u32 *count,
 				    const struct bpf_map_batch_opts *opts);
+
+/**
+ * @brief **bpf_map_lookup_batch()** allows for batch lookup of BPF map elements.
+ *
+ * The parameter *in_batch* is the address of the first element in the batch to read.
+ * *out_batch* is an output parameter that should be passed as *in_batch* to subsequent
+ * calls to **bpf_map_lookup_batch()**. NULL can be passed for *in_batch* to indicate
+ * that the batched lookup starts from the beginning of the map.
+ *
+ * The *keys* and *values* are output parameters which must point to memory large enough to
+ * hold *count* items based on the key and value size of the map *map_fd*. The *keys*
+ * buffer must be of *key_size* * *count*. The *values* buffer must be of
+ * *value_size* * *count*.
+ *
+ * @param fd BPF map file descriptor
+ * @param in_batch address of the first element in batch to read, can pass NULL to
+ * indicate that the batched lookup starts from the beginning of the map.
+ * @param out_batch output parameter that should be passed to next call as *in_batch*
+ * @param keys pointer to an array large enough for *count* keys
+ * @param values pointer to an array large enough for *count* values
+ * @param count number of elements in the map to read in batch. If ENOENT is
+ * returned, count will be set as the number of elements that were read before
+ * running out of entries in the map
+ * @param opts options for configuring the way the batch lookup works
+ * @return 0, on success; negative error code, otherwise (errno is also set to
+ * the error code)
+ */
 LIBBPF_API int bpf_map_lookup_batch(int fd, void *in_batch, void *out_batch,
 				    void *keys, void *values, __u32 *count,
 				    const struct bpf_map_batch_opts *opts);
+
+/**
+ * @brief **bpf_map_lookup_and_delete_batch()** allows for batch lookup and deletion
+ * of BPF map elements where each element is deleted after being retrieved.
+ *
+ * Note that *count* is an input and output parameter, where on output it
+ * represents how many elements were successfully deleted. Also note that if
+ * **EFAULT** is returned up to *count* elements may have been deleted without
+ * being returned via the *keys* and *values* output parameters. If **ENOENT**
+ * is returned then *count* will be set to the number of elements that were read
+ * before running out of entries in the map.
+ *
+ * @param fd BPF map file descriptor
+ * @param in_batch address of the first element in batch to read, can pass NULL to
+ * get address of the first element in *out_batch*
+ * @param out_batch output parameter that should be passed to next call as *in_batch*
+ * @param keys pointer to an array of *count* keys
+ * @param values pointer to an array large enough for *count* values
+ * @param count input and output parameter; on input it's the number of elements
+ * in the map to read and delete in batch; on output it represents number of elements
+ * that were successfully read and deleted
+ * If ENOENT is returned, count will be set as the number of elements that were
+ * read before running out of entries in the map
+ * @param opts options for configuring the way the batch lookup and delete works
+ * @return 0, on success; negative error code, otherwise (errno is also set to
+ * the error code)
+ */
 LIBBPF_API int bpf_map_lookup_and_delete_batch(int fd, void *in_batch,
 					void *out_batch, void *keys,
 					void *values, __u32 *count,
 					const struct bpf_map_batch_opts *opts);
-LIBBPF_API int bpf_map_update_batch(int fd, void *keys, void *values,
+
+/**
+ * @brief **bpf_map_update_batch()** updates multiple elements in a map
+ * by specifying keys and their corresponding values.
+ *
+ * The *keys* and *values* parameters must point to memory large enough
+ * to hold *count* items based on the key and value size of the map.
+ *
+ * The *opts* parameter can be used to control how *bpf_map_update_batch()*
+ * should handle keys that either do or do not already exist in the map.
+ * In particular the *flags* parameter of *bpf_map_batch_opts* can be
+ * one of the following:
+ *
+ * Note that *count* is an input and output parameter, where on output it
+ * represents how many elements were successfully updated. Also note that if
+ * **EFAULT** then *count* should not be trusted to be correct.
+ *
+ * **BPF_ANY**
+ *     Create new elements or update existing.
+ *
+ * **BPF_NOEXIST**
+ *    Create new elements only if they do not exist.
+ *
+ * **BPF_EXIST**
+ *    Update existing elements.
+ *
+ * **BPF_F_LOCK**
+ *    Update spin_lock-ed map elements. This must be
+ *    specified if the map value contains a spinlock.
+ *
+ * @param fd BPF map file descriptor
+ * @param keys pointer to an array of *count* keys
+ * @param values pointer to an array of *count* values
+ * @param count input and output parameter; on input it's the number of elements
+ * in the map to update in batch; on output it represents the number of elements
+ * that were successfully updated. If EFAULT is returned, *count* should not
+ * be trusted to be correct.
+ * @param opts options for configuring the way the batch update works
+ * @return 0, on success; negative error code, otherwise (errno is also set to
+ * the error code)
+ */
+LIBBPF_API int bpf_map_update_batch(int fd, const void *keys, const void *values,
 				    __u32 *count,
 				    const struct bpf_map_batch_opts *opts);
 
+
 LIBBPF_API int bpf_obj_pin(int fd, const char *pathname);
 LIBBPF_API int bpf_obj_get(const char *pathname);