On 06/04/2018 9:27 PM, Adrien Mazarguil wrote:
On Fri, Apr 06, 2018 at 01:24:03PM +0100, Declan Doherty wrote:
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.dohe...@intel.com>

Is there already HW support for extraction and subsequent matching?
> Otherwise it looks overkill. If it's meant to be implemented in
software by
PMDs, isn't the existing MARK action with arbitrary value good enough?
All it needs is a corresponding MARK pattern item (which reminds me I forgot
to add it...)

My suggestion would be to submit this new item/action combo at the same
time as the first actual PMD implementation. In the meantime I think a much
simpler MARK pattern item could satisfy the most pressing needs.


I admit this was a bit future looking :) and probably overkill for what is needed today. I'll add the suggested MARK pattern item which will be enough to handle the OVS-DPDK tunnel encap/decap requirements.

---
  doc/guides/prog_guide/rte_flow.rst | 85 ++++++++++++++++++++++++++++++++++++++
  lib/librte_ether/rte_flow.h        | 42 +++++++++++++++++++
  2 files changed, 127 insertions(+)

diff --git a/doc/guides/prog_guide/rte_flow.rst 
b/doc/guides/prog_guide/rte_flow.rst
index 2f0a47a..9b2b0e3 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.
     +--------------+---------------------------------+
+

You keep adding these empty lines :)

sorry.... what can I say I love whitespace :D


+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.

Usual comment [1] regarding this property.

[1] "ethdev: alter behavior of flow API actions"
     http://dpdk.org/ml/archives/dev/2018-April/095779.html

+
+.. _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.

VxLAN => VXLAN

+
+.. _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 968a23b..f75dee7 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

Please add the trailing comma to ease subsequent contributions.

  };
/**
@@ -815,6 +822,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 */
+};

One last nit regarding this definition (assuming we keep it), please remove
extra spacing before comments (no alignment) and add caps to format it like
the rest of that file.

+
+/**
   * Matching pattern item definition.
   *
   * A pattern is formed by stacking items starting from the lowest protocol
@@ -1266,6 +1284,30 @@ struct rte_flow_action_group {
  };
/**
+ * RTE_FLOW_ACTION_TYPE_METADATA

Definition in enum rte_flow_action_type should be part of this commit.

+ *
+ * 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 */
+};

Same comment as in prior commit regarding this structure. You should use
struct rte_flow_item directly, except this time a pointer to a pattern
with multiple items seems warranted.

+
+/**
   * Definition of a single action.
   *
   * A list of actions is terminated by a END action.
--
2.7.4



Reply via email to