From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id CDC69A00E6 for ; Wed, 10 Jul 2019 14:26:47 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 972FD2C60; Wed, 10 Jul 2019 14:26:46 +0200 (CEST) Received: from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com [66.111.4.26]) by dpdk.org (Postfix) with ESMTP id 423AE1D7 for ; Wed, 10 Jul 2019 14:26:45 +0200 (CEST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 77BA321E97; Wed, 10 Jul 2019 08:26:44 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Wed, 10 Jul 2019 08:26:44 -0400 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=mesmtp; bh=Z0i3TJKjmIjAJbmngDCww61sU4M2wA4wFuErjQZKG0c=; b=cQSmSoXWK3l9 /dAt7sIBDu0euWNoaNeRyA+8TuxKsInOjKr+Lc90eWcWjslpM2FN/QPSCTZELxUx bKIJMuI+IZR9blcaj7o9YIUBHnv6r05QDv2XOa6HqcQ/Lca5apdmDg945oKUcl9o Pf4qOm120mjCOMthTw4XynQ2PE50TOU= 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=fm3; bh=Z0i3TJKjmIjAJbmngDCww61sU4M2wA4wFuErjQZKG 0c=; b=x2KmED5BGtBKw1mOQ5a0HzxOheg1NjBhyg/M9C06HNfcjrGDbwfmYl6/6 t9U7FaVM5WnR4Ek9TmUnZ/yLEa/75GsILvwFwfd9VuSobAn6KszJy7H0gP0KofuE MKVm9/c7Xyh3Q64xaV+Ub71io44Q5n2GsXQNh5t+kChuE83L4EZfpZtHcBSDLzZr lNjA77fXFXRLDkRHfxQiMfhYOmlz4CsdbUFf3aRWNdDOUKqSlQ/1N2da7FuyvZdR 7drLRs5Z+HGyEbP6oqByxt95Dhvr/YwAAxHC4PIPyxLvAGuGXLwhIb1ql67Zakg4 vmrRVuzf21c/ibpADAArLgWlcP8hQ== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduvddrgeeigdehfecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefhvffufffkjghfggfgtgesthfuredttddtvdenucfhrhhomhepvfhhohhmrghs ucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenucfkph epudelfedrvdehtddrkeegrddvgeegnecurfgrrhgrmhepmhgrihhlfhhrohhmpehthhho mhgrshesmhhonhhjrghlohhnrdhnvghtnecuvehluhhsthgvrhfuihiivgeptd X-ME-Proxy: Received: from xps.localnet (lfbn-1-8944-244.w193-250.abo.wanadoo.fr [193.250.84.244]) by mail.messagingengine.com (Postfix) with ESMTPA id D011F80064; Wed, 10 Jul 2019 08:26:41 -0400 (EDT) From: Thomas Monjalon To: Bruce Richardson , Olivier Matz , arybchenko@solarflare.com, adrien.mazarguil@6wind.com Cc: Yongseok Koh , shahafs@mellanox.com, ferruh.yigit@intel.com, dev@dpdk.org, viacheslavo@mellanox.com Date: Wed, 10 Jul 2019 14:26:38 +0200 Message-ID: <6507101.CIctlFmaPS@xps> In-Reply-To: <20190710120128.GC505@bricha3-MOBL.ger.corp.intel.com> References: <20190603213231.27020-1-yskoh@mellanox.com> <20190710100743.z5ioyxish4wnh3s4@glumotte.dev.6wind.com> <20190710120128.GC505@bricha3-MOBL.ger.corp.intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH] ethdev: extend flow metadata 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 10/07/2019 14:01, Bruce Richardson: > On Wed, Jul 10, 2019 at 12:07:43PM +0200, Olivier Matz wrote: > > On Wed, Jul 10, 2019 at 10:55:34AM +0100, Bruce Richardson wrote: > > > On Wed, Jul 10, 2019 at 11:31:56AM +0200, Olivier Matz wrote: > > > > On Thu, Jul 04, 2019 at 04:21:22PM -0700, Yongseok Koh wrote: > > > > > Currently, metadata can be set on egress path via mbuf tx_meatadata field > > > > > with PKT_TX_METADATA flag and RTE_FLOW_ITEM_TYPE_RX_META matches metadata. > > > > > > > > > > This patch extends the usability. > > > > > > > > > > 1) RTE_FLOW_ACTION_TYPE_SET_META > > > > > > > > > > When supporting multiple tables, Tx metadata can also be set by a rule and > > > > > matched by another rule. This new action allows metadata to be set as a > > > > > result of flow match. > > > > > > > > > > 2) Metadata on ingress > > > > > > > > > > There's also need to support metadata on packet Rx. Metadata can be set by > > > > > SET_META action and matched by META item like Tx. The final value set by > > > > > the action will be delivered to application via mbuf metadata field with > > > > > PKT_RX_METADATA ol_flag. > > > > > > > > > > For this purpose, mbuf->tx_metadata is moved as a separate new field and > > > > > renamed to 'metadata' to support both Rx and Tx metadata. > > > > > > > > > > For loopback/hairpin packet, metadata set on Rx/Tx may or may not be > > > > > propagated to the other path depending on HW capability. > > > > > > > > > > Signed-off-by: Yongseok Koh > > > > > > > > > --- a/lib/librte_mbuf/rte_mbuf.h > > > > > +++ b/lib/librte_mbuf/rte_mbuf.h > > > > > @@ -648,17 +653,6 @@ struct rte_mbuf { > > > > > /**< User defined tags. See rte_distributor_process() */ > > > > > uint32_t usr; > > > > > } hash; /**< hash information */ > > > > > - struct { > > > > > - /** > > > > > - * Application specific metadata value > > > > > - * for egress flow rule match. > > > > > - * Valid if PKT_TX_METADATA is set. > > > > > - * Located here to allow conjunct use > > > > > - * with hash.sched.hi. > > > > > - */ > > > > > - uint32_t tx_metadata; > > > > > - uint32_t reserved; > > > > > - }; > > > > > }; > > > > > > > > > > /** Outer VLAN TCI (CPU order), valid if PKT_RX_QINQ is set. */ > > > > > @@ -727,6 +721,11 @@ struct rte_mbuf { > > > > > */ > > > > > struct rte_mbuf_ext_shared_info *shinfo; > > > > > > > > > > + /** Application specific metadata value for flow rule match. > > > > > + * Valid if PKT_RX_METADATA or PKT_TX_METADATA is set. > > > > > + */ > > > > > + uint32_t metadata; > > > > > + > > > > > } __rte_cache_aligned; > > > > > > > > This will break the ABI, so we cannot put it in 19.08, and we need a > > > > deprecation notice. > > > > > > > Does it actually break the ABI? Adding a new field to the mbuf should only > > > break the ABI if it either causes new fields to move or changes the > > > structure size. Since this is at the end, it's not going to move any older > > > fields, and since everything is cache-aligned I don't think the structure > > > size changes either. > > > > I think it does break the ABI: in previous version, when the PKT_TX_METADATA > > flag is set, the associated value is put in m->tx_metadata (offset 44 on > > x86-64), and in the next version, it will be in m->metadata (offset 112). So, > > these 2 versions are not binary compatible. > > > > Anyway, at least it breaks the API. > > Ok, I misunderstood. I thought it was the structure change itself you were > saying broke the ABI. Yes, putting the data in a different place is indeed > an ABI break. We could add the new field and keep the old one unused, so it does not break the ABI. However I suppose everybody will prefer a version using dynamic fields. Is someone against using dynamic field for such usage?