Hi,

On 13/10/2021 04:25, Alexander Kozyrev wrote:
From: Ivan Malov <ivan.ma...@oktetlabs.ru> On Wednesday, October 6, 2021 13:25
On 06/10/2021 07:48, Alexander Kozyrev wrote:
Treating every single flow rule as a completely independent and separate
entity negatively impacts the flow rules insertion rate. Oftentimes in an
application, many flow rules share a common structure (the same item
mask
and/or action list) so they can be grouped and classified together.
This knowledge may be used as a source of optimization by a PMD/HW.

The item template defines common matching fields (the item mask)
without
values. The action template holds a list of action types that will be used
together in the same rule. The specific values for items and actions will
be given only during the rule creation.

A table combines item and action templates along with shared flow rule
attributes (group ID, priority and traffic direction). This way a PMD/HW
can prepare all the resources needed for efficient flow rules creation in
the datapath. To avoid any hiccups due to memory reallocation, the
maximum
number of flow rules is defined at table creation time.

The flow rule creation is done by selecting a table, an item template
and an action template (which are bound to the table), and setting unique
values for the items and actions.

Signed-off-by: Alexander Kozyrev <akozy...@nvidia.com>
Suggested-by: Ori Kam <or...@nvidia.com>
---
   lib/ethdev/rte_flow.h | 268
++++++++++++++++++++++++++++++++++++++++++
   1 file changed, 268 insertions(+)

diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index c69d503b90..ba3204b17e 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -4358,6 +4358,274 @@ int
   rte_flow_configure(uint16_t port_id,
                   const struct rte_flow_port_attr *port_attr,
                   struct rte_flow_error *error);
+
+/**
+ * Opaque type returned after successfull creation of item template.

Typo: "successfull" --> "successful".
Thanks for noticing, will correct.

+ * This handle can be used to manage the created item template.
+ */
+struct rte_flow_item_template;
+
+__extension__
+struct rte_flow_item_template_attr {
+       /**
+        * Version of the struct layout, should be 0.
+        */
+       uint32_t version;
+       /* No attributes so far. */
+};
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Create item template.
+ * The item template defines common matching fields (item mask)
without values.
+ * For example, matching on 5 tuple TCP flow, the template will be
+ * eth(null) + IPv4(source + dest) + TCP(s_port + d_port),
+ * while values for each rule will be set during the flow rule creation.
+ * The order of items in the template must be the same at rule insertion.
+ *
+ * @param port_id
+ *   Port identifier of Ethernet device.
+ * @param[in] attr
+ *   Item template attributes.

Please consider adding meaningful prefixes to "attr" here and below.
This is needed to avoid confusion with "struct rte_flow_attr".

Example: "template_attr".
No problem.

+ * @param[in] items
+ *   Pattern specification (list terminated by the END pattern item).
+ *   The spec member of an item is not used unless the end member is
used.
+ * @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.
+ */
+__rte_experimental
+struct rte_flow_item_template *
+rte_flow_item_template_create(uint16_t port_id,
+                             const struct rte_flow_item_template_attr *attr,
+                             const struct rte_flow_item items[],
+                             struct rte_flow_error *error);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Destroy item template.
+ * This function may be called only when
+ * there are no more tables referencing this template.
+ *
+ * @param port_id
+ *   Port identifier of Ethernet device.
+ * @param[in] template
+ *   Handle to the template to be destroyed.

Perhaps "handle OF the template"?
You are right.

+ * @param[out] error
+ *   Perform verbose error reporting if not NULL.
+ *   PMDs initialize this structure in case of error only.
+ *
+ * @return
+ *   0 on success, a negative errno value otherwise and rte_errno is set.
+ */
+__rte_experimental
+int
+rte_flow_item_template_destroy(uint16_t port_id,
+                              struct rte_flow_item_template *template,
+                              struct rte_flow_error *error);
+
+/**
+ * Opaque type returned after successfull creation of action template.

Single "l" in "successful".
Ditto.

+ * This handle can be used to manage the created action template.
+ */
+struct rte_flow_action_template;
+
+__extension__
+struct rte_flow_action_template_attr {
+       /**
+        * Version of the struct layout, should be 0.
+        */
+       uint32_t version;
+       /* No attributes so far. */
+};
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Create action template.
+ * The action template holds a list of action types without values.
+ * For example, the template to change TCP ports is TCP(s_port + d_port),
+ * while values for each rule will be set during the flow rule creation.
+ *
+ * The order of the action in the template must be kept when inserting
rules.
+ *
+ * @param port_id
+ *   Port identifier of Ethernet device.
+ * @param[in] attr
+ *   Template attributes.

