Re: [RFC] ethdev: introduce protocol type based header split

[Thomas Monjalon <thomas@monjalon.net>]

03/03/2022 07:01, xuan.ding@intel.com:
> From: Xuan Ding <xuan.ding@intel.com>
> 
> Header split consists of splitting a received packet into two separate
> regions based on the packet content. Splitting is usually between the
> packet header that can be posted to a dedicated buffer and the packet
> payload that can be posted to a different buffer. This kind of splitting
> is useful in some use cases, such as GPU. GPU can directly process the
> payload part and improve the performance significantly.
> 
> Currently, Rx buffer split supports length and offset based packet split.
> This is not suitable for some NICs that do split based on protocol types.
> Tunneling makes the conversion from offset to protocol inaccurate.
> 
> This patch extends the current buffer split to support protocol based
> header split. A new proto field is introduced in the rte_eth_rxseg_split
> structure reserved field to specify header split type.
> 
> With Rx offload flag RTE_ETH_RX_OFFLOAD_HEADER_SPLIT enabled and protocol
> type configured, PMD will split the ingress packets into two separate
> regions. Currently, L2/L3/L4 level header split is supported.
> 
> Signed-off-by: Xuan Ding <xuan.ding@intel.com>
> Signed-off-by: Yuan Wang <yuanx.wang@intel.com>
> ---
>  lib/ethdev/rte_ethdev.c |  2 +-
>  lib/ethdev/rte_ethdev.h | 17 ++++++++++++++++-
>  2 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> index 70c850a2f1..d37c8f9d7e 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -1784,7 +1784,7 @@ rte_eth_rx_queue_setup(uint16_t port_id, uint16_t rx_queue_id,
>  							   &dev_info);
>  			if (ret != 0)
>  				return ret;
> -		} else {
> +		} else if (!(rx_conf->offloads & RTE_ETH_RX_OFFLOAD_HEADER_SPLIT)) {
>  			RTE_ETHDEV_LOG(ERR, "No Rx segmentation offload configured\n");
>  			return -EINVAL;
>  		}
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index c2d1f9a972..6743648c22 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -1202,7 +1202,8 @@ struct rte_eth_rxseg_split {
>  	struct rte_mempool *mp; /**< Memory pool to allocate segment from. */
>  	uint16_t length; /**< Segment data length, configures split point. */
>  	uint16_t offset; /**< Data offset from beginning of mbuf data buffer. */
> -	uint32_t reserved; /**< Reserved field. */
> +	uint16_t proto;

If it is not explicitly documented, it cannot be accepted.

What happens if we have a non-0 proto and length/offset defined?

> +	uint16_t reserved; /**< Reserved field. */
>  };
[...]
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this structure may change without prior notice.

This is not a structure.

> + * This enum indicates the header split protocol type
> + */
> +enum rte_eth_rx_header_split_protocol_type {
> +	RTE_ETH_RX_HEADER_SPLIT_DEFAULT = 0,
> +	RTE_ETH_RX_HEADER_SPLIT_INNER_L2,
> +	RTE_ETH_RX_HEADER_SPLIT_OUTER_L2,
> +	RTE_ETH_RX_HEADER_SPLIT_IP,
> +	RTE_ETH_RX_HEADER_SPLIT_TCP_UDP,
> +	RTE_ETH_RX_HEADER_SPLIT_SCTP
> +};

Lack of documentation.
Where the split should happen? before or after the header?
What means DEFAULT?
What means IP, TCP_UDP and SCTP? Is it inner or outer?