[RFC] ethdev: add template table insertion and matching types

Message ID 20221214022110.393410-1-akozyrev@nvidia.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series [RFC] ethdev: add template table insertion and matching types |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS

Commit Message

Alexander Kozyrev Dec. 14, 2022, 2:21 a.m. UTC
  Bring more flexibility and control over both flow rule insertion
and packet matching mechanisms. Introduce 2 new flow table types:

1. Allow a user to specify the insertion type used in template tables.
The insertion type is responsible for choosing the appropriate key
value used to map inserted flow rules into a template table.

Flow rules can be inserted by calculating the hash value for
the pattern or inserted by index via the new create_by_index() API.
The idea of the index-based insertion is to avoid additional matches
and simply execute predefined actions after jumping to the index.

The insertion to an already existing index is undefined and
depends on PMD implementation. An old rule must be destroyed first.
The index cannot be bigger than the size of the table.

2. Allow a user to specify the hash calculation function used in template
tables. The hash calculation type is responsible for the calculation of
the flow rule index a packet would hit upon arrival at the table.

Control over this is useful for applications with custom RSS algorithms,
for example. An application can select various packet fields to serve as
a hash calculation source and jump to the appropriate flow rule location.
The RSS hash result will be used as the index in the table. For the linear
hash function, the mapping is one-to-one and the hash result is the index.
For other hash functions, the index is the hash result module table size.
The RSS hash result can be retrieved via modify_field API: HASH_RESULT.

Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com>
---
 doc/guides/prog_guide/rte_flow.rst | 20 +++++++
 lib/ethdev/rte_flow.c              | 24 ++++++++
 lib/ethdev/rte_flow.h              | 96 ++++++++++++++++++++++++++++++
 lib/ethdev/rte_flow_driver.h       | 11 ++++
 lib/ethdev/version.map             |  3 +
 5 files changed, 154 insertions(+)
  

Comments

Ori Kam Jan. 8, 2023, 2:22 p.m. UTC | #1
Hi Alexander,

> -----Original Message-----
> From: Alexander Kozyrev <akozyrev@nvidia.com>
> Sent: Wednesday, 14 December 2022 4:21
> 
> Bring more flexibility and control over both flow rule insertion
> and packet matching mechanisms. Introduce 2 new flow table types:
> 
> 1. Allow a user to specify the insertion type used in template tables.
> The insertion type is responsible for choosing the appropriate key
> value used to map inserted flow rules into a template table.
> 
> Flow rules can be inserted by calculating the hash value for
> the pattern or inserted by index via the new create_by_index() API.
> The idea of the index-based insertion is to avoid additional matches
> and simply execute predefined actions after jumping to the index.
> 
> The insertion to an already existing index is undefined and
> depends on PMD implementation. An old rule must be destroyed first.
> The index cannot be bigger than the size of the table.
> 
> 2. Allow a user to specify the hash calculation function used in template
> tables. The hash calculation type is responsible for the calculation of
> the flow rule index a packet would hit upon arrival at the table.
> 
> Control over this is useful for applications with custom RSS algorithms,
> for example. An application can select various packet fields to serve as
> a hash calculation source and jump to the appropriate flow rule location.
> The RSS hash result will be used as the index in the table. For the linear
> hash function, the mapping is one-to-one and the hash result is the index.
> For other hash functions, the index is the hash result module table size.
> The RSS hash result can be retrieved via modify_field API: HASH_RESULT.
> 
> Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com>
> ---

Acked-by: Ori Kam <orika@nvidia.com>
Best,
Ori
  
Thomas Monjalon Jan. 18, 2023, 8:50 a.m. UTC | #2
14/12/2022 03:21, Alexander Kozyrev:
> Bring more flexibility and control over both flow rule insertion
> and packet matching mechanisms. Introduce 2 new flow table types:
> 
> 1. Allow a user to specify the insertion type used in template tables.
> The insertion type is responsible for choosing the appropriate key
> value used to map inserted flow rules into a template table.
> 
> Flow rules can be inserted by calculating the hash value for
> the pattern or inserted by index via the new create_by_index() API.
> The idea of the index-based insertion is to avoid additional matches
> and simply execute predefined actions after jumping to the index.

