Subject: Re: [PATCH v12 13/26] Documentation: add ULP DDP offload documentation

From: Simon Horman <[email-masked]>
Date: Sat, 15 Jul 2023 11:32:51 +0100

On Wed, Jul 12, 2023 at 04:15:00PM +0000, Aurelien Aptel wrote:
> From: Yoray Zack <[email-masked]>
> 
> Document the new ULP DDP API and add it under "networking".
> Use NVMe-TCP implementation as an example.

...

> diff --git a/Documentation/networking/ulp-ddp-offload.rst b/Documentation/networking/ulp-ddp-offload.rst
> new file mode 100644
> index 000000000000..b8d7c9c4d6b0
> --- /dev/null
> +++ b/Documentation/networking/ulp-ddp-offload.rst

...

> +Device configuration
> +====================
> +
> +During driver initialization the driver sets the following
> +:c:type:`struct net_device <net_device>` properties:
> +
> +* The ULP DDP capabilities it supports
> +  in :c:type:`struct ulp_ddp_netdev_caps <ulp_ddp_caps>`
> +* The ULP DDP operations pointer in :c:type:`struct ulp_ddp_dev_ops`.

'make htmldocs' seems a little unhappy about this for some reason:

  .../ulp-ddp-offload.rst:74: WARNING: Unparseable C cross-reference: 'struct ulp_ddp_dev_ops'
  Invalid C declaration: Expected identifier in nested name, got keyword: struct [error at 6]
     struct ulp_ddp_dev_ops
     ------^

> +
> +The current list of capabilities is represented as a bitset:
> +
> +.. code-block:: c
> +
> +  enum {
> +	ULP_DDP_C_NVME_TCP_BIT,
> +	ULP_DDP_C_NVME_TCP_DDGST_RX_BIT,
> +	/* add capabilities above */
> +	ULP_DDP_C_COUNT,
> +  };
> +
> +The enablement of capabilities can be controlled from userspace via
> +netlink. See Documentation/networking/ethtool-netlink.rst for more
> +details.
> +
> +Later, after the L5P completes its handshake, the L5P queries the
> +driver for its runtime limitations via the :c:member:`limits` operation:
> +
> +.. code-block:: c
> +
> + int (*limits)(struct net_device *netdev,
> +	       struct ulp_ddp_limits *lim);
> +
> +
> +All L5P share a common set of limits and parameters (:c:type:`struct ulp_ddp_limits`):

Likewise, here:

  .../ulp-ddp-offload.rst:100: WARNING: Unparseable C cross-reference: 'struct ulp_ddp_limits'
  Invalid C declaration: Expected identifier in nested name, got keyword: struct [error at 6]
    struct ulp_ddp_limits
    ------^
> +
> +.. code-block:: c
> +
> + /**
> +  * struct ulp_ddp_limits - Generic ulp ddp limits: tcp ddp
> +  * protocol limits.
> +  * Add new instances of ulp_ddp_limits in the union below (nvme-tcp, etc.).
> +  *
> +  * @type:		type of this limits struct
> +  * @max_ddp_sgl_len:	maximum sgl size supported (zero means no limit)
> +  * @io_threshold:	minimum payload size required to offload
> +  * @tls:		support for ULP over TLS
> +  * @nvmeotcp:		NVMe-TCP specific limits
> +  */
> + struct ulp_ddp_limits {
> +	enum ulp_ddp_type	type;
> +	int			max_ddp_sgl_len;
> +	int			io_threshold;
> +	bool			tls:1;
> +	union {
> +		/* ... protocol-specific limits ... */
> +		struct nvme_tcp_ddp_limits nvmeotcp;
> +	};
> + };

...

> +Asynchronous teardown
> +---------------------
> +
> +To teardown the association between tags and buffers and allow tag reuse NIC HW
> +is called by the NIC driver during `teardown`. This operation may be
> +performed either synchronously or asynchronously. In asynchronous teardown,
> +`teardown` returns immediately without unmapping NIC HW buffers. Later,
> +when the unmapping completes by NIC HW, the NIC driver will call up to L5P
> +using :c:member:`ddp_teardown_done` of :c:type:`struct ulp_ddp_ulp_ops`:

And here:

  .../ulp-ddp-offload.rst:283: WARNING: Unparseable C cross-reference: 'struct ulp_ddp_ulp_ops'
  Invalid C declaration: Expected identifier in nested name, got keyword: struct [error at 6]
    struct ulp_ddp_ulp_ops
    ------^

> +
> +.. code-block:: c
> +
> + void (*ddp_teardown_done)(void *ddp_ctx);
> +
> +The `ddp_ctx` parameter passed in `ddp_teardown_done` is the same on provided
> +in `teardown` and it is used to carry some context about the buffers
> +and tags that are released.
> +
> +Resync handling
> +===============
> +
> +RX
> +--
> +In presence of packet drops or network packet reordering, the device may lose
> +synchronization between the TCP stream and the L5P framing, and require a
> +resync with the kernel's TCP stack. When the device is out of sync, no offload
> +takes place, and packets are passed as-is to software. Resync is very similar
> +to TLS offload (see documentation at Documentation/networking/tls-offload.rst)
> +
> +If only packets with L5P data are lost or reordered, then resynchronization may
> +be avoided by NIC HW that keeps tracking PDU headers. If, however, PDU headers
> +are reordered, then resynchronization is necessary.
> +
> +To resynchronize hardware during traffic, we use a handshake between hardware
> +and software. The NIC HW searches for a sequence of bytes that identifies L5P
> +headers (i.e., magic pattern).  For example, in NVMe-TCP, the PDU operation
> +type can be used for this purpose.  Using the PDU header length field, the NIC
> +HW will continue to find and match magic patterns in subsequent PDU headers. If
> +the pattern is missing in an expected position, then searching for the pattern
> +starts anew.
> +
> +The NIC will not resume offload when the magic pattern is first identified.
> +Instead, it will request L5P software to confirm that indeed this is a PDU
> +header. To request confirmation the NIC driver calls up to L5P using
> +:c:member:`*resync_request` of :c:type:`struct ulp_ddp_ulp_ops`:

And here:

  .../ulp-ddp-offload.rst:321: WARNING: Unparseable C cross-reference: '*resync_request'
  Invalid C declaration: Expected identifier in nested name. [error at 0]
    *resync_request
    ^
  .../ulp-ddp-offload.rst:321: WARNING: Unparseable C cross-reference: 'struct ulp_ddp_ulp_ops'
  Invalid C declaration: Expected identifier in nested name, got keyword: struct [error at 6]
    struct ulp_ddp_ulp_ops
    ------^

> +
> +.. code-block:: c
> +
> +  bool (*resync_request)(struct sock *sk, u32 seq, u32 flags);
> +
> +The `seq` parameter contains the TCP sequence of the last byte in the PDU header.
> +The `flags` parameter contains a flag (`ULP_DDP_RESYNC_PENDING`) indicating whether
> +a request is pending or not.
> +L5P software will respond to this request after observing the packet containing
> +TCP sequence `seq` in-order. If the PDU header is indeed there, then L5P
> +software calls the NIC driver using the :c:member:`resync` function of
> +the :c:type:`struct ulp_ddp_dev_ops <ulp_ddp_ops>` inside the :c:type:`struct
> +net_device <net_device>` while passing the same `seq` to confirm it is a PDU
> +header.
> +
> +.. code-block:: c
> +
> + void (*resync)(struct net_device *netdev,
> +		struct sock *sk, u32 seq);

...