[bpf-next,v4] docs/bpf: Add documentation for BPF_MAP_TYPE_SK_STORAGE

Commit Message

Donald Hunter Dec. 9, 2022, 11:24 a.m. UTC

Add documentation for the BPF_MAP_TYPE_SK_STORAGE including
kernel version introduced, usage and examples.

Signed-off-by: Donald Hunter <[e-mail]>
---
v3 -> v4:
- Update intro paragraph with detail about storage locality.
- Remove confusing text from bpf_map_update_elem()
  as reported by David Vernet
- Updated BPF_EXIST and BPF_NOEXIST behaviour as suggested
  by David Vernet
- Fixed extra space in function signature as reported by
  David Vernet
- Added reference to selftests for complete examples as
  suggested by Yonghong Song
v2 -> v3:
- Fix void * return, reported by Yonghong Song
- Add tracing programs to API note, reported by Yonghong Song
v1 -> v2:
- Fix bpf_sk_storage_* function signatures, reported by Yonghong Song
- Fix NULL return on failure, reported by Yonghong Song

 Documentation/bpf/map_sk_storage.rst | 155 +++++++++++++++++++++++++++
 1 file changed, 155 insertions(+)
 create mode 100644 Documentation/bpf/map_sk_storage.rst

Comments

David Vernet Dec. 9, 2022, 4:14 p.m. UTC | #1

On Fri, Dec 09, 2022 at 11:24:01AM +0000, Donald Hunter wrote:
> Add documentation for the BPF_MAP_TYPE_SK_STORAGE including
> kernel version introduced, usage and examples.
> 
> Signed-off-by: Donald Hunter <[e-mail]>

Looks great, thanks Donald!

Acked-by: David Vernet <[e-mail]>

[e-mail] Dec. 9, 2022, 5:50 p.m. UTC | #2

Hello:

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

On Fri,  9 Dec 2022 11:24:01 +0000 you wrote:
> Add documentation for the BPF_MAP_TYPE_SK_STORAGE including
> kernel version introduced, usage and examples.
> 
> Signed-off-by: Donald Hunter <[e-mail]>
> ---
> v3 -> v4:
> - Update intro paragraph with detail about storage locality.
> - Remove confusing text from bpf_map_update_elem()
>   as reported by David Vernet
> - Updated BPF_EXIST and BPF_NOEXIST behaviour as suggested
>   by David Vernet
> - Fixed extra space in function signature as reported by
>   David Vernet
> - Added reference to selftests for complete examples as
>   suggested by Yonghong Song
> v2 -> v3:
> - Fix void * return, reported by Yonghong Song
> - Add tracing programs to API note, reported by Yonghong Song
> v1 -> v2:
> - Fix bpf_sk_storage_* function signatures, reported by Yonghong Song
> - Fix NULL return on failure, reported by Yonghong Song
> 
> [...]

Here is the summary with links:
  - [bpf-next,v4] docs/bpf: Add documentation for BPF_MAP_TYPE_SK_STORAGE
    https://git.kernel.org/bpf/bpf-next/c/f3212ad5b7e9

You are awesome, thank you!

Martin KaFai Lau Dec. 9, 2022, 5:52 p.m. UTC | #3

On 12/9/22 3:24 AM, Donald Hunter wrote:
> Add documentation for the BPF_MAP_TYPE_SK_STORAGE including
> kernel version introduced, usage and examples.

Thanks for writing the doc for sk_storage!

> +User space
> +----------
> +
> +bpf_map_update_elem()
> +~~~~~~~~~~~~~~~~~~~~~
> +
> +.. code-block:: c
> +
> +   int bpf_map_update_elem(int map_fd, const void *key, const void *value, __u64 flags)
> +
> +Socket-local storage for the socket identified by ``key`` belonging to
> +``map_fd`` can be added or updated using the ``bpf_map_update_elem()`` libbpf > +function. ``key`` must be a pointer to a valid ``fd`` in the user space
> +program. The ``flags`` parameter can be used to control the update behaviour:

The "``key`` belonging to ``map_fd``" seems confusing.  Also, it is useful to 
highlight the ``key`` is a _socket_ ``fd``.

May be something like:

A socket-local storage can be added/updated locally to a socket identified by a 
_socket_ ``fd`` stored in the pointer ``key``.  The pointer ``value`` has the 
data to be added/updated to the socket ``fd``.  The type and size of ``value`` 
should be the same as the value type of the map definition.

Feel free to rephrase the above in a better way.

