[bpf-next,v2] bpf: clarify batch lookup semantics

Commit Message

Martin Kelly Feb. 21, 2024, 9:18 p.m. UTC

The batch lookup and lookup_and_delete APIs have two parameters,
in_batch and out_batch, to facilitate iterative
lookup/lookup_and_deletion operations for supported maps. Except NULL
for in_batch at the start of these two batch operations, both parameters
need to point to memory equal or larger than the respective map key
size, except for various hashmaps (hash, percpu_hash, lru_hash,
lru_percpu_hash) where the in_batch/out_batch memory size should be
at least 4 bytes.

Document these semantics to clarify the API.

Signed-off-by: Martin Kelly <[e-mail]>
---
 include/uapi/linux/bpf.h       |  6 +++++-
 tools/include/uapi/linux/bpf.h |  6 +++++-
 tools/lib/bpf/bpf.h            | 17 ++++++++++++-----
 3 files changed, 22 insertions(+), 7 deletions(-)

Comments

Martin Kelly Feb. 21, 2024, 9:38 p.m. UTC | #1

On 2/21/24 13:18, Martin Kelly wrote:
> The batch lookup and lookup_and_delete APIs have two parameters,
> in_batch and out_batch, to facilitate iterative
> lookup/lookup_and_deletion operations for supported maps. Except NULL
> for in_batch at the start of these two batch operations, both parameters
> need to point to memory equal or larger than the respective map key
> size, except for various hashmaps (hash, percpu_hash, lru_hash,
> lru_percpu_hash) where the in_batch/out_batch memory size should be
> at least 4 bytes.
>
> Document these semantics to clarify the API.
>
> Signed-off-by: Martin Kelly <[e-mail]>
> ---
>   include/uapi/linux/bpf.h       |  6 +++++-
>   tools/include/uapi/linux/bpf.h |  6 +++++-
>   tools/lib/bpf/bpf.h            | 17 ++++++++++++-----
>   3 files changed, 22 insertions(+), 7 deletions(-)

Yonghong, looks like I missed your comment to change from "clarify batch 
lookup semantics" to "Clarify batch lookup/lookup_and_delete semantics"; 
sorry about that. Feel free to change it if you merge this, or I can 
include it in a v3 if needed.

