From: Yonghong Song <yonghong.song@linux.dev>
To: Martin Kelly <martin.kelly@crowdstrike.com>, bpf@vger.kernel.org
Cc: Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Andrii Nakryiko <andrii@kernel.org>
Subject: Re: [PATCH bpf-next v2] bpf: clarify batch lookup semantics
Date: Wed, 21 Feb 2024 15:33:58 -0800 [thread overview]
Message-ID: <7ef09893-c990-420d-8e0d-ff7be38c7fa2@linux.dev> (raw)
In-Reply-To: <a382f71d-3677-4545-a4e2-4e93f0ae3864@crowdstrike.com>
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 <martin.kelly@crowdstrike.com>
>> ---
>> 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 <yonghong.song@linux.dev>
>
>>
>> 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
next prev parent reply other threads:[~2024-02-21 23:34 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-21 21:18 [PATCH bpf-next v2] bpf: clarify batch lookup semantics Martin Kelly
2024-02-21 21:38 ` Martin Kelly
2024-02-21 23:33 ` Yonghong Song [this message]
2024-02-22 18:30 ` patchwork-bot+netdevbpf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7ef09893-c990-420d-8e0d-ff7be38c7fa2@linux.dev \
--to=yonghong.song@linux.dev \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=martin.kelly@crowdstrike.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).