From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (xvm-189-124.dc0.ghst.net [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 817D1A0524; Sat, 9 Jan 2021 02:01:29 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 58981140EA0; Sat, 9 Jan 2021 02:01:29 +0100 (CET) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by mails.dpdk.org (Postfix) with ESMTP id 334C8140E9A for ; Sat, 9 Jan 2021 02:01:27 +0100 (CET) IronPort-SDR: 4Oj920UvoL7owVeiRf7uQIcmKr4kJrZVt+ohDVu0GzJZTncdxSDiKlrXywkrp4DJLI7C0HSiwv RGg8FwD/Ernw== X-IronPort-AV: E=McAfee;i="6000,8403,9858"; a="174166708" X-IronPort-AV: E=Sophos;i="5.79,333,1602572400"; d="scan'208";a="174166708" Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Jan 2021 17:01:25 -0800 IronPort-SDR: 18LfxHDY201xhwhHol7ZxhyBSEvPhcWU88c9pE6h8zhs6o9/SWkspM76nD0jiqDA1gaKWe1EGJ eqsPfjU32sog== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.79,333,1602572400"; d="scan'208";a="403517374" Received: from fmsmsx605.amr.corp.intel.com ([10.18.126.85]) by FMSMGA003.fm.intel.com with ESMTP; 08 Jan 2021 17:01:25 -0800 Received: from shsmsx603.ccr.corp.intel.com (10.109.6.143) by fmsmsx605.amr.corp.intel.com (10.18.126.85) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Fri, 8 Jan 2021 17:01:24 -0800 Received: from shsmsx601.ccr.corp.intel.com (10.109.6.141) by SHSMSX603.ccr.corp.intel.com (10.109.6.143) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Sat, 9 Jan 2021 09:01:23 +0800 Received: from shsmsx601.ccr.corp.intel.com ([10.109.6.141]) by SHSMSX601.ccr.corp.intel.com ([10.109.6.141]) with mapi id 15.01.1713.004; Sat, 9 Jan 2021 09:01:23 +0800 From: "Zhang, Qi Z" To: Thomas Monjalon , "Yigit, Ferruh" , "Guo, Jia" , Andrew Rybchenko CC: "Wu, Jingjing" , "Yang, Qiming" , "Wang, Haiyue" , "dev@dpdk.org" , "orika@nvidia.com" , "getelson@nvidia.com" Thread-Topic: [dpdk-dev] [dpdk-dev v2 1/2] ethdev: add new tunnel type for ecpri Thread-Index: AQHW2cLnLDO52W/MnUi6XmgEhPhIWqoauFcAgAC+FICAAAryAIAAqDkQ//+QKICAAIuhsP//rcCAgAEHCvCAAATsgIAACNMAgAASpwCAAXQdcA== Date: Sat, 9 Jan 2021 01:01:22 +0000 Message-ID: References: <20201216085854.7842-1-jia.guo@intel.com> <29e83a3d-4c22-c200-ba03-7aae54a7e07b@intel.com> <3eb1ed08-01fe-d7d1-4540-e09a67e3077c@oktetlabs.ru> <1974177.35SpgSOlpP@thomas> In-Reply-To: <1974177.35SpgSOlpP@thomas> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-reaction: no-action dlp-version: 11.5.1.3 dlp-product: dlpe-windows x-originating-ip: [10.239.127.36] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [dpdk-dev v2 1/2] ethdev: add new tunnel type for ecpri X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 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" > -----Original Message----- > From: Thomas Monjalon > Sent: Friday, January 8, 2021 6:36 PM > To: Yigit, Ferruh ; Zhang, Qi Z ; > Guo, Jia ; Andrew Rybchenko > > Cc: Wu, Jingjing ; Yang, Qiming > ; Wang, Haiyue ; > dev@dpdk.org; orika@nvidia.com; getelson@nvidia.com > Subject: Re: [dpdk-dev] [dpdk-dev v2 1/2] ethdev: add new tunnel type for= ecpri >=20 > 08/01/2021 10:29, Andrew Rybchenko: > > On 1/8/21 11:57 AM, Ferruh Yigit wrote: > > > On 1/8/2021 1:41 AM, Zhang, Qi Z wrote: > > >> From: Thomas Monjalon > > >>> 07/01/2021 16:24, Zhang, Qi Z: > > >>>> From: Thomas Monjalon > > >>>>> 07/01/2021 13:47, Zhang, Qi Z: > > >>>>>> From: Thomas Monjalon > > >>>>>>> 07/01/2021 10:32, Guo, Jia: > > >>>>>>>> From: Thomas Monjalon > > >>>>>>>>> 24/12/2020 07:59, Jeff Guo: > > >>>>>>>>>> --- a/lib/librte_ethdev/rte_ethdev.h > > >>>>>>>>>> +++ b/lib/librte_ethdev/rte_ethdev.h > > >>>>>>>>>> @@ -1219,6 +1219,7 @@ enum rte_eth_tunnel_type { > > >>>>>>>>>> RTE_TUNNEL_TYPE_IP_IN_GRE, > > >>>>>>>>>> RTE_L2_TUNNEL_TYPE_E_TAG, > > >>>>>>>>>> RTE_TUNNEL_TYPE_VXLAN_GPE, > > >>>>>>>>>> + RTE_TUNNEL_TYPE_ECPRI, > > >>>>>>>>>> RTE_TUNNEL_TYPE_MAX, > > >>>>>>>>>> }; > > >>>>>>>>> > > >>>>>>>>> We tried to remove all these legacy API in DPDK 20.11. > > >>>>>>>>> Andrew decided to not remove this one because it is not yet > > >>>>>>>>> completely replaced by rte_flow in all drivers. > > >>>>>>>>> However, I am against continuing to update this API. > > >>>>>>>>> The opposite work should be done: migrate to rte_flow. > > >>>>>>>> > > >>>>>>>> Agree but seems that the legacy api and driver legacy > > >>>>>>>> implementation still keep in this release, and there is no a > > >>>>>>>> general way to replace the legacy by rte_flow right now. > > >>>>>>> > > >>>>>>> I think rte_flow is a complete replacement with more features. > > >>>>>> > > >>>>>> Thomas, I may not agree with this. > > >>>>>> > > >>>>>> Actually the "enum rte_eth_tunnel_type" is used by > > >>>>>> rte_eth_dev_udp_tunnel_port_add A packet with specific dst udp > > >>>>>> port will be recognized as a specific tunnel packet type (e.g. > > >>>>>> vxlan, vxlan-gpe, > > >>>>> ecpri...) In Intel NIC, the API actually changes the > > >>>>> configuration of the packet parser in HW but not add a filter > > >>>>> rule and I guess all other devices may enable it in a similar way= . > > >>>>>> so naturally it should be a device (port) level configuration > > >>>>>> but not a rte_flow > > >>>>> rule for match, encap, decap... > > >>>>> > > >>>>> I don't understand how it helps to identify an UDP port if there > > >>>>> is no rule for this tunnel. > > >>>>> What is the usage? > > >>>> > > >>>> Yes, in general It is a rule, it matches a udp packet's dst port > > >>>> and the action is > > >>> "now the packet is identified as vxlan packet" then all other > > >>> rte_flow rules that match for a vlxan as pattern will take effect. > > >>> but somehow, I think they are not rules in the same domain, just > > >>> like we have dedicate API for mac/vlan filter, we'd better have a > > >>> dedicate API for this also. ( RFC for Vxlan explains why we need > > >>> this. https://tools.ietf.org/html/rfc7348). > > >>>> > > >>>> "Destination Port: IANA has assigned the value 4789 for the VXLAN > > >>>> UDP port, and this value SHOULD be used by default as the > > >>>> destination UDP port. Some early implementations of VXLAN have > > >>>> used other values for the destination port. To enable > > >>>> interoperability with these implementations, the destination port > SHOULD be configurable." > > >>> > > >>> Yes the port number is free. > > >>> But isn't it more natural to specify this port number as part of > > >>> the rte_flow rule? > > >> > > >> I think if we have a rte_flow action type that can be used to set a > > >> packet's tunnel type xxx, like below #flow create eth/ipv4/udp port > > >> is 4789/... action set_tunnel_type VxLAN / end then we may replace > > >> it with rte_flow, but I'm not sure if it's necessary, please share > > >> if you have a better idea. >=20 > Of course we can specify the UDP port in rte_flow rule. > Please check rte_flow_item_udp. > That's a basic of rte_flow. Its not about the pattern match, it's about the action, what we need is a r= te_flow action to "define a packet's tunnel type", but we don't have. #flow create eth/ipv4/udp port is 4789/... action set_tunnel_type VxLAN I think rte_eth_dev_udp_tunnel_port_add does this job well already, if we p= lan to move it to rte_flow, at least we need a replacement solution. >=20 >=20 > > > Isn't this more a device configuration than filtering, not sure > > > about using rte_flow for this. > > > > +1 >=20 > A device configuration? No, setting an UDP port is a stack configuration. >=20 >=20 > > >> BTW, are we going to move all other filter like mac , VLAN > > >> filter/strip/insert into rte_flow finally? >=20 > Yes I think it should be the direction. > All of this can be achieved with rte_flow. >=20 >=20 > > >> if that's the plan, though I don't have much inputs for this right > > >> now, but I think we may not need to prevent new features be added > > >> based on current API if it does not introduce more complexity and > > >> not break anything. >=20 > If we continue updating old API, we are just forking ourself: > having 2 APIs for the same feature is a non-sense. > We must remove APIs which are superseded by rte_flow. >=20