Re: [PATCH bpf-next v2 1/1] docs: BPF_MAP_TYPE_XSKMAP

[Akira Yokosawa <akiyks@gmail.com>]

Hi Maryam,

On Thu, 17 Nov 2022 10:44:46 -0500, mtahhan@redhat.com wrote:
> From: Maryam Tahhan <mtahhan@redhat.com>
> 
> Add documentation for BPF_MAP_TYPE_XSKMAP
> including kernel version introduced, usage
> and examples.
> 
> Signed-off-by: Maryam Tahhan <mtahhan@redhat.com>
> 
> ---
> v2:
> - Fixed typos + incorrect return type references.
> - Adjusted examples to use __u32 and fixed references to key_size.
> - Changed `AF_XDP socket` references to XSK.
> - Added note re map key and value size.
> ---
>  Documentation/bpf/map_xskmap.rst | 167 +++++++++++++++++++++++++++++++
>  1 file changed, 167 insertions(+)
>  create mode 100644 Documentation/bpf/map_xskmap.rst
> 
> diff --git a/Documentation/bpf/map_xskmap.rst b/Documentation/bpf/map_xskmap.rst
> new file mode 100644
[...]
> +Kernel BPF
> +----------
> +.. c:function::
> +     long bpf_redirect_map(struct bpf_map *map, u32 key, u64 flags)
> +
> + Redirect the packet to the endpoint referenced by ``map`` at index ``key``.
> + For ``BPF_MAP_TYPE_XSKMAP`` this map contains references to XSK FDs
> + for sockets attached to a netdev's queues.
> +
> + .. note::
> +    If the map is empty at an index, the packet is dropped. This means that it is
> +    necessary to have an XDP program loaded with at least one XSK in the
> +    XSKMAP to be able to get any traffic to user space through the socket.
> +
> +.. c:function::
> +    void *bpf_map_lookup_elem(struct bpf_map *map, const void *key)
> +
> + XSK entry references of type ``struct xdp_sock *`` can be retrieved using the
> + ``bpf_map_lookup_elem()`` helper.
> +
> +Userspace
> +---------
> +.. note::
> +    XSK entries can only be updated/deleted from user space and not from
> +    an eBPF program. Trying to call these functions from a kernel eBPF program will
> +    result in the program failing to load and a verifier warning.
> +
> +.. c:function::
> +	int bpf_map_update_elem(int fd, const void *key, const void *value, __u64 flags)
> +
> + XSK entries can be added or updated using the ``bpf_map_update_elem()``
> + helper. The ``key`` parameter is equal to the queue_id of the queue the XSK
> + is attaching to. And the ``value`` parameter is the FD value of that socket.
> +
> + Under the hood, the XSKMAP update function uses the XSK FD value to retrieve the
> + associated ``struct xdp_sock`` instance.
> +
> + The flags argument can be one of the following:
> +
> +  - BPF_ANY: Create a new element or update an existing element.
> +  - BPF_NOEXIST: Create a new element only if it did not exist.
> +  - BPF_EXIST: Update an existing element.
> +
> +.. c:function::
> +    int bpf_map_lookup_elem(int fd, const void *key, void *value)
> +
So you have two declarations of bpf_map_lookup_elem() in map_xskmap.rst.

This will cause "make htmldocs" with Sphinx >=3.1 to emit a warning of:

/linux/Documentation/bpf/map_xskmap.rst:100: WARNING: Duplicate C declaration, also defined at map_xskmap:71.
Declaration is '.. c:function:: int bpf_map_lookup_elem(int fd, const void *key, void *value)'.

, in addition to a bunch of similar warnings observed at bpf-next:

/linux/Documentation/bpf/map_cpumap.rst:50: WARNING: Duplicate C declaration, also defined at map_array:43.
Declaration is '.. c:function:: int bpf_map_update_elem(int fd, const void *key, const void *value, __u64 flags);'.
/linux/Documentation/bpf/map_cpumap.rst:72: WARNING: Duplicate C declaration, also defined at map_array:35.
Declaration is '.. c:function:: int bpf_map_lookup_elem(int fd, const void *key, void *value);'.
/linux/Documentation/bpf/map_hash.rst:37: WARNING: Duplicate C declaration, also defined at map_array:43.
Declaration is '.. c:function:: long bpf_map_update_elem(struct bpf_map *map, const void *key, const void *value, u64 flags)'.
... [bunch of similar warnings]


You might want to say you don't care, but they would annoy those
who do test "make htmldocs".

So let me explain why sphinx complains.

C domain declarations in kernel documentation are for kernel APIs.
By default, c:function declarations belong to the top-level namespace,
which is intended for kernel APIs.

