From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 54A3D2952 for ; Thu, 22 Sep 2016 03:29:13 +0200 (CEST) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga102.jf.intel.com with ESMTP; 21 Sep 2016 18:29:12 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.30,376,1470726000"; d="scan'208";a="1034460291" Received: from shwdeisgchi083.ccr.corp.intel.com (HELO [10.239.67.193]) ([10.239.67.193]) by orsmga001.jf.intel.com with ESMTP; 21 Sep 2016 18:29:09 -0700 To: "Ananyev, Konstantin" , "dev@dpdk.org" 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> Cc: "thomas.monjalon@6wind.com" , "De Lara Guarch, Pablo" , "Wu, Jingjing" , "Zhang, Helin" , "Tao, Zhe" From: "Tan, Jianfeng" Message-ID: Date: Thu, 22 Sep 2016 09:29:08 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: <2601191342CEEE43887BDE71AB9772583F0B9C94@irsmsx105.ger.corp.intel.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit 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 01:29:13 -0000 Hi Konstantin, 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 == 1) { >>>> ol_flags |= 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 == 1) { >>> if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM) { >>> m->outer_l2_len = info.outer_l2_len; >>> m->outer_l3_len = info.outer_l3_len; >>> m->l2_len = info.l2_len; >>> m->l3_len = info.l3_len; >>> m->l4_len = 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 = info.outer_l2_len + >>> info.outer_l3_len + info.l2_len; >>> m->l3_len = info.l3_len; >>> m->l4_len = info.l4_len; >>> } >>> >>> >>> ? >>> >>> In particular shouldn't it be something like: >>> if ((testpmd_ol_flags & TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM) != 0 || >>> ((testmpd_ol_flags & TESTPMD_TX_OFFLOAD_PARSE_TUNNEL) != 0 && >>> info.tso_segsz != 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 checksum 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 would 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_tunnel 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, but 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 possible, correct? > Konstantin 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 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. And original "tso set 800 0" will only control tso of non-tunneled packets. ? Thanks, Jianfeng