Re: [v4 1/3] ethdev: introduce protocol type based header split

[Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>]

On 4/12/22 19:40, Ding, Xuan wrote:
> Hi Jacob,
> 
>> -----Original Message-----
>> From: Jerin Jacob <jerinjacobk@gmail.com>
>> Sent: Thursday, April 7, 2022 9:27 PM
>> To: Wu, WenxuanX <wenxuanx.wu@intel.com>
>> Cc: Thomas Monjalon <thomas@monjalon.net>; Andrew Rybchenko
>> <andrew.rybchenko@oktetlabs.ru>; Li, Xiaoyun <xiaoyun.li@intel.com>;
>> Singh, Aman Deep <aman.deep.singh@intel.com>; Zhang, Yuying
>> <yuying.zhang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; dpdk-dev
>> <dev@dpdk.org>; Stephen Hemminger <stephen@networkplumber.org>;
>> Morten Brørup <mb@smartsharesystems.com>; Viacheslav Ovsiienko
>> <viacheslavo@nvidia.com>; Yu, Ping <ping.yu@intel.com>; Ding, Xuan
>> <xuan.ding@intel.com>; Wang, YuanX <yuanx.wang@intel.com>
>> Subject: Re: [v4 1/3] ethdev: introduce protocol type based header split
>>
>> On Sat, Apr 2, 2022 at 4:33 PM <wenxuanx.wu@intel.com> wrote:
>>>
>>> From: Xuan Ding <xuan.ding@intel.com>
>>>
>>> Header split consists of splitting a received packet into two separate
>>> regions based on the packet content. The split happens after the
>>> packet header and before the packet payload. 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.
>>>
>>> Currently, Rx buffer split supports length and offset based packet split.
>>> Although header split is a subset of buffer split, configuring buffer
>>> split based on length is not suitable for NICs that do split based on
>>> header protocol types. Because tunneling makes the conversion from
>>> length to protocol type impossible.
>>>
>>> This patch extends the current buffer split to support protocol type
>>> and offset based header split. A new proto field is introduced in the
>>> rte_eth_rxseg_split structure reserved field to specify header
>>> protocol 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, both inner and outer
>>> L2/L3/L4 level header split can be supported.
>>>
>>> For example, let's suppose we configured the Rx queue with the
>>> following segments:
>>>      seg0 - pool0, off0=2B
>>>      seg1 - pool1, off1=128B
>>>
>>> With header split type configured with RTE_ETH_RX_HEADER_SPLIT_UDP,
>>> the packet consists of MAC_IP_UDP_PAYLOAD will be split like following:
>>>      seg0 - udp header @ RTE_PKTMBUF_HEADROOM + 2 in mbuf from pool0
>>
>> If we set rte_eth_rxseg_split::proto = RTE_ETH_RX_HEADER_SPLIT_UDP and
>> rte_eth_rxseg_split.offset = 2, What will be the content for seg0, Will it be,
>> - offset as Starts atUDP Header
>> - size of segment as MAX(size of UDP header + 2, 128(as seg 1 start from128).
>> Right? If not, Please describe
> 
> Proto defines the location in packet for split.
> Offset defines data buffer from beginning of mbuf data buffer, it can be zero.
> With proto and offset configured, packets received will be split into two segments.
> 
> So in this configuration, the seg0 content is UDP header, the seg1 content is the payload.
> Size of seg0 is size of UDP header, size of seg1 is size of payload.
> rte_eth_rxseg_split.offset = 2/128 decides the mbuf offset, rather than segment size.

Above discussion proves that definition of the struct
rte_eth_rxseg_split is misleading. It is hard to catch
from naming that length defines a maximum data amount
to be copied, but office is a an offset in destination
mbuf. The structure is still experimental and I think
we should improve naming: offset -> mbuf_offset?

> 
>>
>> Also, I don't think we need duplate
>> rte_eth_rx_header_split_protocol_type instead we can reuse existing
>> RTE_PTYPE_*  flags.
> 
> That's a good idea. Yes, I can use the RTE_PTYPE_* here. My only
> concern is the 32-bits RTE_PTYPE_* will run out of the 32-bits reserved fields.
> If this proposal is agreed, I will use RTE_PTYPE_* instead of rte_eth_rx_header_split_protocol_type.
> 
> Best Regards,
> Xuan
> 
>>
>>
>>>      seg1 - payload @ 128 in mbuf from pool1
>>>
>>> The memory attributes for the split parts may differ either - for
>>> example the mempool0 and mempool1 belong to dpdk memory and
>> external
>>> memory, respectively.