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

["Ding, Xuan" <xuan.ding@intel.com>]

Hi Thomas,

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: 2022年3月3日 16:55
> To: Ding, Xuan <xuan.ding@intel.com>
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; andrew.rybchenko@oktetlabs.ru;
> dev@dpdk.org; viacheslavo@nvidia.com; Zhang, Qi Z <qi.z.zhang@intel.com>;
> Yu, Ping <ping.yu@intel.com>; Ding, Xuan <xuan.ding@intel.com>; Wang,
> YuanX <yuanx.wang@intel.com>; ajit.khaparde@broadcom.com;
> jerinj@marvell.com
> Subject: Re: [RFC] ethdev: introduce protocol type based header split
> 
> 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.

Thanks for your suggestion. The documentation will be enriched in next version.
Let me give a brief introduction here.

> 
> What happens if we have a non-0 proto and length/offset defined?
 
As Morten said, the proto field is exclude from the length field here.
For buffer split, the length/offset is needed.
For header split, the proto is needed.
As for offset field in header split, by default it is zero, it can also be
configured to decide the beginning of mbuf data buffer.

In conclusion, non-0 proto indicates PMDs can do header split. 
Length defines PMDs can do buffer split.

> 
> > +	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? 

When header split is configured, the split happens at the boundary of header and payload.
So, after the header, before payload.

> What means DEFAULT?
 
DEFAULT means no header split protocol type was defined.
As this time, even header split offload is configured in Rx queue,
the PMD won't do header split. Actually, using NONE is more accurate.

> What means IP, TCP_UDP and SCTP? Is it inner or outer?

Since header split happens after the header, so the IP/TCP/UDP/SCTP defines
the header type.
When take inner and outer into consideration, The definition should be refined here.
For example:
rte_eth_rx_header_split_protocol_type {
	RTE_ETH_RX_HEADER_SPLIT_NONE = 0,
	RTE_ETH_RX_HEADER_SPLIT_MAC,
	RTE_ETH_RX_HEADER_SPLIT_IPV4,
	RTE_ETH_RX_HEADER_SPLIT_IPV6,
	RTE_ETH_RX_HEADER_SPLIT_L3,
	RTE_ETH_RX_HEADER_SPLIT_TCP,
	RTE_ETH_RX_HEADER_SPLIT_UDP,
	RTE_ETH_RX_HEADER_SPLIT_SCTP,
	RTE_ETH_RX_HEADER_SPLIT_L4,
	RTE_ETH_RX_HEADER_SPLIT_INNER_MAC,
	RTE_ETH_RX_HEADER_SPLIT_INNER_IPV4,
	RTE_ETH_RX_HEADER_SPLIT_INNER_IPV6,
	RTE_ETH_RX_HEADER_SPLIT_INNER_L3,
	RTE_ETH_RX_HEADER_SPLIT_INNER_TCP,
	RTE_ETH_RX_HEADER_SPLIT_INNER_UDP,
	RTE_ETH_RX_HEADER_SPLIT_INNER_SCTP,
	RTE_ETH_RX_HEADER_SPLIT_INNER_L4,
};

Considering some NICs don’t distinguish the L2/L3/L4 in header split,
a separate L2/L3/L4 is also defined.

Thanks,
Xuan


>