From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [148.163.129.52]) by dpdk.org (Postfix) with ESMTP id 1F8631B49B for ; Wed, 10 Oct 2018 11:31:20 +0200 (CEST) X-Virus-Scanned: Proofpoint Essentials engine Received: from webmail.solarflare.com (uk.solarflare.com [193.34.186.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by mx1-us1.ppe-hosted.com (Proofpoint Essentials ESMTP Server) with ESMTPS id A5102A80061; Wed, 10 Oct 2018 09:31:18 +0000 (UTC) Received: from [192.168.38.17] (91.220.146.112) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Wed, 10 Oct 2018 10:31:10 +0100 To: Ori Kam , Ferruh Yigit , "stephen@networkplumber.org" , Adrien Mazarguil , Declan Doherty CC: "dev@dpdk.org" , Dekel Peled , "Thomas Monjalon" , =?UTF-8?Q?N=c3=a9lio_Laranjeiro?= , Yongseok Koh , "Shahaf Shuler" References: <1537995646-95260-1-git-send-email-orika@mellanox.com> <1538917054-68283-1-git-send-email-orika@mellanox.com> <9760f054-bbe9-2036-dd5d-d39edd906496@intel.com> <1165fc19-c68b-f13c-e2a6-eeb3f6937922@solarflare.com> From: Andrew Rybchenko Message-ID: <2356fed7-ebf3-490b-7dc0-4179ddd9e8eb@solarflare.com> Date: Wed, 10 Oct 2018 12:30:29 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-GB X-Originating-IP: [91.220.146.112] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ukex01.SolarFlarecom.com (10.17.10.4) X-TM-AS-Product-Ver: SMEX-12.5.0.1300-8.5.1010-24146.003 X-TM-AS-Result: No-25.712900-8.000000-10 X-TMASE-MatchedRID: u6ojmU07PKx6DVqDv3PkvSa1MaKuob8Pt3aeg7g/usAutoY2UtFqGHB+ Y9O4SgcBV0uuapArhUon5flxzz7NDl3vqo4Ydbl4nhHKNOYbLL4xmbT6wQT2awv/nTOPQovsLF9 jbxvK1+0Ql6u/ZcsOvJs6sZ5vv7JIr4Tjl93LJlfjqquJ0I50YCEdaywSZvzOZ5yuplze9psPC6 iSWnZppXZ6R0vN+ixkhnsLi4UH4WdNC83U+LUUUOBKa0pUJhagVX7TxgU0dK4G1X6GwRrpozBXX b/qS263MQYGeOA0f3ZuLiPwsvJdEnCDFh6Q58XiICsP4rpeGceSeMExjuIoEhn7x7Zy0SLbrJEN mkGdOdcuS+yQ2qXph4eh0kgcXU+7iwjsyrTVNP4dxBAG5/hkW/iH64jt3FfEW/Kb+C9IfiIaCVs Php6QDRnrhoFYpSQGpChz7rvY4LNdFBQXHE6OF7E3FpMbg63SX6IRwqkp2m7KY//WmIj/oZlFk0 YRtezKEFkFTNc5fDXfic2v2vbXmu736qTQrFTqbc297PAGtWabKpAlY2y6SS8zQZ2rR/OpCKyML VHvlDArhAqjQ5f9abZ4zjiJYhd95m6xK1EoFXa8coKUcaOOvTpA2zZYJjv1qPm/sjj9KBjdpHlT Zyt6414fMqUWS0RxhtfOGOSyK2m/WXZS/HqJ2kY41YX/o/8KFpG8QcMOW5kXeuQCqIxlebxAi7j PoeEQftwZ3X11IV0= X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--25.712900-8.000000 X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-24146.003 X-MDID: 1539163879-jIbbHkHpoW9u Subject: Re: [dpdk-dev] [PATCH v3 0/3] ethdev: add generic L2/L3 tunnel encapsulation actions 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: , X-List-Received-Date: Wed, 10 Oct 2018 09:31:21 -0000 On 10/10/18 12:00 PM, Ori Kam wrote: >> -----Original Message----- >> From: Andrew Rybchenko >> Sent: Wednesday, October 10, 2018 9:45 AM >> To: Ferruh Yigit ; Ori Kam ; >> stephen@networkplumber.org; Adrien Mazarguil >> ; Declan Doherty >> Cc: dev@dpdk.org; Dekel Peled ; Thomas Monjalon >> ; Nélio Laranjeiro ; >> Yongseok Koh ; Shahaf Shuler >> >> Subject: Re: [PATCH v3 0/3] ethdev: add generic L2/L3 tunnel encapsulation >> actions >> >> On 10/9/18 7:48 PM, Ferruh Yigit wrote: >> On 10/7/2018 1:57 PM, Ori Kam wrote: >> This series implement the generic L2/L3 tunnel encapsulation actions >> and is based on rfc [1] "add generic L2/L3 tunnel encapsulation actions" >> >> Currenlty the encap/decap actions only support encapsulation >> of VXLAN and NVGRE L2 packets (L2 encapsulation is where >> the inner packet has a valid Ethernet header, while L3 encapsulation >> is where the inner packet doesn't have the Ethernet header). >> In addtion the parameter to to the encap action is a list of rte items, >> this results in 2 extra translation, between the application to the action >> and from the action to the NIC. This results in negetive impact on the >> insertion performance. >> >> Looking forward there are going to be a need to support many more tunnel >> encapsulations. For example MPLSoGRE, MPLSoUDP. >> Adding the new encapsulation will result in duplication of code. >> For example the code for handling NVGRE and VXLAN are exactly the same, >> and each new tunnel will have the same exact structure. >> >> This series introduce a generic encapsulation for L2 tunnel types, and >> generic encapsulation for L3 tunnel types. In addtion the new >> encapsulations commands are using raw buffer inorder to save the >> converstion time, both for the application and the PMD. >> >> [1]https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail >> s.dpdk.org%2Farchives%2Fdev%2F2018- >> August%2F109944.html&data=02%7C01%7Corika%40mellanox.com%7C468bfe >> a033d642c3af5a08d62e7c0bd2%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0 >> %7C0%7C636747507642154668&sdata=cAMr3ThkyhjFrGv6K%2FvIDKHAskzMZI >> E8cpRTWiBl1eA%3D&reserved=0 >> >> v3: >> * rebase on tip. >> >> v2: >> * add missing decap_l3 structure. >> * fix typo. >> >> >> Ori Kam (3): >> ethdev: add generic L2/L3 tunnel encapsulation actions >> app/testpmd: convert testpmd encap commands to new API >> ethdev: remove vxlan and nvgre encapsulation commands >> >> Reminder of this patchset, any reviews welcome. >> >> I've added the author of previous actions in recipients. >> >> I like the idea to generalize encap/decap actions. It makes a bit harder >> for reader to find which encap/decap actions are supported in fact, >> but it changes nothing for automated usage in the code - just try it >> (as a generic way used in rte_flow). >> > Even now the user doesn't know which encapsulation is supported since > it is PMD and sometime kernel related. On the other end it simplify adding > encapsulation to specific costumers with some time just FW update. I was just trying to say that previous way is a bit easier to understand from sources that PMD pretends to support VXLAN or NVGRE encap/decap. In any case it is not that important, so OK to have the new way. >> Arguments about a way of encap/decap headers specification (flow items >> vs raw) sound sensible, but I'm not sure about it. >> It would be simpler if the tunnel header is added appended or removed >> as is, but as I understand it is not true. For example, IPv4 ID will be >> different in incoming packets to be decapsulated and different values >> should be used on encapsulation. Checksums will be different (but >> offloaded in any case). >> > I'm not sure I understand your comment. > Decapsulation is independent of encapsulation, for example if we decap > L2 tunnel type then there is no parameter at all the NIC just removes > the outer layers. OK, I've just mixed filtering and action parameters for decaps. My bad. The argument for encapsulation still makes sense since the header is not appended just as is. IP IDs change, lengths change, checksums change, however, I agree that it is natural and expected behaviour. >> Current way allows to specify which fields do not matter and which one >> must match. It allows to say that, for example, VNI match is sufficient >> to decapsulate. >> > The encapsulation according to definition, is a list of headers that should > encapsulate the packet. So I don't understand your comment about matching > fields. The matching is based on the flow and the encapsulation is just data > that should be added on top of the packet. Yes, my bad as I've described above. >> Also arguments assume that action input is accepted as is by the HW. >> It could be true, but could be obviously false and HW interface may >> require parsed input (i.e. driver must parse the input buffer and extract >> required fields of packet headers). >> > You are correct there some PMD even Mellanox (for the E-Switch) require to parsed input > There is no driver that knows rte_flow structure so in any case there should be > Some translation between the encapsulation data and the NIC data. > I agree that writing the code for translation can be harder in this approach, > but the code is only written once is the insertion speed is much higher this way. > Also like I said some Virtual Switches are already store this data in raw buffer > (they update only specific fields) so this will also save time for the application when > creating a rule. Yes, makes sense. >> So, I'd say no. It should be better motivated if we change existing >> approach (even advertised as experimental). > I think the reasons I gave are very good motivation to change the approach > please also consider that there is no implementation yet that supports the old approach. > while we do have code that uses the new approach. It is really bad practice that features are accepted without at least one implementation/usage. Thanks for the reply. I'll provide my comments on patches.