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

[Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx>]

On Fri, Dec 9, 2022 at 9:52 AM Martin KaFai Lau <martin.lau@xxxxxxxxx> 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?