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

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

Hi Andrew,

> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Wednesday, April 20, 2022 10:40 PM
> To: Ding, Xuan <xuan.ding@intel.com>; Jerin Jacob <jerinjacobk@gmail.com>;
> Wu, WenxuanX <wenxuanx.wu@intel.com>
> Cc: Thomas Monjalon <thomas@monjalon.net>; 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>; Wang, YuanX
> <yuanx.wang@intel.com>
> Subject: Re: [v4 1/3] ethdev: introduce protocol type based header split
> 
> 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?

Yes, you are right. In rte_eth_rxseg_split structure, even the length and offset
are documented, it is hard to understand just from the naming.

Thanks,
Xuan

> 
> >
> >>
> >> 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.