From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 704B641B96; Wed, 1 Feb 2023 04:14:52 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 00F884113F; Wed, 1 Feb 2023 04:14:52 +0100 (CET) Received: from smtpbguseast2.qq.com (smtpbguseast2.qq.com [54.204.34.130]) by mails.dpdk.org (Postfix) with ESMTP id A45834021F; Wed, 1 Feb 2023 04:14:49 +0100 (CET) X-QQ-mid: Yeas52t1675221282t528t26386 Received: from 7082A6556EBF4E69829842272A565F7C (jiawenwu@trustnetic.com [183.129.236.74]) X-QQ-SSF: 00400000000000F0FK9000000000000 From: =?utf-8?b?Smlhd2VuIFd1?= To: "'Ferruh Yigit'" , Cc: References: <20230118060039.3074016-1-jiawenwu@trustnetic.com> <20230118060039.3074016-4-jiawenwu@trustnetic.com> <731ecbc6-b338-899e-62be-ecda963951a1@amd.com> In-Reply-To: <731ecbc6-b338-899e-62be-ecda963951a1@amd.com> Subject: RE: [PATCH 3/8] net/txgbe: fix packet type to parse from offload flags Date: Wed, 1 Feb 2023 11:14:37 +0800 Message-ID: <027501d935eb$50bdc120$f2394360$@trustnetic.com> MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Mailer: Microsoft Outlook 16.0 Content-Language: zh-cn Thread-Index: AQIBtVZXYxbb1ShePjndn/GGR7Jz2QIgAuTUAd745r2uSGuDwA== X-QQ-SENDSIZE: 520 Feedback-ID: Yeas:trustnetic.com:qybglogicsvr:qybglogicsvr5 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Friday, January 27, 2023 11:37 PM, Ferruh Yigit wrote: > On 1/18/2023 6:00 AM, Jiawen Wu wrote: > > In some external applications, developers may fill in wrong > > packet_type in rte_mbuf for transmission. It will result in Tx ring > > hang when Tx checksum offload is on. So change it to parse from = ol_flags. > > >=20 > Can you please give more information on what packet_type value is = causing > problem in Tx path? >=20 > > Fixes: ca46fcd753b1 ("net/txgbe: support Tx with hardware offload") > > Cc: stable@dpdk.org > > > > Signed-off-by: Jiawen Wu > > --- > > drivers/net/txgbe/txgbe_rxtx.c | 19 ++++++++++++++----- > > 1 file changed, 14 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/net/txgbe/txgbe_rxtx.c > > b/drivers/net/txgbe/txgbe_rxtx.c index ae70ca3beb..e91aaf60ef 100644 > > --- a/drivers/net/txgbe/txgbe_rxtx.c > > +++ b/drivers/net/txgbe/txgbe_rxtx.c > > @@ -516,20 +516,21 @@ tx_desc_ol_flags_to_cmdtype(uint64_t ol_flags) > > return cmdtype; > > } > > > > -static inline uint8_t > > -tx_desc_ol_flags_to_ptid(uint64_t oflags, uint32_t ptype) > > +static inline uint32_t > > +tx_desc_ol_flags_to_ptype(uint64_t oflags) > > { > > + uint32_t ptype; > > bool tun; > > > > - if (ptype) > > - return txgbe_encode_ptype(ptype); > > - > > /* Only support flags in TXGBE_TX_OFFLOAD_MASK */ > > tun =3D !!(oflags & RTE_MBUF_F_TX_TUNNEL_MASK); > > > > /* L2 level */ > > ptype =3D RTE_PTYPE_L2_ETHER; > > if (oflags & RTE_MBUF_F_TX_VLAN) > > + ptype |=3D (tun ? RTE_PTYPE_INNER_L2_ETHER_VLAN : > > +RTE_PTYPE_L2_ETHER_VLAN); > > + > > + if (oflags & RTE_MBUF_F_TX_QINQ) //tun + qinq is not supported >=20 > checkpatch is complaining about c99 comment syntax ('//'). >=20 > > ptype |=3D RTE_PTYPE_L2_ETHER_VLAN; > > > > /* L3 level */ > > @@ -587,6 +588,14 @@ tx_desc_ol_flags_to_ptid(uint64_t oflags, > > uint32_t ptype) > > break; > > } > > > > + return ptype; > > +} > > + > > +static inline uint8_t > > +tx_desc_ol_flags_to_ptid(uint64_t oflags, uint32_t ptype) { > > + ptype =3D tx_desc_ol_flags_to_ptype(oflags); > > + >=20 > This function get 'ptype' as parameter and immediately overwrites is = with > calculated value, what is the point of getting 'ptype' as argument. >=20 > > return txgbe_encode_ptype(ptype); > > } > > >=20 > Overall why 'ptype' is calculated for Tx path, I see this value is = used to see if it is > tunneled packet or not, is there any other usage of ptype in this = path? If not why > parse all packet types? >=20 If Tx checksum offload or TSO is on, a context descriptor is needed for = our hardware, which contains the length of each packet layer and the = packet type. If the packet type and length do not strictly match, it will cause Tx = ring hang. It's not just for tunnel packets. But most cases are caused = by developers encap/decap at the application layer. However, we can flexibly set the packet type. For example, if there is = no inner checksum for a VXLAN packet, we can only regard it as a UDP = packet.