> +
> +- ``BPF_ANY`` will create storage for ``fd`` or update existing storage.
> +- ``BPF_NOEXIST`` will create storage for ``fd`` only if it did not already
> +  exist, otherwise the call will fail with ``-EEXIST``.
> +- ``BPF_EXIST`` will update existing storage for ``fd`` if it already exists,
> +  otherwise the call will fail with ``-ENOENT``.
> +
> +Returns ``0`` on success, or negative error in case of failure.
> +
> +bpf_map_lookup_elem()
> +~~~~~~~~~~~~~~~~~~~~~
> +
> +.. code-block:: c
> +
> +   int bpf_map_lookup_elem(int map_fd, const void *key, void *value)
> +
> +Socket-local storage for the socket identified by ``key`` belonging to
> +``map_fd`` can be retrieved using the ``bpf_map_lookup_elem()`` libbpf
> +function. ``key`` must be a pointer to a valid ``fd`` in the user space

Same here.

> +program. Returns ``0`` on success, or negative error in case of failure.
> +
> +bpf_map_delete_elem()
> +~~~~~~~~~~~~~~~~~~~~~
> +
> +.. code-block:: c
> +
> +   int bpf_map_delete_elem(int map_fd, const void *key)
> +
> +Socket-local storage for the socket identified by ``key`` belonging to
> +``map_fd`` can be deleted using the ``bpf_map_delete_elem()`` libbpf
> +function. Returns ``0`` on success, or negative error in case of failure.

Same here.

Alexei Starovoitov Dec. 9, 2022, 5:54 p.m. UTC | #4

On Fri, Dec 9, 2022 at 9:52 AM Martin KaFai Lau <[e-mail]> wrote:
>
> On 12/9/22 3:24 AM, Donald Hunter wrote:
> > Add documentation for the BPF_MAP_TYPE_SK_STORAGE including
> > kernel version introduced, usage and examples.
>
> Thanks for writing the doc for sk_storage!
>
> > +User space
> > +----------
> > +
> > +bpf_map_update_elem()
> > +~~~~~~~~~~~~~~~~~~~~~
> > +
> > +.. code-block:: c
> > +
> > +   int bpf_map_update_elem(int map_fd, const void *key, const void *value, __u64 flags)
> > +
> > +Socket-local storage for the socket identified by ``key`` belonging to
> > +``map_fd`` can be added or updated using the ``bpf_map_update_elem()`` libbpf > +function. ``key`` must be a pointer to a valid ``fd`` in the user space
> > +program. The ``flags`` parameter can be used to control the update behaviour:
>
> The "``key`` belonging to ``map_fd``" seems confusing.  Also, it is useful to
> highlight the ``key`` is a _socket_ ``fd``.
>
> May be something like:
>
> A socket-local storage can be added/updated locally to a socket identified by a
> _socket_ ``fd`` stored in the pointer ``key``.  The pointer ``value`` has the
> data to be added/updated to the socket ``fd``.  The type and size of ``value``
> should be the same as the value type of the map definition.
>
> Feel free to rephrase the above in a better way.
>
> > +
> > +- ``BPF_ANY`` will create storage for ``fd`` or update existing storage.
> > +- ``BPF_NOEXIST`` will create storage for ``fd`` only if it did not already
> > +  exist, otherwise the call will fail with ``-EEXIST``.
> > +- ``BPF_EXIST`` will update existing storage for ``fd`` if it already exists,
> > +  otherwise the call will fail with ``-ENOENT``.
> > +
> > +Returns ``0`` on success, or negative error in case of failure.
> > +
> > +bpf_map_lookup_elem()
> > +~~~~~~~~~~~~~~~~~~~~~
> > +
> > +.. code-block:: c
> > +
> > +   int bpf_map_lookup_elem(int map_fd, const void *key, void *value)
> > +
> > +Socket-local storage for the socket identified by ``key`` belonging to
> > +``map_fd`` can be retrieved using the ``bpf_map_lookup_elem()`` libbpf
> > +function. ``key`` must be a pointer to a valid ``fd`` in the user space
>
> Same here.
>
> > +program. Returns ``0`` on success, or negative error in case of failure.
> > +
> > +bpf_map_delete_elem()
> > +~~~~~~~~~~~~~~~~~~~~~
> > +
> > +.. code-block:: c
> > +
> > +   int bpf_map_delete_elem(int map_fd, const void *key)
> > +
> > +Socket-local storage for the socket identified by ``key`` belonging to
> > +``map_fd`` can be deleted using the ``bpf_map_delete_elem()`` libbpf
> > +function. Returns ``0`` on success, or negative error in case of failure.
>
> Same here.


Sorry Martin. I just applied it without seeing your comments.
Should I revert or this can be done in the follow up?

Martin KaFai Lau Dec. 9, 2022, 5:58 p.m. UTC | #5

