From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 1771EA04BA; Wed, 7 Oct 2020 18:00:56 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 65FE51BD17; Wed, 7 Oct 2020 18:00:52 +0200 (CEST) Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by dpdk.org (Postfix) with ESMTP id EAE381BCE4 for ; Wed, 7 Oct 2020 18:00:50 +0200 (CEST) IronPort-SDR: CgZSolE3GE0ixo3TbxCreGgQYgCGK/W1fCcK4/SGE9mzDxf7kz5a4VA9N3Yn4hESEQWE3pN00j gg3qM6xh4btw== X-IronPort-AV: E=McAfee;i="6000,8403,9767"; a="144380804" X-IronPort-AV: E=Sophos;i="5.77,347,1596524400"; d="scan'208";a="144380804" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 07 Oct 2020 09:00:30 -0700 IronPort-SDR: 0DCh0a8DnfWLPQ6vng9Pc636etXP3fMrhARlZZCIMcOfZ/s9Hoz3ZjmXTuaaCnwLZLEgdsKOEk QZ3eOlq7zX+g== X-IronPort-AV: E=Sophos;i="5.77,347,1596524400"; d="scan'208";a="316270855" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.213.226.27]) ([10.213.226.27]) by orsmga006-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 07 Oct 2020 09:00:28 -0700 To: Ophir Munk , "dev@dpdk.org" , Wenzhuo Lu , Beilei Xing , Bernard Iremonger , Olivier Matz References: <20200915131717.18252-2-ophirmu@nvidia.com> <20200918141735.18488-1-ophirmu@nvidia.com> From: Ferruh Yigit Message-ID: <9d96b69a-e20a-3829-9323-9c827a9e2637@intel.com> Date: Wed, 7 Oct 2020 17:00:24 +0100 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v5 0/3] Add GENEVE protocol parsing to testpmd X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 10/7/2020 4:43 PM, Ophir Munk wrote: > Hi Ferruh, > I sent V6 which addresses your last review. > > Please see more comments inline. > >> -----Original Message----- >> From: Ferruh Yigit >> Sent: Tuesday, October 6, 2020 5:59 PM >> On 9/18/2020 3:17 PM, Ophir Munk wrote: >>> v1: >>> Initial version >>> v2: >>> Rebased + Minor update in protocol options field: >>> char opts[0] ===> uint8_t opts[] >>> v3: >>> Rebase >>> document "geneve-port=N" parameter >>> v4: >>> Mispelling corrections >>> v5: >>> Rebase + Updates following review >>> >> https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatch >>> >> es.dpdk.org%2Fpatch%2F77734%2F&data=02%7C01%7Cophirmu%40nv >> idia.com >>> >> %7C1b01b5de39d24d61e38008d86a086ae2%7C43083d15727340c1b7db39e >> fd9ccc17a >>> >> %7C0%7C0%7C637375931691373733&sdata=csLb5OdTmWlpv1k4Z7ZZ >> YN1b1d2cd8 >>> %2BTxxnydgNnyQ4%3D&reserved=0 >>> >>> Ophir Munk (3): >>> app/testpmd: add GENEVE parsing >>> app/testpmd: enable configuring GENEVE port >>> app/testpmd: reduce tunnel parsing code duplication >>> >> >> Hi Ophir, >> >> The patchset looks good except a few comments I put into the patches. >> >> But I have two highlevel questions/comments, >> >> 1) The testpmd tunnel parsing feature is not documented properly, there are >> various related commands but there is no documentation to put all together. >> What do you think putting a new section for it under the "Testpmd Runtime >> Functions" (testpmd_funcs.rst) with this patchset? > > I prefer this to be in a separate patchset. > Geneve in testpmd is not complete yet. I know that there is current work in progress to add geneve options to flows. > Maybe its worth waiting till Geneve is finalized in testpmd. > >> >> 2) The 'csum' forwarding engine seems become forwarding engine for the >> case where packet payload needs to be parsed, like gro/gso, tunnel parse. >> Even the description of the forwarding engine in the documentation is not >> accurate now. >> I wonder if we should rename the forwarding engine at this stage? > > Can you please be more specific to which renaming you are referring to? > Can you give examples? > I am asking about forwarding engine name, it is 'csum' and in documentation it is described as: " Changes the checksum field with hardware or software methods depending on the offload flags on the packet. " So the initial purpose of the forwarding engine is to calculate checksums (or offload calculation to HW) before sending packets, but now it detects and parses the tunneling protocols and updates packet accordingly and logs it. It also does GRO and GSO. Is the forwarding engine name 'csum' still make sense, or should we find something else. I don't have any suggestion, just the question.