ethdev: Additions to support tunnel encap/decap offload

Add new RTE_FLOW_ACTION_TYPE_GROUP_COUNT action type to enable shared counters across multiple flows on a single port or across multiple flows on multiple ports within the same switch domain.

Introduce new API rte_flow_query_group_count to allow querying of group counters.

Signed-off-by Declan Doherty <[email protected]>

---
 doc/guides/prog_guide/rte_flow.rst      | 35 +++++++++++++++++++++
 lib/librte_ether/rte_ethdev_version.map |  8 +++++
 lib/librte_ether/rte_flow.c             | 21 +++++++++++++
 lib/librte_ether/rte_flow.h             | 56 ++++++++++++++++++++++++++++++++-
 lib/librte_ether/rte_flow_driver.h      |  6 ++++
 5 files changed, 125 insertions(+), 1 deletion(-)

doc/guides/prog_guide/rte_flow.rst +0 -0

index 961943d..fd33d19 100644

doc/guides/prog_guide/rte_flow.rst +24 -0

+Group Count Query
+~~~~~~~~~~~~~~~~~
+
+Query group counter which can be associated with multiple flows on a specified
+port.
+
+This function allows retrieving of group counters. A group counter is a
+counter which can be shared among multiple flows on a single port or among
+multiple flows on multiple ports within the same switch domain. Data is
+gathered by special actions which must be present in the flow rule
+definition.
+
+.. code-block:: c
+
+   int
+   rte_flow_query_group_count(uint16_t port_id,
+			   uint32_t group_counter_id,
+			   struct rte_flow_query_count *count,
+               struct rte_flow_error *error);
+
+Arguments:
+
+- ``port_id``: port identifier of Ethernet device.
+- ``group_counter_id``: group counter identifier.

lib/librte_ether/rte_ethdev_version.map +6 -0

+EXPERIMENTAL {
+	global:
+
+	rte_flow_query_group_count
+
+} DPDK_18.05;

lib/librte_ether/rte_flow.c +0 -0

index 38f2d27..e10b1d0 100644

lib/librte_ether/rte_flow.c +15 -0

+{
+	struct rte_eth_dev *dev = &rte_eth_devices[port_id];
+	const struct rte_flow_ops *ops = rte_flow_ops_get(port_id, error);
+
+	if (!ops)
+		return -rte_errno;
+	if (likely(!!ops->query_group_count))
+		return flow_err(port_id,
+				ops->query_group_count(dev, group_count_id,
+						       count, error),
+				error);
+	return rte_flow_error_set(error, ENOSYS,
+				  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+				  NULL, rte_strerror(ENOSYS));
+}

lib/librte_ether/rte_flow.h +3 -1

index 13e4202..7d1f89d 100644
@@ -1010,7 +1010,19 @@ enum rte_flow_action_type {
 	 *
 	 * See struct rte_flow_action_security.
 	 */
-	RTE_FLOW_ACTION_TYPE_SECURITY
+	RTE_FLOW_ACTION_TYPE_SECURITY,
+
+	/**

lib/librte_ether/rte_flow.h +12 -0

 };
 
 /**

@@ -1149,6 +1161,18 @@ struct rte_flow_action_security {
 };
 
 /**
+ * RTE_FLOW_ACTION_TYPE_GROUP_COUNT
+ *
+ * A packet/byte counter which can be shared across a group of flows programmed
+ * on the same port/switch domain.
+ *
+ * Non-terminating by default.
+ */
+struct rte_flow_action_group_count {
+	uint32_t id;
+};
+
+/**
  * Definition of a single action.
  *
  * A list of actions is terminated by a END action.

lib/librte_ether/rte_flow.h +29 -0

+/**
+ * Get hit/bytes count for group counter.
+ *
+ * A group counter is a counter which can be shared among multiple flows on a
+ * single port or among multiple flows on multiple ports within the same
+ * switch domain.
+ *
+ * In the case of ports within the same switch domain a global name space is
+ * assumed for group_count_id value.
+ *
+ * @param[in]	port_id
+ *   Port identifier of Ethernet device.
+ * @param[in]	group_count_id
+ *   Group counter identifier to query
+ * @param[out]	count
+ *   Group counter value
+ * @param[out]	error
+ *   Perform verbose error reporting if not NULL. PMDs initialize this
+ *   structure in case of error only.
+ *
+ * @return
+ *   Negative error code (errno value) and rte_errno is set.
+ */
+int __rte_experimental
+rte_flow_query_group_count(uint16_t port_id,
+			   uint32_t group_count_id,
+			   struct rte_flow_query_count *count,
+			   struct rte_flow_error *error);
+
 #ifdef __cplusplus
 }
 #endif

lib/librte_ether/rte_flow_driver.h +6 -0

index 7778c8e..ef09465 100644
@@ -96,6 +96,12 @@ struct rte_flow_ops {
 		(struct rte_eth_dev *,
 		 int,
 		 struct rte_flow_error *);
+	/** See rte_flow_query_group_count(). */
+	int (*query_group_count)
+		(struct rte_eth_dev *,
+		 uint32_t,
+		 struct rte_flow_query_count *,
+		 struct rte_flow_error *);
 };
 
 /**

Signature · 1 line

2.7.4

Quoted 8 lines · Declan Doherty

Add new RTE_FLOW_ACTION_TYPE_GROUP_COUNT action type to enable shared counters across multiple flows on a single port or across multiple flows on multiple ports within the same switch domain.

Introduce new API rte_flow_query_group_count to allow querying of group counters.

Signed-off-by Declan Doherty <[email protected]>

Both features are definitely needed, however I suggest to enhance the existing action type and query function instead, given the rte_flow ABI won't be maintained for the 18.05 release [1].

Counters and query support were defined as a kind of PoC in preparation for future requirements back in DPDK 17.02 and so far few PMDs have implemented the query callback (mlx5 and failsafe, and the latter isn't really a PMD).

Due to the behavior change of action lists [2], providing an action type as a query parameter is not specific enough anymore, for instance if a list contains multiple COUNT, the application should be able to tell which needs to be queried.

Therefore I suggest to redefine the query function as follows:

int
rte_flow_query(uint16_t port_id,
struct rte_flow *flow,
const struct rte_flow_action *action, void *data,
struct rte_flow_error *error);

Third argument is an action definition with the same configuration (if any) as previously defined in the action list originally used to create the flow rule (not necessarily the same pointer, only the contents matter).

It means two perfectly identical actions can't be distinguished, and that's how group counters will work.
Instead of adding a new action type to distinguish groups, a configuration structure is added to the existing RTE_FLOW_ACTION_TYPE_COUNT, with non-shared counters as a default behavior:

  struct rte_flow_action_count {
          uint32_t shared:1; /**< Share counter ID with other flow rules. */
          uint32_t reserved:31; /**< Reserved, must be zero. */
          uint32_t id; /**< Counter ID. */
  };

Doing so will impact some existing code in mlx5 and librte_flow_classify, but that shouldn't be much of an issue.

Keep in mind testpmd and its documentation must be updated as well.

Thoughts?

A few nits below for the sake of commenting.

[1] "Flow API overhaul for switch offloads"

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

Quoted 14 lines

---
  doc/guides/prog_guide/rte_flow.rst      | 35 +++++++++++++++++++++
  lib/librte_ether/rte_ethdev_version.map |  8 +++++
  lib/librte_ether/rte_flow.c             | 21 +++++++++++++
  lib/librte_ether/rte_flow.h             | 56 ++++++++++++++++++++++++++++++++-
  lib/librte_ether/rte_flow_driver.h      |  6 ++++
  5 files changed, 125 insertions(+), 1 deletion(-)

doc/guides/prog_guide/rte_flow.rst +0 -0

index 961943d..fd33d19 100644
@@ -1698,6 +1698,41 @@ Return values:
 

doc/guides/prog_guide/rte_flow.rst Return values: · line 1701

Quoted 3 lines

doc/guides/prog_guide/rte_flow.rst +1 -0

  - 0 on success, a negative errno value otherwise and ``rte_errno`` is set.
 
+

Unnecessary empty line.

Quoted 25 lines

doc/guides/prog_guide/rte_flow.rst +24 -0

+Group Count Query
+~~~~~~~~~~~~~~~~~
+
+Query group counter which can be associated with multiple flows on a specified
+port.
+
+This function allows retrieving of group counters. A group counter is a
+counter which can be shared among multiple flows on a single port or among
+multiple flows on multiple ports within the same switch domain. Data is
+gathered by special actions which must be present in the flow rule
+definition.
+
+.. code-block:: c
+
+   int
+   rte_flow_query_group_count(uint16_t port_id,
+			   uint32_t group_counter_id,
+			   struct rte_flow_query_count *count,
+               struct rte_flow_error *error);
+
+Arguments:
+
+- ``port_id``: port identifier of Ethernet device.
+- ``group_counter_id``: group counter identifier.

doc/guides/prog_guide/rte_flow.rst Return values: · line 1735

Quoted 10 lines

doc/guides/prog_guide/rte_flow.rst +10 -0

+- ``count``: group counter parameters.
+- ``error``: perform verbose error reporting if not NULL. PMDs initialize
+  this structure in case of error only.
+
+Return values:
+
+- 0 on success, a negative errno value otherwise and ``rte_errno`` is set.
+
+
+

More unnecessary empty lines.

