DPDK patches and discussions
 help / color / mirror / Atom feed
From: Slava Ovsiienko <viacheslavo@mellanox.com>
To: Olivier Matz <olivier.matz@6wind.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>, Matan Azrad <matan@mellanox.com>,
	Raslan Darawsheh <rasland@mellanox.com>,
	Thomas Monjalon <thomas@monjalon.net>,
	"arybchenko@solarflare.com" <arybchenko@solarflare.com>,
	Ori Kam <orika@mellanox.com>, Yongseok Koh <yskoh@mellanox.com>
Subject: Re: [dpdk-dev] [PATCH v7 1/2] ethdev: extend flow metadata
Date: Thu, 31 Oct 2019 16:13:27 +0000	[thread overview]
Message-ID: <AM4PR05MB3265EB0864E8D1E7FF87A495D2630@AM4PR05MB3265.eurprd05.prod.outlook.com> (raw)
In-Reply-To: <20191031154728.m2yrkydqbo4jwglf@platinum>

Hi Olivier,

> -----Original Message-----
> From: Olivier Matz <olivier.matz@6wind.com>
> Sent: Thursday, October 31, 2019 17:47
> To: Slava Ovsiienko <viacheslavo@mellanox.com>
> Cc: dev@dpdk.org; Matan Azrad <matan@mellanox.com>; Raslan
> Darawsheh <rasland@mellanox.com>; Thomas Monjalon
> <thomas@monjalon.net>; arybchenko@solarflare.com; Ori Kam
> <orika@mellanox.com>; Yongseok Koh <yskoh@mellanox.com>
> Subject: Re: [PATCH v7 1/2] ethdev: extend flow metadata
> 
> Hi Slava,
> 
> One comment at the end.
> 
> On Thu, Oct 31, 2019 at 01:05:20PM +0000, Viacheslav Ovsiienko wrote:
> > Currently, metadata can be set on egress path via mbuf tx_metadata
> > field with PKT_TX_METADATA flag and RTE_FLOW_ITEM_TYPE_META
> matches metadata.
> >
> > This patch extends the metadata feature 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 ingress. 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 metadata
> > dynamic field of mbuf which can be accessed by
> > RTE_FLOW_DYNF_METADATA() macro or with
> > rte_flow_dynf_metadata_set() and rte_flow_dynf_metadata_get() helper
> > routines. PKT_RX_DYNF_METADATA flag will be set along with the data.
> >
> > The mbuf dynamic field must be registered by calling
> > rte_flow_dynf_metadata_register() prior to use SET_META action.
> >
> > The availability of dynamic mbuf metadata field can be checked with
> > rte_flow_dynf_metadata_avail() routine.
> >
> > If application is going to engage the metadata feature it registers
> > the metadata  dynamic fields, then PMD checks the metadata field
> > availability and handles the appropriate fields in datapath.
> >
> > For loopback/hairpin packet, metadata set on Rx/Tx may or may not be
> > propagated to the other path depending on hardware capability.
> >
> > MARK and METADATA look similar and might operate in similar way, but
> > not interacting.
> >
> > Initially, there were proposed two metadata related actions:
> >
> > - RTE_FLOW_ACTION_TYPE_FLAG
> > - RTE_FLOW_ACTION_TYPE_MARK
> >
> > These actions set the special flag in the packet metadata, MARK action
> > stores some specified value in the metadata storage, and, on the
> > packet receiving PMD puts the flag and value to the mbuf and
> > applications can see the packet was threated inside flow engine
> > according to the appropriate RTE flow(s). MARK and FLAG are like some
> > kind of gateway to transfer some per-packet information from the flow
> > engine to the application via receiving datapath. Also, there is the
> > item of type RTE_FLOW_ITEM_TYPE_MARK provided. It allows us to
> extend
> > the flow match pattern with the capability to match the metadata values
> set by MARK/FLAG actions on other flows.
> >
> > From the datapath point of view, the MARK and FLAG are related to the
> > receiving side only. It would useful to have the same gateway on the
> > transmitting side and there was the feature of type
> > RTE_FLOW_ITEM_TYPE_META was proposed. The application can fill the
> > field in mbuf and this value will be transferred to some field in the
> > packet metadata inside the flow engine. It did not matter whether
> > these metadata fields are shared because of MARK and META items
> > belonged to different domains (receiving and
> > transmitting) and could be vendor-specific.
> >
> > So far, so good, DPDK proposes some entities to control metadata
> > inside the flow engine and gateways to exchange these values on a
> > per-packet basis via datapaths.
> >
> > As we can see, the MARK and META means are not symmetric, there is
> > absent action which would allow us to set META value on the transmitting
> path.
> > So, the action of type:
> >
> > - RTE_FLOW_ACTION_TYPE_SET_META was proposed.
> >
> > The next, applications raise the new requirements for packet metadata.
> > The flow ngines are getting more complex, internal switches are
> > introduced, multiple ports might be supported within the same flow engine
> namespace.
> > From the DPDK points of view, it means the packets might be sent on
> > one eth_dev port and received on the other one, and the packet path
> > inside the flow engine entirely belongs to the same hardware device.
> > The simplest example is SR-IOV with PF, VFs and the representors. And
> > there is a brilliant opportunity to provide some out-of-band channel
> > to transfer some extra data from one port to another one, besides the
> > packet data itself. And applications would like to use this opportunity.
> >
> > It is supposed for application to use trials (with rte_flow_validate)
> > to detect which metadata features (FLAG, MARK, META) actually
> > supported by PMD and underlying hardware. It might depend on PMD
> > configuration, system software, hardware settings, etc., and should be
> > detected in run time.
> >
> > Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> > Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> > Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> > Acked-by: Ori Kam <orika@mellanox.com>
> > ---
> > v7: - updated release notes in collateral patch
> >
> > v6: -
> https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatche
> s.dpdk.org%2Fpatch%2F62245%2F&amp;data=02%7C01%7Cviacheslavo%40
> mellanox.com%7C1390029b214e4543f1e708d75e19a404%7Ca652971c7d2e
> 4d9ba6a4d149256f461b%7C0%7C0%7C637081336535150897&amp;sdata=lZ
> SRRw5DmT1cGeJ7Q57x8qIJSsFLPC%2FrJpzH3Yp%2Bf2k%3D&amp;reserved=
> 0
> >     - minor code style issues
> >     - is combined in series with followed egress metadata patch
> >
> > v5: -
> https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatche
> s.dpdk.org%2Fpatch%2F62179%2F&amp;data=02%7C01%7Cviacheslavo%40
> mellanox.com%7C1390029b214e4543f1e708d75e19a404%7Ca652971c7d2e
> 4d9ba6a4d149256f461b%7C0%7C0%7C637081336535150897&amp;sdata=L
> EL91A7XH%2BXYJr19%2FxpcEUmzr2ZMITfUJu%2FY8IfnLR0%3D&amp;reserv
> ed=0
> >     - addressed code style issues from comments
> >     - Tx metadata deprecation notice removed
> >       (dedicated tx_metadata patch is coming)
> >     - MBUF_DYNF_METADATA_NAME is splitted into FIELD and FLAG
> >       dedicated ones, RTE suffix is added
> >     - metadata historic retrospective is added to log message
> >     - rebased
> >
> > v4: -
> https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatche
> s.dpdk.org%2Fpatch%2F62065%2F&amp;data=02%7C01%7Cviacheslavo%40
> mellanox.com%7C1390029b214e4543f1e708d75e19a404%7Ca652971c7d2e
> 4d9ba6a4d149256f461b%7C0%7C0%7C637081336535150897&amp;sdata=z
> wSoQ0IhWjalmLqSv5CLSNtBt96FJwxmnZQOL%2FLIupw%3D&amp;reserved=
> 0
> >     - documentation comments addressed
> >     - deprecation notice for Tx metadata offload flag
> >     - rebased
> >
> > v3: -
> https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatche
> s.dpdk.org%2Fpatch%2F61902%2F&amp;data=02%7C01%7Cviacheslavo%40
> mellanox.com%7C1390029b214e4543f1e708d75e19a404%7Ca652971c7d2e
> 4d9ba6a4d149256f461b%7C0%7C0%7C637081336535150897&amp;sdata=
> %2FvKJWyFvCaBeDQVfpsKM4HL8fzXFoGKHWbQn%2FjBtulM%3D&amp;reser
> ved=0
> >     - rebased, neat updates
> >
> > v2: -
> >
> https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatch
> >
> es.dpdk.org%2Fpatch%2F60909%2F&amp;data=02%7C01%7Cviacheslavo%4
> 0mellan
> >
> ox.com%7C1390029b214e4543f1e708d75e19a404%7Ca652971c7d2e4d9ba
> 6a4d14925
> >
> 6f461b%7C0%7C0%7C637081336535150897&amp;sdata=lEb4sLRWVXuvaL
> WLHENHZNMr
> > 4EwFuFs8LoglfikJqFE%3D&amp;reserved=0
> >
> > v1: -
> https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatche
> s.dpdk.org%2Fpatch%2F56104%2F&amp;data=02%7C01%7Cviacheslavo%40
> mellanox.com%7C1390029b214e4543f1e708d75e19a404%7Ca652971c7d2e
> 4d9ba6a4d149256f461b%7C0%7C0%7C637081336535150897&amp;sdata=
> %2FNYTARUn%2FqNn9BRBK09juCiGYS1eACb1OxEZKJJMSY4%3D&amp;reser
> ved=0
> >     - rfc:
> >
> https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatch
> >
> es.dpdk.org%2Fpatch%2F54271%2F&amp;data=02%7C01%7Cviacheslavo%4
> 0mellan
> >
> ox.com%7C1390029b214e4543f1e708d75e19a404%7Ca652971c7d2e4d9ba
> 6a4d14925
> >
> 6f461b%7C0%7C0%7C637081336535150897&amp;sdata=u0faxda7EuxmoHk
> Ty0r0d4Yw
> > z6JfPvkiQuZEss6oo%2B4%3D&amp;reserved=0
> >
> >  app/test-pmd/cmdline_flow.c              |  57 ++++++++++++++++-
> >  app/test-pmd/util.c                      |   5 ++
> >  doc/guides/prog_guide/rte_flow.rst       |  72 ++++++++++++++++-----
> >  doc/guides/rel_notes/release_19_11.rst   |  13 ++++
> >  lib/librte_ethdev/rte_ethdev_version.map |   3 +
> >  lib/librte_ethdev/rte_flow.c             |  40 ++++++++++++
> >  lib/librte_ethdev/rte_flow.h             | 103
> +++++++++++++++++++++++++++++--
> >  lib/librte_mbuf/rte_mbuf_dyn.h           |   8 ++-
> >  8 files changed, 279 insertions(+), 22 deletions(-)
> >
> > diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> > index 0d0bc0a..e4ef066 100644
> > --- a/app/test-pmd/cmdline_flow.c
> > +++ b/app/test-pmd/cmdline_flow.c
> > @@ -316,6 +316,9 @@ enum index {
> >  	ACTION_RAW_ENCAP_INDEX_VALUE,
> >  	ACTION_RAW_DECAP_INDEX,
> >  	ACTION_RAW_DECAP_INDEX_VALUE,
> > +	ACTION_SET_META,
> > +	ACTION_SET_META_DATA,
> > +	ACTION_SET_META_MASK,
> >  };
> >
> >  /** Maximum size for pattern in struct rte_flow_item_raw. */ @@
> > -1067,6 +1070,7 @@ struct parse_action_priv {
> >  	ACTION_DEC_TCP_ACK,
> >  	ACTION_RAW_ENCAP,
> >  	ACTION_RAW_DECAP,
> > +	ACTION_SET_META,
> >  	ZERO,
> >  };
> >
> > @@ -1265,6 +1269,13 @@ struct parse_action_priv {
> >  	ZERO,
> >  };
> >
> > +static const enum index action_set_meta[] = {
> > +	ACTION_SET_META_DATA,
> > +	ACTION_SET_META_MASK,
> > +	ACTION_NEXT,
> > +	ZERO,
> > +};
> > +
> >  static int parse_set_raw_encap_decap(struct context *, const struct token
> *,
> >  				     const char *, unsigned int,
> >  				     void *, unsigned int);
> > @@ -1329,6 +1340,10 @@ static int
> > parse_vc_action_raw_encap_index(struct context *,  static int
> parse_vc_action_raw_decap_index(struct context *,
> >  					   const struct token *, const char *,
> >  					   unsigned int, void *, unsigned int);
> > +static int parse_vc_action_set_meta(struct context *ctx,
> > +				    const struct token *token, const char *str,
> > +				    unsigned int len, void *buf,
> > +				    unsigned int size);
> >  static int parse_destroy(struct context *, const struct token *,
> >  			 const char *, unsigned int,
> >  			 void *, unsigned int);
> > @@ -3378,7 +3393,31 @@ static int comp_set_raw_index(struct context
> *, const struct token *,
> >  		.help = "index of raw_encap/raw_decap data",
> >  		.next = NEXT(next_item),
> >  		.call = parse_port,
> > -	}
> > +	},
> > +	[ACTION_SET_META] = {
> > +		.name = "set_meta",
> > +		.help = "set metadata",
> > +		.priv = PRIV_ACTION(SET_META,
> > +			sizeof(struct rte_flow_action_set_meta)),
> > +		.next = NEXT(action_set_meta),
> > +		.call = parse_vc_action_set_meta,
> > +	},
> > +	[ACTION_SET_META_DATA] = {
> > +		.name = "data",
> > +		.help = "metadata value",
> > +		.next = NEXT(action_set_meta, NEXT_ENTRY(UNSIGNED)),
> > +		.args = ARGS(ARGS_ENTRY_HTON
> > +			     (struct rte_flow_action_set_meta, data)),
> > +		.call = parse_vc_conf,
> > +	},
> > +	[ACTION_SET_META_MASK] = {
> > +		.name = "mask",
> > +		.help = "mask for metadata value",
> > +		.next = NEXT(action_set_meta, NEXT_ENTRY(UNSIGNED)),
> > +		.args = ARGS(ARGS_ENTRY_HTON
> > +			     (struct rte_flow_action_set_meta, mask)),
> > +		.call = parse_vc_conf,
> > +	},
> >  };
> >
> >  /** Remove and return last entry from argument stack. */ @@ -4818,6
> > +4857,22 @@ static int comp_set_raw_index(struct context *, const struct
> token *,
> >  	return ret;
> >  }
> >
> > +static int
> > +parse_vc_action_set_meta(struct context *ctx, const struct token *token,
> > +			 const char *str, unsigned int len, void *buf,
> > +			 unsigned int size)
> > +{
> > +	int ret;
> > +
> > +	ret = parse_vc(ctx, token, str, len, buf, size);
> > +	if (ret < 0)
> > +		return ret;
> > +	ret = rte_flow_dynf_metadata_register();
> > +	if (ret < 0)
> > +		return -1;
> > +	return len;
> > +}
> > +
> >  /** Parse tokens for destroy command. */  static int
> > parse_destroy(struct context *ctx, const struct token *token, diff
> > --git a/app/test-pmd/util.c b/app/test-pmd/util.c index
> > f20531d..56075b3 100644
> > --- a/app/test-pmd/util.c
> > +++ b/app/test-pmd/util.c
> > @@ -82,6 +82,11 @@
> >  			       mb->vlan_tci, mb->vlan_tci_outer);
> >  		else if (ol_flags & PKT_RX_VLAN)
> >  			printf(" - VLAN tci=0x%x", mb->vlan_tci);
> > +		if (ol_flags & PKT_TX_METADATA)
> > +			printf(" - Tx metadata: 0x%x", mb->tx_metadata);
> > +		if (ol_flags & PKT_RX_DYNF_METADATA)
> > +			printf(" - Rx metadata: 0x%x",
> > +			       *RTE_FLOW_DYNF_METADATA(mb));
> >  		if (mb->packet_type) {
> >  			rte_get_ptype_name(mb->packet_type, buf,
> sizeof(buf));
> >  			printf(" - hw ptype: %s", buf);
> > diff --git a/doc/guides/prog_guide/rte_flow.rst
> > b/doc/guides/prog_guide/rte_flow.rst
> > index 159ce19..c943aca 100644
> > --- a/doc/guides/prog_guide/rte_flow.rst
> > +++ b/doc/guides/prog_guide/rte_flow.rst
> > @@ -658,6 +658,32 @@ the physical device, with virtual groups in the
> PMD or not at all.
> >     | ``mask`` | ``id``   | zeroed to match any value |
> >     +----------+----------+---------------------------+
> >
> > +Item: ``META``
> > +^^^^^^^^^^^^^^^^^
> > +
> > +Matches 32 bit metadata item set.
> > +
> > +On egress, metadata can be set either by mbuf metadata field with
> > +PKT_TX_METADATA flag or ``SET_META`` action. On ingress,
> ``SET_META``
> > +action sets metadata for a packet and the metadata will be reported
> > +via ``metadata`` dynamic field of ``rte_mbuf`` with
> PKT_RX_DYNF_METADATA flag.
> > +
> > +- Default ``mask`` matches the specified Rx metadata value.
> > +
> > +.. _table_rte_flow_item_meta:
> > +
> > +.. table:: META
> > +
> > +   +----------+----------+---------------------------------------+
> > +   | Field    | Subfield | Value                                 |
> > +
> +==========+==========+=======================================
> +
> > +   | ``spec`` | ``data`` | 32 bit metadata value                 |
> > +   +----------+----------+---------------------------------------+
> > +   | ``last`` | ``data`` | upper range value                     |
> > +   +----------+----------+---------------------------------------+
> > +   | ``mask`` | ``data`` | bit-mask applies to "spec" and "last" |
> > +   +----------+----------+---------------------------------------+
> > +
> >  Data matching item types
> >  ~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > @@ -1232,21 +1258,6 @@ Matches a PPPoE session protocol identifier.
> >  - ``proto_id``: PPP protocol identifier.
> >  - Default ``mask`` matches proto_id only.
> >
> > -
> > -.. _table_rte_flow_item_meta:
> > -
> > -.. table:: META
> > -
> > -   +----------+----------+---------------------------------------+
> > -   | Field    | Subfield | Value                                 |
> > -
> +==========+==========+=======================================
> +
> > -   | ``spec`` | ``data`` | 32 bit metadata value                 |
> > -   +----------+--------------------------------------------------+
> > -   | ``last`` | ``data`` | upper range value                     |
> > -   +----------+----------+---------------------------------------+
> > -   | ``mask`` | ``data`` | bit-mask applies to "spec" and "last" |
> > -   +----------+----------+---------------------------------------+
> > -
> >  Item: ``NSH``
> >  ^^^^^^^^^^^^^
> >
> > @@ -2466,6 +2477,37 @@ Value to decrease TCP acknowledgment
> number by is a big-endian 32 bit integer.
> >
> >  Using this action on non-matching traffic will result in undefined behavior.
> >
> > +Action: ``SET_META``
> > +^^^^^^^^^^^^^^^^^^^^^^^
> > +
> > +Set metadata. Item ``META`` matches metadata.
> > +
> > +Metadata set by mbuf metadata field with PKT_TX_METADATA flag on
> > +egress will be overridden by this action. On ingress, the metadata
> > +will be carried by ``metadata`` dynamic field of ``rte_mbuf`` which
> > +can be accessed by ``RTE_FLOW_DYNF_METADATA()``.
> PKT_RX_DYNF_METADATA
> > +flag will be set along with the data.
> > +
> > +The mbuf dynamic field must be registered by calling
> > +``rte_flow_dynf_metadata_register()`` prior to use ``SET_META`` action.
> > +
> > +Altering partial bits is supported with ``mask``. For bits which have
> > +never been set, unpredictable value will be seen depending on driver
> > +implementation. For loopback/hairpin packet, metadata set on Rx/Tx
> > +may or may not be propagated to the other path depending on HW
> capability.
> > +
> > +.. _table_rte_flow_action_set_meta:
> > +
> > +.. table:: SET_META
> > +
> > +   +----------+----------------------------+
> > +   | Field    | Value                      |
> > +   +==========+============================+
> > +   | ``data`` | 32 bit metadata value      |
> > +   +----------+----------------------------+
> > +   | ``mask`` | bit-mask applies to "data" |
> > +   +----------+----------------------------+
> > +
> >  Negative types
> >  ~~~~~~~~~~~~~~
> >
> > diff --git a/doc/guides/rel_notes/release_19_11.rst
> > b/doc/guides/rel_notes/release_19_11.rst
> > index f6e90cb..963c4f8 100644
> > --- a/doc/guides/rel_notes/release_19_11.rst
> > +++ b/doc/guides/rel_notes/release_19_11.rst
> > @@ -237,6 +237,14 @@ New Features
> >    On supported NICs, we can now setup haipin queue which will offload
> packets
> >    from the wire, backto the wire.
> >
> > +* **Extended metadata support in rte_flow.**
> > +
> > +  Flow metadata is extended to both Rx and Tx.
> > +
> > +  * Tx metadata can also be set by SET_META action of rte_flow.
> > +  * Rx metadata is delivered to host via a dynamic field of ``rte_mbuf``
> with
> > +    PKT_RX_DYNF_METADATA.
> > +
> >
> >  Removed Items
> >  -------------
> > @@ -344,6 +352,11 @@ API Changes
> >    has been introduced in this release is used when used when all the
> packets
> >    enqueued in the tx adapter are destined for the same Ethernet port & Tx
> queue.
> >
> > +* metadata: RTE_FLOW_ITEM_TYPE_META data endianness altered to
> host one.
> > +  Due to the new dynamic metadata field in mbuf is host-endian
> > +either, there
> > +  is the minor compatibility issue for applications in case of 32-bit
> > +values
> > +  supported.
> > +
> >  * sched: The pipe nodes configuration parameters such as number of
> pipes,
> >    pipe queue sizes, pipe profiles, etc., are moved from port level structure
> >    to subport level. This allows different subports of the same port
> > to diff --git a/lib/librte_ethdev/rte_ethdev_version.map
> > b/lib/librte_ethdev/rte_ethdev_version.map
> > index 48b5389..e593f34 100644
> > --- a/lib/librte_ethdev/rte_ethdev_version.map
> > +++ b/lib/librte_ethdev/rte_ethdev_version.map
> > @@ -291,4 +291,7 @@ EXPERIMENTAL {
> >  	rte_eth_rx_hairpin_queue_setup;
> >  	rte_eth_tx_hairpin_queue_setup;
> >  	rte_eth_dev_hairpin_capability_get;
> > +	rte_flow_dynf_metadata_offs;
> > +	rte_flow_dynf_metadata_mask;
> > +	rte_flow_dynf_metadata_register;
> >  };
> > diff --git a/lib/librte_ethdev/rte_flow.c
> > b/lib/librte_ethdev/rte_flow.c index ca0f680..b0490cd 100644
> > --- a/lib/librte_ethdev/rte_flow.c
> > +++ b/lib/librte_ethdev/rte_flow.c
> > @@ -12,10 +12,18 @@
> >  #include <rte_errno.h>
> >  #include <rte_branch_prediction.h>
> >  #include <rte_string_fns.h>
> > +#include <rte_mbuf.h>
> > +#include <rte_mbuf_dyn.h>
> >  #include "rte_ethdev.h"
> >  #include "rte_flow_driver.h"
> >  #include "rte_flow.h"
> >
> > +/* Mbuf dynamic field name for metadata. */ int
> > +rte_flow_dynf_metadata_offs = -1;
> > +
> > +/* Mbuf dynamic field flag bit number for metadata. */ uint64_t
> > +rte_flow_dynf_metadata_mask;
> > +
> >  /**
> >   * Flow elements description tables.
> >   */
> > @@ -157,8 +165,40 @@ struct rte_flow_desc_data {
> >  	MK_FLOW_ACTION(DEC_TCP_SEQ, sizeof(rte_be32_t)),
> >  	MK_FLOW_ACTION(INC_TCP_ACK, sizeof(rte_be32_t)),
> >  	MK_FLOW_ACTION(DEC_TCP_ACK, sizeof(rte_be32_t)),
> > +	MK_FLOW_ACTION(SET_META, sizeof(struct
> rte_flow_action_set_meta)),
> >  };
> >
> > +int
> > +rte_flow_dynf_metadata_register(void)
> > +{
> > +	int offset;
> > +	int flag;
> > +
> > +	static const struct rte_mbuf_dynfield desc_offs = {
> > +		.name = RTE_MBUF_DYNFIELD_METADATA_NAME,
> > +		.size = sizeof(uint32_t),
> > +		.align = __alignof__(uint32_t),
> > +	};
> > +	static const struct rte_mbuf_dynflag desc_flag = {
> > +		.name = RTE_MBUF_DYNFLAG_METADATA_NAME,
> > +	};
> > +
> > +	offset = rte_mbuf_dynfield_register(&desc_offs);
> > +	if (offset < 0)
> > +		goto error;
> > +	flag = rte_mbuf_dynflag_register(&desc_flag);
> > +	if (flag < 0)
> > +		goto error;
> > +	rte_flow_dynf_metadata_offs = offset;
> > +	rte_flow_dynf_metadata_mask = (1ULL << flag);
> > +	return 0;
> > +
> > +error:
> > +	rte_flow_dynf_metadata_offs = -1;
> > +	rte_flow_dynf_metadata_mask = 0ULL;
> > +	return -rte_errno;
> > +}
> > +
> >  static int
> >  flow_err(uint16_t port_id, int ret, struct rte_flow_error *error)  {
> > diff --git a/lib/librte_ethdev/rte_flow.h
> > b/lib/librte_ethdev/rte_flow.h index 4fee105..f6e050c 100644
> > --- a/lib/librte_ethdev/rte_flow.h
> > +++ b/lib/librte_ethdev/rte_flow.h
> > @@ -28,6 +28,8 @@
> >  #include <rte_byteorder.h>
> >  #include <rte_esp.h>
> >  #include <rte_higig.h>
> > +#include <rte_mbuf.h>
> > +#include <rte_mbuf_dyn.h>
> >
> >  #ifdef __cplusplus
> >  extern "C" {
> > @@ -418,7 +420,8 @@ enum rte_flow_item_type {
> >  	/**
> >  	 * [META]
> >  	 *
> > -	 * Matches a metadata value specified in mbuf metadata field.
> > +	 * Matches a metadata value.
> > +	 *
> >  	 * See struct rte_flow_item_meta.
> >  	 */
> >  	RTE_FLOW_ITEM_TYPE_META,
> > @@ -1263,18 +1266,23 @@ struct rte_flow_item_icmp6_nd_opt_tla_eth {
> > #endif
> >
> >  /**
> > - * RTE_FLOW_ITEM_TYPE_META.
> > + * RTE_FLOW_ITEM_TYPE_META
> >   *
> > - * Matches a specified metadata value.
> > + * Matches a specified metadata value. On egress, metadata can be set
> > + either by
> > + * mbuf tx_metadata field with PKT_TX_METADATA flag or
> > + * RTE_FLOW_ACTION_TYPE_SET_META. On ingress,
> > + RTE_FLOW_ACTION_TYPE_SET_META sets
> > + * metadata for a packet and the metadata will be reported via mbuf
> > + metadata
> > + * dynamic field with PKT_RX_DYNF_METADATA flag. The dynamic mbuf
> > + field must be
> > + * registered in advance by rte_flow_dynf_metadata_register().
> >   */
> >  struct rte_flow_item_meta {
> > -	rte_be32_t data;
> > +	uint32_t data;
> >  };
> >
> >  /** Default mask for RTE_FLOW_ITEM_TYPE_META. */  #ifndef
> __cplusplus
> > static const struct rte_flow_item_meta rte_flow_item_meta_mask = {
> > -	.data = RTE_BE32(UINT32_MAX),
> > +	.data = UINT32_MAX,
> >  };
> >  #endif
> >
> > @@ -1942,6 +1950,13 @@ enum rte_flow_action_type {
> >  	 * undefined behavior.
> >  	 */
> >  	RTE_FLOW_ACTION_TYPE_DEC_TCP_ACK,
> > +
> > +	/**
> > +	 * Set metadata on ingress or egress path.
> > +	 *
> > +	 * See struct rte_flow_action_set_meta.
> > +	 */
> > +	RTE_FLOW_ACTION_TYPE_SET_META,
> >  };
> >
> >  /**
> > @@ -2429,6 +2444,57 @@ struct rte_flow_action_set_mac {
> >  	uint8_t mac_addr[RTE_ETHER_ADDR_LEN];  };
> >
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this structure may change without prior notice
> > + *
> > + * RTE_FLOW_ACTION_TYPE_SET_META
> > + *
> > + * Set metadata. Metadata set by mbuf tx_metadata field with
> > + * PKT_TX_METADATA flag on egress will be overridden by this action.
> > +On
> > + * ingress, the metadata will be carried by mbuf metadata dynamic
> > +field
> > + * with PKT_RX_DYNF_METADATA flag if set.  The dynamic mbuf field
> > +must be
> > + * registered in advance by rte_flow_dynf_metadata_register().
> > + *
> > + * Altering partial bits is supported with mask. For bits which have
> > +never
> > + * been set, unpredictable value will be seen depending on driver
> > + * implementation. For loopback/hairpin packet, metadata set on Rx/Tx
> > +may
> > + * or may not be propagated to the other path depending on HW
> capability.
> > + *
> > + * RTE_FLOW_ITEM_TYPE_META matches metadata.
> > + */
> > +struct rte_flow_action_set_meta {
> > +	uint32_t data;
> > +	uint32_t mask;
> > +};
> > +
> > +/* Mbuf dynamic field offset for metadata. */ extern int
> > +rte_flow_dynf_metadata_offs;
> > +
> > +/* Mbuf dynamic field flag mask for metadata. */ extern uint64_t
> > +rte_flow_dynf_metadata_mask;
> > +
> > +/* Mbuf dynamic field pointer for metadata. */ #define
> > +RTE_FLOW_DYNF_METADATA(m) \
> > +	RTE_MBUF_DYNFIELD((m), rte_flow_dynf_metadata_offs, uint32_t
> *)
> > +
> > +/* Mbuf dynamic flag for metadata. */ #define PKT_RX_DYNF_METADATA
> > +(rte_flow_dynf_metadata_mask)
> > +
> > +__rte_experimental
> > +static inline uint32_t
> > +rte_flow_dynf_metadata_get(struct rte_mbuf *m) {
> > +	return *RTE_FLOW_DYNF_METADATA(m);
> > +}
> > +
> > +__rte_experimental
> > +static inline void
> > +rte_flow_dynf_metadata_set(struct rte_mbuf *m, uint32_t v) {
> > +	*RTE_FLOW_DYNF_METADATA(m) = v;
> > +}
> > +
> >  /*
> >   * Definition of a single action.
> >   *
> > @@ -2662,6 +2728,33 @@ enum rte_flow_conv_op {  };
> >
> >  /**
> > + * Check if mbuf dynamic field for metadata is registered.
> > + *
> > + * @return
> > + *   True if registered, false otherwise.
> > + */
> > +__rte_experimental
> > +static inline int
> > +rte_flow_dynf_metadata_avail(void)
> > +{
> > +	return !!rte_flow_dynf_metadata_mask; }
> > +
> > +/**
> > + * Register mbuf dynamic field and flag for metadata.
> > + *
> > + * This function must be called prior to use SET_META action in order
> > +to
> > + * register the dynamic mbuf field. Otherwise, the data cannot be
> > +delivered to
> > + * application.
> > + *
> > + * @return
> > + *   0 on success, a negative errno value otherwise and rte_errno is set.
> > + */
> > +__rte_experimental
> > +int
> > +rte_flow_dynf_metadata_register(void);
> > +
> > +/**
> >   * Check whether a flow rule can be created on a given port.
> >   *
> >   * The flow rule is validated for correctness and whether it could be
> > accepted diff --git a/lib/librte_mbuf/rte_mbuf_dyn.h
> > b/lib/librte_mbuf/rte_mbuf_dyn.h index 2e9d418..de651c1 100644
> > --- a/lib/librte_mbuf/rte_mbuf_dyn.h
> > +++ b/lib/librte_mbuf/rte_mbuf_dyn.h
> > @@ -234,6 +234,12 @@ int rte_mbuf_dynflag_lookup(const char *name,
> > __rte_experimental  void rte_mbuf_dyn_dump(FILE *out);
> >
> > -/* Placeholder for dynamic fields and flags declarations. */
> > +/*
> > + * Placeholder for dynamic fields and flags declarations.
> > + * This is centralizing point to gather all field names
> > + * and parameters together.
> > + */
> > +#define RTE_MBUF_DYNFIELD_METADATA_NAME
> "rte_flow_dynfield_metadata"
> > +#define RTE_MBUF_DYNFLAG_METADATA_NAME
> "rte_flow_dynflag_metadata"
> >
> 
> Sorry if it was not clear in my first comment, but I think we should have some
> words in this file about what are these field/flag.
Sure, I'll add some not-too-wordy comment here.

> 
> After that:
> Acked-by: Olivier Matz <olivier.matz@6wind.com>
Thanks a lot for the review, we polished the patch and made one better.

With best regards, Slava

  reply	other threads:[~2019-10-31 16:13 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
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 [this message]
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=AM4PR05MB3265EB0864E8D1E7FF87A495D2630@AM4PR05MB3265.eurprd05.prod.outlook.com \
    --to=viacheslavo@mellanox.com \
    --cc=arybchenko@solarflare.com \
    --cc=dev@dpdk.org \
    --cc=matan@mellanox.com \
    --cc=olivier.matz@6wind.com \
    --cc=orika@mellanox.com \
    --cc=rasland@mellanox.com \
    --cc=thomas@monjalon.net \
    --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).