Re: [PATCH v7 2/4] ethdev: introduce protocol hdr based buffer split

From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
To: "Wang, YuanX" <yuanx.wang@intel.com>,
	"dev@dpdk.org" <dev@dpdk.org>,
	Thomas Monjalon <thomas@monjalon.net>,
	Ferruh Yigit <ferruh.yigit@amd.com>
Cc: "ferruh.yigit@xilinx.com" <ferruh.yigit@xilinx.com>,
	"mdr@ashroe.eu" <mdr@ashroe.eu>,
	"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>,
	"Yang, Qiming" <qiming.yang@intel.com>,
	"jerinjacobk@gmail.com" <jerinjacobk@gmail.com>,
	"viacheslavo@nvidia.com" <viacheslavo@nvidia.com>,
	"stephen@networkplumber.org" <stephen@networkplumber.org>,
	"Ding, Xuan" <xuan.ding@intel.com>,
	"hpothula@marvell.com" <hpothula@marvell.com>,
	"Tang, Yaqi" <yaqi.tang@intel.com>
Subject: Re: [PATCH v7 2/4] ethdev: introduce protocol hdr based buffer split
Date: Tue, 4 Oct 2022 11:22:44 +0300	[thread overview]
Message-ID: <6420052f-5f21-2416-362a-f1a0527924d2@oktetlabs.ru> (raw)
In-Reply-To: <PH7PR11MB69537A73E3AF44D24C05842E855A9@PH7PR11MB6953.namprd11.prod.outlook.com>

On 10/4/22 05:48, Wang, YuanX wrote:
> Hi Andrew,
> 
>> -----Original Message-----
>> On 10/2/22 00:05, Yuan Wang wrote:
>>> +
>>> +			/* skip the payload */
>>
>> Sorry, it is confusing. What do you mean here?
> 
> Because setting n proto_hdr will generate (n+1) segments. If we want to split the packet into n segments, we only need to check the first (n-1) proto_hdr.
> For example, for ETH-IPV4-UDP-PAYLOAD, if we want to split after the UDP header, we only need to set and check the UDP header in the first segment.
> 
> Maybe mask is not a good way, so we will use index to filter out the check of proto_hdr inside the last segment.

I see your point and understand the problem now.
Thinking a bit more about it I realize that consistency check
here should be more sophisticated.
It should not allow:
  - seg1 - length-based, seg2 - proto-based, seg3 - payload
  - seg1 - proto-based, seg2 - legnth-based, seg3 - proto-based, seg4 - 
payload
I.e. no protocol-based split after length-based.
But should allow:
  - seg1 - proto-based, seg2 - legnth-based, seg3 - payload
I.e. length based split after protocol-based.

Taking the last point above into account, proto_hdr in the last
segment should be 0 like in length-based split (not
RTE_PTYPE_ALL_MASK).

It is an interesting question how to request:
  - seg1 - ETH, seg2 - IPv4, seg3 - UDP, seg4 - payload
Should we really repeat ETH in seg2->proto_hdr and
seg3->proto_hdr header and IPv4 in seg3->proto_hdr again?
I tend to say no since when packet comes to seg2 it already
has no ETH header.

If so, how to handle configuration when ETH is repeat in seg2?
For example,
   - seg1 ETH+IPv4+UDP
   - seg2 ETH+IPv6+UDP
   - seg2 0
Should we deny it or should we define behaviour like.
If a packet does not match segX proto_hdr, the segment is
skipped and segX+1 considered.
Of course, not all drivers/HW supports it. If so, such
configuration should be just discarded by the driver itself.