lib/librte_ether/rte_ethdev_version.map EXPERIMENTAL { · line 233

Quoted 13 lines

doc/guides/prog_guide/rte_flow.rst +0 -0

  Isolated mode
  -------------
 

lib/librte_ether/rte_ethdev_version.map +2 -0

index 34df6c8..cff6807 100644
@@ -229,3 +229,11 @@ EXPERIMENTAL {
  	rte_mtr_stats_update;
 
  } DPDK_17.11;
+
+

One more.

Quoted 11 lines

lib/librte_ether/rte_ethdev_version.map +6 -0

+EXPERIMENTAL {
+	global:
+
+	rte_flow_query_group_count
+
+} DPDK_18.05;

lib/librte_ether/rte_flow.c +0 -0

index 38f2d27..e10b1d0 100644

lib/librte_ether/rte_flow.c rte_flow_copy(struct rte_flow_desc *desc, size_t len, · line 426

Quoted 9 lines

lib/librte_ether/rte_flow.c +6 -0

@@ -418,3 +418,24 @@ rte_flow_copy(struct rte_flow_desc *desc, size_t len,
  	}
  	return 0;
  }
+
+int __rte_experimental
+rte_flow_query_group_count(uint16_t port_id,
+	uint32_t group_count_id,
+	struct rte_flow_query_count *count,
+	struct rte_flow_error *error)

This function lacks a short documentation comment (see rte_flow_query()).

Quoted 27 lines

lib/librte_ether/rte_flow.c +15 -0

+{
+	struct rte_eth_dev *dev = &rte_eth_devices[port_id];
+	const struct rte_flow_ops *ops = rte_flow_ops_get(port_id, error);
+
+	if (!ops)
+		return -rte_errno;
+	if (likely(!!ops->query_group_count))
+		return flow_err(port_id,
+				ops->query_group_count(dev, group_count_id,
+						       count, error),
+				error);
+	return rte_flow_error_set(error, ENOSYS,
+				  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+				  NULL, rte_strerror(ENOSYS));
+}

lib/librte_ether/rte_flow.h +3 -1

index 13e4202..7d1f89d 100644
@@ -1010,7 +1010,19 @@ enum rte_flow_action_type {
  	 *
  	 * See struct rte_flow_action_security.
  	 */
-	RTE_FLOW_ACTION_TYPE_SECURITY
+	RTE_FLOW_ACTION_TYPE_SECURITY,
+
+	/**

lib/librte_ether/rte_flow.h enum rte_flow_action_type { · line 1025

Quoted 10 lines

lib/librte_ether/rte_flow.h +10 -0

+	 * Enable a shared flow group counter for flow. Group counters can be
+	 * associated with multiples flows on the same port or on port within
+	 * the same switch domain if supported by that device.
+	 *
+	 * Group counters can be retrieved and reset through
+	 * rte_flow_query_group_count()
+	 *
+	 * See struct rte_flow_action_group_count.
+	 */
+	RTE_FLOW_ACTION_TYPE_GROUP_COUNT

Don't forget the trailing comma.

Quoted 23 lines

lib/librte_ether/rte_flow.h +12 -0

  };
 
  /**

@@ -1149,6 +1161,18 @@ struct rte_flow_action_security {
  };
 
  /**
+ * RTE_FLOW_ACTION_TYPE_GROUP_COUNT
+ *
+ * A packet/byte counter which can be shared across a group of flows programmed
+ * on the same port/switch domain.
+ *
+ * Non-terminating by default.
+ */
+struct rte_flow_action_group_count {
+	uint32_t id;
+};
+
+/**
   * Definition of a single action.
   *
   * A list of actions is terminated by a END action.

lib/librte_ether/rte_flow.h rte_flow_copy(struct rte_flow_desc *fd, size_t len, · line 1503

Quoted 4 lines

lib/librte_ether/rte_flow.h +1 -0

@@ -1476,6 +1500,36 @@ rte_flow_copy(struct rte_flow_desc *fd, size_t len,
  	      const struct rte_flow_item *items,
  	      const struct rte_flow_action *actions);
 
+

Caught another empty line.

Quoted 52 lines

lib/librte_ether/rte_flow.h +29 -0

+/**
+ * Get hit/bytes count for group counter.
+ *
+ * A group counter is a counter which can be shared among multiple flows on a
+ * single port or among multiple flows on multiple ports within the same
+ * switch domain.
+ *
+ * In the case of ports within the same switch domain a global name space is
+ * assumed for group_count_id value.
+ *
+ * @param[in]	port_id
+ *   Port identifier of Ethernet device.
+ * @param[in]	group_count_id
+ *   Group counter identifier to query
+ * @param[out]	count
+ *   Group counter value
+ * @param[out]	error
+ *   Perform verbose error reporting if not NULL. PMDs initialize this
+ *   structure in case of error only.
+ *
+ * @return
+ *   Negative error code (errno value) and rte_errno is set.
+ */
+int __rte_experimental
+rte_flow_query_group_count(uint16_t port_id,
+			   uint32_t group_count_id,
+			   struct rte_flow_query_count *count,
+			   struct rte_flow_error *error);
+
  #ifdef __cplusplus
  }
  #endif

lib/librte_ether/rte_flow_driver.h +6 -0

index 7778c8e..ef09465 100644
@@ -96,6 +96,12 @@ struct rte_flow_ops {
  		(struct rte_eth_dev *,
  		 int,
  		 struct rte_flow_error *);
+	/** See rte_flow_query_group_count(). */
+	int (*query_group_count)
+		(struct rte_eth_dev *,
+		 uint32_t,
+		 struct rte_flow_query_count *,
+		 struct rte_flow_error *);
  };
 
  /**

Signature · 1 line

2.7.4

Hi Adrien,

Quoted 47 lines · Adrien Mazarguil

Quoted 8 lines · Declan Doherty

Add new RTE_FLOW_ACTION_TYPE_GROUP_COUNT action type to enable shared counters across multiple flows on a single port or across multiple flows on multiple ports within the same switch domain.

Introduce new API rte_flow_query_group_count to allow querying of group counters.

Signed-off-by Declan Doherty <[email protected]>

Both features are definitely needed, however I suggest to enhance the existing action type and query function instead, given the rte_flow ABI won't be maintained for the 18.05 release [1].

Counters and query support were defined as a kind of PoC in preparation for future requirements back in DPDK 17.02 and so far few PMDs have implemented the query callback (mlx5 and failsafe, and the latter isn't really a PMD).

Due to the behavior change of action lists [2], providing an action type as a query parameter is not specific enough anymore, for instance if a list contains multiple COUNT, the application should be able to tell which needs to be queried.

Therefore I suggest to redefine the query function as follows:

int
rte_flow_query(uint16_t port_id,
struct rte_flow *flow,
const struct rte_flow_action *action, void *data,
struct rte_flow_error *error);

Third argument is an action definition with the same configuration (if any) as previously defined in the action list originally used to create the flow rule (not necessarily the same pointer, only the contents matter).

It means two perfectly identical actions can't be distinguished, and that's how group counters will work.
Instead of adding a new action type to distinguish groups, a configuration structure is added to the existing RTE_FLOW_ACTION_TYPE_COUNT, with non-shared counters as a default behavior:

  struct rte_flow_action_count {
          uint32_t shared:1; /**< Share counter ID with other flow rules. */
          uint32_t reserved:31; /**< Reserved, must be zero. */
          uint32_t id; /**< Counter ID. */
  };

Quoted 6 lines

Doing so will impact some existing code in mlx5 and librte_flow_classify, but that shouldn't be much of an issue.

Keep in mind testpmd and its documentation must be updated as well.

Thoughts?

Please correct me if I am wrong but I think we are talking two different things here.
If I understood you correctly, you are proposing to pass a list of actions (instead if a action type) in the third parameter to perform multiple actions in the same query call. Lets take an example for 100 ingress flows. So, if we want to query the counter for all the flows, we can get them by a single query providing a list (of size 100) of action_count in the 3rd param.

On the other hand, we are saying that all the flows are belongs to same tunnel end-point (we are talking only 1 TEP here), then the PMD will be responsible to increment the counter of TEP for matching all the flows (100 flows). So, using one group query by passing one action_count in 3rd param, we can get the count of the TEP.

This case is generic enough for sure for simple flows but may not be suitable for tunnel cases, as application needs to track the counters for all the flows, and needs to build the list of action each time the flows added/deleted.

Quoted 6 lines

A few nits below for the sake of commenting.

[1] "Flow API overhaul for switch offloads"

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

Add new flow action types and associated action data structures to support the encapsulation and decapsulation of the virtual tunnel endpoints.

The RTE_FLOW_ACTION_TYPE_TUNNEL_ENCAP action will cause the matching flow to be encapsulated in the virtual tunnel endpoint overlay defined in the tunnel_encap action data.

The RTE_FLOW_ACTION_TYPE_TUNNEL_DECAP action will cause all virtual tunnel endpoint overlays up to and including the first instance of the flow item type defined in the tunnel_decap action data for the matching flows.

Signed-off-by Declan Doherty <[email protected]>

---
 doc/guides/prog_guide/rte_flow.rst | 77 ++++++++++++++++++++++++++++++++--
 lib/librte_ether/rte_flow.h        | 84 ++++++++++++++++++++++++++++++++++++--
 2 files changed, 155 insertions(+), 6 deletions(-)

doc/guides/prog_guide/rte_flow.rst +4 -2

index fd33d19..106fb93 100644
@@ -997,9 +997,11 @@ Actions
 
 Each possible action is represented by a type. Some have associated
 configuration structures. Several actions combined in a list can be assigned
-to a flow rule. That list is not ordered.
+to a flow rule. That list is not ordered, with the exception of  actions which
+modify the packet itself, these packet modification actions must be specified
+in the explicit order in which they are to be executed.
 
-They fall in three categories:
+They fall in four categories:
 
 - Terminating actions (such as QUEUE, DROP, RSS, PF, VF) that prevent
   processing matched packets by subsequent flow rules, unless overridden

doc/guides/prog_guide/rte_flow.rst +5 -0

+
+.. _table_rte_flow_action_tunnel_encap:
+
+.. table:: TUNNEL_ENCAP
+

doc/guides/prog_guide/rte_flow.rst +4 -0

+
+   +-------+--------------------------+------------+
+   | Index | Flow Item Type           | Flow Item  |
+   +=======+==========================+============+

lib/librte_ether/rte_flow.h +4 -2

 /**
  * Action types.
  *
  * Each possible action is represented by a type. Some have associated
  * configuration structures. Several actions combined in a list can be
- * affected to a flow rule. That list is not ordered.
+ * affected to a flow rule. That list is not ordered, with the exception of
+ * actions which modify the packet itself, these packet modification actions
+ * must be specified in the explicit order in which they are to be executed.
  *
- * They fall in three categories:
+ * They fall in four categories:
  *
  * - Terminating actions (such as QUEUE, DROP, RSS, PF, VF) that prevent
  *   processing matched packets by subsequent flow rules, unless overridden

lib/librte_ether/rte_flow.h +25 -0

+	/**
+	 * Encapsulate flow with tunnel defined in
+	 * rte_flow_action_tunnel_encap structure.
+	 *
+	 * See struct rte_flow_action_tunnel_encap.
+	 */
+	RTE_FLOW_ACTION_TYPE_TUNNEL_ENCAP,
+
+	/**
+	 * Decapsulate all the headers of the tunnel
+	 *
+	 * See struct rte_flow_action_tunnel_decap.
+	 */
+	RTE_FLOW_ACTION_TYPE_TUNNEL_DECAP,
+
+	/**
+	 * Redirects packets to the logical group of the current device.
+	 *
+	 * In a logical hierarchy of groups, which can be used to represent a
+	 * physical of logical chaining of flow tables, this action allows the
+	 * terminating action to be a logical group of the same device.
+	 *
+	 * See struct rte_flow_action_group.
+	 */
+	RTE_FLOW_ACTION_TYPE_GROUP,

lib/librte_ether/rte_flow.h +5 -0