IIUC, most APIs described in map*.rst files don't belong to kernel.
So I think the way to go is to use the c:namespace directive.

See: https://www.sphinx-doc.org/en/master/usage/restructuredtext/domains.html#namespacing

As mentioned there, namespacing works with Sphinx >=3.1.
Currently, kernel documentation build scripts support only the
"c:namespace" directive, which means you can't switch namespaces in the
middle of a .rst file. This limitation comes from the fact that Sphinx
1.7.9 is still in the list for htmldocs at the moment and build scripts
emulate namespacing for Sphinx <3.1 in a limited way.

So please avoid putting function declarations of the same name in
a .rst file.

The other duplicate warnings shown above can be silenced by the
change attached below. It is only as a suggestion and I'm not putting
a S-o-b tag.

Hope this helps,

Akira

--------
diff --git a/Documentation/bpf/map_array.rst b/Documentation/bpf/map_array.rst
index 97bb80333254..68545702ca78 100644
--- a/Documentation/bpf/map_array.rst
+++ b/Documentation/bpf/map_array.rst
@@ -1,6 +1,8 @@
 .. SPDX-License-Identifier: GPL-2.0-only
 .. Copyright (C) 2022 Red Hat, Inc.
 
+.. c:namespace:: BPF.MAP_ARRAY
+
 ================================================
 BPF_MAP_TYPE_ARRAY and BPF_MAP_TYPE_PERCPU_ARRAY
 ================================================
diff --git a/Documentation/bpf/map_cpumap.rst b/Documentation/bpf/map_cpumap.rst
index 61a797a86342..25e05d14ec82 100644
--- a/Documentation/bpf/map_cpumap.rst
+++ b/Documentation/bpf/map_cpumap.rst
@@ -1,6 +1,8 @@
 .. SPDX-License-Identifier: GPL-2.0-only
 .. Copyright (C) 2022 Red Hat, Inc.
 
+.. c:namespace:: BPF.MAP_CPUMAP
+
 ===================
 BPF_MAP_TYPE_CPUMAP
 ===================
diff --git a/Documentation/bpf/map_hash.rst b/Documentation/bpf/map_hash.rst
index e85120878b27..3ac93ccf2b0e 100644
--- a/Documentation/bpf/map_hash.rst
+++ b/Documentation/bpf/map_hash.rst
@@ -1,6 +1,8 @@
 .. SPDX-License-Identifier: GPL-2.0-only
 .. Copyright (C) 2022 Red Hat, Inc.
 
+.. c:namespace:: BPF.MAP_HASH
+
 ===============================================
 BPF_MAP_TYPE_HASH, with PERCPU and LRU Variants
 ===============================================
diff --git a/Documentation/bpf/map_lpm_trie.rst b/Documentation/bpf/map_lpm_trie.rst
index 31be1aa7ba2c..c934c3e2bcb7 100644
--- a/Documentation/bpf/map_lpm_trie.rst
+++ b/Documentation/bpf/map_lpm_trie.rst
@@ -1,6 +1,8 @@
 .. SPDX-License-Identifier: GPL-2.0-only
 .. Copyright (C) 2022 Red Hat, Inc.
 
+.. c:namespace:: BPF.MAP_LPM_TRIE
+
 =====================
 BPF_MAP_TYPE_LPM_TRIE
 =====================
diff --git a/Documentation/bpf/map_of_maps.rst b/Documentation/bpf/map_of_maps.rst
index 07212b9227a9..f59cd0e3a72c 100644
--- a/Documentation/bpf/map_of_maps.rst
+++ b/Documentation/bpf/map_of_maps.rst
@@ -1,6 +1,8 @@
 .. SPDX-License-Identifier: GPL-2.0-only
 .. Copyright (C) 2022 Red Hat, Inc.
 
+.. c:namespace:: BPF.MAP_OF_MAPS
+
 ========================================================
 BPF_MAP_TYPE_ARRAY_OF_MAPS and BPF_MAP_TYPE_HASH_OF_MAPS
 ========================================================
diff --git a/Documentation/bpf/map_queue_stack.rst b/Documentation/bpf/map_queue_stack.rst
index f20e31a647b9..abc8ed569900 100644
--- a/Documentation/bpf/map_queue_stack.rst
+++ b/Documentation/bpf/map_queue_stack.rst
@@ -1,6 +1,8 @@
 .. SPDX-License-Identifier: GPL-2.0-only
 .. Copyright (C) 2022 Red Hat, Inc.
 
+.. c:namespace:: BPF.MAP_QUEUE_STACK
+
 =========================================
 BPF_MAP_TYPE_QUEUE and BPF_MAP_TYPE_STACK
 =========================================
--