From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id 798DB5595 for ; Wed, 21 Sep 2016 14:36:55 +0200 (CEST) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga104.fm.intel.com with ESMTP; 21 Sep 2016 05:36:54 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.30,373,1470726000"; d="scan'208";a="882270637" Received: from shwdeisgchi083.ccr.corp.intel.com (HELO [10.239.67.193]) ([10.239.67.193]) by orsmga003.jf.intel.com with ESMTP; 21 Sep 2016 05:36:53 -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> Cc: "thomas.monjalon@6wind.com" , "De Lara Guarch, Pablo" , "Wu, Jingjing" , "Zhang, Helin" , "Tao, Zhe" From: "Tan, Jianfeng" Message-ID: <312c9e7b-03eb-6b77-7d2e-0d984d337980@intel.com> Date: Wed, 21 Sep 2016 20:36:51 +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: <2601191342CEEE43887BDE71AB9772583F0B57BD@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: Wed, 21 Sep 2016 12:36:56 -0000 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 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. As far as I can see, the new command will always have semantic overlapping with existing commands, because it indeed depends on the two different things. Thanks, Jianfeng > > Konstantin >