+};
+
+/**
+ * RTE_FLOW_ACTION_TYP_TUNNEL_DECAP
+ *

lib/librte_ether/rte_flow.h +2 -0

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

Signature · 1 line

2.7.4

Quoted 8 lines · Declan Doherty

Add new flow action types and associated action data structures to support the encapsulation and decapsulation of the virtual tunnel endpoints.

The RTE_FLOW_ACTION_TYPE_TUNNEL_ENCAP action will cause the matching flow to be encapsulated in the virtual tunnel endpoint overlay defined in the tunnel_encap action data.

Quoted 6 lines

The RTE_FLOW_ACTION_TYPE_TUNNEL_DECAP action will cause all virtual tunnel endpoint overlays up to and including the first instance of the flow item type defined in the tunnel_decap action data for the matching flows.

Signed-off-by Declan Doherty <[email protected]>

This generic approach looks flexible enough to cover the use cases that immediately come to mind (VLAN, VXLAN), its design is sound.

However, while I'm aware it's not a concern at this point, it won't be able to deal with stateful tunnel or encapsulation types (e.g. IPsec or TCP) which will require additional meta data or some run-time assistance from the application.

Eventually for more complex use cases, dedicated encap/decap actions will have to appear, so the issue I wanted to raise before going further is this:

Going generic inevitably trades some of the usability; flat structures dedicated to VXLAN encap/decap with only the needed info to get the job done would likely be easier to implement in PMDs and use in applications. Any number of such actions can be added to rte_flow without ABI impact.

If VXLAN is the only use case at this point, my suggestion would be to go with simpler RTE_FLOW_ACTION_TYPE_VXLAN_(ENCAP|DECAP) actions, with fixed L2/L3/L4/L5 header definitions to prepend according to RFC 7348.

Now we can start with the generic approach, see how it fares and add dedicated encap/decap later as needed.

More comments below.

Quoted 24 lines

---
  doc/guides/prog_guide/rte_flow.rst | 77 ++++++++++++++++++++++++++++++++--
  lib/librte_ether/rte_flow.h        | 84 ++++++++++++++++++++++++++++++++++++--
  2 files changed, 155 insertions(+), 6 deletions(-)

doc/guides/prog_guide/rte_flow.rst +4 -2

index fd33d19..106fb93 100644
@@ -997,9 +997,11 @@ Actions
 
  Each possible action is represented by a type. Some have associated
  configuration structures. Several actions combined in a list can be assigned
-to a flow rule. That list is not ordered.
+to a flow rule. That list is not ordered, with the exception of  actions which
+modify the packet itself, these packet modification actions must be specified
+in the explicit order in which they are to be executed.
 
-They fall in three categories:
+They fall in four categories:
 
  - Terminating actions (such as QUEUE, DROP, RSS, PF, VF) that prevent
    processing matched packets by subsequent flow rules, unless overridden

doc/guides/prog_guide/rte_flow.rst They fall in three categories: · line 1017

Quoted 9 lines

doc/guides/prog_guide/rte_flow.rst +4 -1

@@ -1008,8 +1010,11 @@ They fall in three categories:
  - Non-terminating actions (PASSTHRU, DUP) that leave matched packets up for
    additional processing by subsequent flow rules.
 
+- Non-terminating meta actions that do not affect the fate of packets but result
+  in modification of the packet itself (SECURITY, TUNNEL_ENCAP, TUNNEL_DECAP).
+
  - Other non-terminating meta actions that do not affect the fate of packets
-  (END, VOID, MARK, FLAG, COUNT, SECURITY).
+  (END, VOID, MARK, FLAG, COUNT).

The above changes are not necessary anymore [1][2].

[1] "ethdev: clarify flow API pattern items and actions"

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

doc/guides/prog_guide/rte_flow.rst fields in the pattern items. · line 1494

Quoted 8 lines

doc/guides/prog_guide/rte_flow.rst +1 -0

  When several actions are combined in a flow rule, they should all have
  different types (e.g. dropping a packet twice is not possible).

@@ -1486,6 +1491,72 @@ fields in the pattern items.
     | 1     | END      |
     +-------+----------+
 
+

Nit: titles in this file are separated by a single empty line.

doc/guides/prog_guide/rte_flow.rst fields in the pattern items. · line 1505

Quoted 12 lines

doc/guides/prog_guide/rte_flow.rst +11 -0

+Action: ``TUNNEL_ENCAP``
+^^^^^^^^^^^^^^^^^^^^^^
+
+Performs an encapsulation action by encapsulating the flows matched by the
+pattern items according to the network overlay defined in the
+``rte_flow_action_tunnel_encap`` pattern items.
+
+This action modifies the payload of matched flows. The pattern items specified
+in the ``rte_flow_action_tunnel_encap`` action structure must defined a valid
+set of overlay headers, from the Ethernet header up to the overlay header. The
+pattern must be terminated with the RTE_FLOW_ITEM_TYPE_END item type.

Regarding the use of a pattern list, if you consider PMDs are already iterating on a list of actions when encountering RTE_FLOW_ACTION_TYPE_TUNNEL_ENCAP, it adds yet another inner loop.

How about making each encountered RTE_FLOW_ACTION_TYPE_TUNNEL_ENCAP provide exactly one item instead (in encap, i.e. reverse order)?

In which case perhaps "GENERIC" would be a better fit than "TUNNEL".

doc/guides/prog_guide/rte_flow.rst fields in the pattern items. · line 1507

Quoted 2 lines

doc/guides/prog_guide/rte_flow.rst +2 -0

+
+- Non-terminating by default.

There's no such property anymore [2].

Quoted 6 lines

doc/guides/prog_guide/rte_flow.rst +5 -0

+
+.. _table_rte_flow_action_tunnel_encap:
+
+.. table:: TUNNEL_ENCAP
+

doc/guides/prog_guide/rte_flow.rst fields in the pattern items. · line 1522

Quoted 10 lines

doc/guides/prog_guide/rte_flow.rst +10 -0

+   +-------------+---------------------------------------------+
+   | Field       | Value                                       |
+   +=============+=============================================+
+   | ``pattern`` | Virtual tunnel end-point pattern definition |
+   +-------------+---------------------------------------------+
+
+
+.. _table_rte_flow_action_tunnel_encap_example:
+
+.. table:: IPv4 VxLAN flow pattern example.

VxLAN => VXLAN

Quoted 5 lines

doc/guides/prog_guide/rte_flow.rst +4 -0

+
+   +-------+--------------------------+------------+
+   | Index | Flow Item Type           | Flow Item  |
+   +=======+==========================+============+

doc/guides/prog_guide/rte_flow.rst fields in the pattern items. · line 1536

Quoted 10 lines

doc/guides/prog_guide/rte_flow.rst +10 -0

+   | 0     | RTE_FLOW_ITEM_TYPE_ETH   | eth item   |
+   +-------+--------------------------+------------+
+   | 1     | RTE_FLOW_ITEM_TYPE_IPV4  | ipv4 item  |
+   +-------+--------------------------+------------+
+   | 2     | RTE_FLOW_ITEM_TYPE_UDP   | udp item   |
+   +-------+--------------------------+------------+
+   | 3     | RTE_FLOW_ITEM_TYPE_VXLAN | vxlan item |
+   +-------+--------------------------+------------+
+   | 4     | RTE_FLOW_ITEM_TYPE_END   | NULL       |
+   +-------+--------------------------+------------+

One possible issue is that it relies on objects normally found on the pattern side of flow rules. Those are supposed to match something, they are not intended for packet header generation. While their "spec" and "mask" fields might make sense in this context, the "last" field is odd.

You must define them without leaving anything open for interpretation by PMDs and users alike. Defining things as "undefined" is fine as long as it's covered.

doc/guides/prog_guide/rte_flow.rst fields in the pattern items. · line 1538

Quoted 3 lines

doc/guides/prog_guide/rte_flow.rst +2 -0

+
+

Nit: only one empty line necessary here.

doc/guides/prog_guide/rte_flow.rst fields in the pattern items. · line 1544

Quoted 7 lines

doc/guides/prog_guide/rte_flow.rst +6 -0

+Action: ``TUNNEL_DECAP``
+^^^^^^^^^^^^^^^^^^^^^^
+
+Performs a decapsulation action by stripping all headers of the virtual tunnel
+end-point overlay up to the header defined by the flow item type of flows
+matched by the pattern items.

Not necessarily, for instance if one guarantees that flowing traffic only consists of decap'able packets. You must avoid mandatory dependencies between patterns and actions since they are normally unrelated.

What you can document on the other hand is that the behavior is undefined when processing traffic on which the action can't be applied. This is how RSS level is documented [3].

[3] https://dpdk.org/ml/archives/dev/2018-April/095783.html

doc/guides/prog_guide/rte_flow.rst fields in the pattern items. · line 1550

Quoted 7 lines

doc/guides/prog_guide/rte_flow.rst +6 -0

+
+This action modifies the payload of matched flows. The flow item type specified
+in the ``rte_flow_action_tunnel_decap`` action structure must defined a valid
+set of overlay header type.
+
+- Non-terminating by default.

See [2].

doc/guides/prog_guide/rte_flow.rst fields in the pattern items. · line 1558

Quoted 9 lines

doc/guides/prog_guide/rte_flow.rst +8 -0

+
+.. _table_rte_flow_action_tunnel_decap:
+
+   +---------------+----------------------------------------------+
+   | Field         | Value                                        |
+   +===============+==============================================+
+   | ``item type`` | Item type of tunnel end-point to decapsulate |
+   +---------------+----------------------------------------------+

"item type" should be the exact name used in the structure.

Quoted 13 lines

doc/guides/prog_guide/rte_flow.rst +1 -0

+
  Negative types
  ~~~~~~~~~~~~~~
 

lib/librte_ether/rte_flow.h +0 -0

index 7d1f89d..6d94423 100644
@@ -854,14 +854,17 @@ struct rte_flow_item {
  	const void *mask; /**< Bit-mask applied to spec and last. */
  };
 