I don't understand the idea.
Why is it avoiding additional matches?

> The insertion to an already existing index is undefined and

Why is it undefined?

> depends on PMD implementation. An old rule must be destroyed first.
> The index cannot be bigger than the size of the table.
> 
> 2. Allow a user to specify the hash calculation function used in template
> tables. The hash calculation type is responsible for the calculation of
> the flow rule index a packet would hit upon arrival at the table.
> 
> Control over this is useful for applications with custom RSS algorithms,
> for example. An application can select various packet fields to serve as
> a hash calculation source and jump to the appropriate flow rule location.
> The RSS hash result will be used as the index in the table. For the linear
> hash function, the mapping is one-to-one and the hash result is the index.
> For other hash functions, the index is the hash result module table size.
> The RSS hash result can be retrieved via modify_field API: HASH_RESULT.
> 
> Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com>
> ---
>  doc/guides/prog_guide/rte_flow.rst | 20 +++++++
>  lib/ethdev/rte_flow.c              | 24 ++++++++
>  lib/ethdev/rte_flow.h              | 96 ++++++++++++++++++++++++++++++
>  lib/ethdev/rte_flow_driver.h       | 11 ++++
>  lib/ethdev/version.map             |  3 +
>  5 files changed, 154 insertions(+)

Is there a PMD implementation available on the mailing list?
This is required to accept a new API.
  
Alexander Kozyrev Jan. 20, 2023, 11:06 p.m. UTC | #3
> 14/12/2022 03:21, Alexander Kozyrev:
> > Bring more flexibility and control over both flow rule insertion
> > and packet matching mechanisms. Introduce 2 new flow table types:
> >
> > 1. Allow a user to specify the insertion type used in template tables.
> > The insertion type is responsible for choosing the appropriate key
> > value used to map inserted flow rules into a template table.
> >
> > Flow rules can be inserted by calculating the hash value for
> > the pattern or inserted by index via the new create_by_index() API.
> > The idea of the index-based insertion is to avoid additional matches
> > and simply execute predefined actions after jumping to the index.
> 
> I don't understand the idea.
> Why is it avoiding additional matches?

If you jump directly to an index in a table, you don't need to match anything.
For the regular pattern-based table you must calculate the hash and match on it.

> > The insertion to an already existing index is undefined and
> 
> Why is it undefined?

The behavior is up to a PMD to decide. It is free to replace the rule or throw an error.

> > depends on PMD implementation. An old rule must be destroyed first.
> > The index cannot be bigger than the size of the table.
> >
> > 2. Allow a user to specify the hash calculation function used in template
> > tables. The hash calculation type is responsible for the calculation of
> > the flow rule index a packet would hit upon arrival at the table.
> >
> > Control over this is useful for applications with custom RSS algorithms,
> > for example. An application can select various packet fields to serve as
> > a hash calculation source and jump to the appropriate flow rule location.
> > The RSS hash result will be used as the index in the table. For the linear
> > hash function, the mapping is one-to-one and the hash result is the index.
> > For other hash functions, the index is the hash result module table size.
> > The RSS hash result can be retrieved via modify_field API: HASH_RESULT.
> >
> > Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com>
> > ---
> >  doc/guides/prog_guide/rte_flow.rst | 20 +++++++
> >  lib/ethdev/rte_flow.c              | 24 ++++++++
> >  lib/ethdev/rte_flow.h              | 96 ++++++++++++++++++++++++++++++
> >  lib/ethdev/rte_flow_driver.h       | 11 ++++
> >  lib/ethdev/version.map             |  3 +
> >  5 files changed, 154 insertions(+)
> 
> Is there a PMD implementation available on the mailing list?
> This is required to accept a new API.

This is RFC. PMD implementation will follow in a v1 patches shortly.
  
