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 9722F43AB3; Fri, 9 Feb 2024 14:44:39 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 7E08C42E64; Fri, 9 Feb 2024 14:44:39 +0100 (CET) Received: from wfhigh5-smtp.messagingengine.com (wfhigh5-smtp.messagingengine.com [64.147.123.156]) by mails.dpdk.org (Postfix) with ESMTP id 2F6D640697 for ; Fri, 9 Feb 2024 14:44:38 +0100 (CET) Received: from compute2.internal (compute2.nyi.internal [10.202.2.46]) by mailfhigh.west.internal (Postfix) with ESMTP id 030B818000D0; Fri, 9 Feb 2024 08:44:35 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute2.internal (MEProxy); Fri, 09 Feb 2024 08:44:36 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= cc:cc:content-transfer-encoding:content-type:content-type:date :date:from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to; s=fm3; t=1707486275; x=1707572675; bh=PJYf/UMvPgYRXB2OEiMmX6rzTqkug3pfypoWn8GUCQg=; b= I9xDx1s22dhwZUnS/CdPyXYy0bxeC1yl1J0li6aeDe6GEuOkPSEgOMbRtGANfPbh LSI+CjGOPh8LzDHHhqrK+KCFlDDwGmyC+33wHzXl7KLyhgd904tuMM09GRvEyt16 gEbXS0mhjHsR07XNUkb+hDP8bhNOLvg3eC7uwSXG2GCSqL/t0pSLBMzsbhL4w2PP XSi0jqLqjMdcZmNT33/XxuXU5r7O/mLp9Uu4kb2XF7eUsMzrjT5ZV61PaDRaQndZ iK1vhcmupU8MYc+Kp2P35Vh0RgmdsSU38qYDhQsCWXBIzxPP2nLaXHJytFRhtQX9 s/UtIT2VRbdKUck2PtuB5w== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to:x-me-proxy:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm3; t=1707486275; x= 1707572675; bh=PJYf/UMvPgYRXB2OEiMmX6rzTqkug3pfypoWn8GUCQg=; b=p loFVWiezAujyc15BSBCTKBX0AXS+HZwue1O/RLcD8+gQiK5x+kIvXpfMKWDKqbLZ muqiWK4wE9ofQUeMbxKXgCQ+zSizXc6353fsYIrZ2UuQ9/rIsUzaJpPaPy4mLlwT z9X7xPtKZoYmmoXbTFfFVYiQfQnGLTjRXjxWGzZJDggbmCMC0TSuOqu4FD0BdfF6 A04C07CNZFnrko6lP0rjD8Ms+NJvttMt290ysXSVnkh0p7YvV0+69i9TlDsOfuz2 jO+HmpcsarzzwvBSuPhokcbWNkxiTLm4sZ0p/QouwnTQK2M1Tc1f4S9Iziomh4lH G6TPu436WgBv3RQ6Ns7sA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvledrtdeigdehgecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefhvfevufffkfgjfhgggfgtsehtqhertddttdejnecuhfhrohhmpefvhhhomhgr shcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecugg ftrfgrthhtvghrnhepgedttdeljeejgeffkeekkedtjeevtdehvedtkeeivdeuuedviedu vdelveejueejnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrh homhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvth X-ME-Proxy: Feedback-ID: i47234305:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 9 Feb 2024 08:44:34 -0500 (EST) From: Thomas Monjalon To: Gavin Li , orika@nvidia.com, andrew.rybchenko@oktetlabs.ru, Ferruh Yigit Cc: dev@dpdk.org, jiaweiw@nvidia.com Subject: Re: [RFC V1 1/1] net: extend VXLAN header to support more extensions Date: Fri, 09 Feb 2024 14:44:32 +0100 Message-ID: <4126957.6PsWsQAL7t@thomas> In-Reply-To: <59a69287-cde3-4e9e-8174-c454f964fe5e@amd.com> References: <20240130112520.1971315-1-gavinl@nvidia.com> <2384008.yKrmzQ4Hd0@thomas> <59a69287-cde3-4e9e-8174-c454f964fe5e@amd.com> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="UTF-8" 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 09/02/2024 13:11, Ferruh Yigit: > On 2/9/2024 10:12 AM, Thomas Monjalon wrote: > > 09/02/2024 00:54, Ferruh Yigit: > >> On 1/30/2024 11:25 AM, Gavin Li wrote: > >>> Currently, DPDK supports VXLAN and VXLAN-GPE with similar header > >>> structures and we are working on adding support for VXLAN-GBP which is > >>> another extension to VXLAN. More extension of VXLAN may be added in t= he > >>> future. > >>> > >>> VXLAN and VXLAN-GBP use the same UDP port(4789) while VXLAN-GPE uses a > >>> different one, 4790. The three protocols have the same header length = and > >>> overall similar header structure as below. > >>> 0 1 2 3 > >>> 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 > >>> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > >>> |R|R|R|R|I|R|R|R| Reserved | > >>> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > >>> | VXLAN Network Identifier (VNI) | Reserved | > >>> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > >>> > >>> Figure 1: VXLAN Header > >>> > >>> 0 1 2 3 > >>> 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 > >>> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > >>> |R|R|Ver|I|P|B|O| Reserved |Next Protocol | > >>> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > >>> | VXLAN Network Identifier (VNI) | Reserved | > >>> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > >>> > >>> Figure 2: VXLAN-GPE Header > >>> > >>> 0 1 2 3 > >>> 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 > >>> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > >>> |G|R|R|R|I|R|R|R|R|D|R|R|A|R|R|R| Group Policy ID | > >>> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > >>> | VXLAN Network Identifier (VNI) | Reserved | > >>> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > >>> > >>> Figure 3: VXLAN-GBP Extension > >>> > >>> Both VXLAN-GPE and VXLAN-GBP extended VXLAN by redefining its reserved > >>> bits, which means the packets can be processed with same pattern and = most > >>> of the code can be reused. Instead of adding more new items by > >>> copying/pasting code for the VXLAN extensions in the future, it=E2=80= =99s better > >>> to use existing VXLAN infrastructure and add support code in it. > >>> > >> > >> Hi Gavin, > >> > >> The motivation is to prevent code duplication, and the code mentioned = is > >> the driver code, right? > >=20 > > The motivation is mainly to provide a unified and more explicit API. > >=20 >=20 > From user perspective, I think existing approach is more explicit, > because it sets VXLAN or VXLAN_GPE flow types. >=20 > I am trying to understand the benefit, how unifying flow type in the API > helps to the user? >=20 > >> Overall OK to unify "struct rte_vxlan_hdr" although it makes the struct > >> a little complex, perhaps we can consider extraction some nested struc= ts > >> as named struct, no strong opinion. > >> > >> > >> But not sure about removing the flow item types for VXLAN-GPE, or not > >> adding for VXLAN-GBP. > >> > >> Think about a case user adding a rule, which has a item type as VXLAN > >> and in the protocol header some bits are set, lets say first word, last > >> byte is set, how driver will know if to take it as GPE "next protocol" > >> or "group policy id". > >=20 > > The driver may decide depending on the UDP port and some distinguishing= flags. > > If you want to match on GBP, you should includes the GBP flag in your p= attern, > > no need to use a separate item. > >=20 >=20 > Why not be more explicit? > It helps to driver to know more about the pattern to be able to create > proper flow rule, if there is an obvious way for driver to differentiate > these protocol extensions, and flow item type is redundant, I can > understand the proposal, but otherwise I think better to keep flow items > for extensions. In any case we need the simple VXLAN item. If we have GPE and GBP specialized items, what means a match on the simple VXLAN? Does it include packets with other extensions or exclude them? Matching the bits in the protocol make such decision explicit. > When a rule is set in HW, HW may not care about the protocol, as long as > bits in the rule match with the packet, HW can apply the action. > But for driver to be able to set the rule properly, it needs more > explicit information. Yes information is in the pattern to match. > Lets assume driver API get a pattern with 'RTE_FLOW_ITEM_TYPE_VXLAN' > type and "struct rte_flow_item_vxlan", at this point driver doesn't know > if it is VXLAN or any of the extensions. Yes it knows because of the matched bits in the pattern. If the rule specify a match on GBP flag =3D 1, it is GBP only. If the rule specify a match on GBP flag =3D 0, it excludes GBP. If the rule does not mask GBP flag, it includes GBP. > Should driver go and check GBP flags to deduce if it is GBP flag, and Yes > what if they are all zero, how can driver can tell if this is GBP flag > that is zero or is it VXLAN header and bit is reserved. As explained above, value & mask does the matching. > Or I will just make up a sample, it may not be accurate but please take > it as a case to prove a point. > Lets assume driver API again get "struct rte_flow_item_vxlan" whose > first word's last bit is set, which union in the struct will driver > check to detect this, GPE one or GBP one? It depends on the G flag. > This can be GPE-"next protocol" or GBP-"Group policy id", if driver > knows this it can set the mask better for the field in the rule. > Driver may try to deduce this extension information from other > information, but why not proper specific flow type for each extension > instead? I believe relying on bit matching is a cleaner approach than creating a new item each time a protocol has a variation. And it makes include/exclude policy easier to understand thanks to the value/mask logic. > >> And if a specific HW detects VXLAN-GPE and VXLAN-GBP protocols as two > >> separate protocols, won't only having capability to describe VXLAN > >> protocol in SW be a limitation. > >=20 > > I think the driver may know based on the flow rule. > >=20 > > That's a good question about the item granularity. > > What is the separation between a different protocol and protocol option= s? > > How flow item should match protocol variations? > > My current thinking is that we should minimize the number of items. > >=20 > >> If the target is to remove code duplication in the driver, can it be > >> accomplished by extracting code that parse the common fields of these > >> protocols? > >=20 > > Driver code is not a concern as far as any driver can implement the API. > >=20 >=20 > Got it, I want to clarify this one. > Where the code duplication via copy/paste that commit log mention occurs? >=20 > >=20 > >>> In this patch, all the VXLAN extension header will be merged with VXL= AN as > >>> union if the overlapped field has different format among protocols. T= he > >>> existing VXLAN-GPE will be marked as deprecated and new extensions of > >>> VXLAN should be added to VXLAN instead of a new RTE item. > >>> > >>> Signed-off-by: Gavin Li > >>> --- > >>> doc/guides/rel_notes/deprecation.rst | 5 +++ > >>> lib/ethdev/rte_flow.h | 13 +++++- > >>> lib/net/rte_vxlan.h | 67 ++++++++++++++++++++++++++= =2D- > >>> 3 files changed, 80 insertions(+), 5 deletions(-) > >>> > >>> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_no= tes/deprecation.rst > >>> index 81b93515cb..f9cf931b77 100644 > >>> --- a/doc/guides/rel_notes/deprecation.rst > >>> +++ b/doc/guides/rel_notes/deprecation.rst > >>> @@ -95,6 +95,11 @@ Deprecation Notices > >>> - ``rte_flow_item_pppoe`` > >>> - ``rte_flow_item_pppoe_proto_id`` > >>> =20 > >>> +* ethdev: The flow item ``RTE_FLOW_ITEM_TYPE_VXLAN_GPE`` is replaced= with ``RTE_FLOW_ITEM_TYPE_VXLAN``. > >>> + The item ``RTE_FLOW_ITEM_TYPE_VXLAN_GPE``, the struct ``rte_flow_i= tem_vxlan_gpe``, its mask ``rte_flow_item_vxlan_gpe_mask``, > >>> + and the header struct ``rte_vxlan_gpe_hdr`` with the macro ``RTE_E= THER_VXLAN_GPE_HLEN`` > >>> + will be removed in DPDK 25.11. > >>> + > >>> > >> > >> We have 24.11 in between. > >=20 > > Isn't it too soon? > > Should we remove at all?