From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 0BF86A04B5; Mon, 11 Jan 2021 10:23:42 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 87413140CB8; Mon, 11 Jan 2021 10:23:41 +0100 (CET) Received: from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com [66.111.4.25]) by mails.dpdk.org (Postfix) with ESMTP id 7A0D3140CB8 for ; Mon, 11 Jan 2021 10:23:40 +0100 (CET) Received: from compute2.internal (compute2.nyi.internal [10.202.2.42]) by mailout.nyi.internal (Postfix) with ESMTP id C44DF5C009D; Mon, 11 Jan 2021 04:23:39 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute2.internal (MEProxy); Mon, 11 Jan 2021 04:23:39 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=fm3; bh= 6GsaazfvdK8gce35ZNwXK+vEQLwz9dtpMdSu0mK1CiI=; b=iLofG6oj42t2kQDQ i4rRD5KwsIrUrYgyZMDD8AthjEH/5uSfL/UazlrfTakyNz4+rCelBVBkSGeIqbPB DlEVNnUBLvLMA7lruWsnReKbyUu9mw3cAwteqWk7DEQBA9kWEktx5nEdD8EdqxhU bhFAK2ev96SKV+ect/IAgTBS4YzWrBhWJndu5TKZzW6VinzkP7nUwxwEaHcObgtK JvFerMIyFRG3lBI4sK2PVUxKYqe3BJxWt2aO5xYYVvTiCeY7gQ3xnDkW4ap0aG2a /dyLar7/feCNErcN6c2rkN464vaqsIB+dn11iBU0maD28t+cOxLviBUSuNMXDmh1 s7A3ag== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; bh=6GsaazfvdK8gce35ZNwXK+vEQLwz9dtpMdSu0mK1C iI=; b=A1JZOLh6rq35c/Gzl/1pYXbuH+O9dRaGPpc0l4rr/cfmG9cnC0nEkEpxZ t1TievggbllsU9Jpxw9OSTg62KzCFaiVEuXht92SCKiVoMQS2cO2x49eWZHnK8jz /1qaNU7F3wfGIYfVzEaTbUlg44iAzULLu9u1x6189MIXeLlYOTnL+6RjY2yKyltl oIt1R3hQJ5oK807pZcLJw1/GToQV1XtonJNAcxlusnXr/iFjbiExAsKTDK7ApjLi a5gn0ojNtHLMdWx6Y4bQKNmr5k8ub4fXO4v8IaVQScaA1N5c4XyYPfS3KtAhxWXE yJs/OoERgnnbzBZzwmAvioCoLA2tQ== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedujedrvdehuddgtdefucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhephffvufffkfgjfhgggfgtsehtufertddttddvnecuhfhrohhmpefvhhhomhgr shcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecugg ftrfgrthhtvghrnhepueevjefgkedtudffleehhfeuvdeigfdtjeeljedvheffveejheeu heegffekudeunecuffhomhgrihhnpehouhhtlhhoohhkrdgtohhmnecukfhppeejjedrud efgedrvddtfedrudekgeenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgr ihhlfhhrohhmpehthhhomhgrshesmhhonhhjrghlohhnrdhnvght X-ME-Proxy: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id 6DC131080064; Mon, 11 Jan 2021 04:23:37 -0500 (EST) From: Thomas Monjalon To: "Zhang, Qi Z" , "Yigit, Ferruh" , "Guo, Jia" , Andrew Rybchenko , Ori Kam Cc: "Wu, Jingjing" , "Yang, Qiming" , "Wang, Haiyue" , "dev@dpdk.org" , Gregory Etelson Date: Mon, 11 Jan 2021 10:23:36 +0100 Message-ID: <8947440.8VpXZ41EjM@thomas> In-Reply-To: References: <20201216085854.7842-1-jia.guo@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" 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" 10/01/2021 11:46, Ori Kam: > Hi, > > > -----Original Message----- > > From: Zhang, Qi Z > > Sent: Saturday, January 9, 2021 3:01 AM > > Subject: RE: [dpdk-dev] [dpdk-dev v2 1/2] ethdev: add new tunnel type for ecpri > > > > > > > > > -----Original Message----- > > > From: Thomas Monjalon > > > Subject: Re: [dpdk-dev] [dpdk-dev v2 1/2] ethdev: add new tunnel type for > > ecpri > > > > > > 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://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftools.ietf > > .org%2Fhtml%2Frfc7348&data=04%7C01%7Corika%40nvidia.com%7C46b2 > > d8f48944422f0d9008d8b43a2293%7C43083d15727340c1b7db39efd9ccc17a%7 > > C0%7C0%7C637457509081543237%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC > > 4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&a > > mp;sdata=RYWFMjuxkcUZ982kK2s44tBAjf%2FTkDyaa7VEybCtxOo%3D&res > > erved=0). > > > > >>>> > > > > >>>> "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. > > > > > > 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 > > rte_flow action to "define a packet's tunnel type", but we don't have. A packet type alone is meaningless. It is always associated to an action, this is what rte_flow does. > > #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 plan > > to move it to rte_flow, at least we need a replacement solution. The documentation does not say why it is useful. With rte_flow you don't need it because a flow is specified with its action. > Let me see if I understand it correctly. > In your case, the issue is that you need to configure the HW to parse the packet correctly right? > It is not about the matching it is about the configuration of the HW, you wish to tell > the HW that the packet should be parsed by different means correct? > > If this is the case it sounds to me that you should use rte_flow and if the > user adds the following rule: > #flow create pattern eth / ivp4 / udp port is 4789 / .. action ..... > You simply need to configure your HW to support the ecpri configuration. > > > > > > > > > > > > > Isn't this more a device configuration than filtering, not sure > > > > > about using rte_flow for this. > > > > > > > > +1 > > > > > > A device configuration? No, setting an UDP port is a stack configuration. > > > > > > > > > > >> BTW, are we going to move all other filter like mac , VLAN > > > > >> filter/strip/insert into rte_flow finally? > > > > > > Yes I think it should be the direction. > > > All of this can be achieved with rte_flow. > > > > > > > > > > >> 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. > > > > > > 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. > > > > >