Thomas Monjalon Jan. 22, 2023, 8:55 p.m. UTC | #4
21/01/2023 00:06, Alexander Kozyrev:
> > 14/12/2022 03:21, Alexander Kozyrev:
> > > Bring more flexibility and control over both flow rule insertion
> > > and packet matching mechanisms. Introduce 2 new flow table types:
> > >
> > > 1. Allow a user to specify the insertion type used in template tables.
> > > The insertion type is responsible for choosing the appropriate key
> > > value used to map inserted flow rules into a template table.
> > >
> > > Flow rules can be inserted by calculating the hash value for
> > > the pattern or inserted by index via the new create_by_index() API.
> > > The idea of the index-based insertion is to avoid additional matches
> > > and simply execute predefined actions after jumping to the index.
> > 
> > I don't understand the idea.
> > Why is it avoiding additional matches?
> 
> If you jump directly to an index in a table, you don't need to match anything.
> For the regular pattern-based table you must calculate the hash and match on it.

I don't get it. I think it would be simpler with an example.

> > > The insertion to an already existing index is undefined and
> > 
> > Why is it undefined?
> 
> The behavior is up to a PMD to decide. It is free to replace the rule or throw an error.

An undefined behavior of an API makes applications not portable.
Why not deciding of a behavior? You are not sure what is best?
  
Alexander Kozyrev Jan. 23, 2023, 10:02 p.m. UTC | #5
> 21/01/2023 00:06, Alexander Kozyrev:
> > > 14/12/2022 03:21, Alexander Kozyrev:
> > > > Bring more flexibility and control over both flow rule insertion
> > > > and packet matching mechanisms. Introduce 2 new flow table types:
> > > >
> > > > 1. Allow a user to specify the insertion type used in template tables.
> > > > The insertion type is responsible for choosing the appropriate key
> > > > value used to map inserted flow rules into a template table.
> > > >
> > > > Flow rules can be inserted by calculating the hash value for
> > > > the pattern or inserted by index via the new create_by_index() API.
> > > > The idea of the index-based insertion is to avoid additional matches
> > > > and simply execute predefined actions after jumping to the index.
> > >
> > > I don't understand the idea.
> > > Why is it avoiding additional matches?
> >
> > If you jump directly to an index in a table, you don't need to match
> anything.
> > For the regular pattern-based table you must calculate the hash and match
> on it.
> 
> I don't get it. I think it would be simpler with an example.

This is how the regular pattern-based table works:
you calculate hash on 5-tuple, go to the table entry with the corresponding hash,
check if there is a collision and search for the proper entry if so, execute actions.
Index-based table:
take an index value from a specified field, go to this index, and execute actions.

> > > > The insertion to an already existing index is undefined and
> > >
> > > Why is it undefined?
> >
> > The behavior is up to a PMD to decide. It is free to replace the rule or throw
> an error.
> 
> An undefined behavior of an API makes applications not portable.
> Why not deciding of a behavior? You are not sure what is best?

We don't want to restrict a PMD behavior here. 
Personally, I would prefer throwing an error in this case.
Do you think returning EEXIST is better?
  
Thomas Monjalon Jan. 30, 2023, 2:47 p.m. UTC | #6
23/01/2023 23:02, Alexander Kozyrev:
> > 21/01/2023 00:06, Alexander Kozyrev:
> > > > 14/12/2022 03:21, Alexander Kozyrev:
> > > > > Bring more flexibility and control over both flow rule insertion
> > > > > and packet matching mechanisms. Introduce 2 new flow table types:
> > > > >
> > > > > 1. Allow a user to specify the insertion type used in template tables.
> > > > > The insertion type is responsible for choosing the appropriate key
> > > > > value used to map inserted flow rules into a template table.
> > > > >
> > > > > Flow rules can be inserted by calculating the hash value for
> > > > > the pattern or inserted by index via the new create_by_index() API.
> > > > > The idea of the index-based insertion is to avoid additional matches
> > > > > and simply execute predefined actions after jumping to the index.
> > > >
> > > > I don't understand the idea.
> > > > Why is it avoiding additional matches?
> > >
> > > If you jump directly to an index in a table, you don't need to match
> > anything.
> > > For the regular pattern-based table you must calculate the hash and match
> > on it.
> > 
> > I don't get it. I think it would be simpler with an example.
> 
> This is how the regular pattern-based table works:
> you calculate hash on 5-tuple, go to the table entry with the corresponding hash,
> check if there is a collision and search for the proper entry if so, execute actions.
> Index-based table:
> take an index value from a specified field, go to this index, and execute actions.

OK, please make sure the explanation is clear in the patch.


