[PATCH bpf-next 2/2] bpf: update uapi/linux/bpf.h docs on the batch map ops

Anton Protopopov posted 2 patches 2 years, 6 months ago
[PATCH bpf-next 2/2] bpf: update uapi/linux/bpf.h docs on the batch map ops
Posted by Anton Protopopov 2 years, 6 months ago
The map_lookup{,_and_delete}_batch operations return same values. Make
this clear in documentation. Also, update the comments so that this is
more clear that -ENOENT is a valid return value in case of success. (In
fact, this is the most common return value, as this is reasonable to do
map_lookup_batch(MAX_ENTRIES), which, in case of success, will always
return -ENOENT.)

Signed-off-by: Anton Protopopov <aspsk@isovalent.com>
---
 include/uapi/linux/bpf.h | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 600d0caebbd8..9e6e277bedab 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -632,17 +632,19 @@ union bpf_iter_link_info {
  *			returning the lock. This must be specified if the
  *			elements contain a spinlock.
  *
- *		On success, *count* elements from the map are copied into the
- *		user buffer, with the keys copied into *keys* and the values
- *		copied into the corresponding indices in *values*.
- *
- *		If an error is returned and *errno* is not **EFAULT**, *count*
- *		is set to the number of successfully processed elements.
+ *		On success, up to *count* elements from the map are copied into
+ *		the user buffer, with the keys copied into *keys* and the
+ *		values copied into the corresponding indices in *values*.
  *
  *	Return
  *		Returns zero on success. On error, -1 is returned and *errno*
  *		is set appropriately.
  *
+ *		If an error is returned and *errno* is not **EFAULT**, then
+ *		*count* is set to the number of successfully processed
+ *		elements. In particular, the *errno* may be set to **ENOENT**
+ *		in case of success to indicate that the end of map is reached.
+ *
  *		May set *errno* to **ENOSPC** to indicate that *keys* or
  *		*values* is too small to dump an entire bucket during
  *		iteration of a hash-based map type.
@@ -655,15 +657,15 @@ union bpf_iter_link_info {
  *		**BPF_MAP_LOOKUP_BATCH** with two exceptions:
  *
  *		* Every element that is successfully returned is also deleted
- *		  from the map. This is at least *count* elements. Note that
- *		  *count* is both an input and an output parameter.
+ *		  from the map. The *count* parameter is set to the number of
+ *		  returned elements. This value can be less than the actual
+ *		  number of deleted elements, see the next item.
  *		* Upon returning with *errno* set to **EFAULT**, up to
  *		  *count* elements may be deleted without returning the keys
  *		  and values of the deleted elements.
  *
  *	Return
- *		Returns zero on success. On error, -1 is returned and *errno*
- *		is set appropriately.
+ *		Same as the BPF_MAP_LOOKUP_BATCH return values.
  *
  * BPF_MAP_UPDATE_BATCH
  *	Description
-- 
2.34.1
Re: [PATCH bpf-next 2/2] bpf: update uapi/linux/bpf.h docs on the batch map ops
Posted by Hou Tao 2 years, 6 months ago
Hi,

On 7/17/2023 7:43 PM, Anton Protopopov wrote:
> The map_lookup{,_and_delete}_batch operations return same values. Make
> this clear in documentation. Also, update the comments so that this is
> more clear that -ENOENT is a valid return value in case of success. (In
> fact, this is the most common return value, as this is reasonable to do
> map_lookup_batch(MAX_ENTRIES), which, in case of success, will always
> return -ENOENT.)
>
> Signed-off-by: Anton Protopopov <aspsk@isovalent.com>
> ---
>  include/uapi/linux/bpf.h | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 600d0caebbd8..9e6e277bedab 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -632,17 +632,19 @@ union bpf_iter_link_info {
>   *			returning the lock. This must be specified if the
>   *			elements contain a spinlock.
>   *
> - *		On success, *count* elements from the map are copied into the
> - *		user buffer, with the keys copied into *keys* and the values
> - *		copied into the corresponding indices in *values*.
> - *
> - *		If an error is returned and *errno* is not **EFAULT**, *count*
> - *		is set to the number of successfully processed elements.
> + *		On success, up to *count* elements from the map are copied into
> + *		the user buffer, with the keys copied into *keys* and the
> + *		values copied into the corresponding indices in *values*.
>   *
>   *	Return
>   *		Returns zero on success. On error, -1 is returned and *errno*
>   *		is set appropriately.
>   *
> + *		If an error is returned and *errno* is not **EFAULT**, then

Maybe we should say "is not EFAULT or EINVAL" ? Otherwise, the update
looks good to me, so

Acked-by: Hou Tao <houtao1@huawei.com>

> + *		*count* is set to the number of successfully processed
> + *		elements. In particular, the *errno* may be set to **ENOENT**
> + *		in case of success to indicate that the end of map is reached.
> + *
>   *		May set *errno* to **ENOSPC** to indicate that *keys* or
>   *		*values* is too small to dump an entire bucket during
>   *		iteration of a hash-based map type.
> @@ -655,15 +657,15 @@ union bpf_iter_link_info {
>   *		**BPF_MAP_LOOKUP_BATCH** with two exceptions:
>   *
>   *		* Every element that is successfully returned is also deleted
> - *		  from the map. This is at least *count* elements. Note that
> - *		  *count* is both an input and an output parameter.
> + *		  from the map. The *count* parameter is set to the number of
> + *		  returned elements. This value can be less than the actual
> + *		  number of deleted elements, see the next item.
>   *		* Upon returning with *errno* set to **EFAULT**, up to
>   *		  *count* elements may be deleted without returning the keys
>   *		  and values of the deleted elements.
>   *
>   *	Return
> - *		Returns zero on success. On error, -1 is returned and *errno*
> - *		is set appropriately.
> + *		Same as the BPF_MAP_LOOKUP_BATCH return values.
>   *
>   * BPF_MAP_UPDATE_BATCH
>   *	Description