DPDK patches and discussions
 help / color / mirror / Atom feed
From: Bruce Richardson <bruce.richardson@intel.com>
To: Olivier Matz <olivier.matz@6wind.com>
Cc: Yongseok Koh <yskoh@mellanox.com>,
	shahafs@mellanox.com, thomas@monjalon.net,
	ferruh.yigit@intel.com, arybchenko@solarflare.com,
	adrien.mazarguil@6wind.com, dev@dpdk.org,
	viacheslavo@mellanox.com
Subject: Re: [dpdk-dev] [PATCH] ethdev: extend flow metadata
Date: Wed, 10 Jul 2019 13:01:29 +0100	[thread overview]
Message-ID: <20190710120128.GC505@bricha3-MOBL.ger.corp.intel.com> (raw)
In-Reply-To: <20190710100743.z5ioyxish4wnh3s4@glumotte.dev.6wind.com>

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:
> > > 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 <yskoh@mellanox.com>
> > > 
> > > (...)
> > > 
> > > > --- 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.

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.

/Bruce

  reply	other threads:[~2019-07-10 12:01 UTC|newest]

Thread overview: 98+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-03 21:32 [dpdk-dev] [RFC 1/3] " Yongseok Koh
2019-06-03 21:32 ` [dpdk-dev] [RFC 2/3] ethdev: add flow modify mark action Yongseok Koh
2019-06-06 10:35   ` Jerin Jacob Kollanukkaran
2019-06-06 18:33     ` Yongseok Koh
2019-06-03 21:32 ` [dpdk-dev] [RFC 3/3] ethdev: add flow tag Yongseok Koh
2019-07-04 23:23   ` [dpdk-dev] [PATCH] " Yongseok Koh
2019-07-05 13:54     ` Adrien Mazarguil
2019-07-05 18:05       ` Yongseok Koh
2019-07-08 23:32         ` Yongseok Koh
2019-07-09  8:38         ` Adrien Mazarguil
2019-07-11  1:59           ` Yongseok Koh
2019-10-08 12:57             ` Yigit, Ferruh
2019-10-08 13:18               ` Slava Ovsiienko
2019-10-10 16:09     ` [dpdk-dev] [PATCH v2] " Viacheslav Ovsiienko
2019-10-24 13:12       ` [dpdk-dev] [PATCH v3] " Viacheslav Ovsiienko
2019-10-27 16:38         ` Ori Kam
2019-10-27 18:42         ` [dpdk-dev] [PATCH v4] " Viacheslav Ovsiienko
2019-10-27 19:11           ` Ori Kam
2019-10-31 18:57             ` Ferruh Yigit
2019-06-09 14:23 ` [dpdk-dev] [RFC 1/3] ethdev: extend flow metadata Andrew Rybchenko
2019-06-10  3:19   ` Wang, Haiyue
2019-06-10  7:20     ` Andrew Rybchenko
2019-06-11  0:06       ` Yongseok Koh
2019-06-19  9:05         ` Andrew Rybchenko
2019-07-04 23:21 ` [dpdk-dev] [PATCH] " Yongseok Koh
2019-07-10  9:31   ` Olivier Matz
2019-07-10  9:55     ` Bruce Richardson
2019-07-10 10:07       ` Olivier Matz
2019-07-10 12:01         ` Bruce Richardson [this message]
2019-07-10 12:26           ` Thomas Monjalon
2019-07-10 16:37             ` Yongseok Koh
2019-07-11  7:44               ` Adrien Mazarguil
2019-07-14 11:46                 ` Andrew Rybchenko
2019-07-29 15:06                   ` Adrien Mazarguil
2019-10-08 12:51                     ` Yigit, Ferruh
2019-10-08 13:17                       ` Slava Ovsiienko
2019-10-10 16:02   ` [dpdk-dev] [PATCH v2] " Viacheslav Ovsiienko
2019-10-18  9:22     ` Olivier Matz
2019-10-19 19:47       ` Slava Ovsiienko
2019-10-21 16:37         ` Olivier Matz
2019-10-24  6:49           ` Slava Ovsiienko
2019-10-24  9:22             ` Olivier Matz
2019-10-24 12:30               ` Slava Ovsiienko
2019-10-24 13:08     ` [dpdk-dev] [PATCH v3] " Viacheslav Ovsiienko
2019-10-27 16:56       ` Ori Kam
2019-10-27 18:40       ` [dpdk-dev] [PATCH v4] " Viacheslav Ovsiienko
2019-10-27 19:10         ` Ori Kam
2019-10-29 16:22         ` Andrew Rybchenko
2019-10-29 17:19           ` Slava Ovsiienko
2019-10-29 18:30             ` Thomas Monjalon
2019-10-29 18:35               ` Slava Ovsiienko
2019-10-30  6:28               ` Andrew Rybchenko
2019-10-30  7:35             ` Andrew Rybchenko
2019-10-30  8:59               ` Slava Ovsiienko
2019-10-30  9:20                 ` Andrew Rybchenko
2019-10-30 10:05                   ` Slava Ovsiienko
2019-10-30 10:03                 ` Slava Ovsiienko
2019-10-30 15:49               ` Olivier Matz
2019-10-31  9:25                 ` Andrew Rybchenko
2019-10-29 16:25         ` Olivier Matz
2019-10-29 16:33           ` Olivier Matz
2019-10-29 17:53             ` Slava Ovsiienko
2019-10-29 17:43           ` Slava Ovsiienko
2019-10-29 19:31         ` [dpdk-dev] [PATCH v5] " Viacheslav Ovsiienko
2019-10-30  8:02           ` Andrew Rybchenko
2019-10-30 14:40             ` Slava Ovsiienko
2019-10-30 14:46               ` Slava Ovsiienko
2019-10-30 15:20                 ` Olivier Matz
2019-10-30 15:57                   ` Thomas Monjalon
2019-10-30 15:58                   ` Slava Ovsiienko
2019-10-30 16:13                     ` Olivier Matz
2019-10-30  8:35           ` Ori Kam
2019-10-30 17:12           ` [dpdk-dev] [PATCH v6 0/2] extend flow metadata feature Viacheslav Ovsiienko
2019-10-30 17:12             ` [dpdk-dev] [PATCH v6 1/2] ethdev: extend flow metadata Viacheslav Ovsiienko
2019-10-31  9:19               ` Andrew Rybchenko
2019-10-31 13:05               ` [dpdk-dev] [PATCH v7 0/2] extend flow metadata feature Viacheslav Ovsiienko
2019-10-31 13:05                 ` [dpdk-dev] [PATCH v7 1/2] ethdev: extend flow metadata Viacheslav Ovsiienko
2019-10-31 15:47                   ` Olivier Matz
2019-10-31 16:13                     ` Slava Ovsiienko
2019-10-31 16:48                   ` [dpdk-dev] [PATCH v8 0/2] extend flow metadata feature Viacheslav Ovsiienko
2019-10-31 16:48                     ` [dpdk-dev] [PATCH v8 1/2] ethdev: extend flow metadata Viacheslav Ovsiienko
2019-11-04  6:13                       ` [dpdk-dev] [PATCH v9 0/2] extend flow metadata feature Viacheslav Ovsiienko
2019-11-04  6:13                         ` [dpdk-dev] [PATCH v9 1/2] ethdev: extend flow metadata Viacheslav Ovsiienko
2019-11-05 14:19                           ` [dpdk-dev] [PATCH v10 0/2] extend flow metadata feature Viacheslav Ovsiienko
2019-11-05 14:19                             ` [dpdk-dev] [PATCH v10 1/2] ethdev: extend flow metadata Viacheslav Ovsiienko
2019-11-05 14:19                             ` [dpdk-dev] [PATCH v10 2/2] ethdev: move egress metadata to dynamic field Viacheslav Ovsiienko
2019-11-06 15:49                             ` [dpdk-dev] [PATCH v10 0/2] extend flow metadata feature Ferruh Yigit
2019-11-04  6:13                         ` [dpdk-dev] [PATCH v9 2/2] ethdev: move egress metadata to dynamic field Viacheslav Ovsiienko
2019-10-31 16:48                     ` [dpdk-dev] [PATCH v8 " Viacheslav Ovsiienko
2019-10-31 17:21                       ` Olivier Matz
2019-11-01 12:34                       ` Andrew Rybchenko
2019-10-31 13:05                 ` [dpdk-dev] [PATCH v7 " Viacheslav Ovsiienko
2019-10-31 13:33                   ` Ori Kam
2019-10-31 15:51                   ` Olivier Matz
2019-10-31 16:07                     ` Slava Ovsiienko
2019-10-30 17:12             ` [dpdk-dev] [PATCH v6 " Viacheslav Ovsiienko
2019-10-31  9:01               ` Andrew Rybchenko
2019-10-31 10:54                 ` Slava Ovsiienko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190710120128.GC505@bricha3-MOBL.ger.corp.intel.com \
    --to=bruce.richardson@intel.com \
    --cc=adrien.mazarguil@6wind.com \
    --cc=arybchenko@solarflare.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=olivier.matz@6wind.com \
    --cc=shahafs@mellanox.com \
    --cc=thomas@monjalon.net \
    --cc=viacheslavo@mellanox.com \
    --cc=yskoh@mellanox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).