>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index d96708380e52..d2e6c5fcec01 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -617,7 +617,11 @@ union bpf_iter_link_info {
>    *		to NULL to begin the batched operation. After each subsequent
>    *		**BPF_MAP_LOOKUP_BATCH**, the caller should pass the resultant
>    *		*out_batch* as the *in_batch* for the next operation to
> - *		continue iteration from the current point.
> + *		continue iteration from the current point. Both *in_batch* and
> + *		*out_batch* must point to memory large enough to hold a key,
> + *		except for maps of type **BPF_MAP_TYPE_{HASH, PERCPU_HASH,
> + *		LRU_HASH, LRU_PERCPU_HASH}**, for which batch parameters
> + *		must be at least 4 bytes wide regardless of key size.
>    *
>    *		The *keys* and *values* are output parameters which must point
>    *		to memory large enough to hold *count* items based on the key
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index d96708380e52..d2e6c5fcec01 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -617,7 +617,11 @@ union bpf_iter_link_info {
>    *		to NULL to begin the batched operation. After each subsequent
>    *		**BPF_MAP_LOOKUP_BATCH**, the caller should pass the resultant
>    *		*out_batch* as the *in_batch* for the next operation to
> - *		continue iteration from the current point.
> + *		continue iteration from the current point. Both *in_batch* and
> + *		*out_batch* must point to memory large enough to hold a key,
> + *		except for maps of type **BPF_MAP_TYPE_{HASH, PERCPU_HASH,
> + *		LRU_HASH, LRU_PERCPU_HASH}**, for which batch parameters
> + *		must be at least 4 bytes wide regardless of key size.
>    *
>    *		The *keys* and *values* are output parameters which must point
>    *		to memory large enough to hold *count* items based on the key
> diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
> index ab2570d28aec..df0db2f0cdb7 100644
> --- a/tools/lib/bpf/bpf.h
> +++ b/tools/lib/bpf/bpf.h
> @@ -190,10 +190,14 @@ LIBBPF_API int bpf_map_delete_batch(int fd, const void *keys,
>   /**
>    * @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 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. Both *in_batch* and *out_batch* must point to memory large enough to
> + * hold a single key, except for maps of type **BPF_MAP_TYPE_{HASH, PERCPU_HASH,
> + * LRU_HASH, LRU_PERCPU_HASH}**, for which the memory size must be at
> + * least 4 bytes wide regardless of key size.
>    *
>    * 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*
> @@ -226,7 +230,10 @@ LIBBPF_API int bpf_map_lookup_batch(int fd, void *in_batch, void *out_batch,
>    *
>    * @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*
> + * get address of the first element in *out_batch*. If not NULL, must be large
> + * enough to hold a key. For **BPF_MAP_TYPE_{HASH, PERCPU_HASH, LRU_HASH,
> + * LRU_PERCPU_HASH}**, the memory size must be at least 4 bytes wide regardless
> + * of key size.
>    * @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

Yonghong Song Feb. 21, 2024, 11:33 p.m. UTC | #2

On 2/21/24 1:38 PM, Martin Kelly wrote:
> On 2/21/24 13:18, Martin Kelly wrote:
>> The batch lookup and lookup_and_delete APIs have two parameters,
>> in_batch and out_batch, to facilitate iterative
>> lookup/lookup_and_deletion operations for supported maps. Except NULL
>> for in_batch at the start of these two batch operations, both parameters
>> need to point to memory equal or larger than the respective map key
>> size, except for various hashmaps (hash, percpu_hash, lru_hash,
>> lru_percpu_hash) where the in_batch/out_batch memory size should be
>> at least 4 bytes.
>>
>> Document these semantics to clarify the API.
>>
>> Signed-off-by: Martin Kelly <[e-mail]>
>> ---
>>   include/uapi/linux/bpf.h       |  6 +++++-
>>   tools/include/uapi/linux/bpf.h |  6 +++++-
>>   tools/lib/bpf/bpf.h            | 17 ++++++++++++-----
>>   3 files changed, 22 insertions(+), 7 deletions(-)
>
> Yonghong, looks like I missed your comment to change from "clarify 
> batch lookup semantics" to "Clarify batch lookup/lookup_and_delete 
> semantics"; sorry about that. Feel free to change it if you merge 
> this, or I can include it in a v3 if needed.

Ok, LGTM except the subject. I guess it is up to maintainers who will either change the subject
or asking for another revision if more change is needed. From my part,

Acked-by: Yonghong Song <[e-mail]>

>
>>
>> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>> index d96708380e52..d2e6c5fcec01 100644
>> --- a/include/uapi/linux/bpf.h
>> +++ b/include/uapi/linux/bpf.h
>> @@ -617,7 +617,11 @@ union bpf_iter_link_info {
>>    *        to NULL to begin the batched operation. After each 
>> subsequent
>>    *        **BPF_MAP_LOOKUP_BATCH**, the caller should pass the 
>> resultant
>>    *        *out_batch* as the *in_batch* for the next operation to
>> - *        continue iteration from the current point.
>> + *        continue iteration from the current point. Both *in_batch* 
>> and
>> + *        *out_batch* must point to memory large enough to hold a key,
>> + *        except for maps of type **BPF_MAP_TYPE_{HASH, PERCPU_HASH,
>> + *        LRU_HASH, LRU_PERCPU_HASH}**, for which batch parameters
>> + *        must be at least 4 bytes wide regardless of key size.
>>    *
>>    *        The *keys* and *values* are output parameters which must 
>> point
>>    *        to memory large enough to hold *count* items based on the 
>> key
>> diff --git a/tools/include/uapi/linux/bpf.h 
>> b/tools/include/uapi/linux/bpf.h
>> index d96708380e52..d2e6c5fcec01 100644
>> --- a/tools/include/uapi/linux/bpf.h
>> +++ b/tools/include/uapi/linux/bpf.h
>> @@ -617,7 +617,11 @@ union bpf_iter_link_info {
>>    *        to NULL to begin the batched operation. After each 
>> subsequent
>>    *        **BPF_MAP_LOOKUP_BATCH**, the caller should pass the 
>> resultant
>>    *        *out_batch* as the *in_batch* for the next operation to
>> - *        continue iteration from the current point.
>> + *        continue iteration from the current point. Both *in_batch* 
>> and
>> + *        *out_batch* must point to memory large enough to hold a key,
>> + *        except for maps of type **BPF_MAP_TYPE_{HASH, PERCPU_HASH,
>> + *        LRU_HASH, LRU_PERCPU_HASH}**, for which batch parameters
>> + *        must be at least 4 bytes wide regardless of key size.
>>    *
>>    *        The *keys* and *values* are output parameters which must 
>> point
>>    *        to memory large enough to hold *count* items based on the 
>> key
>> diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
>> index ab2570d28aec..df0db2f0cdb7 100644
>> --- a/tools/lib/bpf/bpf.h
>> +++ b/tools/lib/bpf/bpf.h
>> @@ -190,10 +190,14 @@ LIBBPF_API int bpf_map_delete_batch(int fd, 
>> const void *keys,
>>   /**
>>    * @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 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. Both *in_batch* and *out_batch* must point to memory 
>> large enough to
>> + * hold a single key, except for maps of type **BPF_MAP_TYPE_{HASH, 
>> PERCPU_HASH,
>> + * LRU_HASH, LRU_PERCPU_HASH}**, for which the memory size must be at
>> + * least 4 bytes wide regardless of key size.
>>    *
>>    * 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*
>> @@ -226,7 +230,10 @@ LIBBPF_API int bpf_map_lookup_batch(int fd, void 
>> *in_batch, void *out_batch,
>>    *
>>    * @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*
>> + * get address of the first element in *out_batch*. If not NULL, 
>> must be large
>> + * enough to hold a key. For **BPF_MAP_TYPE_{HASH, PERCPU_HASH, 
>> LRU_HASH,
>> + * LRU_PERCPU_HASH}**, the memory size must be at least 4 bytes wide 
>> regardless
>> + * of key size.
>>    * @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

[e-mail] Feb. 22, 2024, 6:30 p.m. UTC | #3

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Martin KaFai Lau <[e-mail]>:

On Wed, 21 Feb 2024 13:18:38 -0800 you wrote:
> The batch lookup and lookup_and_delete APIs have two parameters,
> in_batch and out_batch, to facilitate iterative
> lookup/lookup_and_deletion operations for supported maps. Except NULL
> for in_batch at the start of these two batch operations, both parameters
> need to point to memory equal or larger than the respective map key
> size, except for various hashmaps (hash, percpu_hash, lru_hash,
> lru_percpu_hash) where the in_batch/out_batch memory size should be
> at least 4 bytes.
> 
> [...]

Here is the summary with links:
  - [bpf-next,v2] bpf: clarify batch lookup semantics
    https://git.kernel.org/bpf/bpf-next/c/58fd62e0aa50

You are awesome, thank you!

Patch

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index d96708380e52..d2e6c5fcec01 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -617,7 +617,11 @@  union bpf_iter_link_info {
  *		to NULL to begin the batched operation. After each subsequent
  *		**BPF_MAP_LOOKUP_BATCH**, the caller should pass the resultant
  *		*out_batch* as the *in_batch* for the next operation to
- *		continue iteration from the current point.
+ *		continue iteration from the current point. Both *in_batch* and
+ *		*out_batch* must point to memory large enough to hold a key,
+ *		except for maps of type **BPF_MAP_TYPE_{HASH, PERCPU_HASH,
+ *		LRU_HASH, LRU_PERCPU_HASH}**, for which batch parameters
+ *		must be at least 4 bytes wide regardless of key size.
  *
  *		The *keys* and *values* are output parameters which must point
  *		to memory large enough to hold *count* items based on the key
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index d96708380e52..d2e6c5fcec01 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -617,7 +617,11 @@  union bpf_iter_link_info {
  *		to NULL to begin the batched operation. After each subsequent
  *		**BPF_MAP_LOOKUP_BATCH**, the caller should pass the resultant
  *		*out_batch* as the *in_batch* for the next operation to
- *		continue iteration from the current point.
+ *		continue iteration from the current point. Both *in_batch* and
+ *		*out_batch* must point to memory large enough to hold a key,
+ *		except for maps of type **BPF_MAP_TYPE_{HASH, PERCPU_HASH,
+ *		LRU_HASH, LRU_PERCPU_HASH}**, for which batch parameters
+ *		must be at least 4 bytes wide regardless of key size.
  *
  *		The *keys* and *values* are output parameters which must point
  *		to memory large enough to hold *count* items based on the key
diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index ab2570d28aec..df0db2f0cdb7 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -190,10 +190,14 @@  LIBBPF_API int bpf_map_delete_batch(int fd, const void *keys,
 /**
  * @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 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. Both *in_batch* and *out_batch* must point to memory large enough to
+ * hold a single key, except for maps of type **BPF_MAP_TYPE_{HASH, PERCPU_HASH,
+ * LRU_HASH, LRU_PERCPU_HASH}**, for which the memory size must be at
+ * least 4 bytes wide regardless of key size.
  *
  * 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*
@@ -226,7 +230,10 @@  LIBBPF_API int bpf_map_lookup_batch(int fd, void *in_batch, void *out_batch,
  *
  * @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*
+ * get address of the first element in *out_batch*. If not NULL, must be large
+ * enough to hold a key. For **BPF_MAP_TYPE_{HASH, PERCPU_HASH, LRU_HASH,
+ * LRU_PERCPU_HASH}**, the memory size must be at least 4 bytes wide regardless
+ * of key size.
  * @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