lib/librte_ether/rte_flow.h struct rte_flow_item { · line 857

Quoted 1 line

lib/librte_ether/rte_flow.h +1 -0

+

Unnecessary empty line.

Quoted 17 lines

lib/librte_ether/rte_flow.h +4 -2

  /**
   * Action types.
   *
   * Each possible action is represented by a type. Some have associated
   * configuration structures. Several actions combined in a list can be
- * affected to a flow rule. That list is not ordered.
+ * affected to a flow rule. That list is not ordered, with the exception of
+ * actions which modify the packet itself, these packet modification actions
+ * must be specified in the explicit order in which they are to be executed.
   *
- * They fall in three categories:
+ * They fall in four categories:
   *
   * - Terminating actions (such as QUEUE, DROP, RSS, PF, VF) that prevent
   *   processing matched packets by subsequent flow rules, unless overridden

@@ -870,6 +873,10 @@ struct rte_flow_item {

lib/librte_ether/rte_flow.h struct rte_flow_item { · line 879

Quoted 7 lines

lib/librte_ether/rte_flow.h +4 -0

   * - Non terminating actions (PASSTHRU, DUP) that leave matched packets up
   *   for additional processing by subsequent flow rules.
   *
+ * - Non terminating meta actions that do not affect the fate of
+ *   packets but result in modification of the packet itself (SECURITY,
+ *   TUNNEL_ENCAP, TUNNEL_DECAP).
+ *

Same comment as above [1][2].

lib/librte_ether/rte_flow.h enum rte_flow_action_type { · line 1032

Quoted 10 lines

lib/librte_ether/rte_flow.h +1 -1

   * - Other non terminating meta actions that do not affect the fate of
   *   packets (END, VOID, MARK, FLAG, COUNT).
   *

@@ -1022,7 +1029,42 @@ enum rte_flow_action_type {
  	 *
  	 * See struct rte_flow_action_group_count.
  	 */
-	RTE_FLOW_ACTION_TYPE_GROUP_COUNT
+	RTE_FLOW_ACTION_TYPE_GROUP_COUNT,

An empty line would have been needed here (if we agree about no more GROUP_COUNT.)

Quoted 30 lines

lib/librte_ether/rte_flow.h +29 -0

+	/**
+	 * Encapsulate flow with tunnel defined in
+	 * rte_flow_action_tunnel_encap structure.
+	 *
+	 * See struct rte_flow_action_tunnel_encap.
+	 */
+	RTE_FLOW_ACTION_TYPE_TUNNEL_ENCAP,
+
+	/**
+	 * Decapsulate all the headers of the tunnel
+	 *
+	 * See struct rte_flow_action_tunnel_decap.
+	 */
+	RTE_FLOW_ACTION_TYPE_TUNNEL_DECAP,
+
+	/**
+	 * Redirects packets to the logical group of the current device.
+	 *
+	 * In a logical hierarchy of groups, which can be used to represent a
+	 * physical of logical chaining of flow tables, this action allows the
+	 * terminating action to be a logical group of the same device.
+	 *
+	 * See struct rte_flow_action_group.
+	 */
+	RTE_FLOW_ACTION_TYPE_GROUP,
+
+	/**
+	 * [META]
+	 *

lib/librte_ether/rte_flow.h enum rte_flow_action_type { · line 1067

Quoted 6 lines

lib/librte_ether/rte_flow.h +6 -0

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

These two actions should be part of the next patch, I won't comment them here.

Quoted 7 lines

lib/librte_ether/rte_flow.h +0 -0

  };
 
  /**

@@ -1173,6 +1215,42 @@ struct rte_flow_action_group_count {
  };
 

lib/librte_ether/rte_flow.h struct rte_flow_action_group_count { · line 1222

Quoted 6 lines

lib/librte_ether/rte_flow.h +5 -0

  /**
+ * RTE_FLOW_ACTION_TYPE_TUNNEL_ENCAP
+ *
+ * Virtual tunnel end-point encapsulation action data.
+ *
+ * Non-terminating action by default.

See [2].

lib/librte_ether/rte_flow.h struct rte_flow_action_group_count { · line 1231

Quoted 10 lines

lib/librte_ether/rte_flow.h +9 -0

+ */
+struct rte_flow_action_tunnel_encap {
+	struct rte_flow_action_item {
+		enum rte_flow_item_type type;
+		/**< Flow item type. */
+		const void *item;
+		/**< Flow item definition which points to the data of
+		 * corresponding rte_flow_item_type.
+		 */

I see it's a new action type, albeit a bit confusing (there is no RTE_FLOW_ACTION_TYPE_ITEM).

I suggest the standard pattern item type since you're going with enum rte_flow_item_type anyway. Keep in mind you need some kind of mask to tell what fields are relevant. An application might otherwise want to encap with unsupported properties (e.g. specific IPv4 ToS field and whatnot).

How about a single "struct rte_flow_pattern_item item", neither const and neither a pointer. It's generic enough, enclosed spec/last/mask pointers take care of the specifics. You just need to define what's supposed to happen when "last" is set.

lib/librte_ether/rte_flow.h struct rte_flow_action_group_count { · line 1236

Quoted 6 lines

lib/librte_ether/rte_flow.h +5 -0

+	} *pattern;
+	/**<
+	 * Tunnel pattern specification (list terminated by the END pattern
+	 * item).
+	 */

As previously suggested, how about a single item per encap?

Quoted 10 lines

lib/librte_ether/rte_flow.h +9 -0

+};
+
+/**
+ * RTE_FLOW_ACTION_TYP_TUNNEL_DECAP
+ *
+ * Virtual tunnel end-point decapsulation action data.
+ *
+ * Non-terminating action by default.
+ */

lib/librte_ether/rte_flow.h struct rte_flow_action_group_count { · line 1251

Quoted 6 lines

lib/librte_ether/rte_flow.h +6 -0

+struct rte_flow_action_tunnel_decap {
+	enum rte_flow_item_type type;
+	/**<
+	 * Flow item type of virtual tunnel end-point to be decapsulated
+	 */
+};

Note that contrary to ENCAP, DECAP wouldn't necessarily need repeated actions to peel each layer off. The current definition is fine.

Quoted 9 lines

lib/librte_ether/rte_flow.h +2 -0

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

Signature · 1 line

2.7.4

Quoted 31 lines · Adrien Mazarguil

Quoted 8 lines · Declan Doherty

Add new flow action types and associated action data structures to support the encapsulation and decapsulation of the virtual tunnel endpoints.

The RTE_FLOW_ACTION_TYPE_TUNNEL_ENCAP action will cause the matching flow to be encapsulated in the virtual tunnel endpoint overlay defined in the tunnel_encap action data.

Quoted 6 lines

The RTE_FLOW_ACTION_TYPE_TUNNEL_DECAP action will cause all virtual tunnel endpoint overlays up to and including the first instance of the flow item type defined in the tunnel_decap action data for the matching flows.

Signed-off-by Declan Doherty <[email protected]>

This generic approach looks flexible enough to cover the use cases that immediately come to mind (VLAN, VXLAN), its design is sound.

However, while I'm aware it's not a concern at this point, it won't be able to deal with stateful tunnel or encapsulation types (e.g. IPsec or TCP) which will require additional meta data or some run-time assistance from the application.

Eventually for more complex use cases, dedicated encap/decap actions will have to appear, so the issue I wanted to raise before going further is this:

Going generic inevitably trades some of the usability; flat structures dedicated to VXLAN encap/decap with only the needed info to get the job done would likely be easier to implement in PMDs and use in applications. Any number of such actions can be added to rte_flow without ABI impact.

Quoted 3 lines

If VXLAN is the only use case at this point, my suggestion would be to go with simpler RTE_FLOW_ACTION_TYPE_VXLAN_(ENCAP|DECAP) actions, with fixed L2/L3/L4/L5 header definitions to prepend according to RFC 7348.

We can go this way and this will increase the action for more and more tunneling protocols being added. Current proposal is already a generic approach which specifies as a tunnel for all the tunneling protocols.

Quoted 4 lines

Now we can start with the generic approach, see how it fares and add dedicated encap/decap later as needed.

More comments below.

Quoted 6 lines

Quoted 6 lines

doc/guides/prog_guide/rte_flow.rst +6 -0

+Action: ``TUNNEL_ENCAP``
+^^^^^^^^^^^^^^^^^^^^^^
+
+Performs an encapsulation action by encapsulating the flows matched by the
+pattern items according to the network overlay defined in the
+``rte_flow_action_tunnel_encap`` pattern items.

Quoted 8 lines

doc/guides/prog_guide/rte_flow.rst fields in the pattern items. · line 1505

Quoted 5 lines

doc/guides/prog_guide/rte_flow.rst +5 -0

+
+This action modifies the payload of matched flows. The pattern items specified
+in the ``rte_flow_action_tunnel_encap`` action structure must defined a valid
+set of overlay headers, from the Ethernet header up to the overlay header. The
+pattern must be terminated with the RTE_FLOW_ITEM_TYPE_END item type.

Regarding the use of a pattern list, if you consider PMDs are already iterating on a list of actions when encountering RTE_FLOW_ACTION_TYPE_TUNNEL_ENCAP, it adds yet another inner loop.

We understand that it is implementation specifics. If we do not go for another inner loop, all the bundling need to be handled in the same function, which seems more clumsy to me. This also breaks the tunnel endpoint concept.

Quoted 1 line

Quoted 2 lines

How about making each encountered RTE_FLOW_ACTION_TYPE_TUNNEL_ENCAP provide exactly one item instead (in encap, i.e. reverse order)?

Again, if we have tunnel action, security action, and other actions, all the processing and tracking need to be done in one function. Now we will need ETH_ENCAP/DECAP, UDP_ENCAP/DECAP, NVGRE_ENCAP/DECAP, etc.

Quoted 3 lines

In which case perhaps "GENERIC" would be a better fit than "TUNNEL".

Quoted 14 lines

Quoted 4 lines

doc/guides/prog_guide/rte_flow.rst +4 -0

+
+   +-------+--------------------------+------------+
+   | Index | Flow Item Type           | Flow Item  |
+   +=======+==========================+============+

doc/guides/prog_guide/rte_flow.rst fields in the pattern items. · line 1536

Quoted 10 lines

doc/guides/prog_guide/rte_flow.rst +10 -0

+   | 0     | RTE_FLOW_ITEM_TYPE_ETH   | eth item   |
+   +-------+--------------------------+------------+
+   | 1     | RTE_FLOW_ITEM_TYPE_IPV4  | ipv4 item  |
+   +-------+--------------------------+------------+
+   | 2     | RTE_FLOW_ITEM_TYPE_UDP   | udp item   |
+   +-------+--------------------------+------------+
+   | 3     | RTE_FLOW_ITEM_TYPE_VXLAN | vxlan item |
+   +-------+--------------------------+------------+
+   | 4     | RTE_FLOW_ITEM_TYPE_END   | NULL       |
+   +-------+--------------------------+------------+

Quoted 8 lines

One possible issue is that it relies on objects normally found on the pattern side of flow rules. Those are supposed to match something, they are not intended for packet header generation. While their "spec" and "mask" fields might make sense in this context, the "last" field is odd.

You must define them without leaving anything open for interpretation by PMDs and users alike. Defining things as "undefined" is fine as long as it's covered.

Please note that the "void *item" in the "rte_flow_action_tunnel_encap.pattern" points to the data structure defined for the corresponding rte_flow_item_type instead of a rte_flow_item structure. As an example, for the rte_flow_item_eth type, the "void *item" will point to a "struct rte_flow_item_eth" instance. Thats why we have defined struct rte_flow_action_item inside struct rte_flow_action_tunnel_encap. So, no question of spec, mask, last anymore.

Quoted 17 lines

lib/librte_ether/rte_flow.h struct rte_flow_action_group_count { · line 1231

Quoted 9 lines

+ */

lib/librte_ether/rte_flow.h +8 -0

+struct rte_flow_action_tunnel_encap {
+	struct rte_flow_action_item {
+		enum rte_flow_item_type type;
+		/**< Flow item type. */
+		const void *item;
+		/**< Flow item definition which points to the data of
+		 * corresponding rte_flow_item_type.
+		 */

I see it's a new action type, albeit a bit confusing (there is no RTE_FLOW_ACTION_TYPE_ITEM).

I suggest the standard pattern item type since you're going with enum rte_flow_item_type anyway. Keep in mind you need some kind of mask to tell what fields are relevant. An application might otherwise want to encap with unsupported properties (e.g. specific IPv4 ToS field and whatnot).

Quoted 4 lines

How about a single "struct rte_flow_pattern_item item", neither const and neither a pointer. It's generic enough, enclosed spec/last/mask pointers take care of the specifics. You just need to define what's supposed to happen when "last" is set.

Please see the comment above regarding this field.

Quoted 9 lines

Quoted 9 lines

lib/librte_ether/rte_flow.h +9 -0

+};
+
+/**
+ * RTE_FLOW_ACTION_TYP_TUNNEL_DECAP
+ *
+ * Virtual tunnel end-point decapsulation action data.
+ *
+ * Non-terminating action by default.
+ */

Quoted 8 lines

lib/librte_ether/rte_flow.h struct rte_flow_action_group_count { · line 1251

Quoted 6 lines

lib/librte_ether/rte_flow.h +6 -0

+struct rte_flow_action_tunnel_decap {
+	enum rte_flow_item_type type;
+	/**<
+	 * Flow item type of virtual tunnel end-point to be decapsulated
+	 */
+};

Note that contrary to ENCAP, DECAP wouldn't necessarily need repeated actions to peel each layer off. The current definition is fine.

To clarify, the the decap is upto the PMD to remove all the header for a specified type. For example, for

rte_flow_item_type type=RTE_FLOW_ITEM_TYPE_VXLAN, the PMD will peel off (ETH, IPV4, UDP, VXLAN) header all together.

Quoted 9 lines

Quoted 9 lines

lib/librte_ether/rte_flow.h +2 -0

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

Signature · 1 line

2.7.4

Quoted 19 lines · Adrien Mazarguil

Quoted 18 lines · Declan Doherty

Quoted 8 lines

Add new flow action types and associated action data structures to support the encapsulation and decapsulation of the virtual tunnel endpoints.

The RTE_FLOW_ACTION_TYPE_TUNNEL_ENCAP action will cause the matching flow to be encapsulated in the virtual tunnel endpoint overlay defined in the tunnel_encap action data.

Quoted 6 lines

The RTE_FLOW_ACTION_TYPE_TUNNEL_DECAP action will cause all virtual tunnel endpoint overlays up to and including the first instance of the flow item type defined in the tunnel_decap action data for the matching flows.

Signed-off-by Declan Doherty <[email protected]>

This generic approach looks flexible enough to cover the use cases that immediately come to mind (VLAN, VXLAN), its design is sound.

Quoted 7 lines

Quoted 7 lines

However, while I'm aware it's not a concern at this point, it won't be able to deal with stateful tunnel or encapsulation types (e.g. IPsec or TCP) which will require additional meta data or some run-time assistance from the application.

Eventually for more complex use cases, dedicated encap/decap actions will have to appear, so the issue I wanted to raise before going further is

this:

Quoted 5 lines

Quoted 5 lines

Going generic inevitably trades some of the usability; flat structures dedicated to VXLAN encap/decap with only the needed info to get the job done would likely be easier to implement in PMDs and use in applications. Any number of such actions can be added to rte_flow

without ABI impact.

Quoted 3 lines

Quoted 3 lines

If VXLAN is the only use case at this point, my suggestion would be to go with simpler RTE_FLOW_ACTION_TYPE_VXLAN_(ENCAP|DECAP)

actions,

Quoted 5 lines

Quoted 2 lines

with fixed
L2/L3/L4/L5 header definitions to prepend according to RFC 7348.

We can go this way and this will increase the action for more and more tunneling protocols being added. Current proposal is already a generic approach which specifies as a tunnel for all the tunneling protocols.

Right, on the other hand there are not that many standard encapsulations offloaded by existing devices. rte_flow could easily handle dedicated actions for each of them without problem.

My point is that many of those (will eventually) have their own quirks to manage when doing encap/decap, it's not just a matter of prepending or removing a bunch of header definitions, otherwise we could as well let applications simply provide an arbitrary buffer to prepend.

Consider that the "generic" part is already built into rte_flow as the way patterns and action are handled. Adding another generic layer on top of that could make things more inconvenient than necessary to applications (my main concern).

You'd need another layer of validation/error reporting machinery to properly let applications know they cannot encap VXLAN on top of TCP on top of QinQinQinQinQ for instance. Either a single bounded encapsulation definition or a combination at the action list level is needed to avoid that.

Quoted 4 lines

Quoted 4 lines

Now we can start with the generic approach, see how it fares and add dedicated encap/decap later as needed.

More comments below.

<snip>

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

+Action: ``TUNNEL_ENCAP``
+^^^^^^^^^^^^^^^^^^^^^^

Quoted 9 lines

Quoted 9 lines

Review

Quoted 9 lines

(quoted diff) +9 -0

+
+Performs an encapsulation action by encapsulating the flows
+matched by the pattern items according to the network overlay
+defined in the ``rte_flow_action_tunnel_encap`` pattern items.
+
+This action modifies the payload of matched flows. The pattern
+items specified in the ``rte_flow_action_tunnel_encap`` action
+structure must defined a valid set of overlay headers, from the
+Ethernet header up to the overlay header. The pattern must be

terminated with the RTE_FLOW_ITEM_TYPE_END item type.

Quoted 3 lines

Quoted 3 lines

Regarding the use of a pattern list, if you consider PMDs are already iterating on a list of actions when encountering RTE_FLOW_ACTION_TYPE_TUNNEL_ENCAP, it adds yet another inner

loop.

Quoted 6 lines

We understand that it is implementation specifics. If we do not go for another inner loop, all the bundling need to be handled in the same function, which seems more clumsy to me. This also breaks the tunnel endpoint concept.

Quoted 2 lines

How about making each encountered

RTE_FLOW_ACTION_TYPE_TUNNEL_ENCAP

Quoted 4 lines

Quoted 1 line

provide exactly one item instead (in encap, i.e. reverse order)?

Again, if we have tunnel action, security action, and other actions, all the processing and tracking need to be done in one function. Now we will need ETH_ENCAP/DECAP, UDP_ENCAP/DECAP,

NVGRE_ENCAP/DECAP, etc.

Well, the number of DECAP actions doesn't need to perfectly reflect that of ENCAP since it implies all preceding layers. No problem with that.

Regarding multiple dedicated actions, my suggestion was for a single generic one as in this patch, but each instance on the ENCAP side would deal with a single protocol layer, instead of having a single ENCAP action with multiple inner layers (and thus an inner loop).

PMDs also gain the ability to precisely report which encap step fails by making rte_flow_error point to the problematic object to ease debugging of flow rules on the application side.

Why would that break the tunnel idea and more importantly, how would it prevent PMD developers from splitting their processing into multiple functions?

Quoted 3 lines

Quoted 3 lines

In which case perhaps "GENERIC" would be a better fit than "TUNNEL".

<snip>

Quoted 14 lines

Quoted 14 lines

Quoted 14 lines

doc/guides/prog_guide/rte_flow.rst +14 -0

+
+   +-------+--------------------------+------------+
+   | Index | Flow Item Type           | Flow Item  |
+   +=======+==========================+============+
+   | 0     | RTE_FLOW_ITEM_TYPE_ETH   | eth item   |
+   +-------+--------------------------+------------+
+   | 1     | RTE_FLOW_ITEM_TYPE_IPV4  | ipv4 item  |
+   +-------+--------------------------+------------+
+   | 2     | RTE_FLOW_ITEM_TYPE_UDP   | udp item   |
+   +-------+--------------------------+------------+
+   | 3     | RTE_FLOW_ITEM_TYPE_VXLAN | vxlan item |
+   +-------+--------------------------+------------+
+   | 4     | RTE_FLOW_ITEM_TYPE_END   | NULL       |
+   +-------+--------------------------+------------+

Quoted 3 lines

Quoted 3 lines

One possible issue is that it relies on objects normally found on the pattern side of flow rules. Those are supposed to match something, they are not intended for packet header generation. While

their "spec" and "mask"

Quoted 9 lines

Quoted 5 lines

fields might make sense in this context, the "last" field is odd.

You must define them without leaving anything open for interpretation by PMDs and users alike. Defining things as "undefined" is fine as long as it's covered.

Please note that the "void *item" in the "rte_flow_action_tunnel_encap.pattern" points to the data structure defined for the corresponding rte_flow_item_type instead of a rte_flow_item structure. As an example, for the rte_flow_item_eth type,

the "void *item"

Quoted 3 lines

will point to a "struct rte_flow_item_eth" instance. Thats why we have defined struct rte_flow_action_item inside struct rte_flow_action_tunnel_encap. So, no question of spec, mask, last

anymore.

Right, I noticed that after commenting its structure definition below.

I think I won't be the only one confused by this approach, also because a mask is needed in addition to a specification structure, otherwise how do you plan to tell what fields are relevant in application-provided protocol headers?

An application might set unusual IPv4/UDP/VXLAN fields and expect them to be part of the encapsulated traffic. Without a mask, a PMD must take headers verbatim, and I don't think many devices are ready for that yet.

Hence my other suggestion: defining inflexible $PROTOCOL_(ENCAP|DECAP) actions that do not allow more than what's defined by official RFCs for $PROTOCOL.

<snip>

Quoted 12 lines

Quoted 12 lines

lib/librte_ether/rte_flow.h struct rte_flow_action_group_count { · line 1231

Quoted 9 lines

+ */

lib/librte_ether/rte_flow.h +8 -0

+struct rte_flow_action_tunnel_encap {
+	struct rte_flow_action_item {
+		enum rte_flow_item_type type;
+		/**< Flow item type. */
+		const void *item;
+		/**< Flow item definition which points to the data of
+		 * corresponding rte_flow_item_type.
+		 */

I see it's a new action type, albeit a bit confusing (there is no RTE_FLOW_ACTION_TYPE_ITEM).

Quoted 4 lines

Quoted 4 lines

I suggest the standard pattern item type since you're going with enum rte_flow_item_type anyway. Keep in mind you need some kind of mask to tell what fields are relevant. An application might otherwise want to encap with unsupported properties (e.g. specific IPv4

ToS field and whatnot).

Quoted 6 lines

Quoted 5 lines

How about a single "struct rte_flow_pattern_item item", neither const and neither a pointer. It's generic enough, enclosed spec/last/mask pointers take care of the specifics. You just need to define what's supposed to happen when "last" is set.

Please see the comment above regarding this field.

Point still stands, if you need to distinguish spec and mask, a more complete structure is needed. Instead of adding a new (confusing) type, you should use rte_flow_item and define what happens when "last" is set.

You should define it as reserved for now, any non-NULL value is an error. This field might also be useful later.

<snip>

Quoted 17 lines

Quoted 9 lines

Quoted 9 lines

lib/librte_ether/rte_flow.h +9 -0

+};
+
+/**
+ * RTE_FLOW_ACTION_TYP_TUNNEL_DECAP
+ *
+ * Virtual tunnel end-point decapsulation action data.
+ *
+ * Non-terminating action by default.
+ */

Quoted 8 lines

lib/librte_ether/rte_flow.h struct rte_flow_action_group_count { · line 1251

Quoted 6 lines

lib/librte_ether/rte_flow.h +6 -0

+struct rte_flow_action_tunnel_decap {
+	enum rte_flow_item_type type;
+	/**<
+	 * Flow item type of virtual tunnel end-point to be decapsulated
+	 */
+};

Note that contrary to ENCAP, DECAP wouldn't necessarily need repeated actions to peel each layer off. The current definition is fine.

Quoted 4 lines

To clarify, the the decap is upto the PMD to remove all the header for a specified type. For example, for

rte_flow_item_type type=RTE_FLOW_ITEM_TYPE_VXLAN, the PMD will

peel off (ETH, IPV4, UDP, VXLAN) header all together.

Yep, that's fine, whether we use multiple actions or a single one doing multiple things, a single DECAP can peel them off all at once :)

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

lib/librte_ether/rte_flow.h +2 -1

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

2.7.4

If the reasons I gave did not manage to convince you about choosing between
either fixed (VLAN|VXLAN)_(ENCAP|DECAP) actions or generic encap/decap actions that deal with a single protocol layer at once instead of the proposed approach, I'm ready to try it out as an experimental API (all new objects tagged as experimental) *if* you address the lack of mask, which remains an open issue.

--
Adrien Mazarguil
6WIND

Quoted 8 lines · Declan Doherty

Add new flow action types and associated action data structures to support the encapsulation and decapsulation of the virtual tunnel endpoints.

The RTE_FLOW_ACTION_TYPE_TUNNEL_ENCAP action will cause the matching flow to be encapsulated in the virtual tunnel endpoint overlay defined in the tunnel_encap action data.

Quoted 6 lines

The RTE_FLOW_ACTION_TYPE_TUNNEL_DECAP action will cause all virtual tunnel endpoint overlays up to and including the first instance of the flow item type defined in the tunnel_decap action data for the matching flows.

Signed-off-by Declan Doherty <[email protected]>

This generic approach looks flexible enough to cover the use cases that immediately come to mind (VLAN, VXLAN), its design is sound.

However, while I'm aware it's not a concern at this point, it won't be able to deal with stateful tunnel or encapsulation types (e.g. IPsec or TCP) which will require additional meta data or some run-time assistance from the application.

Eventually for more complex use cases, dedicated encap/decap actions will have to appear, so the issue I wanted to raise before going further is this:

Going generic inevitably trades some of the usability; flat structures dedicated to VXLAN encap/decap with only the needed info to get the job done would likely be easier to implement in PMDs and use in applications. Any number of such actions can be added to rte_flow without ABI impact.

If VXLAN is the only use case at this point, my suggestion would be to go with simpler RTE_FLOW_ACTION_TYPE_VXLAN_(ENCAP|DECAP) actions, with fixed L2/L3/L4/L5 header definitions to prepend according to RFC 7348.

Now we can start with the generic approach, see how it fares and add dedicated encap/decap later as needed.

More comments below.

Quoted 24 lines

---
  doc/guides/prog_guide/rte_flow.rst | 77 ++++++++++++++++++++++++++++++++--
  lib/librte_ether/rte_flow.h        | 84 ++++++++++++++++++++++++++++++++++++--
  2 files changed, 155 insertions(+), 6 deletions(-)

doc/guides/prog_guide/rte_flow.rst +4 -2

index fd33d19..106fb93 100644
@@ -997,9 +997,11 @@ Actions
 
  Each possible action is represented by a type. Some have associated
  configuration structures. Several actions combined in a list can be assigned
-to a flow rule. That list is not ordered.
+to a flow rule. That list is not ordered, with the exception of  actions which
+modify the packet itself, these packet modification actions must be specified
+in the explicit order in which they are to be executed.
 
-They fall in three categories:
+They fall in four categories:
 
  - Terminating actions (such as QUEUE, DROP, RSS, PF, VF) that prevent
    processing matched packets by subsequent flow rules, unless overridden

doc/guides/prog_guide/rte_flow.rst They fall in three categories: · line 1017

Quoted 9 lines

doc/guides/prog_guide/rte_flow.rst +4 -1

@@ -1008,8 +1010,11 @@ They fall in three categories:
  - Non-terminating actions (PASSTHRU, DUP) that leave matched packets up for
    additional processing by subsequent flow rules.
 
+- Non-terminating meta actions that do not affect the fate of packets but result
+  in modification of the packet itself (SECURITY, TUNNEL_ENCAP, TUNNEL_DECAP).
+
  - Other non-terminating meta actions that do not affect the fate of packets
-  (END, VOID, MARK, FLAG, COUNT, SECURITY).
+  (END, VOID, MARK, FLAG, COUNT).

The above changes are not necessary anymore [1][2].

[1] "ethdev: clarify flow API pattern items and actions"

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

doc/guides/prog_guide/rte_flow.rst fields in the pattern items. · line 1494

Quoted 7 lines

doc/guides/prog_guide/rte_flow.rst +1 -0

  When several actions are combined in a flow rule, they should all have
  different types (e.g. dropping a packet twice is not possible).

@@ -1486,6 +1491,72 @@ fields in the pattern items.
     | 1     | END      |
     +-------+----------+
 
+

Nit: titles in this file are separated by a single empty line.

doc/guides/prog_guide/rte_flow.rst fields in the pattern items. · line 1505

Quoted 11 lines

doc/guides/prog_guide/rte_flow.rst +11 -0

+Action: ``TUNNEL_ENCAP``
+^^^^^^^^^^^^^^^^^^^^^^
+
+Performs an encapsulation action by encapsulating the flows matched by the
+pattern items according to the network overlay defined in the
+``rte_flow_action_tunnel_encap`` pattern items.
+
+This action modifies the payload of matched flows. The pattern items specified
+in the ``rte_flow_action_tunnel_encap`` action structure must defined a valid
+set of overlay headers, from the Ethernet header up to the overlay header. The
+pattern must be terminated with the RTE_FLOW_ITEM_TYPE_END item type.

Regarding the use of a pattern list, if you consider PMDs are already iterating on a list of actions when encountering RTE_FLOW_ACTION_TYPE_TUNNEL_ENCAP, it adds yet another inner loop.

How about making each encountered RTE_FLOW_ACTION_TYPE_TUNNEL_ENCAP provide exactly one item instead (in encap, i.e. reverse order)?

In which case perhaps "GENERIC" would be a better fit than "TUNNEL".

doc/guides/prog_guide/rte_flow.rst fields in the pattern items. · line 1507

Quoted 2 lines

doc/guides/prog_guide/rte_flow.rst +2 -0

+
+- Non-terminating by default.

There's no such property anymore [2].

Quoted 5 lines

doc/guides/prog_guide/rte_flow.rst +5 -0

+
+.. _table_rte_flow_action_tunnel_encap:
+
+.. table:: TUNNEL_ENCAP
+

doc/guides/prog_guide/rte_flow.rst fields in the pattern items. · line 1522

Quoted 10 lines

doc/guides/prog_guide/rte_flow.rst +10 -0

+   +-------------+---------------------------------------------+
+   | Field       | Value                                       |
+   +=============+=============================================+
+   | ``pattern`` | Virtual tunnel end-point pattern definition |
+   +-------------+---------------------------------------------+
+
+
+.. _table_rte_flow_action_tunnel_encap_example:
+
+.. table:: IPv4 VxLAN flow pattern example.

VxLAN => VXLAN

Quoted 4 lines

doc/guides/prog_guide/rte_flow.rst +4 -0

+
+   +-------+--------------------------+------------+
+   | Index | Flow Item Type           | Flow Item  |
+   +=======+==========================+============+

doc/guides/prog_guide/rte_flow.rst fields in the pattern items. · line 1536

Quoted 10 lines

doc/guides/prog_guide/rte_flow.rst +10 -0

+   | 0     | RTE_FLOW_ITEM_TYPE_ETH   | eth item   |
+   +-------+--------------------------+------------+
+   | 1     | RTE_FLOW_ITEM_TYPE_IPV4  | ipv4 item  |
+   +-------+--------------------------+------------+
+   | 2     | RTE_FLOW_ITEM_TYPE_UDP   | udp item   |
+   +-------+--------------------------+------------+
+   | 3     | RTE_FLOW_ITEM_TYPE_VXLAN | vxlan item |
+   +-------+--------------------------+------------+
+   | 4     | RTE_FLOW_ITEM_TYPE_END   | NULL       |
+   +-------+--------------------------+------------+

One possible issue is that it relies on objects normally found on the pattern side of flow rules. Those are supposed to match something, they are not intended for packet header generation. While their "spec" and "mask" fields might make sense in this context, the "last" field is odd.

You must define them without leaving anything open for interpretation by PMDs and users alike. Defining things as "undefined" is fine as long as it's covered.

doc/guides/prog_guide/rte_flow.rst fields in the pattern items. · line 1538

Quoted 2 lines

doc/guides/prog_guide/rte_flow.rst +2 -0

+
+

Nit: only one empty line necessary here.

doc/guides/prog_guide/rte_flow.rst fields in the pattern items. · line 1544

Quoted 6 lines

doc/guides/prog_guide/rte_flow.rst +6 -0

+Action: ``TUNNEL_DECAP``
+^^^^^^^^^^^^^^^^^^^^^^
+
+Performs a decapsulation action by stripping all headers of the virtual tunnel
+end-point overlay up to the header defined by the flow item type of flows
+matched by the pattern items.

Not necessarily, for instance if one guarantees that flowing traffic only consists of decap'able packets. You must avoid mandatory dependencies between patterns and actions since they are normally unrelated.

What you can document on the other hand is that the behavior is undefined when processing traffic on which the action can't be applied. This is how RSS level is documented [3].

[3] https://dpdk.org/ml/archives/dev/2018-April/095783.html

doc/guides/prog_guide/rte_flow.rst fields in the pattern items. · line 1550

Quoted 6 lines

doc/guides/prog_guide/rte_flow.rst +6 -0

+
+This action modifies the payload of matched flows. The flow item type specified
+in the ``rte_flow_action_tunnel_decap`` action structure must defined a valid
+set of overlay header type.
+
+- Non-terminating by default.

See [2].

doc/guides/prog_guide/rte_flow.rst fields in the pattern items. · line 1558

Quoted 8 lines

doc/guides/prog_guide/rte_flow.rst +8 -0

+
+.. _table_rte_flow_action_tunnel_decap:
+
+   +---------------+----------------------------------------------+
+   | Field         | Value                                        |
+   +===============+==============================================+
+   | ``item type`` | Item type of tunnel end-point to decapsulate |
+   +---------------+----------------------------------------------+

"item type" should be the exact name used in the structure.

lib/librte_ether/rte_flow.h struct rte_flow_item { · line 857

Quoted 13 lines

doc/guides/prog_guide/rte_flow.rst +1 -0

+
  Negative types
  ~~~~~~~~~~~~~~
 

lib/librte_ether/rte_flow.h +1 -0

index 7d1f89d..6d94423 100644
@@ -854,14 +854,17 @@ struct rte_flow_item {
  	const void *mask; /**< Bit-mask applied to spec and last. */
  };
 
+

Unnecessary empty line.

Quoted 16 lines

lib/librte_ether/rte_flow.h +4 -2

  /**
   * Action types.
   *
   * Each possible action is represented by a type. Some have associated
   * configuration structures. Several actions combined in a list can be
- * affected to a flow rule. That list is not ordered.
+ * affected to a flow rule. That list is not ordered, with the exception of
+ * actions which modify the packet itself, these packet modification actions
+ * must be specified in the explicit order in which they are to be executed.
   *
- * They fall in three categories:
+ * They fall in four categories:
   *
   * - Terminating actions (such as QUEUE, DROP, RSS, PF, VF) that prevent
   *   processing matched packets by subsequent flow rules, unless overridden

lib/librte_ether/rte_flow.h struct rte_flow_item { · line 879

Quoted 7 lines

lib/librte_ether/rte_flow.h +4 -0

@@ -870,6 +873,10 @@ struct rte_flow_item {
   * - Non terminating actions (PASSTHRU, DUP) that leave matched packets up
   *   for additional processing by subsequent flow rules.
   *
+ * - Non terminating meta actions that do not affect the fate of
+ *   packets but result in modification of the packet itself (SECURITY,
+ *   TUNNEL_ENCAP, TUNNEL_DECAP).
+ *

Same comment as above [1][2].

lib/librte_ether/rte_flow.h enum rte_flow_action_type { · line 1032

Quoted 9 lines

lib/librte_ether/rte_flow.h +1 -1

   * - Other non terminating meta actions that do not affect the fate of
   *   packets (END, VOID, MARK, FLAG, COUNT).
   *

@@ -1022,7 +1029,42 @@ enum rte_flow_action_type {
  	 *
  	 * See struct rte_flow_action_group_count.
  	 */
-	RTE_FLOW_ACTION_TYPE_GROUP_COUNT
+	RTE_FLOW_ACTION_TYPE_GROUP_COUNT,

An empty line would have been needed here (if we agree about no more GROUP_COUNT.)

Quoted 30 lines

lib/librte_ether/rte_flow.h +30 -0

+	/**
+	 * Encapsulate flow with tunnel defined in
+	 * rte_flow_action_tunnel_encap structure.
+	 *
+	 * See struct rte_flow_action_tunnel_encap.
+	 */
+	RTE_FLOW_ACTION_TYPE_TUNNEL_ENCAP,
+
+	/**
+	 * Decapsulate all the headers of the tunnel
+	 *
+	 * See struct rte_flow_action_tunnel_decap.
+	 */
+	RTE_FLOW_ACTION_TYPE_TUNNEL_DECAP,
+
+	/**
+	 * Redirects packets to the logical group of the current device.
+	 *
+	 * In a logical hierarchy of groups, which can be used to represent a
+	 * physical of logical chaining of flow tables, this action allows the
+	 * terminating action to be a logical group of the same device.
+	 *
+	 * See struct rte_flow_action_group.
+	 */
+	RTE_FLOW_ACTION_TYPE_GROUP,
+
+	/**
+	 * [META]
+	 *
+	 * Set specific metadata field associated with packet which is then

lib/librte_ether/rte_flow.h enum rte_flow_action_type { · line 1067

Quoted 5 lines

lib/librte_ether/rte_flow.h +5 -0

+	 * available to further pipeline stages.
+	 *
+	 * See struct rte_flow_action_metadata.
+	 */
+	RTE_FLOW_ACTION_TYPE_METADATA

These two actions should be part of the next patch, I won't comment them here.

lib/librte_ether/rte_flow.h struct rte_flow_action_group_count { · line 1222

Quoted 12 lines

lib/librte_ether/rte_flow.h +5 -0

  };
 
  /**

@@ -1173,6 +1215,42 @@ struct rte_flow_action_group_count {
  };
 
  /**
+ * RTE_FLOW_ACTION_TYPE_TUNNEL_ENCAP
+ *
+ * Virtual tunnel end-point encapsulation action data.
+ *
+ * Non-terminating action by default.

See [2].

lib/librte_ether/rte_flow.h struct rte_flow_action_group_count { · line 1231

Quoted 9 lines

lib/librte_ether/rte_flow.h +9 -0

+ */
+struct rte_flow_action_tunnel_encap {
+	struct rte_flow_action_item {
+		enum rte_flow_item_type type;
+		/**< Flow item type. */
+		const void *item;
+		/**< Flow item definition which points to the data of
+		 * corresponding rte_flow_item_type.
+		 */

I see it's a new action type, albeit a bit confusing (there is no RTE_FLOW_ACTION_TYPE_ITEM).

I suggest the standard pattern item type since you're going with enum rte_flow_item_type anyway. Keep in mind you need some kind of mask to tell what fields are relevant. An application might otherwise want to encap with unsupported properties (e.g. specific IPv4 ToS field and whatnot). sure, that makes sense.

How about a single "struct rte_flow_pattern_item item", neither const and neither a pointer. It's generic enough, enclosed spec/last/mask pointers take care of the specifics. You just need to define what's supposed to happen when "last" is set.

lib/librte_ether/rte_flow.h struct rte_flow_action_group_count { · line 1236

Quoted 6 lines

lib/librte_ether/rte_flow.h +5 -0

+	} *pattern;
+	/**<
+	 * Tunnel pattern specification (list terminated by the END pattern
+	 * item).
+	 */

As previously suggested, how about a single item per encap?

Quoted 5 lines

lib/librte_ether/rte_flow.h +5 -0

+};
+
+/**
+ * RTE_FLOW_ACTION_TYP_TUNNEL_DECAP
+ *

lib/librte_ether/rte_flow.h struct rte_flow_action_group_count { · line 1251

Quoted 10 lines

lib/librte_ether/rte_flow.h +10 -0

+ * Virtual tunnel end-point decapsulation action data.
+ *
+ * Non-terminating action by default.
+ */
+struct rte_flow_action_tunnel_decap {
+	enum rte_flow_item_type type;
+	/**<
+	 * Flow item type of virtual tunnel end-point to be decapsulated
+	 */
+};

Note that contrary to ENCAP, DECAP wouldn't necessarily need repeated actions to peel each layer off. The current definition is fine.

Quoted 8 lines

lib/librte_ether/rte_flow.h +2 -0

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

Signature · 1 line

2.7.4

Add group action type which defines a terminating action which allows a matched flow to be redirect to a group. This allows logical flow table hierarchies to be managed through rte_flow.

Signed-off-by Declan Doherty <[email protected]>

---
 doc/guides/prog_guide/rte_flow.rst | 23 +++++++++++++++++++++++
 lib/librte_ether/rte_flow.h        | 15 +++++++++++++++
 2 files changed, 38 insertions(+)

doc/guides/prog_guide/rte_flow.rst +0 -0

index 106fb93..2f0a47a 100644

doc/guides/prog_guide/rte_flow.rst +2 -0

+
+
 Negative types
 ~~~~~~~~~~~~~~
 

lib/librte_ether/rte_flow.h +0 -0

index 6d94423..968a23b 100644

lib/librte_ether/rte_flow.h +3 -0

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

Signature · 1 line

2.7.4

Quoted 5 lines · Declan Doherty

Add group action type which defines a terminating action which allows a matched flow to be redirect to a group. This allows logical flow table hierarchies to be managed through rte_flow.

Signed-off-by Declan Doherty <[email protected]>

OK, I'm wondering if perhaps with the addition of this action, we should redefine groups as unlinked by default?

Currently traffic enters through the flow rule with the lowest priority of the group with the lowest ID and iterates through subsequent flow rules and groups until matched by a flow rule without PASSTHRU (according to latest definition [1]).

This would make jumps between groups always explicit, not necessarily a bad idea given no PMD implements groups as of yet. Thoughts?

Also as a rather fundamental API addition, I suggest to add it after RTE_FLOW_ACTION_TYPE_PASSTHRU. It's OK because ABI is already broken. You just need to mention it in the commit log [1].

Another suggestion would be to rename it "JUMP" (reasons below).

[1] "ethdev: alter behavior of flow API actions"

     http://dpdk.org/ml/archives/dev/2018-April/095779.html

Quoted 10 lines

---
  doc/guides/prog_guide/rte_flow.rst | 23 +++++++++++++++++++++++
  lib/librte_ether/rte_flow.h        | 15 +++++++++++++++
  2 files changed, 38 insertions(+)

doc/guides/prog_guide/rte_flow.rst +0 -0

index 106fb93..2f0a47a 100644

doc/guides/prog_guide/rte_flow.rst set of overlay header type. · line 1560

Quoted 4 lines

doc/guides/prog_guide/rte_flow.rst +1 -0

@@ -1557,6 +1557,29 @@ set of overlay header type.
     | ``item type`` | Item type of tunnel end-point to decapsulate |
     +---------------+----------------------------------------------+
 
+

Unnecessary empty line.

doc/guides/prog_guide/rte_flow.rst set of overlay header type. · line 1570

Quoted 10 lines

doc/guides/prog_guide/rte_flow.rst +10 -0

+Action: ``GROUP``
+^^^^^^^^^^^^^^^^^
+
+Redirects packets to a group on the current device.
+
+In a hierarchy of groups, which can be used to represent physical or logical
+flow tables on the device, this action allows the terminating action to be a
+group on that device.
+
+- Terminating by default.

Keep in mind there's no such thing as a terminating action anymore [1].

doc/guides/prog_guide/rte_flow.rst set of overlay header type. · line 1580

Quoted 10 lines

doc/guides/prog_guide/rte_flow.rst +10 -0

+
+.. _table_rte_flow_action_group:
+
+.. table:: GROUP
+
+   +--------------+---------------------------------+
+   | Field        | Value                           |
+   +==============+=================================+
+   | ``id``       | Group ID to redirect packets to |
+   +--------------+---------------------------------+

"Field" column can be shrunk somewhat.

Quoted 12 lines

doc/guides/prog_guide/rte_flow.rst +2 -0

+
+
  Negative types
  ~~~~~~~~~~~~~~
 

lib/librte_ether/rte_flow.h +0 -0

index 6d94423..968a23b 100644
@@ -1251,6 +1251,21 @@ struct rte_flow_action_tunnel_decap {
  };
 

lib/librte_ether/rte_flow.h struct rte_flow_action_tunnel_decap { · line 1254

Quoted 2 lines

lib/librte_ether/rte_flow.h +1 -0

  /**
+ * RTE_FLOW_ACTION_TYPE_GROUP

Its addition to enum rte_flow_action_type should be part of this commit.

lib/librte_ether/rte_flow.h struct rte_flow_action_tunnel_decap { · line 1262

Quoted 8 lines

lib/librte_ether/rte_flow.h +8 -0

+ *
+ * Redirects packets to a group on the current device.
+ *
+ * In a hierarchy of groups, which can be used to represent physical or logical
+ * flow tables on the device, this action allows the terminating action to be a
+ * group on that device.
+ *
+ * Terminating by default.

See [1].

lib/librte_ether/rte_flow.h struct rte_flow_action_tunnel_decap { · line 1265

Quoted 3 lines

lib/librte_ether/rte_flow.h +3 -0

+ */
+struct rte_flow_action_group {
+	uint32_t id;

Assuming this structure is named rte_flow_action_jump, naming this field "group" would match the attribute of the same name.

Quoted 9 lines

lib/librte_ether/rte_flow.h +3 -0

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

Signature · 1 line

2.7.4

Don't forget testpmd code and documentation update.

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 <[email protected]>

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

doc/guides/prog_guide/rte_flow.rst +0 -0

index 2f0a47a..9b2b0e3 100644

doc/guides/prog_guide/rte_flow.rst +35 -0

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

doc/guides/prog_guide/rte_flow.rst +27 -0

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

lib/librte_ether/rte_flow.h +0 -0

index 968a23b..f75dee7 100644

lib/librte_ether/rte_flow.h +0 -0

 };
 
 /**

@@ -815,6 +822,17 @@ static const struct rte_flow_item_geneve rte_flow_item_geneve_mask = {
 #endif
 

lib/librte_ether/rte_flow.h +11 -0

+ *
+ * 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 {

lib/librte_ether/rte_flow.h +2 -0

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

Signature · 1 line

2.7.4

Quoted 5 lines · Declan Doherty

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.

Quoted 5 lines

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 <[email protected]>

Is there already HW support for extraction and subsequent matching?

Quoted 1 line

Otherwise it looks overkill. If it's meant to be implemented in

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.

Quoted 13 lines

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

doc/guides/prog_guide/rte_flow.rst +0 -0

index 2f0a47a..9b2b0e3 100644
@@ -1580,6 +1580,91 @@ group on that device.
     +--------------+---------------------------------+
 
 

doc/guides/prog_guide/rte_flow.rst group on that device. · line 1583

Quoted 1 line

doc/guides/prog_guide/rte_flow.rst +1 -0

+

You keep adding these empty lines :)

doc/guides/prog_guide/rte_flow.rst group on that device. · line 1595

Quoted 13 lines

doc/guides/prog_guide/rte_flow.rst +12 -0

+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

Quoted 35 lines

doc/guides/prog_guide/rte_flow.rst +35 -0

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

doc/guides/prog_guide/rte_flow.rst group on that device. · line 1640

Quoted 10 lines

doc/guides/prog_guide/rte_flow.rst +10 -0

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

Quoted 35 lines

doc/guides/prog_guide/rte_flow.rst +27 -0

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

lib/librte_ether/rte_flow.h +0 -0

index 968a23b..f75dee7 100644

lib/librte_ether/rte_flow.h enum rte_flow_item_type { · line 332

Quoted 10 lines

lib/librte_ether/rte_flow.h +7 -0

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

Quoted 6 lines

lib/librte_ether/rte_flow.h +0 -0

  };
 
  /**

@@ -815,6 +822,17 @@ static const struct rte_flow_item_geneve rte_flow_item_geneve_mask = {
  #endif
 

lib/librte_ether/rte_flow.h static const struct rte_flow_item_geneve rte_flow_item_geneve_mask = { · line 833

Quoted 10 lines

lib/librte_ether/rte_flow.h +9 -0

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

lib/librte_ether/rte_flow.h struct rte_flow_action_group { · line 1287

Quoted 10 lines

lib/librte_ether/rte_flow.h +3 -0

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

Quoted 11 lines

lib/librte_ether/rte_flow.h +11 -0

+ *
+ * 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 {

lib/librte_ether/rte_flow.h struct rte_flow_action_group { · line 1308

Quoted 10 lines

lib/librte_ether/rte_flow.h +10 -0

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

Quoted 8 lines

lib/librte_ether/rte_flow.h +2 -0

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

Signature · 1 line

2.7.4

-----Original Message-----
From: dev <[email protected]> On Behalf Of Declan Doherty
Sent: Friday, April 6, 2018 8:24 PM
To: [email protected]
Cc: Declan Doherty <[email protected]>
Subject: [dpdk-dev] [PATCH v3 4/4] ethdev: Add metadata flow and action items support

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 <[email protected]>

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

doc/guides/prog_guide/rte_flow.rst +89 -0

index 2f0a47a..9b2b0e3 100644
@@ -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
 ~~~~~~~~~~~~~~

lib/librte_ether/rte_flow.h +43 -1

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

@@ -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 */
+};
+
+/**
  * 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
+ *
+ * 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.
  *
  * A list of actions is terminated by a END action.
--

2.7.4

Add new flow action types and associated action data structure to support the encapsulation and decapsulation of VXLAN/NVGRE tunnel endpoints.

The RTE_FLOW_ACTION_TYPE_VXLAN_ENCAP or RTE_FLOW_ACTION_TYPE_NVGRE_ENCAP actions will cause the matching flow to be encapsulated in the tunnel endpoint overlay defined in the rte_flow_action_tunnel_encap action definition.

The RTE_FLOW_ACTION_TYPE_VXLAN_DECAP and RTE_FLOW_ACTION_TYPE_NVGRE_DECAP actions will cause all headers associated with the outermost VXLAN/NVGRE tunnel overlay to be decapsulated from the matching flow.

Signed-off-by Declan Doherty <[email protected]>

---
 doc/guides/prog_guide/rte_flow.rst | 89 ++++++++++++++++++++++++++++++++++++++
 lib/librte_ether/rte_flow.h        | 67 +++++++++++++++++++++++++++-
 2 files changed, 155 insertions(+), 1 deletion(-)

doc/guides/prog_guide/rte_flow.rst +0 -0

index 961943dda..672473d33 100644

doc/guides/prog_guide/rte_flow.rst +32 -0

+VLXAN network overlay which conforms with RFC 7348 (Virtual eXtensible Local Area
+Network (VXLAN): A Framework for Overlaying Virtualized Layer 2 Networks over
+Layer 3 Networks). The pattern must be terminated with
+the RTE_FLOW_ITEM_TYPE_END item type.
+
+- Non-terminating by default.
+
+.. _table_rte_flow_action_tunnel_encap:
+
+.. table:: TUNNEL_ENCAP
+
+   +----------------+-------------------------------------+
+   | Field          | Value                               |
+   +================+=====================================+
+   | ``definition`` | Tunnel end-point overlay definition |
+   +----------------+-------------------------------------+
+
+.. _table_rte_flow_action_tunnel_encap_example:
+
+.. table:: IPv4 VxLAN flow pattern example.
+
+   +-------+----------+
+   | Index | Item     |
+   +=======+==========+
+   | 0     | Ethernet |
+   +-------+----------+
+   | 1     | IPv4     |
+   +-------+----------+
+   | 2     | UDP      |
+   +-------+----------+
+   | 3     | VXLAN    |
+   +-------+----------+

doc/guides/prog_guide/rte_flow.rst +15 -0

+
+.. _table_rte_flow_action_tunnel_encap_example:
+
+.. table:: IPv4 NVGRE flow pattern example.
+
+   +-------+----------+
+   | Index | Item     |
+   +=======+==========+
+   | 0     | Ethernet |
+   +-------+----------+
+   | 1     | IPv4     |
+   +-------+----------+
+   | 2     | NVGRE    |
+   +-------+----------+
+   | 3     | END      |

doc/guides/prog_guide/rte_flow.rst +0 -0

 Negative types
 ~~~~~~~~~~~~~~
 

lib/librte_ether/rte_flow.h +19 -1

index 56c733451..537e1bfba 100644
@@ -1010,7 +1010,35 @@ enum rte_flow_action_type {
 	 *
 	 * See struct rte_flow_action_security.
 	 */
-	RTE_FLOW_ACTION_TYPE_SECURITY
+	RTE_FLOW_ACTION_TYPE_SECURITY,
+
+	/**
+	 * Encapsulate flow in VXLAN tunnel as defined in RFC7348
+	 * using headers defined in rte_flow_action_tunnel_encap
+	 * structure.
+	 *
+	 * See struct rte_flow_action_tunnel_encap.
+	 */
+	RTE_FLOW_ACTION_TYPE_VXLAN_ENCAP,
+
+	/**
+	 * Decapsulate outer most VXLAN tunnel headers from flow
+	 */
+	RTE_FLOW_ACTION_TYPE_VXLAN_DECAP,
+
+	/**
+	 * Encapsulate flow in NVGRE tunnel as defined in RFC7637
+	 * using header defined in the rte_flow_action_tunnel_encap

lib/librte_ether/rte_flow.h +19 -0

 };
 
 /**

@@ -1148,6 +1176,43 @@ struct rte_flow_action_security {
 	void *security_session; /**< Pointer to security session structure. */
 };
 
+/**
+ * RTE_FLOW_ACTION_TYPE_TUNNEL_ENCAP
+ *
+ * Tunnel end-point encapsulation data definition
+ *
+ * The tunnel definition is provided through the flow item pattern pattern, the
+ * provided pattern must conform with the corresponding RFC for the
+ * tunnel encapsulation action specified by the action type. The flow
+ * definition must be provided in order from the RTE_FLOW_ITEM_TYPE_ETH
+ * definition up the end item which is specified by RTE_FLOW_ITEM_TYPE_END.
+ *
+ * The mask field allows user to specify which fields in the flow item
+ * definitions can be ignored and which have valid data and can be used
+ * verbatim.
+ *
+ * Note: the last field is not used in the definition of a tunnel and can be
+ * ignored.
+ *
+ * For example if the actions type was RTE_FLOW_ACTION_TYPE_VXLAN_ENCAP

lib/librte_ether/rte_flow.h +2 -0

+};
+
 /**
  * Definition of a single action.
  *

Signature · 1 line

2.14.3

Subject: Re: [dpdk-dev] [PATCH v4 1/6] ethdev: Add tunnel encap/decap actions

On Wed, Apr 18, 2018 at 10:04:18PM +0100, Declan Doherty wrote:

I'm still unsure about the definition involving a list of items. I imagined these actions even less generic than that. How about tagging them EXPERIMENTAL until sorted out through the first PMD implementation?

(We could even provide forward declarations only to prevent applications from using them in the meantime.)

--
Adrien Mazarguil
6WIND

Thursday, April 19, 2018 4:03 PM, Adrien Mazarguil:

Quoted 4 lines

Subject: Re: [dpdk-dev] [PATCH v4 1/6] ethdev: Add tunnel encap/decap actions

On Wed, Apr 18, 2018 at 10:04:18PM +0100, Declan Doherty wrote:

[...]

Quoted 1 line

Quoted 3 lines

I'm still unsure about the definition involving a list of items. I imagined these actions even less generic than that. How about tagging them EXPERIMENTAL until sorted out through the first PMD implementation?

I think every new API should be marked as experimental. At least for one release. Here in specific, because there is no actual implementation of those APIs in any of the drivers.

Quoted 7 lines

(We could even provide forward declarations only to prevent applications from using them in the meantime.)

--
Adrien Mazarguil
6WIND

Quoted 9 lines · Shahaf Shuler

Thursday, April 19, 2018 4:03 PM, Adrien Mazarguil:

Quoted 4 lines

Subject: Re: [dpdk-dev] [PATCH v4 1/6] ethdev: Add tunnel encap/decap actions

On Wed, Apr 18, 2018 at 10:04:18PM +0100, Declan Doherty wrote:

[...]

Quoted 1 line

Quoted 7 lines

Quoted 3 lines

I'm still unsure about the definition involving a list of items. I imagined these actions even less generic than that. How about tagging them EXPERIMENTAL until sorted out through the first PMD implementation?

I think every new API should be marked as experimental. At least for one release. Here in specific, because there is no actual implementation of those APIs in any of the drivers.

On this, since these are just new structures to use with existing APIs is using the following tag in the doxygen comments sufficient?

/**
* @warning
* @b EXPERIMENTAL: this structure may change without prior notice */

Quoted 2 lines

Quoted 2 lines

(We could even provide forward declarations only to prevent applications from using them in the meantime.)

Add jump action type which defines an action which allows a matched flow to be redirect to the specified group. This allows physical and logical flow table/group hierarchies to be managed through rte_flow.

Signed-off-by Declan Doherty <[email protected]>

---
 doc/guides/prog_guide/rte_flow.rst | 26 ++++++++++++++++++++++++--
 lib/librte_ether/rte_flow.h        | 27 +++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 2 deletions(-)

doc/guides/prog_guide/rte_flow.rst +0 -0

index 672473d33..325010544 100644