[dpdk-dev,v2,4/4] ethdev: Add metadata flow and action items support

Message ID 20180405135148.16388-5-declan.doherty@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Doherty, Declan April 5, 2018, 1:51 p.m. UTC
Introduces a new action type RTE_FLOW_ACTION_TYPE_METADATA which enables
metadata extraction from a packet into a specified metadata container
for consumption on further pipeline stages or for propagation to the host
interface.

As complementary function to the new metadata action type this patch also
introduces a new flow item type which enables flow patterns to specify a
specific metadata container as a matching criteria for a flow rule.

Signed-off-by: Declan Doherty <declan.doherty@intel.com>
---
 doc/guides/prog_guide/rte_flow.rst | 85 ++++++++++++++++++++++++++++++++++++++
 lib/librte_ether/rte_flow.h        | 54 +++++++++++++++++++++++-
 2 files changed, 138 insertions(+), 1 deletion(-)
  

Comments

Thomas Monjalon April 5, 2018, 4:49 p.m. UTC | #1
05/04/2018 15:51, Declan Doherty:
> +struct rte_flow_item_metadata {
> +       uint32_t id;            /**< field identifier */
> +       uint32_t size;          /**< field size */
> +       uint8_t bytes[];        /**< field value */
> +};

Spotted C99 syntax of flexible array.
Are we OK with all supported compilers?
  
Adrien Mazarguil April 6, 2018, 12:20 p.m. UTC | #2
On Thu, Apr 05, 2018 at 06:49:32PM +0200, Thomas Monjalon wrote:
> 05/04/2018 15:51, Declan Doherty:
> > +struct rte_flow_item_metadata {
> > +       uint32_t id;            /**< field identifier */
> > +       uint32_t size;          /**< field size */
> > +       uint8_t bytes[];        /**< field value */
> > +};
> 
> Spotted C99 syntax of flexible array.
> Are we OK with all supported compilers?

I also thought they were a good idea at first but got rid of them in
rte_flow [1] for the following reasons:

- Not valid/standard C++.
- Can't be statically initialized.

Both can be overcome by relying on compiler extensions, however their use
should be restricted to a minimum in public APIs for portability reasons.

[1] http://dpdk.org/ml/archives/dev/2018-April/095307.html
  
Mohammad Abdul Awal April 6, 2018, 1:47 p.m. UTC | #3
On 05/04/2018 17:49, Thomas Monjalon wrote:
> 05/04/2018 15:51, Declan Doherty:
>> +struct rte_flow_item_metadata {
>> +       uint32_t id;            /**< field identifier */
>> +       uint32_t size;          /**< field size */
>> +       uint8_t bytes[];        /**< field value */
>> +};
> Spotted C99 syntax of flexible array.
> Are we OK with all supported compilers?
>
Used "uint8_t *bytes;" instead of "uint8_t bytes[];"
  
Thomas Monjalon April 6, 2018, 3:57 p.m. UTC | #4
06/04/2018 15:47, Mohammad Abdul Awal:
> 
> On 05/04/2018 17:49, Thomas Monjalon wrote:
> > 05/04/2018 15:51, Declan Doherty:
> >> +struct rte_flow_item_metadata {
> >> +       uint32_t id;            /**< field identifier */
> >> +       uint32_t size;          /**< field size */
> >> +       uint8_t bytes[];        /**< field value */
> >> +};
> > Spotted C99 syntax of flexible array.
> > Are we OK with all supported compilers?
> >
> Used "uint8_t *bytes;" instead of "uint8_t bytes[];"

Why this change? It is changing the size of the structure, isn't it?
  