On 12/9/22 9:54 AM, Alexei Starovoitov wrote:
> On Fri, Dec 9, 2022 at 9:52 AM Martin KaFai Lau <[e-mail]> wrote:
>>
>> On 12/9/22 3:24 AM, Donald Hunter wrote:
>>> Add documentation for the BPF_MAP_TYPE_SK_STORAGE including
>>> kernel version introduced, usage and examples.
>>
>> Thanks for writing the doc for sk_storage!
>>
>>> +User space
>>> +----------
>>> +
>>> +bpf_map_update_elem()
>>> +~~~~~~~~~~~~~~~~~~~~~
>>> +
>>> +.. code-block:: c
>>> +
>>> +   int bpf_map_update_elem(int map_fd, const void *key, const void *value, __u64 flags)
>>> +
>>> +Socket-local storage for the socket identified by ``key`` belonging to
>>> +``map_fd`` can be added or updated using the ``bpf_map_update_elem()`` libbpf > +function. ``key`` must be a pointer to a valid ``fd`` in the user space
>>> +program. The ``flags`` parameter can be used to control the update behaviour:
>>
>> The "``key`` belonging to ``map_fd``" seems confusing.  Also, it is useful to
>> highlight the ``key`` is a _socket_ ``fd``.
>>
>> May be something like:
>>
>> A socket-local storage can be added/updated locally to a socket identified by a
>> _socket_ ``fd`` stored in the pointer ``key``.  The pointer ``value`` has the
>> data to be added/updated to the socket ``fd``.  The type and size of ``value``
>> should be the same as the value type of the map definition.
>>
>> Feel free to rephrase the above in a better way.
>>
>>> +
>>> +- ``BPF_ANY`` will create storage for ``fd`` or update existing storage.
>>> +- ``BPF_NOEXIST`` will create storage for ``fd`` only if it did not already
>>> +  exist, otherwise the call will fail with ``-EEXIST``.
>>> +- ``BPF_EXIST`` will update existing storage for ``fd`` if it already exists,
>>> +  otherwise the call will fail with ``-ENOENT``.
>>> +
>>> +Returns ``0`` on success, or negative error in case of failure.
>>> +
>>> +bpf_map_lookup_elem()
>>> +~~~~~~~~~~~~~~~~~~~~~
>>> +
>>> +.. code-block:: c
>>> +
>>> +   int bpf_map_lookup_elem(int map_fd, const void *key, void *value)
>>> +
>>> +Socket-local storage for the socket identified by ``key`` belonging to
>>> +``map_fd`` can be retrieved using the ``bpf_map_lookup_elem()`` libbpf
>>> +function. ``key`` must be a pointer to a valid ``fd`` in the user space
>>
>> Same here.
>>
>>> +program. Returns ``0`` on success, or negative error in case of failure.
>>> +
>>> +bpf_map_delete_elem()
>>> +~~~~~~~~~~~~~~~~~~~~~
>>> +
>>> +.. code-block:: c
>>> +
>>> +   int bpf_map_delete_elem(int map_fd, const void *key)
>>> +
>>> +Socket-local storage for the socket identified by ``key`` belonging to
>>> +``map_fd`` can be deleted using the ``bpf_map_delete_elem()`` libbpf
>>> +function. Returns ``0`` on success, or negative error in case of failure.
>>
>> Same here.
> 
> 
> Sorry Martin. I just applied it without seeing your comments.
> Should I revert or this can be done in the follow up?
Ah, just noticed that also.  My bad that only catching up till v4.  It can 
definitely be a followup.

Patch

