Message ID | 20200918141735.18488-1-ophirmu@nvidia.com (mailing list archive) |
---|---|
Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id B4CEEA04C8; Fri, 18 Sep 2020 16:17:43 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 5929A1D9B3; Fri, 18 Sep 2020 16:17:43 +0200 (CEST) Received: from mellanox.co.il (mail-il-dmz.mellanox.com [193.47.165.129]) by dpdk.org (Postfix) with ESMTP id 201221D9AF for <dev@dpdk.org>; Fri, 18 Sep 2020 16:17:42 +0200 (CEST) Received: from Internal Mail-Server by MTLPINE1 (envelope-from ophirmu@nvidia.com) with SMTP; 18 Sep 2020 17:17:41 +0300 Received: from nvidia.com (pegasus05.mtr.labs.mlnx [10.210.16.100]) by labmailer.mlnx (8.13.8/8.13.8) with ESMTP id 08IEHff2014370; Fri, 18 Sep 2020 17:17:41 +0300 From: Ophir Munk <ophirmu@nvidia.com> To: dev@dpdk.org, Wenzhuo Lu <wenzhuo.lu@intel.com>, Beilei Xing <beilei.xing@intel.com>, Bernard Iremonger <bernard.iremonger@intel.com>, Ferruh Yigit <ferruh.yigit@intel.com>, Olivier Matz <olivier.matz@6wind.com> Cc: Ophir Munk <ophirmu@nvidia.com> Date: Fri, 18 Sep 2020 14:17:32 +0000 Message-Id: <20200918141735.18488-1-ophirmu@nvidia.com> X-Mailer: git-send-email 2.8.4 In-Reply-To: <20200915131717.18252-2-ophirmu@nvidia.com> References: <20200915131717.18252-2-ophirmu@nvidia.com> Subject: [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 <dev.dpdk.org> List-Unsubscribe: <https://mails.dpdk.org/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://mails.dpdk.org/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <https://mails.dpdk.org/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Series |
Add GENEVE protocol parsing to testpmd
|
|
Message
Ophir Munk
Sept. 18, 2020, 2:17 p.m. UTC
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 http://patches.dpdk.org/patch/77734/ Ophir Munk (3): app/testpmd: add GENEVE parsing app/testpmd: enable configuring GENEVE port app/testpmd: reduce tunnel parsing code duplication app/test-pmd/csumonly.c | 120 ++++++++++++++++++++++------------ app/test-pmd/parameters.c | 14 +++- app/test-pmd/testpmd.h | 1 + doc/guides/testpmd_app_ug/run_app.rst | 5 ++ lib/librte_net/meson.build | 3 +- lib/librte_net/rte_geneve.h | 66 +++++++++++++++++++ lib/librte_net/rte_vxlan.h | 1 + 7 files changed, 165 insertions(+), 45 deletions(-) create mode 100644 lib/librte_net/rte_geneve.h
Comments
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 > http://patches.dpdk.org/patch/77734/ > > 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? 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?
Hi Ferruh, I sent V6 which addresses your last review. Please see more comments inline. > -----Original Message----- > From: Ferruh Yigit <ferruh.yigit@intel.com> > 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?
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 <ferruh.yigit@intel.com> >> 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.