From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 6600B23D; Tue, 24 Apr 2018 12:56:14 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 24 Apr 2018 03:56:12 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.49,322,1520924400"; d="scan'208";a="50360015" Received: from irsmsx110.ger.corp.intel.com ([163.33.3.25]) by orsmga001.jf.intel.com with ESMTP; 24 Apr 2018 03:56:11 -0700 Received: from irsmsx155.ger.corp.intel.com (163.33.192.3) by irsmsx110.ger.corp.intel.com (163.33.3.25) with Microsoft SMTP Server (TLS) id 14.3.319.2; Tue, 24 Apr 2018 11:56:10 +0100 Received: from irsmsx102.ger.corp.intel.com ([169.254.2.164]) by irsmsx155.ger.corp.intel.com ([169.254.14.143]) with mapi id 14.03.0319.002; Tue, 24 Apr 2018 11:56:09 +0100 From: "Ananyev, Konstantin" To: Ophir Munk , "Hu, Jiayu" , "dev@dpdk.org" CC: Thomas Monjalon , Olga Shern , Pascal Mazon , "stable@dpdk.org" Thread-Topic: [PATCH v1] gso: fix marking TCP checksum flag in TCP segments Thread-Index: AQHT2rl94CoPk27HYUCsaOLb2rnsLKQPm2oAgAAjyvA= Date: Tue, 24 Apr 2018 10:56:09 +0000 Message-ID: <2601191342CEEE43887BDE71AB977258AEA5081C@IRSMSX102.ger.corp.intel.com> References: <1524406859-29585-1-git-send-email-ophirmu@mellanox.com> In-Reply-To: Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZGYzOGY3YWQtMTI0MC00NWMzLWE2NDYtOTI2ZmE4YmU0YWIxIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6IlVoZjhodHVqKzVLRGFlYmxEMXBFS3ExMzlkWm1TQ3dmSjQ2VVwvdXhTM0d3PSJ9 x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.200.100 dlp-reaction: no-action x-originating-ip: [163.33.239.180] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-stable] [PATCH v1] gso: fix marking TCP checksum flag in TCP segments X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 24 Apr 2018 10:56:15 -0000 > -----Original Message----- > From: Ophir Munk [mailto:ophirmu@mellanox.com] > Sent: Tuesday, April 24, 2018 10:44 AM > To: Hu, Jiayu ; dev@dpdk.org; Ananyev, Konstantin > Cc: Thomas Monjalon ; Olga Shern ; Pascal Mazon ; > stable@dpdk.org > Subject: RE: [PATCH v1] gso: fix marking TCP checksum flag in TCP segment= s >=20 > Hi Jiayu, > Please find comments inline >=20 > > -----Original Message----- > > From: Hu, Jiayu [mailto:jiayu.hu@intel.com] > > Sent: Monday, April 23, 2018 7:14 AM > > To: Ophir Munk ; dev@dpdk.org; Ananyev, > > Konstantin > > Cc: Thomas Monjalon ; Olga Shern > > ; Pascal Mazon ; > > stable@dpdk.org > > Subject: RE: [PATCH v1] gso: fix marking TCP checksum flag in TCP segme= nts > > > > Hi Ophir, > > > > In the GSO design, the GSO library doesn't care about checksums, which > > means it doesn't check if input packets have correct checksums, and it > > doesn't do any checksum related work for the output GSO segments. It > > depends on the callers to use HW or SW checksum calculation for output > > packets. This is why the GSO library doesn't set PKT_TX_TCP_CKSUM. So I > > don't think it's a bug. > > >=20 > Can you please reconsider this design? I think the GSO library should imi= tate the HW behavior where TCP segments checksum is > automatically calculated without explicitly requesting it. I am not sayin= g that GSO library itself should calculate the checksums - but at least > it should mark each segment as requiring this calculation. But gso has no idea how this packet will be processed after it. Caller can choose to calculate L3/L4 cksum in SW or might be going to use H= W offloads. In later case nothing stops the caller to update mbuf->ol_flags in a way he= likes (TCP_CKSUM, IP_CKSUM, etc.). Konstantin >=20 > > In my opinion, it's not a good idea to enable HW TCP checksum calculati= on > > silently, and without the aware of the caller. In fact, the caller alwa= ys know it > > does SW TSO (i.e. GSO), instead of real HW TSO. >=20 > This is not correct. Consider net_failsafe with 2 sub-devices: one is a H= W PCI device, the other one is a SW TAP device. Failsafe must work > transparently with these two sub-devices and the caller cannot tell if TS= O is done in SW or HW. >=20 > > If the caller wants HW > > checksum calculation, it can add PKT_TX_TCP_CKSUM to ol_flags before or > > after calling the GSO library. > > >=20 > FYI - TAP TSO patches were submitted to dpdk.org mailing list. These patc= hes use the GSO library. > https://dpdk.org/dev/patchwork/patch/38666/ > https://dpdk.org/dev/patchwork/patch/38667/ >=20 > Running testpmd with TAP TSO is currently broken without the suggested li= brte_gso patch. > Please note testpmd implementation (app/test-pmd/csumonly.c b/app/test-pm= d/csumonly.c) in case *both* TSO and TCP CKSUM are > configured: >=20 > if (tso_segsz) > ol_flags |=3D PKT_TX_TCP_SEG; // *** if TSO is applicable - the = packet flags are only marked with PKT_TX_TCP_SEG and no > PKT_TX_TCP_CKSUM *** > else if (tx_offloads & DEV_TX_OFFLOAD_TCP_CKSUM) > ol_flags |=3D PKT_TX_TCP_CKSUM; // *** PKT_TX_TCP_CKSUM is mar= ked only if TSO is not applicable *** > else { > tcp_hdr->cksum =3D > get_udptcp_checksum(l3_hdr, tcp_hdr, >=20 > In other words - testpmd does not set TCP_CKSUM along with TCP_SEG theref= ore using testpmd with TAP/TSO will result in TCP segments > with 0 (incorrect) TCP checksums. >=20 > In addition - please note the comments in lib/librte_mbuf/rte_mbuf.h whic= h specify that PKT_TX_TCP_SEG flag implies the > PKT_TX_TCP_CKSUM (hence it is not required to be explicitly set by the ca= ller) >=20 > /** > * TCP segmentation offload. To enable this offload feature for a > * packet to be transmitted on hardware supporting TSO: > * - set the PKT_TX_TCP_SEG flag in mbuf->ol_flags (this flag implies > * PKT_TX_TCP_CKSUM) > ... >=20 > > Add Konstantin for more suggestions. > > > > Thanks, > > Jiayu > > > > > -----Original Message----- > > > From: Ophir Munk [mailto:ophirmu@mellanox.com] > > > Sent: Sunday, April 22, 2018 10:21 PM > > > To: dev@dpdk.org; Hu, Jiayu > > > Cc: Thomas Monjalon ; Olga Shern > > > ; Pascal Mazon ; > > Ophir > > > Munk ; stable@dpdk.org > > > Subject: [PATCH v1] gso: fix marking TCP checksum flag in TCP segment= s > > > > > > Large TCP packets which are marked with PKT_TX_TCP_SEG flag are > > > segmented and the flag is cleared in the resulting segments, however, > > > the segments checksum is not updated. It is therefore required to set > > > the PKT_TX_TCP_CKSUM flag in each TCP segment in order to mark for th= e > > > sending driver the need to update the TCP checksum before transmittin= g > > > the segment. > > > > > > Fixes: 119583797b6a ("gso: support TCP/IPv4 GSO") > > > Cc: stable@dpdk.org > > > > > > Signed-off-by: Ophir Munk > > > --- > > > lib/librte_gso/rte_gso.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/lib/librte_gso/rte_gso.c b/lib/librte_gso/rte_gso.c inde= x > > > a44e3d4..e9ce9ce 100644 > > > --- a/lib/librte_gso/rte_gso.c > > > +++ b/lib/librte_gso/rte_gso.c > > > @@ -50,12 +50,14 @@ rte_gso_segment(struct rte_mbuf *pkt, > > > ((IS_IPV4_GRE_TCP4(pkt->ol_flags) && > > > (gso_ctx->gso_types & > > > DEV_TX_OFFLOAD_GRE_TNL_TSO)))) { > > > pkt->ol_flags &=3D (~PKT_TX_TCP_SEG); > > > + pkt->ol_flags |=3D PKT_TX_TCP_CKSUM; > > > ret =3D gso_tunnel_tcp4_segment(pkt, gso_size, ipid_delta, > > > direct_pool, indirect_pool, > > > pkts_out, nb_pkts_out); > > > } else if (IS_IPV4_TCP(pkt->ol_flags) && > > > (gso_ctx->gso_types & > > > DEV_TX_OFFLOAD_TCP_TSO)) { > > > pkt->ol_flags &=3D (~PKT_TX_TCP_SEG); > > > + pkt->ol_flags |=3D PKT_TX_TCP_CKSUM; > > > ret =3D gso_tcp4_segment(pkt, gso_size, ipid_delta, > > > direct_pool, indirect_pool, > > > pkts_out, nb_pkts_out); > > > -- > > > 2.7.4