> > > > > The insertion to an already existing index is undefined and
> > > >
> > > > Why is it undefined?
> > >
> > > The behavior is up to a PMD to decide. It is free to replace the rule or throw
> > an error.
> > 
> > An undefined behavior of an API makes applications not portable.
> > Why not deciding of a behavior? You are not sure what is best?
> 
> We don't want to restrict a PMD behavior here. 
> Personally, I would prefer throwing an error in this case.
> Do you think returning EEXIST is better?

I don't know what is best.
At least you can define an error EEXIST if the PMD forbids it,
and no error if the PMD replaces the rule.
  

Patch

diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 3e6242803d..7d05acb2db 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -3669,6 +3669,26 @@  Enqueueing a flow rule creation operation is similar to simple creation.
 A valid handle in case of success is returned. It must be destroyed later
 by calling ``rte_flow_async_destroy()`` even if the rule is rejected by HW.
 
+Enqueue creation by index operation
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Enqueueing a flow rule creation operation to insert the rule at a table index.
+
+.. code-block:: c
+
+   struct rte_flow *
+   rte_flow_async_create(uint16_t port_id,
+                         uint32_t queue_id,
+                         const struct rte_flow_op_attr *op_attr,
+                         struct rte_flow_template_table *template_table,
+                         uint32_t rule_index,
+                         const struct rte_flow_action actions[],
+                         uint8_t actions_template_index,
+                         void *user_data,
+                         struct rte_flow_error *error);
+
+A valid handle in case of success is returned. It must be destroyed later
+by calling ``rte_flow_async_destroy()`` even if the rule is rejected by HW.
+
 Enqueue destruction operation
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c
index 7d0c24366c..013eb355ca 100644
--- a/lib/ethdev/rte_flow.c
+++ b/lib/ethdev/rte_flow.c
@@ -1765,6 +1765,30 @@  rte_flow_async_create(uint16_t port_id,
 	return flow;
 }
 
