Message-ID: <CAMB2axPuRgPg_mhEW7ZbmjexFEqLkfnfSpVHuCZ-j4U3UxYtTA@mail.gmail.com>
Date: Tue, 1 Jul 2025 15:47:24 -0700
From: Amery Hung <ameryhung@...il.com>
To: Andrii Nakryiko <andrii.nakryiko@...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 Tue, Jul 1, 2025 at 3:02 PM Andrii Nakryiko
<andrii.nakryiko@...il.com> wrote:
>
> 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?
>

It should be fine as the BPF side uses strncmp. So, a TLD can have a
name that is TLD_NAME_LEN-char long, not including the null
terminator.

> > +               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
>

Designated initializers has been supported since C++20, but I can also
just use (tld_key_t){-ENOSPC} to make it less verbose.

> > +}
> > +
> > +/**
> > + * 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?
>

It is a global variable. File-scope is just the terminology used in
the C language standard
https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3220.pdf

> > + * 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