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 E0F32A00E6 for ; Wed, 10 Jul 2019 12:07:47 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id A048A324D; Wed, 10 Jul 2019 12:07:46 +0200 (CEST) Received: from proxy.6wind.com (host.76.145.23.62.rev.coltfrance.com [62.23.145.76]) by dpdk.org (Postfix) with ESMTP id B0ADC2E83 for ; Wed, 10 Jul 2019 12:07:44 +0200 (CEST) Received: from core.dev.6wind.com (unknown [10.0.0.1]) by proxy.6wind.com (Postfix) with ESMTPS id 8A6812E4438; Wed, 10 Jul 2019 12:07:44 +0200 (CEST) Received: from [10.16.0.195] (helo=6wind.com) by core.dev.6wind.com with smtp (Exim 4.84_2) (envelope-from ) id 1hl9Vn-0001Ub-Fv; Wed, 10 Jul 2019 12:07:44 +0200 Received: by 6wind.com (sSMTP sendmail emulation); Wed, 10 Jul 2019 12:07:43 +0200 Date: Wed, 10 Jul 2019 12:07:43 +0200 From: Olivier Matz To: Bruce Richardson Cc: Yongseok Koh , shahafs@mellanox.com, thomas@monjalon.net, ferruh.yigit@intel.com, arybchenko@solarflare.com, adrien.mazarguil@6wind.com, dev@dpdk.org, viacheslavo@mellanox.com Message-ID: <20190710100743.z5ioyxish4wnh3s4@glumotte.dev.6wind.com> References: <20190603213231.27020-1-yskoh@mellanox.com> <20190704232122.19477-1-yskoh@mellanox.com> <20190710093156.va3rk5jmz4oj7hfx@glumotte.dev.6wind.com> <20190710095533.GA505@bricha3-MOBL.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190710095533.GA505@bricha3-MOBL.ger.corp.intel.com> User-Agent: NeoMutt/20170113 (1.7.2) 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" 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: > > Hi Yongseok, > > > > 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 > > > @@ -200,6 +200,11 @@ extern "C" { > > > > > > /* add new RX flags here */ > > > > > > +/** > > > + * Indicate that mbuf has metadata from device. > > > + */ > > > +#define PKT_RX_METADATA (1ULL << 23) > > > + > > > /* add new TX flags here */ > > > > > > /** > > > @@ -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.