From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 9E80C590B for ; Thu, 22 Sep 2016 11:16:02 +0200 (CEST) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga105.fm.intel.com with ESMTP; 22 Sep 2016 02:16:00 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.30,378,1470726000"; d="scan'208";a="1034596821" Received: from irsmsx107.ger.corp.intel.com ([163.33.3.99]) by orsmga001.jf.intel.com with ESMTP; 22 Sep 2016 02:15:59 -0700 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.196]) by IRSMSX107.ger.corp.intel.com ([169.254.10.95]) with mapi id 14.03.0248.002; Thu, 22 Sep 2016 10:15:58 +0100 From: "Ananyev, Konstantin" To: "Tan, Jianfeng" , "dev@dpdk.org" CC: "thomas.monjalon@6wind.com" , "De Lara Guarch, Pablo" , "Wu, Jingjing" , "Zhang, Helin" , "Tao, Zhe" Thread-Topic: [PATCH v4 3/3] app/testpmd: fix Tx offload on tunneling packet Thread-Index: AQHR66jQc3Dgxo+vkkK4jQhFkSeQJ6CBAhuQgAMfd4CAAETRIIAAkvUAgACR3yA= Date: Thu, 22 Sep 2016 09:15:57 +0000 Message-ID: <2601191342CEEE43887BDE71AB9772583F0BA12D@irsmsx105.ger.corp.intel.com> References: <1467752375-25984-1-git-send-email-zhe.tao@intel.com> <1470023815-23108-1-git-send-email-jianfeng.tan@intel.com> <1470023815-23108-4-git-send-email-jianfeng.tan@intel.com> <2601191342CEEE43887BDE71AB9772583F0B57BD@irsmsx105.ger.corp.intel.com> <312c9e7b-03eb-6b77-7d2e-0d984d337980@intel.com> <2601191342CEEE43887BDE71AB9772583F0B9C94@irsmsx105.ger.corp.intel.com> In-Reply-To: Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v4 3/3] app/testpmd: fix Tx offload on tunneling packet X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 22 Sep 2016 09:16:03 -0000 Hi Jianfeng, >=20 > Hi Konstantin, >=20 >=20 > On 9/21/2016 11:47 PM, Ananyev, Konstantin wrote: > > Hi Jianfeng, > > > >> Hi Konstantin, > >> > >> > >> On 9/19/2016 8:09 PM, Ananyev, Konstantin wrote: > >>> Hi Jainfeng, > >>> > >>>> -----Original Message----- > >>>> From: Tan, Jianfeng > >>>> Sent: Monday, August 1, 2016 4:57 AM > >>>> To: dev@dpdk.org > >>>> Cc: thomas.monjalon@6wind.com; De Lara Guarch, Pablo > >>>> ; Ananyev, Konstantin > >>>> ; Wu, Jingjing > >>>> ; Zhang, Helin ; Tan, > >>>> Jianfeng ; Tao, Zhe > >>>> Subject: [PATCH v4 3/3] app/testpmd: fix Tx offload on tunneling > >>>> packet > >>>> > >>>> Tx offload on tunneling packet now requires applications to > >>>> correctly set tunneling type. Without setting it, i40e driver does > >>>> not parse tunneling parameters. Besides that, add a check to see if > >>>> NIC supports TSO on tunneling packet when executing "csum > >> parse_tunnel on _port" > >>>> after "tso set _size _port" or the other way around. > >>>> > >>>> Fixes: b51c47536a9e ("app/testpmd: support TSO in checksum forward > >>>> engine") > >>>> > >>>> Signed-off-by: Zhe Tao > >>>> Signed-off-by: Jianfeng Tan > >>>> --- > >>>> app/test-pmd/cmdline.c | 42 ++++++++++++++++++++++++++++++++++++= ------ > >>>> app/test-pmd/csumonly.c | 37 +++++++++++++++++++++++++++++-------= - > >>>> 2 files changed, 65 insertions(+), 14 deletions(-) > >>>> > >>>> [...] > >>>> > >>>> @@ -745,7 +762,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs= ) > >>>> * processed in hardware. */ > >>>> if (info.is_tunnel =3D=3D 1) { > >>>> ol_flags |=3D process_outer_cksums(outer_l3_hdr, &info, > >>>> - testpmd_ol_flags); > >>>> + testpmd_ol_flags, ol_flags & PKT_TX_TCP_SEG); > >>>> } > >>>> > >>>> /* step 4: fill the mbuf meta data (flags and header lengths) > >>>> */ @@ -806,6 +823,10 @@ > >>> It was a while since I looked a t it closely, but shouldn't you also = update step 4 below: > >>> > >>> if (info.is_tunnel =3D=3D 1) { > >>> if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_O= UTER_IP_CKSUM) { > >>> m->outer_l2_len =3D info.outer_l2_l= en; > >>> m->outer_l3_len =3D info.outer_l3_l= en; > >>> m->l2_len =3D info.l2_len; > >>> m->l3_len =3D info.l3_len; > >>> m->l4_len =3D info.l4_len; > >>> } > >>> else { > >>> /* if there is a outer UDP cksum > >>> processed in sw and the inner in= hw, > >>> the outer checksum will be wrong= as > >>> the payload will be modified by = the > >>> hardware */ > >>> m->l2_len =3D info.outer_l2_len + > >>> info.outer_l3_len + info.l2= _len; > >>> m->l3_len =3D info.l3_len; > >>> m->l4_len =3D info.l4_len; > >>> } > >>> > >>> > >>> ? > >>> > >>> In particular shouldn't it be something like: > >>> if ((testpmd_ol_flags & TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM) !=3D 0 || > >>> ((testmpd_ol_flags & TESTPMD_TX_OFFLOAD_PARSE_TUNNEL) !=3D 0 > >>> && info.tso_segsz !=3D 0)) { .... > >>> ? > >> Sorry for late response, because I also take some time to refresh > >> memory. And, you are right, I missed this corner case. After applying = your way above, it works! > >> > >> The case below settings in testpmd: > >> $ set fwd csum > >> $ csum parse_tunnel on 0 > >> $ tso set 800 0 > >> > > Great :) > > > >> And unfortunately, our previous verification is based on "outer-ip che= cksum offload is hw". > >> > >>> Another thought, might be it is worth to introduce new flag: > >>> TESTPMD_TX_OFFLOAD_TSO_TUNNEL, and new command in cmdline.c, that wou= ld set/clear that flag. > >>> Instead of trying to make assumptions does user wants tso for > >>> tunneled packets based on 2 different things: > >>> - enable/disable tso > >>> - enable/disable tunneled packets parsing ? > >> Currently, if we do parse_tunnel is based on the command "csum parse_t= unnel on/off ". > >> If we add a command like "tso_tunnel set ", it's a > >> little duplicated with "tso set ", and there is too > >> much info to just set a flag like TESTPMD_TX_OFFLOAD_TSO_TUNNEL; If we= add a command like "csum tunnel_tso on ", it > also depends on "csum parse_tunnel on " so that tunnel packets are = parsed. > > But I thought in some cases user might want to enable tunnel parsing, b= ut do tso for non-tunneled packets only. > > I.E. > > - enable tunnel parsing > > - for non-tunneled packets do tso > > - for tunneled packets don't do tso > > My understanding that with current set commands/flags this is not possi= ble, correct? > > Konstantin >=20 > Yes, correct, above case is not supported now. A twin case would be: > - for non-tunneled packets, don't do tso > - for tunneled packets, do tso Yep, you right. >=20 > Considering above two cases, so how about adding a command like; $ tunnel= _tso set 800 0 which needs "csum parse_tunnel on 0" has > been set before it. >=20 > And original "tso set 800 0" will only control tso of non-tunneled packet= s. Looks good for me. Konstantin