+struct rte_flow *
+rte_flow_async_create_by_index(uint16_t port_id,
+			       uint32_t queue_id,
+			       const struct rte_flow_op_attr *op_attr,
+			       struct rte_flow_template_table *template_table,
+			       uint32_t rule_index,
+			       const struct rte_flow_action actions[],
+			       uint8_t actions_template_index,
+			       void *user_data,
+			       struct rte_flow_error *error)
+{
+	struct rte_eth_dev *dev = &rte_eth_devices[port_id];
+	const struct rte_flow_ops *ops = rte_flow_ops_get(port_id, error);
+	struct rte_flow *flow;
+
+	flow = ops->async_create_by_index(dev, queue_id,
+					  op_attr, template_table, rule_index,
+					  actions, actions_template_index,
+					  user_data, error);
+	if (flow == NULL)
+		flow_err(port_id, -rte_errno, error);
+	return flow;
+}
+
 int
 rte_flow_async_destroy(uint16_t port_id,
 		       uint32_t queue_id,
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index b60987db4b..858992893c 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -3528,6 +3528,7 @@  enum rte_flow_field_id {
 	RTE_FLOW_FIELD_IPV6_ECN,	/**< IPv6 ECN. */
 	RTE_FLOW_FIELD_GTP_PSC_QFI,	/**< GTP QFI. */
 	RTE_FLOW_FIELD_METER_COLOR,	/**< Meter color marker. */
+	RTE_FLOW_FIELD_HASH_RESULT,	/**< Hash result. */
 };
 
 /**
@@ -5187,6 +5188,48 @@  rte_flow_actions_template_destroy(uint16_t port_id,
  */
 struct rte_flow_template_table;
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Template table flow rules insertion type.
+ */
+enum rte_flow_table_insertion_type {
+	/**
+	 * Pattern-based insertion.
+	 */
+	RTE_FLOW_TABLE_INSERTION_TYPE_PATTERN,
+	/**
+	 * Index-based insertion.
+	 */
+	RTE_FLOW_TABLE_INSERTION_TYPE_INDEX,
+};
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Template table hash index calculation function.
+ */
+enum rte_flow_table_hash_func {
+	/**
+	 * Default hash calculation.
+	 */
+	RTE_FLOW_TABLE_HASH_FUNC_DEFAULT,
+	/**
+	 * Linear hash calculation.
+	 */
+	RTE_FLOW_TABLE_HASH_FUNC_LINEAR,
+	/*
+	 * 32-bit checksum hash calculation.
+	 */
+	RTE_FLOW_TABLE_HASH_FUNC_CRC32,
+	/*
+	 * 16-bit checksum hash calculation.
+	 */
+	RTE_FLOW_TABLE_HASH_FUNC_CRC16,
+};
+
 /**
  * @warning
  * @b EXPERIMENTAL: this API may change without prior notice.
@@ -5202,6 +5245,14 @@  struct rte_flow_template_table_attr {
 	 * Maximum number of flow rules that this table holds.
 	 */
 	uint32_t nb_flows;
+	/**
+	 * Insertion type for flow rules.
+	 */
+	enum rte_flow_table_insertion_type insertion_type;
+	/**
+	 * Hash calculation function for the packet matching.
+	 */
+	enum rte_flow_table_hash_func hash_func;
 };
 
 /**
@@ -5336,6 +5387,51 @@  rte_flow_async_create(uint16_t port_id,
 		      void *user_data,
 		      struct rte_flow_error *error);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Enqueue rule creation operation.
+ *
+ * @param port_id
+ *   Port identifier of Ethernet device.
+ * @param queue_id
+ *   Flow queue used to insert the rule.
+ * @param[in] op_attr
+ *   Rule creation operation attributes.
+ * @param[in] template_table
+ *   Template table to select templates from.
+ * @param[in] rule_index
+ *   Rule index in the table.
+ *   Inserting a rule to already occupied index results in undefined behavior.
+ * @param[in] actions
+ *   List of actions to be used.
+ *   The list order should match the order in the actions template.
+ * @param[in] actions_template_index
+ *   Actions template index in the table.
+ * @param[in] user_data
+ *   The user data that will be returned on the completion events.
+ * @param[out] error
+ *   Perform verbose error reporting if not NULL.
+ *   PMDs initialize this structure in case of error only.
+ *
+ * @return
+ *   Handle on success, NULL otherwise and rte_errno is set.
+ *   The rule handle doesn't mean that the rule has been populated.
+ *   Only completion result indicates that if there was success or failure.
+ */
+__rte_experimental
+struct rte_flow *
+rte_flow_async_create_by_index(uint16_t port_id,
+			       uint32_t queue_id,
+			       const struct rte_flow_op_attr *op_attr,
+			       struct rte_flow_template_table *template_table,
+			       uint32_t rule_index,
+			       const struct rte_flow_action actions[],
+			       uint8_t actions_template_index,
+			       void *user_data,
+			       struct rte_flow_error *error);
+
 /**
  * @warning
  * @b EXPERIMENTAL: this API may change without prior notice.
diff --git a/lib/ethdev/rte_flow_driver.h b/lib/ethdev/rte_flow_driver.h
index c7d0699c91..b5b597dd28 100644
--- a/lib/ethdev/rte_flow_driver.h
+++ b/lib/ethdev/rte_flow_driver.h
@@ -221,6 +221,17 @@  struct rte_flow_ops {
 		 uint8_t actions_template_index,
 		 void *user_data,
 		 struct rte_flow_error *err);
+	/** See rte_flow_async_create_by_index() */
+	struct rte_flow *(*async_create_by_index)
+		(struct rte_eth_dev *dev,
+		 uint32_t queue_id,
+		 const struct rte_flow_op_attr *op_attr,
+		 struct rte_flow_template_table *template_table,
+		 uint32_t rule_index,
+		 const struct rte_flow_action actions[],
+		 uint8_t actions_template_index,
+		 void *user_data,
+		 struct rte_flow_error *err);
 	/** See rte_flow_async_destroy() */
 	int (*async_destroy)
 		(struct rte_eth_dev *dev,
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index 17201fbe0f..dbc2bffe64 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -298,6 +298,9 @@  EXPERIMENTAL {
 	rte_flow_get_q_aged_flows;
 	rte_mtr_meter_policy_get;
 	rte_mtr_meter_profile_get;
+
+	# added in 23.03
+	rte_flow_async_create_by_index;
 };
 
 INTERNAL {