Message-ID: <CAEf4BzYFdiQX3gz8Nd2T2cGm6NCZPzTVCRh+eh_C2gYd=cEMpA@mail.gmail.com>
Date: Tue, 1 Jul 2025 15:02:31 -0700
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Amery Hung <ameryhung@...il.com>
Cc: bpf@...r.kernel.org, netdev@...r.kernel.org, alexei.starovoitov@...il.com, 
	andrii@...nel.org, daniel@...earbox.net, tj@...nel.org, memxor@...il.com, 
	martin.lau@...nel.org, kernel-team@...a.com
Subject: Re: [PATCH bpf-next v5 1/3] selftests/bpf: Introduce task local data

On Fri, Jun 27, 2025 at 4:40 PM Amery Hung <ameryhung@...il.com> wrote:
>
> Task local data defines an abstract storage type for storing task-
> specific data (TLD). This patch provides user space and bpf
> implementation as header-only libraries for accessing task local data.
>
> Task local data is a bpf task local storage map with two UPTRs:
> 1) u_tld_metadata, shared by all tasks of the same process, consists of
> the total count of TLDs and an array of metadata of TLDs. A metadata of
> a TLD comprises the size and the name. The name is used to identify a
> specific TLD in bpf 2) u_tld_data points to a task-specific memory region
> for storing TLDs.
>
> Below are the core task local data API:
>
>                      User space                           BPF
> Define TLD    TLD_DEFINE_KEY(), tld_create_key()           -
> Get data           tld_get_data()                    tld_get_data()
>
> A TLD is first defined by the user space with TLD_DEFINE_KEY() or
> tld_create_key(). TLD_DEFINE_KEY() defines a TLD statically and allocates
> just enough memory during initialization. tld_create_key() allows
> creating TLDs on the fly, but has a fix memory budget, TLD_DYN_DATA_SIZE.
> Internally, they all go through the metadata array to check if the TLD can
> be added. The total TLD size needs to fit into a page (limited by UPTR),
> and no two TLDs can have the same name. It also calculates the offset, the
> next available space in u_tld_data, by summing sizes of TLDs. If the TLD
> can be added, it increases the count using cmpxchg as there may be other
> concurrent tld_create_key(). After a successful cmpxchg, the last
> metadata slot now belongs to the calling thread and will be updated.
> tld_create_key() returns the offset encapsulated as a opaque object key
> to prevent user misuse.
>
> Then, user space can pass the key to tld_get_data() to get a pointer
> to the TLD. The pointer will remain valid for the lifetime of the
> thread.
>
> BPF programs can also locate the TLD by tld_get_data(), but with both
> name and key. The first time tld_get_data() is called, the name will
> be used to lookup the metadata. Then, the key will be saved to a
> task_local_data map, tld_keys_map. Subsequent call to tld_get_data()
> will use the key to quickly locate the data.
>
> User space task local data library uses a light way approach to ensure
> thread safety (i.e., atomic operation + compiler and memory barriers).
> While a metadata is being updated, other threads may also try to read it.
> To prevent them from seeing incomplete data, metadata::size is used to
> signal the completion of the update, where 0 means the update is still
> ongoing. Threads will wait until seeing a non-zero size to read a
> metadata.
>
> Signed-off-by: Amery Hung <ameryhung@...il.com>
> ---
>  .../bpf/prog_tests/task_local_data.h          | 397 ++++++++++++++++++
>  .../selftests/bpf/progs/task_local_data.bpf.h | 232 ++++++++++
>  2 files changed, 629 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/task_local_data.h
>  create mode 100644 tools/testing/selftests/bpf/progs/task_local_data.bpf.h
>

[...]

> +               /*
> +                * Only one tld_create_key() can increase the current cnt by one and
> +                * takes the latest available slot. Other threads will check again if a new
> +                * TLD can still be added, and then compete for the new slot after the
> +                * succeeding thread update the size.
> +                */
> +               if (!atomic_compare_exchange_strong(&tld_metadata_p->cnt, &cnt, cnt + 1))
> +                       goto retry;
> +
> +               strncpy(tld_metadata_p->metadata[i].name, name, TLD_NAME_LEN);

from man page:

Warning: If there is no null byte among the first n bytes of src, the
string placed in dest will not be null-terminated.

is that a concern?

> +               atomic_store(&tld_metadata_p->metadata[i].size, size);
> +               return (tld_key_t) {.off = (__s16)off};
> +       }
> +
> +       return (tld_key_t) {.off = -ENOSPC};

I don't know if C++ compiler will like this, but in C just
`(tld_key_t){-ENOSPC}` should work fine

> +}
> +
> +/**
> + * TLD_DEFINE_KEY() - Defines a TLD and a file-scope key associated with the TLD.
> + *
> + * @name: The name of the TLD
> + * @size: The size of the TLD
> + * @key: The variable name of the key. Cannot exceed TLD_NAME_LEN
> + *
> + * The macro can only be used in file scope.
> + *
> + * A file-scope key of opaque type, tld_key_t, will be declared and initialized before

what's "file-scope"? it looks like a global (not even static)
variable, so you can even reference it from other files with extern,
no?

> + * main() starts. Use tld_key_is_err() or tld_key_err_or_zero() later to check if the key
> + * creation succeeded. Pass the key to tld_get_data() to get a pointer to the TLD.
> + * bpf programs can also fetch the same key by name.
> + *
> + * The total size of TLDs created using TLD_DEFINE_KEY() cannot exceed a page. Just
> + * enough memory will be allocated for each thread on the first call to tld_get_data().
> + */

[...]

Powered by blists - more mailing lists