Perhaps add a meaningful prefix to "attr".
Sure thing, will rename all the "attr" to "thing_attr".

+ * @param[in] actions
+ *   Associated actions (list terminated by the END action).
+ *   The spec member is only used if the mask is 1.

Maybe "its mask is all ones"?
Not necessarily, just non-zero value would do. Will make it clearer.

+ * @param[in] masks
+ *   List of actions that marks which of the action's member is constant.

Consider the following action example:

struct rte_flow_action_vxlan_encap {
          struct rte_flow_item *definition;
};

So, if "definition" is not NULL, the whole header definition is supposed
to be constant, right? Or am I missing something?
If definition has non-zero value then the action spec will be used in every 
rule created with this template.
In this particular example, yes, this definition is going to be a constant 
header for all the rules.


+ *   A mask has the same format as the corresponding action.
+ *   If the action field in @p masks is not 0,
+ *   the corresponding value in an action from @p actions will be the part
+ *   of the template and used in all flow rules.
+ *   The order of actions in @p masks is the same as in @p actions.
+ *   In case of indirect actions present in @p actions,
+ *   the actual action type should be present in @p mask.
+ * @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.
+ */
+__rte_experimental
+struct rte_flow_action_template *
+rte_flow_action_template_create(uint16_t port_id,
+                       const struct rte_flow_action_template_attr *attr,
+                       const struct rte_flow_action actions[],
+                       const struct rte_flow_action masks[],
+                       struct rte_flow_error *error);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Destroy action template.
+ * This function may be called only when
+ * there are no more tables referencing this template.
+ *
+ * @param port_id
+ *   Port identifier of Ethernet device.
+ * @param[in] template
+ *   Handle to the template to be destroyed.
+ * @param[out] error
+ *   Perform verbose error reporting if not NULL.
+ *   PMDs initialize this structure in case of error only.
+ *
+ * @return
+ *   0 on success, a negative errno value otherwise and rte_errno is set.
+ */
+__rte_experimental
+int
+rte_flow_action_template_destroy(uint16_t port_id,
+                       const struct rte_flow_action_template *template,
+                       struct rte_flow_error *error);
+
+
+/**
+ * Opaque type returned after successfull creation of table.

Redundant "l" in "successful".
Consider this fixed.

+ * This handle can be used to manage the created table.
+ */
+struct rte_flow_table;
+
+enum rte_flow_table_mode {
+       /**
+        * Fixed size, the number of flow rules will be limited.
+        * It is possible that some of the rules will not be inserted
+        * due to conflicts/lack of space.
+        * When rule insertion fails with try again error,
+        * the application may use one of the following ways
+        * to address this state:
+        * 1. Keep this rule processing in the software.
+        * 2. Try to offload this rule at a later time,
+        *    after some rules have been removed from the hardware.
+        * 3. Create a new table and add this rule to the new table.
+        */
+       RTE_FLOW_TABLE_MODE_FIXED,
+       /**
+        * Resizable, the PMD/HW will insert all rules.
+        * No try again error will be received in this mode.
+        */
+       RTE_FLOW_TABLE_MODE_RESIZABLE,
+};
+
+/**
+ * Table attributes.
+ */
+struct rte_flow_table_attr {
+       /**
+        * Version of the struct layout, should be 0.
+        */
+       uint32_t version;
+       /**
+        * Flow attributes that will be used in the table.
+        */
+       struct rte_flow_attr attr;

Perhaps, "flow_attr" then?
As we agreed.

+       /**
+        * Maximum number of flow rules that this table holds.
+        * It can be hard or soft limit depending on the mode.
+        */
+       uint32_t max_rules;

How about "nb_flows_max"?
Just nb_flows maybe?


Probabably OK, too, but I don't have a strong opinion.
One more option: "table_size".

--
Ivan M

Reply via email to