diff --git a/Documentation/bpf/map_sk_storage.rst b/Documentation/bpf/map_sk_storage.rst
new file mode 100644
index 000000000000..047e16c8aaa8
--- /dev/null
+++ b/Documentation/bpf/map_sk_storage.rst
@@ -0,0 +1,155 @@ 
+.. SPDX-License-Identifier: GPL-2.0-only
+.. Copyright (C) 2022 Red Hat, Inc.
+
+=======================
+BPF_MAP_TYPE_SK_STORAGE
+=======================
+
+.. note::
+   - ``BPF_MAP_TYPE_SK_STORAGE`` was introduced in kernel version 5.2
+
+``BPF_MAP_TYPE_SK_STORAGE`` is used to provide socket-local storage for BPF
+programs. A map of type ``BPF_MAP_TYPE_SK_STORAGE`` declares the type of storage
+to be provided and acts as the handle for accessing the socket-local
+storage. The values for maps of type ``BPF_MAP_TYPE_SK_STORAGE`` are stored
+locally with each socket instead of with the map. The kernel is responsible for
+allocating storage for a socket when requested and for freeing the storage when
+either the map or the socket is deleted.
+
+.. note::
+  - The key type must be ``int`` and ``max_entries`` must be set to ``0``.
+  - The ``BPF_F_NO_PREALLOC`` flag must be used when creating a map for
+    socket-local storage.
+
+Usage
+=====
+
+Kernel BPF
+----------
+
+bpf_sk_storage_get()
+~~~~~~~~~~~~~~~~~~~~
+
+.. code-block:: c
+
+   void *bpf_sk_storage_get(struct bpf_map *map, void *sk, void *value, u64 flags)
+
+Socket-local storage can be retrieved using the ``bpf_sk_storage_get()``
+helper. The helper gets the storage from ``sk`` that is associated with ``map``.
+If the ``BPF_LOCAL_STORAGE_GET_F_CREATE`` flag is used then
+``bpf_sk_storage_get()`` will create the storage for ``sk`` if it does not
+already exist. ``value`` can be used together with
+``BPF_LOCAL_STORAGE_GET_F_CREATE`` to initialize the storage value, otherwise it
+will be zero initialized. Returns a pointer to the storage on success, or
+``NULL`` in case of failure.
+
+.. note::
+   - ``sk`` is a kernel ``struct sock`` pointer for LSM or tracing programs.
+   - ``sk`` is a ``struct bpf_sock`` pointer for other program types.
+
+bpf_sk_storage_delete()
+~~~~~~~~~~~~~~~~~~~~~~~
+
+.. code-block:: c
+
+   long bpf_sk_storage_delete(struct bpf_map *map, void *sk)
+
+Socket-local storage can be deleted using the ``bpf_sk_storage_delete()``
+helper. The helper deletes the storage from ``sk`` that is identified by
+``map``. Returns ``0`` on success, or negative error in case of failure.
+
+User space
+----------
+
+bpf_map_update_elem()
+~~~~~~~~~~~~~~~~~~~~~
+
+.. code-block:: c
+
+   int bpf_map_update_elem(int map_fd, const void *key, const void *value, __u64 flags)
+
+Socket-local storage for the socket identified by ``key`` belonging to
+``map_fd`` can be added or updated using the ``bpf_map_update_elem()`` libbpf
+function. ``key`` must be a pointer to a valid ``fd`` in the user space
+program. The ``flags`` parameter can be used to control the update behaviour:
+
+- ``BPF_ANY`` will create storage for ``fd`` or update existing storage.
+- ``BPF_NOEXIST`` will create storage for ``fd`` only if it did not already
+  exist, otherwise the call will fail with ``-EEXIST``.
+- ``BPF_EXIST`` will update existing storage for ``fd`` if it already exists,
+  otherwise the call will fail with ``-ENOENT``.
+
+Returns ``0`` on success, or negative error in case of failure.
+
+bpf_map_lookup_elem()
+~~~~~~~~~~~~~~~~~~~~~
+
+.. code-block:: c
+
+   int bpf_map_lookup_elem(int map_fd, const void *key, void *value)
+
+Socket-local storage for the socket identified by ``key`` belonging to
+``map_fd`` can be retrieved using the ``bpf_map_lookup_elem()`` libbpf
+function. ``key`` must be a pointer to a valid ``fd`` in the user space
+program. Returns ``0`` on success, or negative error in case of failure.
+
+bpf_map_delete_elem()
+~~~~~~~~~~~~~~~~~~~~~
+
+.. code-block:: c
+
+   int bpf_map_delete_elem(int map_fd, const void *key)
+
+Socket-local storage for the socket identified by ``key`` belonging to
+``map_fd`` can be deleted using the ``bpf_map_delete_elem()`` libbpf
+function. Returns ``0`` on success, or negative error in case of failure.
+
+Examples
+========
+
+Kernel BPF
+----------
+
+This snippet shows how to declare socket-local storage in a BPF program:
+
+.. code-block:: c
+
+    struct {
+            __uint(type, BPF_MAP_TYPE_SK_STORAGE);
+            __uint(map_flags, BPF_F_NO_PREALLOC);
+            __type(key, int);
+            __type(value, struct my_storage);
+    } socket_storage SEC(".maps");
+
+This snippet shows how to retrieve socket-local storage in a BPF program:
+
+.. code-block:: c
+
+    SEC("sockops")
+    int _sockops(struct bpf_sock_ops *ctx)
+    {
+            struct my_storage *storage;
+            struct bpf_sock *sk;
+
+            sk = ctx->sk;
+            if (!sk)
+                    return 1;
+
+            storage = bpf_sk_storage_get(&socket_storage, sk, 0,
+                                         BPF_LOCAL_STORAGE_GET_F_CREATE);
+            if (!storage)
+                    return 1;
+
+            /* Use 'storage' here */
+
+            return 1;
+    }
+
+
+Please see the ``tools/testing/selftests/bpf`` directory for functional
+examples.
+
+References
+==========
+
+https://lwn.net/ml/netdev/[e-mail]/