Hi Andrew, > -----Original Message----- > From: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru> > Sent: Tuesday, October 4, 2022 4:23 PM > To: Wang, YuanX <yuanx.w...@intel.com>; dev@dpdk.org; Thomas > Monjalon <tho...@monjalon.net>; Ferruh Yigit <ferruh.yi...@amd.com> > Cc: ferruh.yi...@xilinx.com; m...@ashroe.eu; Li, Xiaoyun > <xiaoyun...@intel.com>; Singh, Aman Deep <aman.deep.si...@intel.com>; > Zhang, Yuying <yuying.zh...@intel.com>; Zhang, Qi Z > <qi.z.zh...@intel.com>; Yang, Qiming <qiming.y...@intel.com>; > jerinjac...@gmail.com; viachesl...@nvidia.com; > step...@networkplumber.org; Ding, Xuan <xuan.d...@intel.com>; > hpoth...@marvell.com; Tang, Yaqi <yaqi.t...@intel.com> > Subject: Re: [PATCH v7 2/4] ethdev: introduce protocol hdr based buffer split > > 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).
Just to confirm, do you mean that the payload as last segment should be treated as a length-based split(proto_hdr == 0)? If so, for this question, 'check that dataroom in the last segment mempool is sufficient> for up to MTU packet if Rx scatter is disabled' Is it not necessary to compare MTU size and mbuf_size? Because the check in length based split is sufficient. We will send v8 soon with above thought, please help to check. > > 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. Here a question that needs to be clarified, whether the segments are sequential or independent. I prefer the former because it's more readable. Furthermore, it consists with length based split, which also configures the lengths sequentially. In this case, the following situation does not exist: - seg1 ETH+IPv4+UDP - seg2 ETH+IPv6+UDP - seg3 0 For the case of repeating ETH, such as - seg1 - ETH, seg2 - IPv4, seg3 - UDP, seg4 - payload, as you suggested, we can omit ETH in the following segment. but IPV4-UDP and IPV6-UDP still need to be distinguished, follow our previous discussion (user wants to split at IPV4-UDP rather than IPV6-UDP although driver supports both). In this case, seg1 - ETH, seg2 - IPv4, seg3 - UDP, seg4 - payload, we set proto_hdr with: seg1 proto_hdr1=RTE_PTYPE_L2_ETHER seg2 proto_hdr2=RTE_PTYPE_L3_IPV4 seg3 proto_hdr3=RTE_PTYPE_L3_IPV4 | RTE_PTYPE_L4_UDP Thanks, Yuan