Mohammad Abdul Awal April 6, 2018, 4:58 p.m. UTC | #5
On 06/04/2018 16:57, Thomas Monjalon wrote:
> 06/04/2018 15:47, Mohammad Abdul Awal:
>> On 05/04/2018 17:49, Thomas Monjalon wrote:
>>> 05/04/2018 15:51, Declan Doherty:
>>>> +struct rte_flow_item_metadata {
>>>> +       uint32_t id;            /**< field identifier */
>>>> +       uint32_t size;          /**< field size */
>>>> +       uint8_t bytes[];        /**< field value */
>>>> +};
>>> Spotted C99 syntax of flexible array.
>>> Are we OK with all supported compilers?
>>>
>> Used "uint8_t *bytes;" instead of "uint8_t bytes[];"
> Why this change? It is changing the size of the structure, isn't it?
It does change the size. I also agree with Adrien's comment as he 
mentioned in the previous mail.
  

Patch

diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 362231829..48e84b374 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -1580,6 +1580,91 @@  group on that device.
    +--------------+---------------------------------+
 
 
+
+Action: ``METADATA``
+^^^^^^^^^^^^^^^^^^^^
+
+Action extracts data from packet into user specified metadata field in pipeline
+for use in downstream processing stages or for propagation to host interface.
+
+The pattern mask is used to define the data which is to be extracted from the
+packet. The mask pattern types defined in the action metadata pattern must
+match the flow pattern definitions up to the last flow item from which data is
+to be extracted.
+
+- Non-terminating by default.
+
+.. _table_rte_flow_action_metadata:
+
+.. table:: METADATA
+
+   +--------------+---------------------------+
+   | Field        | Value                     |
+   +==============+===========================+
+   | ``id``       | Metadata field Identifier |
+   +--------------+---------------------------+
+   | ``pattern``  | Extraction mask pattern   |
+   +--------------+---------------------------+
+
+The example below demonstrates how the extraction mask to extract the source/
+destination IPv4 address, the UDP destination port and and the VxLAN VNI can be
+specified.
+
+.. _table_rte_flow_action_metadata_example:
+
+.. table:: IPv4 VxLAN metadata extraction
+
+   +-------+--------------------------+-----------------------------------+
+   | Index | Flow Item Type           | Flow Mask                         |
+   +=======+==========================+===================================+
+   | 0     | RTE_FLOW_ITEM_TYPE_ETH   | .dst = "\x00\x00\x00\x00\x00\x00" |
+   |       |                          +-----------------------------------+
+   |       |                          | .src = "\x00\x00\x00\x00\x00\x00" |
+   |       |                          +-----------------------------------+
+   |       |                          | .type = RTE_BE16(0x0)             |
+   +-------+--------------------------+-----------------------------------+
+   | 1     | RTE_FLOW_ITEM_TYPE_IPV4  | .src_addr = RTE_BE32(0xffffffff)  |
+   |       |                          +-----------------------------------+
+   |       |                          | .dst_addr = RTE_BE32(0xffffffff)  |
+   +-------+--------------------------+-----------------------------------+
+   | 2     | RTE_FLOW_ITEM_TYPE_UDP   | .src_port = RTE_BE16(0x0)         |
+   |       |                          +-----------------------------------+
+   |       |                          | .dst_port = RTE_BE16(0xffff)      |
+   +-------+--------------------------+-----------------------------------+
+   | 3     | RTE_FLOW_ITEM_TYPE_VXLAN | .vni = "\xff\xff\xff"             |
+   +-------+--------------------------+-----------------------------------+
+   | 4     | RTE_FLOW_ITEM_TYPE_END   | NULL                              |
+   +-------+--------------------------+-----------------------------------+
+
+If only the VxLAN VNI extraction was required then the extraction mask would be
+as follows.
+
+.. _table_rte_flow_action_metadata_example_2:
+
+.. table::  VxLAN VNI metadata extraction
+
+   +-------+--------------------------+-----------------------------------+
+   | Index | Flow Item Type           | Flow Mask                         |
+   +=======+==========================+===================================+
+   | 0     | RTE_FLOW_ITEM_TYPE_ETH   | .dst = "\x00\x00\x00\x00\x00\x00" |
+   |       |                          +-----------------------------------+
+   |       |                          | .src = "\x00\x00\x00\x00\x00\x00" |
+   |       |                          +-----------------------------------+
+   |       |                          | .type = RTE_BE16(0x0)             |
+   +-------+--------------------------+-----------------------------------+
+   | 1     | RTE_FLOW_ITEM_TYPE_IPV4  | .src_addr = RTE_BE32(0x0)         |
+   |       |                          +-----------------------------------+
+   |       |                          | .dst_addr = RTE_BE32(0x0)         |
+   +-------+--------------------------+-----------------------------------+
+   | 2     | RTE_FLOW_ITEM_TYPE_UDP   | .src_port = RTE_BE16(0x0)         |
+   |       |                          +-----------------------------------+
+   |       |                          | .dst_port = RTE_BE16(0x0)         |
+   +-------+--------------------------+-----------------------------------+
+   | 3     | RTE_FLOW_ITEM_TYPE_VXLAN | .vni = "\xff\xff\xff"             |
+   +-------+--------------------------+-----------------------------------+
+   | 4     | RTE_FLOW_ITEM_TYPE_END   | NULL                              |
+   +-------+--------------------------+-----------------------------------+
+
 Negative types
 ~~~~~~~~~~~~~~
 
