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 3E09CA0524; Fri, 8 Jan 2021 10:22:13 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 0F39D140ED3; Fri, 8 Jan 2021 10:22:13 +0100 (CET) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by mails.dpdk.org (Postfix) with ESMTP id E72ED140EC5 for ; Fri, 8 Jan 2021 10:22:10 +0100 (CET) IronPort-SDR: +DyqlHFS7GcvJxPD4g5o1n5L20i3/lBVEOQH/3V2P98OcfeZk9rKnCIArgU5XAlHEfR+cGx4cT O/qmnuZleguw== X-IronPort-AV: E=McAfee;i="6000,8403,9857"; a="239119763" X-IronPort-AV: E=Sophos;i="5.79,330,1602572400"; d="scan'208";a="239119763" Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Jan 2021 01:22:09 -0800 IronPort-SDR: U4xN9qP4BFvGNBHCziRb7t8E6YwKC5kPlu71mRgU5L9X99G1pitxP9CnYC+jE5FvFPQHHA/9/M AbH0ekPhc8wg== X-IronPort-AV: E=Sophos;i="5.79,330,1602572400"; d="scan'208";a="398932703" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.213.237.192]) ([10.213.237.192]) by fmsmga002-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Jan 2021 01:22:07 -0800 To: Thomas Monjalon , "Guo, Jia" , "Zhang, Qi Z" Cc: "Wu, Jingjing" , "Yang, Qiming" , "Wang, Haiyue" , "dev@dpdk.org" , "andrew.rybchenko@oktetlabs.ru" , "orika@nvidia.com" , "getelson@nvidia.com" , Dodji Seketeli References: <20201216085854.7842-1-jia.guo@intel.com> <1693993.jziF1hSf6E@thomas> <27289607.Vi9ZVq1Shk@thomas> From: Ferruh Yigit Message-ID: Date: Fri, 8 Jan 2021 09:22:03 +0000 MIME-Version: 1.0 In-Reply-To: <27289607.Vi9ZVq1Shk@thomas> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit 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" On 1/7/2021 1:33 PM, Thomas Monjalon wrote: > 07/01/2021 13:47, Zhang, Qi Z: >> >>> -----Original Message----- >>> From: Thomas Monjalon >>> Sent: Thursday, January 7, 2021 6:12 PM >>> To: Guo, Jia >>> Cc: Zhang, Qi Z ; Wu, Jingjing ; >>> Yang, Qiming ; Wang, Haiyue >>> ; dev@dpdk.org; Yigit, Ferruh >>> ; andrew.rybchenko@oktetlabs.ru; orika@nvidia.com; >>> getelson@nvidia.com >>> Subject: Re: [dpdk-dev] [dpdk-dev v2 1/2] ethdev: add new tunnel type for ecpri >>> >>> 07/01/2021 10:32, Guo, Jia: >>>> From: Thomas Monjalon >>>>> 24/12/2020 07:59, Jeff Guo: >>>>>> Add type of RTE_TUNNEL_TYPE_ECPRI into the enum of ethdev tunnel >>>>> type. >>>>>> >>>>>> Signed-off-by: Jeff Guo >>>>>> Reviewed-by: Qi Zhang >>>>> [...] >>>>>> --- 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? > >> So I think it's not a good idea to replace >> the rte_eth_dev_udp_tunnel_port_add with rte_flow config >> and also there is no existing rte_flow_action >> can cover this requirement unless we introduce some new one. >> >>> You can match, encap, decap. >>> There is even a new API to get tunnel infos after decap. >>> What is missing? > > I still don't see which use case is missing. > > >>>>> Sorry, it is a nack. >>>>> BTW, it is probably breaking the ABI because of RTE_TUNNEL_TYPE_MAX. >> >> Yes that may break the ABI but fortunately the checking-abi-compatibility tool shows negative :) , thanks Ferruh' s guide. >> https://github.com/ferruhy/dpdk/actions/runs/468859673 > > That's very strange. An enum value is changed. > Why it is not flagged by libabigail? > As long as the enum values not sent to the application and kept within the library, changing their values shouldn't be problem. > >>>> Oh, the ABI break should be a problem. >>>> >>>>> PS: please Cc ethdev maintainers for such patch, thanks. >>>>> tip: use --cc-cmd devtools/get-maintainer.sh >>>> >>>> Thanks for your helpful tip. > > >