diff --git a/lib/librte_ether/rte_flow.h b/lib/librte_ether/rte_flow.h
index 55951b6ca..be1b0348b 100644
--- a/lib/librte_ether/rte_flow.h
+++ b/lib/librte_ether/rte_flow.h
@@ -323,6 +323,13 @@  enum rte_flow_item_type {
 	 * See struct rte_flow_item_geneve.
 	 */
 	RTE_FLOW_ITEM_TYPE_GENEVE,
+
+	/**
+	 * Matches specified pipeline metadata field.
+	 *
+	 * See struct rte_flow_item_metadata.
+	 */
+	RTE_FLOW_ITEM_TYPE_METADATA
 };
 
 /**
@@ -814,6 +821,17 @@  static const struct rte_flow_item_geneve rte_flow_item_geneve_mask = {
 };
 #endif
 
+/**
+ * RTE_FLOW_ITEM_TYPE_METADATA
+ *
+ * Allow arbitrary pipeline metadata to be used in specification flow pattern
+ */
+struct rte_flow_item_metadata {
+	uint32_t id;		/**< field identifier */
+	uint32_t size;		/**< field size */
+	uint8_t bytes[];	/**< field value */
+};
+
 /**
  * Matching pattern item definition.
  *
@@ -1054,7 +1072,17 @@  enum rte_flow_action_type {
 	 *
 	 * See struct rte_flow_action_group.
 	 */
-	RTE_FLOW_ACTION_TYPE_GROUP
+	RTE_FLOW_ACTION_TYPE_GROUP,
+
+	/**
+	 * [META]
+	 *
+	 * Set specific metadata field associated with packet which is then
+	 * available to further pipeline stages.
+	 *
+	 * See struct rte_flow_action_metadata.
+	 */
+	RTE_FLOW_ACTION_TYPE_METADATA
 };
 
 /**
@@ -1252,6 +1280,30 @@  struct rte_flow_action_group {
 	uint32_t id;
 };
 
+/**
+ * RTE_FLOW_ACTION_TYPE_METADATA
+ *
+ * Action extracts data from packet into specified metadata field in pipeline
+ * for use in downstream processing stages or for propagation to host interface.
+ *
+ * Pattern mask is use to define the extraction data for packet. The mask
+ * pattern types defined in the action metadata pattern must match the flow
+ * pattern definitions up to the last item from which data is to be extracted.
+ *
+ * Non-terminating by default.
+ */
+struct rte_flow_action_metadata {
+	uint32_t id;		/**< field identifier */
+
+	struct rte_flow_action_mask_item {
+		enum rte_flow_item_type type;
+		/**< Flow item type. */
+		const void *mask;
+		/**< Flow item mask. */
+	} *pattern;
+	/** metadata extraction pattern mask specification */
+};
+
 /**
  * Definition of a single action.
  *