[v8,02/11] ethdev: add flow item/action templates
Checks
Commit Message
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 pattern template defines common matching fields (the item mask) without
values. The actions 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 pattern and actions 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 the table creation time.
The flow rule creation is done by selecting a table, a pattern template
and an actions template (which are bound to the table), and setting unique
values for the items and actions.
Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com>
Acked-by: Ori Kam <orika@nvidia.com>
---
doc/guides/prog_guide/rte_flow.rst | 135 ++++++++++++
doc/guides/rel_notes/release_22_03.rst | 8 +
lib/ethdev/rte_flow.c | 252 +++++++++++++++++++++++
lib/ethdev/rte_flow.h | 274 +++++++++++++++++++++++++
lib/ethdev/rte_flow_driver.h | 37 ++++
lib/ethdev/version.map | 6 +
6 files changed, 712 insertions(+)
Comments
On 2/20/22 06:44, 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 pattern template defines common matching fields (the item mask) without
> values. The actions 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 pattern and actions 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 the table creation time.
>
> The flow rule creation is done by selecting a table, a pattern template
> and an actions template (which are bound to the table), and setting unique
> values for the items and actions.
>
> Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com>
> Acked-by: Ori Kam <orika@nvidia.com>
[snip]
> +For example, to create an actions template with the same Mark ID
> +but different Queue Index for every rule:
> +
> +.. code-block:: c
> +
> + rte_flow_actions_template_attr attr = {.ingress = 1};
> + struct rte_flow_action act[] = {
> + /* Mark ID is 4 for every rule, Queue Index is unique */
> + [0] = {.type = RTE_FLOW_ACTION_TYPE_MARK,
> + .conf = &(struct rte_flow_action_mark){.id = 4}},
> + [1] = {.type = RTE_FLOW_ACTION_TYPE_QUEUE},
> + [2] = {.type = RTE_FLOW_ACTION_TYPE_END,},
> + };
> + struct rte_flow_action msk[] = {
> + /* Assign to MARK mask any non-zero value to make it constant */
> + [0] = {.type = RTE_FLOW_ACTION_TYPE_MARK,
> + .conf = &(struct rte_flow_action_mark){.id = 1}},
1 looks very strange. I can understand it in the case of
integer and boolean fields, but what to do in the case of
arrays? IMHO, it would be better to use all 0xff's in value.
Anyway, it must be defined very carefully and non-ambiguous.
> + [1] = {.type = RTE_FLOW_ACTION_TYPE_QUEUE},
> + [2] = {.type = RTE_FLOW_ACTION_TYPE_END,},
> + };
> + struct rte_flow_error err;
> +
> + struct rte_flow_actions_template *actions_template =
> + rte_flow_actions_template_create(port, &attr, &act, &msk, &err);
> +
> +The concrete value for Queue Index will be provided at the rule creation.
[snip]
> diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c
> index ffd48e40d5..e9f684eedb 100644
> --- a/lib/ethdev/rte_flow.c
> +++ b/lib/ethdev/rte_flow.c
> @@ -1461,3 +1461,255 @@ rte_flow_configure(uint16_t port_id,
> RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> NULL, rte_strerror(ENOTSUP));
> }
> +
> +struct rte_flow_pattern_template *
> +rte_flow_pattern_template_create(uint16_t port_id,
> + const struct rte_flow_pattern_template_attr *template_attr,
> + const struct rte_flow_item pattern[],
> + 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_pattern_template *template;
> +
> + if (template_attr == NULL) {
> + RTE_FLOW_LOG(ERR,
> + "Port %"PRIu16" template attr is NULL.\n",
> + port_id);
> + rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_ATTR,
> + NULL, rte_strerror(EINVAL));
> + return NULL;
> + }
> + if (pattern == NULL) {
> + RTE_FLOW_LOG(ERR,
> + "Port %"PRIu16" pattern is NULL.\n",
> + port_id);
> + rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_ATTR,
> + NULL, rte_strerror(EINVAL));
> + return NULL;
> + }
> + if (dev->data->flow_configured == 0) {
> + RTE_FLOW_LOG(INFO,
> + "Flow engine on port_id=%"PRIu16" is not configured.\n",
> + port_id);
> + rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_STATE,
> + NULL, rte_strerror(EINVAL));
> + return NULL;
> + }
> + if (unlikely(!ops))
> + return NULL;
See notes about order of checks in previous patch review notes.
> + if (likely(!!ops->pattern_template_create)) {
> + template = ops->pattern_template_create(dev, template_attr,
> + pattern, error);
> + if (template == NULL)
> + flow_err(port_id, -rte_errno, error);
> + return template;
> + }
> + rte_flow_error_set(error, ENOTSUP,
> + RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> + NULL, rte_strerror(ENOTSUP));
> + return NULL;
> +}
> +
> +int
> +rte_flow_pattern_template_destroy(uint16_t port_id,
> + struct rte_flow_pattern_template *pattern_template,
> + 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);
> +
> + if (unlikely(pattern_template == NULL))
> + return 0;
> + if (unlikely(!ops))
> + return -rte_errno;
Same here. I'm afraid it is really important here as well,
since request should not return OK if port_id is invalid.
> + if (likely(!!ops->pattern_template_destroy)) {
> + return flow_err(port_id,
> + ops->pattern_template_destroy(dev,
> + pattern_template,
> + error),
> + error);
> + }
> + return rte_flow_error_set(error, ENOTSUP,
> + RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> + NULL, rte_strerror(ENOTSUP));
> +}
> +
> +struct rte_flow_actions_template *
> +rte_flow_actions_template_create(uint16_t port_id,
> + const struct rte_flow_actions_template_attr *template_attr,
> + const struct rte_flow_action actions[],
> + const struct rte_flow_action masks[],
> + 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_actions_template *template;
> +
> + if (template_attr == NULL) {
> + RTE_FLOW_LOG(ERR,
> + "Port %"PRIu16" template attr is NULL.\n",
> + port_id);
> + rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_ATTR,
> + NULL, rte_strerror(EINVAL));
> + return NULL;
> + }
> + if (actions == NULL) {
> + RTE_FLOW_LOG(ERR,
> + "Port %"PRIu16" actions is NULL.\n",
> + port_id);
> + rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_ATTR,
> + NULL, rte_strerror(EINVAL));
> + return NULL;
> + }
> + if (masks == NULL) {
> + RTE_FLOW_LOG(ERR,
> + "Port %"PRIu16" masks is NULL.\n",
> + port_id);
> + rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_ATTR,
> + NULL, rte_strerror(EINVAL));
> +
> + }
> + if (dev->data->flow_configured == 0) {
> + RTE_FLOW_LOG(INFO,
> + "Flow engine on port_id=%"PRIu16" is not configured.\n",
> + port_id);
> + rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_STATE,
> + NULL, rte_strerror(EINVAL));
> + return NULL;
> + }
> + if (unlikely(!ops))
> + return NULL;
same here
> + if (likely(!!ops->actions_template_create)) {
> + template = ops->actions_template_create(dev, template_attr,
> + actions, masks, error);
> + if (template == NULL)
> + flow_err(port_id, -rte_errno, error);
> + return template;
> + }
> + rte_flow_error_set(error, ENOTSUP,
> + RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> + NULL, rte_strerror(ENOTSUP));
> + return NULL;
> +}
> +
> +int
> +rte_flow_actions_template_destroy(uint16_t port_id,
> + struct rte_flow_actions_template *actions_template,
> + 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);
> +
> + if (unlikely(actions_template == NULL))
> + return 0;
> + if (unlikely(!ops))
> + return -rte_errno;
same here
> + if (likely(!!ops->actions_template_destroy)) {
> + return flow_err(port_id,
> + ops->actions_template_destroy(dev,
> + actions_template,
> + error),
> + error);
> + }
> + return rte_flow_error_set(error, ENOTSUP,
> + RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> + NULL, rte_strerror(ENOTSUP));
> +}
> +
> +struct rte_flow_template_table *
> +rte_flow_template_table_create(uint16_t port_id,
> + const struct rte_flow_template_table_attr *table_attr,
> + struct rte_flow_pattern_template *pattern_templates[],
> + uint8_t nb_pattern_templates,
> + struct rte_flow_actions_template *actions_templates[],
> + uint8_t nb_actions_templates,
> + 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_template_table *table;
> +
> + if (table_attr == NULL) {
> + RTE_FLOW_LOG(ERR,
> + "Port %"PRIu16" table attr is NULL.\n",
> + port_id);
> + rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_ATTR,
> + NULL, rte_strerror(EINVAL));
> + return NULL;
> + }
> + if (pattern_templates == NULL) {
> + RTE_FLOW_LOG(ERR,
> + "Port %"PRIu16" pattern templates is NULL.\n",
> + port_id);
> + rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_ATTR,
> + NULL, rte_strerror(EINVAL));
> + return NULL;
> + }
> + if (actions_templates == NULL) {
> + RTE_FLOW_LOG(ERR,
> + "Port %"PRIu16" actions templates is NULL.\n",
> + port_id);
> + rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_ATTR,
> + NULL, rte_strerror(EINVAL));
> + return NULL;
> + }
> + if (dev->data->flow_configured == 0) {
> + RTE_FLOW_LOG(INFO,
> + "Flow engine on port_id=%"PRIu16" is not configured.\n",
> + port_id);
> + rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_STATE,
> + NULL, rte_strerror(EINVAL));
> + return NULL;
> + }
> + if (unlikely(!ops))
> + return NULL;
Order of checks
> + if (likely(!!ops->template_table_create)) {
> + table = ops->template_table_create(dev, table_attr,
> + pattern_templates, nb_pattern_templates,
> + actions_templates, nb_actions_templates,
> + error);
> + if (table == NULL)
> + flow_err(port_id, -rte_errno, error);
> + return table;
> + }
> + rte_flow_error_set(error, ENOTSUP,
> + RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> + NULL, rte_strerror(ENOTSUP));
> + return NULL;
> +}
> +
> +int
> +rte_flow_template_table_destroy(uint16_t port_id,
> + struct rte_flow_template_table *template_table,
> + 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);
> +
> + if (unlikely(template_table == NULL))
> + return 0;
> + if (unlikely(!ops))
> + return -rte_errno;
> + if (likely(!!ops->template_table_destroy)) {
> + return flow_err(port_id,
> + ops->template_table_destroy(dev,
> + template_table,
> + error),
> + error);
> + }
> + return rte_flow_error_set(error, ENOTSUP,
> + RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> + NULL, rte_strerror(ENOTSUP));
> +}
> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> index cdb7b2be68..776e8ccc11 100644
> --- a/lib/ethdev/rte_flow.h
> +++ b/lib/ethdev/rte_flow.h
> @@ -4983,6 +4983,280 @@ rte_flow_configure(uint16_t port_id,
> const struct rte_flow_port_attr *port_attr,
> struct rte_flow_error *error);
>
> +/**
> + * Opaque type returned after successful creation of pattern template.
> + * This handle can be used to manage the created pattern template.
> + */
> +struct rte_flow_pattern_template;
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Flow pattern template attributes.
Would it be useful to mentioned that at least one direction
bit must be set? Otherwise request does not make sense.
> + */
> +__extension__
> +struct rte_flow_pattern_template_attr {
> + /**
> + * Relaxed matching policy.
> + * - PMD may match only on items with mask member set and skip
> + * matching on protocol layers specified without any masks.
> + * - If not set, PMD will match on protocol layers
> + * specified without any masks as well.
> + * - Packet data must be stacked in the same order as the
> + * protocol layers to match inside packets, starting from the lowest.
> + */
> + uint32_t relaxed_matching:1;
I should notice this earlier, but it looks like a new feature
which sounds unrelated to templates. If so, it makes asymmetry
in sync and async flow rules capabilities.
Am I missing something?
Anyway, the feature looks hidden in the patch.
> + /** Pattern valid for rules applied to ingress traffic. */
> + uint32_t ingress:1;
> + /** Pattern valid for rules applied to egress traffic. */
> + uint32_t egress:1;
> + /** Pattern valid for rules applied to transfer traffic. */
> + uint32_t transfer:1;
> +};
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Create flow pattern template.
> + *
> + * The pattern template defines common matching fields 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 number and order of items in the template must be the same
> + * at the rule creation.
> + *
> + * @param port_id
> + * Port identifier of Ethernet device.
> + * @param[in] template_attr
> + * Pattern template attributes.
> + * @param[in] pattern
> + * 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.
Don't we want to be explicit about used negative error code?
The question is applicable to all functions.
[snip]
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Flow actions template attributes.
Same question about no directions specified.
> + */
> +__extension__
> +struct rte_flow_actions_template_attr {
> + /** Action valid for rules applied to ingress traffic. */
> + uint32_t ingress:1;
> + /** Action valid for rules applied to egress traffic. */
> + uint32_t egress:1;
> + /** Action valid for rules applied to transfer traffic. */
> + uint32_t transfer:1;
> +};
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Create flow actions template.
> + *
> + * The actions 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 number and order of actions in the template must be the same
> + * at the rule creation.
> + *
> + * @param port_id
> + * Port identifier of Ethernet device.
> + * @param[in] template_attr
> + * Template attributes.
> + * @param[in] actions
> + * Associated actions (list terminated by the END action).
> + * The spec member is only used if @p masks spec is non-zero.
> + * @param[in] masks
> + * List of actions that marks which of the action's member is constant.
> + * A mask has the same format as the corresponding action.
> + * If the action field in @p masks is not 0,
Comparison with zero makes sense for integers only.
> + * 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_actions_template *
> +rte_flow_actions_template_create(uint16_t port_id,
> + const struct rte_flow_actions_template_attr *template_attr,
> + const struct rte_flow_action actions[],
> + const struct rte_flow_action masks[],
> + struct rte_flow_error *error);
[snip]
Hi Andrew,
> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Monday, February 21, 2022 12:57 PM
> Subject: Re: [PATCH v8 02/11] ethdev: add flow item/action templates
>
> On 2/20/22 06:44, 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 pattern template defines common matching fields (the item mask) without
> > values. The actions 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 pattern and actions 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 the table creation time.
> >
> > The flow rule creation is done by selecting a table, a pattern template
> > and an actions template (which are bound to the table), and setting unique
> > values for the items and actions.
> >
> > Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com>
> > Acked-by: Ori Kam <orika@nvidia.com>
>
> [snip]
>
> > +For example, to create an actions template with the same Mark ID
> > +but different Queue Index for every rule:
> > +
> > +.. code-block:: c
> > +
> > + rte_flow_actions_template_attr attr = {.ingress = 1};
> > + struct rte_flow_action act[] = {
> > + /* Mark ID is 4 for every rule, Queue Index is unique */
> > + [0] = {.type = RTE_FLOW_ACTION_TYPE_MARK,
> > + .conf = &(struct rte_flow_action_mark){.id = 4}},
> > + [1] = {.type = RTE_FLOW_ACTION_TYPE_QUEUE},
> > + [2] = {.type = RTE_FLOW_ACTION_TYPE_END,},
> > + };
> > + struct rte_flow_action msk[] = {
> > + /* Assign to MARK mask any non-zero value to make it constant */
> > + [0] = {.type = RTE_FLOW_ACTION_TYPE_MARK,
> > + .conf = &(struct rte_flow_action_mark){.id = 1}},
>
> 1 looks very strange. I can understand it in the case of
> integer and boolean fields, but what to do in the case of
> arrays? IMHO, it would be better to use all 0xff's in value.
> Anyway, it must be defined very carefully and non-ambiguous.
>
There is some issues with all 0xff for example in case of pointers or
enums this it will result in invalid value.
So I vote for saving it as is.
I fully agree that it should be defined very clearly.
I think that for arrays with predefined size (I don't think we have such in rte_flow)
it should be declared that that the first element should not be 0.
> > + [1] = {.type = RTE_FLOW_ACTION_TYPE_QUEUE},
> > + [2] = {.type = RTE_FLOW_ACTION_TYPE_END,},
> > + };
> > + struct rte_flow_error err;
> > +
> > + struct rte_flow_actions_template *actions_template =
> > + rte_flow_actions_template_create(port, &attr, &act, &msk, &err);
> > +
> > +The concrete value for Queue Index will be provided at the rule creation.
>
> [snip]
>
> > diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c
> > index ffd48e40d5..e9f684eedb 100644
> > --- a/lib/ethdev/rte_flow.c
> > +++ b/lib/ethdev/rte_flow.c
> > @@ -1461,3 +1461,255 @@ rte_flow_configure(uint16_t port_id,
> > RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> > NULL, rte_strerror(ENOTSUP));
> > }
> > +
> > +struct rte_flow_pattern_template *
> > +rte_flow_pattern_template_create(uint16_t port_id,
> > + const struct rte_flow_pattern_template_attr *template_attr,
> > + const struct rte_flow_item pattern[],
> > + 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_pattern_template *template;
> > +
> > + if (template_attr == NULL) {
> > + RTE_FLOW_LOG(ERR,
> > + "Port %"PRIu16" template attr is NULL.\n",
> > + port_id);
> > + rte_flow_error_set(error, EINVAL,
> > + RTE_FLOW_ERROR_TYPE_ATTR,
> > + NULL, rte_strerror(EINVAL));
> > + return NULL;
> > + }
> > + if (pattern == NULL) {
> > + RTE_FLOW_LOG(ERR,
> > + "Port %"PRIu16" pattern is NULL.\n",
> > + port_id);
> > + rte_flow_error_set(error, EINVAL,
> > + RTE_FLOW_ERROR_TYPE_ATTR,
> > + NULL, rte_strerror(EINVAL));
> > + return NULL;
> > + }
> > + if (dev->data->flow_configured == 0) {
> > + RTE_FLOW_LOG(INFO,
> > + "Flow engine on port_id=%"PRIu16" is not configured.\n",
> > + port_id);
> > + rte_flow_error_set(error, EINVAL,
> > + RTE_FLOW_ERROR_TYPE_STATE,
> > + NULL, rte_strerror(EINVAL));
> > + return NULL;
> > + }
> > + if (unlikely(!ops))
> > + return NULL;
>
> See notes about order of checks in previous patch review notes.
>
> > + if (likely(!!ops->pattern_template_create)) {
> > + template = ops->pattern_template_create(dev, template_attr,
> > + pattern, error);
> > + if (template == NULL)
> > + flow_err(port_id, -rte_errno, error);
> > + return template;
> > + }
> > + rte_flow_error_set(error, ENOTSUP,
> > + RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> > + NULL, rte_strerror(ENOTSUP));
> > + return NULL;
> > +}
> > +
> > +int
> > +rte_flow_pattern_template_destroy(uint16_t port_id,
> > + struct rte_flow_pattern_template *pattern_template,
> > + 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);
> > +
> > + if (unlikely(pattern_template == NULL))
> > + return 0;
> > + if (unlikely(!ops))
> > + return -rte_errno;
>
> Same here. I'm afraid it is really important here as well,
> since request should not return OK if port_id is invalid.
>
>
> > + if (likely(!!ops->pattern_template_destroy)) {
> > + return flow_err(port_id,
> > + ops->pattern_template_destroy(dev,
> > + pattern_template,
> > + error),
> > + error);
> > + }
> > + return rte_flow_error_set(error, ENOTSUP,
> > + RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> > + NULL, rte_strerror(ENOTSUP));
> > +}
> > +
> > +struct rte_flow_actions_template *
> > +rte_flow_actions_template_create(uint16_t port_id,
> > + const struct rte_flow_actions_template_attr *template_attr,
> > + const struct rte_flow_action actions[],
> > + const struct rte_flow_action masks[],
> > + 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_actions_template *template;
> > +
> > + if (template_attr == NULL) {
> > + RTE_FLOW_LOG(ERR,
> > + "Port %"PRIu16" template attr is NULL.\n",
> > + port_id);
> > + rte_flow_error_set(error, EINVAL,
> > + RTE_FLOW_ERROR_TYPE_ATTR,
> > + NULL, rte_strerror(EINVAL));
> > + return NULL;
> > + }
> > + if (actions == NULL) {
> > + RTE_FLOW_LOG(ERR,
> > + "Port %"PRIu16" actions is NULL.\n",
> > + port_id);
> > + rte_flow_error_set(error, EINVAL,
> > + RTE_FLOW_ERROR_TYPE_ATTR,
> > + NULL, rte_strerror(EINVAL));
> > + return NULL;
> > + }
> > + if (masks == NULL) {
> > + RTE_FLOW_LOG(ERR,
> > + "Port %"PRIu16" masks is NULL.\n",
> > + port_id);
> > + rte_flow_error_set(error, EINVAL,
> > + RTE_FLOW_ERROR_TYPE_ATTR,
> > + NULL, rte_strerror(EINVAL));
> > +
> > + }
> > + if (dev->data->flow_configured == 0) {
> > + RTE_FLOW_LOG(INFO,
> > + "Flow engine on port_id=%"PRIu16" is not configured.\n",
> > + port_id);
> > + rte_flow_error_set(error, EINVAL,
> > + RTE_FLOW_ERROR_TYPE_STATE,
> > + NULL, rte_strerror(EINVAL));
> > + return NULL;
> > + }
> > + if (unlikely(!ops))
> > + return NULL;
>
> same here
>
> > + if (likely(!!ops->actions_template_create)) {
> > + template = ops->actions_template_create(dev, template_attr,
> > + actions, masks, error);
> > + if (template == NULL)
> > + flow_err(port_id, -rte_errno, error);
> > + return template;
> > + }
> > + rte_flow_error_set(error, ENOTSUP,
> > + RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> > + NULL, rte_strerror(ENOTSUP));
> > + return NULL;
> > +}
> > +
> > +int
> > +rte_flow_actions_template_destroy(uint16_t port_id,
> > + struct rte_flow_actions_template *actions_template,
> > + 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);
> > +
> > + if (unlikely(actions_template == NULL))
> > + return 0;
> > + if (unlikely(!ops))
> > + return -rte_errno;
>
> same here
>
> > + if (likely(!!ops->actions_template_destroy)) {
> > + return flow_err(port_id,
> > + ops->actions_template_destroy(dev,
> > + actions_template,
> > + error),
> > + error);
> > + }
> > + return rte_flow_error_set(error, ENOTSUP,
> > + RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> > + NULL, rte_strerror(ENOTSUP));
> > +}
> > +
> > +struct rte_flow_template_table *
> > +rte_flow_template_table_create(uint16_t port_id,
> > + const struct rte_flow_template_table_attr *table_attr,
> > + struct rte_flow_pattern_template *pattern_templates[],
> > + uint8_t nb_pattern_templates,
> > + struct rte_flow_actions_template *actions_templates[],
> > + uint8_t nb_actions_templates,
> > + 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_template_table *table;
> > +
> > + if (table_attr == NULL) {
> > + RTE_FLOW_LOG(ERR,
> > + "Port %"PRIu16" table attr is NULL.\n",
> > + port_id);
> > + rte_flow_error_set(error, EINVAL,
> > + RTE_FLOW_ERROR_TYPE_ATTR,
> > + NULL, rte_strerror(EINVAL));
> > + return NULL;
> > + }
> > + if (pattern_templates == NULL) {
> > + RTE_FLOW_LOG(ERR,
> > + "Port %"PRIu16" pattern templates is NULL.\n",
> > + port_id);
> > + rte_flow_error_set(error, EINVAL,
> > + RTE_FLOW_ERROR_TYPE_ATTR,
> > + NULL, rte_strerror(EINVAL));
> > + return NULL;
> > + }
> > + if (actions_templates == NULL) {
> > + RTE_FLOW_LOG(ERR,
> > + "Port %"PRIu16" actions templates is NULL.\n",
> > + port_id);
> > + rte_flow_error_set(error, EINVAL,
> > + RTE_FLOW_ERROR_TYPE_ATTR,
> > + NULL, rte_strerror(EINVAL));
> > + return NULL;
> > + }
> > + if (dev->data->flow_configured == 0) {
> > + RTE_FLOW_LOG(INFO,
> > + "Flow engine on port_id=%"PRIu16" is not configured.\n",
> > + port_id);
> > + rte_flow_error_set(error, EINVAL,
> > + RTE_FLOW_ERROR_TYPE_STATE,
> > + NULL, rte_strerror(EINVAL));
> > + return NULL;
> > + }
> > + if (unlikely(!ops))
> > + return NULL;
>
> Order of checks
>
> > + if (likely(!!ops->template_table_create)) {
> > + table = ops->template_table_create(dev, table_attr,
> > + pattern_templates, nb_pattern_templates,
> > + actions_templates, nb_actions_templates,
> > + error);
> > + if (table == NULL)
> > + flow_err(port_id, -rte_errno, error);
> > + return table;
> > + }
> > + rte_flow_error_set(error, ENOTSUP,
> > + RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> > + NULL, rte_strerror(ENOTSUP));
> > + return NULL;
> > +}
> > +
> > +int
> > +rte_flow_template_table_destroy(uint16_t port_id,
> > + struct rte_flow_template_table *template_table,
> > + 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);
> > +
> > + if (unlikely(template_table == NULL))
> > + return 0;
> > + if (unlikely(!ops))
> > + return -rte_errno;
> > + if (likely(!!ops->template_table_destroy)) {
> > + return flow_err(port_id,
> > + ops->template_table_destroy(dev,
> > + template_table,
> > + error),
> > + error);
> > + }
> > + return rte_flow_error_set(error, ENOTSUP,
> > + RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> > + NULL, rte_strerror(ENOTSUP));
> > +}
> > diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> > index cdb7b2be68..776e8ccc11 100644
> > --- a/lib/ethdev/rte_flow.h
> > +++ b/lib/ethdev/rte_flow.h
> > @@ -4983,6 +4983,280 @@ rte_flow_configure(uint16_t port_id,
> > const struct rte_flow_port_attr *port_attr,
> > struct rte_flow_error *error);
> >
> > +/**
> > + * Opaque type returned after successful creation of pattern template.
> > + * This handle can be used to manage the created pattern template.
> > + */
> > +struct rte_flow_pattern_template;
> > +
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice.
> > + *
> > + * Flow pattern template attributes.
>
> Would it be useful to mentioned that at least one direction
> bit must be set? Otherwise request does not make sense.
>
Agree one direction must be set.
> > + */
> > +__extension__
> > +struct rte_flow_pattern_template_attr {
> > + /**
> > + * Relaxed matching policy.
> > + * - PMD may match only on items with mask member set and skip
> > + * matching on protocol layers specified without any masks.
> > + * - If not set, PMD will match on protocol layers
> > + * specified without any masks as well.
> > + * - Packet data must be stacked in the same order as the
> > + * protocol layers to match inside packets, starting from the lowest.
> > + */
> > + uint32_t relaxed_matching:1;
>
> I should notice this earlier, but it looks like a new feature
> which sounds unrelated to templates. If so, it makes asymmetry
> in sync and async flow rules capabilities.
> Am I missing something?
>
> Anyway, the feature looks hidden in the patch.
>
No this is not hidden feature.
In current API application must specify all the preciding items,
For example application wants to match on udp source port.
The rte flow will look something like eth / ipv4/ udp sport = xxx ..
When PMD gets this pattern it must enforce the after the eth
there will be IPv4 and then UDP and then add the match for the
sport.
This means that the PMD addes extra matching.
If the application already validated that there is udp in the packet
in group 0 and then jump to group 1 it can save the HW those extra matching
by enabling this bit which means that the HW should only match on implicit
masked fields.
> > + /** Pattern valid for rules applied to ingress traffic. */
> > + uint32_t ingress:1;
> > + /** Pattern valid for rules applied to egress traffic. */
> > + uint32_t egress:1;
> > + /** Pattern valid for rules applied to transfer traffic. */
> > + uint32_t transfer:1;
> > +};
> > +
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice.
> > + *
> > + * Create flow pattern template.
> > + *
> > + * The pattern template defines common matching fields 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 number and order of items in the template must be the same
> > + * at the rule creation.
> > + *
> > + * @param port_id
> > + * Port identifier of Ethernet device.
> > + * @param[in] template_attr
> > + * Pattern template attributes.
> > + * @param[in] pattern
> > + * 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.
>
> Don't we want to be explicit about used negative error code?
> The question is applicable to all functions.
>
Same answer as given in other patch.
Since PMD may have different/extra error codes I don't think we should
give them here.
> [snip]
>
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice.
> > + *
> > + * Flow actions template attributes.
>
> Same question about no directions specified.
>
> > + */
> > +__extension__
> > +struct rte_flow_actions_template_attr {
> > + /** Action valid for rules applied to ingress traffic. */
> > + uint32_t ingress:1;
> > + /** Action valid for rules applied to egress traffic. */
> > + uint32_t egress:1;
> > + /** Action valid for rules applied to transfer traffic. */
> > + uint32_t transfer:1;
> > +};
> > +
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice.
> > + *
> > + * Create flow actions template.
> > + *
> > + * The actions 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 number and order of actions in the template must be the same
> > + * at the rule creation.
> > + *
> > + * @param port_id
> > + * Port identifier of Ethernet device.
> > + * @param[in] template_attr
> > + * Template attributes.
> > + * @param[in] actions
> > + * Associated actions (list terminated by the END action).
> > + * The spec member is only used if @p masks spec is non-zero.
> > + * @param[in] masks
> > + * List of actions that marks which of the action's member is constant.
> > + * A mask has the same format as the corresponding action.
> > + * If the action field in @p masks is not 0,
>
> Comparison with zero makes sense for integers only.
>
Why? It can also be with pointers enums.
> > + * 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_actions_template *
> > +rte_flow_actions_template_create(uint16_t port_id,
> > + const struct rte_flow_actions_template_attr *template_attr,
> > + const struct rte_flow_action actions[],
> > + const struct rte_flow_action masks[],
> > + struct rte_flow_error *error);
>
> [snip]
Best,
Ori
On 2/21/22 16:12, Ori Kam wrote:
> Hi Andrew,
>
>> -----Original Message-----
>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>> Sent: Monday, February 21, 2022 12:57 PM
>> Subject: Re: [PATCH v8 02/11] ethdev: add flow item/action templates
>>
>> On 2/20/22 06:44, 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 pattern template defines common matching fields (the item mask) without
>>> values. The actions 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 pattern and actions 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 the table creation time.
>>>
>>> The flow rule creation is done by selecting a table, a pattern template
>>> and an actions template (which are bound to the table), and setting unique
>>> values for the items and actions.
>>>
>>> Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com>
>>> Acked-by: Ori Kam <orika@nvidia.com>
>>
>> [snip]
>>
>>> +For example, to create an actions template with the same Mark ID
>>> +but different Queue Index for every rule:
>>> +
>>> +.. code-block:: c
>>> +
>>> + rte_flow_actions_template_attr attr = {.ingress = 1};
>>> + struct rte_flow_action act[] = {
>>> + /* Mark ID is 4 for every rule, Queue Index is unique */
>>> + [0] = {.type = RTE_FLOW_ACTION_TYPE_MARK,
>>> + .conf = &(struct rte_flow_action_mark){.id = 4}},
>>> + [1] = {.type = RTE_FLOW_ACTION_TYPE_QUEUE},
>>> + [2] = {.type = RTE_FLOW_ACTION_TYPE_END,},
>>> + };
>>> + struct rte_flow_action msk[] = {
>>> + /* Assign to MARK mask any non-zero value to make it constant */
>>> + [0] = {.type = RTE_FLOW_ACTION_TYPE_MARK,
>>> + .conf = &(struct rte_flow_action_mark){.id = 1}},
>>
>> 1 looks very strange. I can understand it in the case of
>> integer and boolean fields, but what to do in the case of
>> arrays? IMHO, it would be better to use all 0xff's in value.
>> Anyway, it must be defined very carefully and non-ambiguous.
>>
> There is some issues with all 0xff for example in case of pointers or
> enums this it will result in invalid value.
> So I vote for saving it as is.
> I fully agree that it should be defined very clearly.
> I think that for arrays with predefined size (I don't think we have such in rte_flow)
> it should be declared that that the first element should not be 0.
It is good that we agree that the aspect should be documented
very carefully. Let's do it.
>
>>> + [1] = {.type = RTE_FLOW_ACTION_TYPE_QUEUE},
>>> + [2] = {.type = RTE_FLOW_ACTION_TYPE_END,},
>>> + };
>>> + struct rte_flow_error err;
>>> +
>>> + struct rte_flow_actions_template *actions_template =
>>> + rte_flow_actions_template_create(port, &attr, &act, &msk, &err);
>>> +
>>> +The concrete value for Queue Index will be provided at the rule creation.
>>
>> [snip]
>>
>>> diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c
>>> index ffd48e40d5..e9f684eedb 100644
>>> --- a/lib/ethdev/rte_flow.c
>>> +++ b/lib/ethdev/rte_flow.c
>>> @@ -1461,3 +1461,255 @@ rte_flow_configure(uint16_t port_id,
>>> RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
>>> NULL, rte_strerror(ENOTSUP));
>>> }
>>> +
>>> +struct rte_flow_pattern_template *
>>> +rte_flow_pattern_template_create(uint16_t port_id,
>>> + const struct rte_flow_pattern_template_attr *template_attr,
>>> + const struct rte_flow_item pattern[],
>>> + 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_pattern_template *template;
>>> +
>>> + if (template_attr == NULL) {
>>> + RTE_FLOW_LOG(ERR,
>>> + "Port %"PRIu16" template attr is NULL.\n",
>>> + port_id);
>>> + rte_flow_error_set(error, EINVAL,
>>> + RTE_FLOW_ERROR_TYPE_ATTR,
>>> + NULL, rte_strerror(EINVAL));
>>> + return NULL;
>>> + }
>>> + if (pattern == NULL) {
>>> + RTE_FLOW_LOG(ERR,
>>> + "Port %"PRIu16" pattern is NULL.\n",
>>> + port_id);
>>> + rte_flow_error_set(error, EINVAL,
>>> + RTE_FLOW_ERROR_TYPE_ATTR,
>>> + NULL, rte_strerror(EINVAL));
>>> + return NULL;
>>> + }
>>> + if (dev->data->flow_configured == 0) {
>>> + RTE_FLOW_LOG(INFO,
>>> + "Flow engine on port_id=%"PRIu16" is not configured.\n",
>>> + port_id);
>>> + rte_flow_error_set(error, EINVAL,
>>> + RTE_FLOW_ERROR_TYPE_STATE,
>>> + NULL, rte_strerror(EINVAL));
>>> + return NULL;
>>> + }
>>> + if (unlikely(!ops))
>>> + return NULL;
>>
>> See notes about order of checks in previous patch review notes.
>>
>>> + if (likely(!!ops->pattern_template_create)) {
>>> + template = ops->pattern_template_create(dev, template_attr,
>>> + pattern, error);
>>> + if (template == NULL)
>>> + flow_err(port_id, -rte_errno, error);
>>> + return template;
>>> + }
>>> + rte_flow_error_set(error, ENOTSUP,
>>> + RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
>>> + NULL, rte_strerror(ENOTSUP));
>>> + return NULL;
>>> +}
>>> +
>>> +int
>>> +rte_flow_pattern_template_destroy(uint16_t port_id,
>>> + struct rte_flow_pattern_template *pattern_template,
>>> + 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);
>>> +
>>> + if (unlikely(pattern_template == NULL))
>>> + return 0;
>>> + if (unlikely(!ops))
>>> + return -rte_errno;
>>
>> Same here. I'm afraid it is really important here as well,
>> since request should not return OK if port_id is invalid.
>>
>>
>>> + if (likely(!!ops->pattern_template_destroy)) {
>>> + return flow_err(port_id,
>>> + ops->pattern_template_destroy(dev,
>>> + pattern_template,
>>> + error),
>>> + error);
>>> + }
>>> + return rte_flow_error_set(error, ENOTSUP,
>>> + RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
>>> + NULL, rte_strerror(ENOTSUP));
>>> +}
>>> +
>>> +struct rte_flow_actions_template *
>>> +rte_flow_actions_template_create(uint16_t port_id,
>>> + const struct rte_flow_actions_template_attr *template_attr,
>>> + const struct rte_flow_action actions[],
>>> + const struct rte_flow_action masks[],
>>> + 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_actions_template *template;
>>> +
>>> + if (template_attr == NULL) {
>>> + RTE_FLOW_LOG(ERR,
>>> + "Port %"PRIu16" template attr is NULL.\n",
>>> + port_id);
>>> + rte_flow_error_set(error, EINVAL,
>>> + RTE_FLOW_ERROR_TYPE_ATTR,
>>> + NULL, rte_strerror(EINVAL));
>>> + return NULL;
>>> + }
>>> + if (actions == NULL) {
>>> + RTE_FLOW_LOG(ERR,
>>> + "Port %"PRIu16" actions is NULL.\n",
>>> + port_id);
>>> + rte_flow_error_set(error, EINVAL,
>>> + RTE_FLOW_ERROR_TYPE_ATTR,
>>> + NULL, rte_strerror(EINVAL));
>>> + return NULL;
>>> + }
>>> + if (masks == NULL) {
>>> + RTE_FLOW_LOG(ERR,
>>> + "Port %"PRIu16" masks is NULL.\n",
>>> + port_id);
>>> + rte_flow_error_set(error, EINVAL,
>>> + RTE_FLOW_ERROR_TYPE_ATTR,
>>> + NULL, rte_strerror(EINVAL));
>>> +
>>> + }
>>> + if (dev->data->flow_configured == 0) {
>>> + RTE_FLOW_LOG(INFO,
>>> + "Flow engine on port_id=%"PRIu16" is not configured.\n",
>>> + port_id);
>>> + rte_flow_error_set(error, EINVAL,
>>> + RTE_FLOW_ERROR_TYPE_STATE,
>>> + NULL, rte_strerror(EINVAL));
>>> + return NULL;
>>> + }
>>> + if (unlikely(!ops))
>>> + return NULL;
>>
>> same here
>>
>>> + if (likely(!!ops->actions_template_create)) {
>>> + template = ops->actions_template_create(dev, template_attr,
>>> + actions, masks, error);
>>> + if (template == NULL)
>>> + flow_err(port_id, -rte_errno, error);
>>> + return template;
>>> + }
>>> + rte_flow_error_set(error, ENOTSUP,
>>> + RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
>>> + NULL, rte_strerror(ENOTSUP));
>>> + return NULL;
>>> +}
>>> +
>>> +int
>>> +rte_flow_actions_template_destroy(uint16_t port_id,
>>> + struct rte_flow_actions_template *actions_template,
>>> + 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);
>>> +
>>> + if (unlikely(actions_template == NULL))
>>> + return 0;
>>> + if (unlikely(!ops))
>>> + return -rte_errno;
>>
>> same here
>>
>>> + if (likely(!!ops->actions_template_destroy)) {
>>> + return flow_err(port_id,
>>> + ops->actions_template_destroy(dev,
>>> + actions_template,
>>> + error),
>>> + error);
>>> + }
>>> + return rte_flow_error_set(error, ENOTSUP,
>>> + RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
>>> + NULL, rte_strerror(ENOTSUP));
>>> +}
>>> +
>>> +struct rte_flow_template_table *
>>> +rte_flow_template_table_create(uint16_t port_id,
>>> + const struct rte_flow_template_table_attr *table_attr,
>>> + struct rte_flow_pattern_template *pattern_templates[],
>>> + uint8_t nb_pattern_templates,
>>> + struct rte_flow_actions_template *actions_templates[],
>>> + uint8_t nb_actions_templates,
>>> + 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_template_table *table;
>>> +
>>> + if (table_attr == NULL) {
>>> + RTE_FLOW_LOG(ERR,
>>> + "Port %"PRIu16" table attr is NULL.\n",
>>> + port_id);
>>> + rte_flow_error_set(error, EINVAL,
>>> + RTE_FLOW_ERROR_TYPE_ATTR,
>>> + NULL, rte_strerror(EINVAL));
>>> + return NULL;
>>> + }
>>> + if (pattern_templates == NULL) {
>>> + RTE_FLOW_LOG(ERR,
>>> + "Port %"PRIu16" pattern templates is NULL.\n",
>>> + port_id);
>>> + rte_flow_error_set(error, EINVAL,
>>> + RTE_FLOW_ERROR_TYPE_ATTR,
>>> + NULL, rte_strerror(EINVAL));
>>> + return NULL;
>>> + }
>>> + if (actions_templates == NULL) {
>>> + RTE_FLOW_LOG(ERR,
>>> + "Port %"PRIu16" actions templates is NULL.\n",
>>> + port_id);
>>> + rte_flow_error_set(error, EINVAL,
>>> + RTE_FLOW_ERROR_TYPE_ATTR,
>>> + NULL, rte_strerror(EINVAL));
>>> + return NULL;
>>> + }
>>> + if (dev->data->flow_configured == 0) {
>>> + RTE_FLOW_LOG(INFO,
>>> + "Flow engine on port_id=%"PRIu16" is not configured.\n",
>>> + port_id);
>>> + rte_flow_error_set(error, EINVAL,
>>> + RTE_FLOW_ERROR_TYPE_STATE,
>>> + NULL, rte_strerror(EINVAL));
>>> + return NULL;
>>> + }
>>> + if (unlikely(!ops))
>>> + return NULL;
>>
>> Order of checks
>>
>>> + if (likely(!!ops->template_table_create)) {
>>> + table = ops->template_table_create(dev, table_attr,
>>> + pattern_templates, nb_pattern_templates,
>>> + actions_templates, nb_actions_templates,
>>> + error);
>>> + if (table == NULL)
>>> + flow_err(port_id, -rte_errno, error);
>>> + return table;
>>> + }
>>> + rte_flow_error_set(error, ENOTSUP,
>>> + RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
>>> + NULL, rte_strerror(ENOTSUP));
>>> + return NULL;
>>> +}
>>> +
>>> +int
>>> +rte_flow_template_table_destroy(uint16_t port_id,
>>> + struct rte_flow_template_table *template_table,
>>> + 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);
>>> +
>>> + if (unlikely(template_table == NULL))
>>> + return 0;
>>> + if (unlikely(!ops))
>>> + return -rte_errno;
>>> + if (likely(!!ops->template_table_destroy)) {
>>> + return flow_err(port_id,
>>> + ops->template_table_destroy(dev,
>>> + template_table,
>>> + error),
>>> + error);
>>> + }
>>> + return rte_flow_error_set(error, ENOTSUP,
>>> + RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
>>> + NULL, rte_strerror(ENOTSUP));
>>> +}
>>> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
>>> index cdb7b2be68..776e8ccc11 100644
>>> --- a/lib/ethdev/rte_flow.h
>>> +++ b/lib/ethdev/rte_flow.h
>>> @@ -4983,6 +4983,280 @@ rte_flow_configure(uint16_t port_id,
>>> const struct rte_flow_port_attr *port_attr,
>>> struct rte_flow_error *error);
>>>
>>> +/**
>>> + * Opaque type returned after successful creation of pattern template.
>>> + * This handle can be used to manage the created pattern template.
>>> + */
>>> +struct rte_flow_pattern_template;
>>> +
>>> +/**
>>> + * @warning
>>> + * @b EXPERIMENTAL: this API may change without prior notice.
>>> + *
>>> + * Flow pattern template attributes.
>>
>> Would it be useful to mentioned that at least one direction
>> bit must be set? Otherwise request does not make sense.
>>
> Agree one direction must be set.
>
>>> + */
>>> +__extension__
>>> +struct rte_flow_pattern_template_attr {
>>> + /**
>>> + * Relaxed matching policy.
>>> + * - PMD may match only on items with mask member set and skip
>>> + * matching on protocol layers specified without any masks.
>>> + * - If not set, PMD will match on protocol layers
>>> + * specified without any masks as well.
>>> + * - Packet data must be stacked in the same order as the
>>> + * protocol layers to match inside packets, starting from the lowest.
>>> + */
>>> + uint32_t relaxed_matching:1;
>>
>> I should notice this earlier, but it looks like a new feature
>> which sounds unrelated to templates. If so, it makes asymmetry
>> in sync and async flow rules capabilities.
>> Am I missing something?
>>
>> Anyway, the feature looks hidden in the patch.
>>
> No this is not hidden feature.
> In current API application must specify all the preciding items,
> For example application wants to match on udp source port.
> The rte flow will look something like eth / ipv4/ udp sport = xxx ..
> When PMD gets this pattern it must enforce the after the eth
> there will be IPv4 and then UDP and then add the match for the
> sport.
> This means that the PMD addes extra matching.
> If the application already validated that there is udp in the packet
> in group 0 and then jump to group 1 it can save the HW those extra matching
> by enabling this bit which means that the HW should only match on implicit
> masked fields.
Old API allows to insert rule to non-0 table as well.
So, similar logic could be applicable. Do we want to
have the same feature in old API?
>
>>> + /** Pattern valid for rules applied to ingress traffic. */
>>> + uint32_t ingress:1;
>>> + /** Pattern valid for rules applied to egress traffic. */
>>> + uint32_t egress:1;
>>> + /** Pattern valid for rules applied to transfer traffic. */
>>> + uint32_t transfer:1;
>>> +};
>>> +
>>> +/**
>>> + * @warning
>>> + * @b EXPERIMENTAL: this API may change without prior notice.
>>> + *
>>> + * Create flow pattern template.
>>> + *
>>> + * The pattern template defines common matching fields 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 number and order of items in the template must be the same
>>> + * at the rule creation.
>>> + *
>>> + * @param port_id
>>> + * Port identifier of Ethernet device.
>>> + * @param[in] template_attr
>>> + * Pattern template attributes.
>>> + * @param[in] pattern
>>> + * 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.
>>
>> Don't we want to be explicit about used negative error code?
>> The question is applicable to all functions.
>>
> Same answer as given in other patch.
> Since PMD may have different/extra error codes I don't think we should
> give them here.
>
>> [snip]
>>
>>> +/**
>>> + * @warning
>>> + * @b EXPERIMENTAL: this API may change without prior notice.
>>> + *
>>> + * Flow actions template attributes.
>>
>> Same question about no directions specified.
>>
>>> + */
>>> +__extension__
>>> +struct rte_flow_actions_template_attr {
>>> + /** Action valid for rules applied to ingress traffic. */
>>> + uint32_t ingress:1;
>>> + /** Action valid for rules applied to egress traffic. */
>>> + uint32_t egress:1;
>>> + /** Action valid for rules applied to transfer traffic. */
>>> + uint32_t transfer:1;
>>> +};
>>> +
>>> +/**
>>> + * @warning
>>> + * @b EXPERIMENTAL: this API may change without prior notice.
>>> + *
>>> + * Create flow actions template.
>>> + *
>>> + * The actions 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 number and order of actions in the template must be the same
>>> + * at the rule creation.
>>> + *
>>> + * @param port_id
>>> + * Port identifier of Ethernet device.
>>> + * @param[in] template_attr
>>> + * Template attributes.
>>> + * @param[in] actions
>>> + * Associated actions (list terminated by the END action).
>>> + * The spec member is only used if @p masks spec is non-zero.
>>> + * @param[in] masks
>>> + * List of actions that marks which of the action's member is constant.
>>> + * A mask has the same format as the corresponding action.
>>> + * If the action field in @p masks is not 0,
>>
>> Comparison with zero makes sense for integers only.
>>
>
> Why? It can also be with pointers enums.
It should be NULL for pointers and enum-specific member of
enum.
>
>>> + * 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_actions_template *
>>> +rte_flow_actions_template_create(uint16_t port_id,
>>> + const struct rte_flow_actions_template_attr *template_attr,
>>> + const struct rte_flow_action actions[],
>>> + const struct rte_flow_action masks[],
>>> + struct rte_flow_error *error);
>>
>> [snip]
>
> Best,
> Ori
>
On Monday, February 21, 2022 8:12 Ori Kam <orika@nvidia.com> wrote:
> > See notes about order of checks in previous patch review notes.
I'll fix order of checks in all patches, thank you for the suggestion.
> > Would it be useful to mentioned that at least one direction
> > bit must be set? Otherwise request does not make sense.
> >
> Agree one direction must be set.
Will add comments about mandatory setting of the direction.
> > > + */
> > > +__extension__
> > > +struct rte_flow_pattern_template_attr {
> > > + /**
> > > + * Relaxed matching policy.
> > > + * - PMD may match only on items with mask member set and skip
> > > + * matching on protocol layers specified without any masks.
> > > + * - If not set, PMD will match on protocol layers
> > > + * specified without any masks as well.
> > > + * - Packet data must be stacked in the same order as the
> > > + * protocol layers to match inside packets, starting from the lowest.
> > > + */
> > > + uint32_t relaxed_matching:1;
> >
> > I should notice this earlier, but it looks like a new feature
> > which sounds unrelated to templates. If so, it makes asymmetry
> > in sync and async flow rules capabilities.
> > Am I missing something?
> >
> > Anyway, the feature looks hidden in the patch.
> >
> No this is not hidden feature.
> In current API application must specify all the preciding items,
> For example application wants to match on udp source port.
> The rte flow will look something like eth / ipv4/ udp sport = xxx ..
> When PMD gets this pattern it must enforce the after the eth
> there will be IPv4 and then UDP and then add the match for the
> sport.
> This means that the PMD addes extra matching.
> If the application already validated that there is udp in the packet
> in group 0 and then jump to group 1 it can save the HW those extra matching
> by enabling this bit which means that the HW should only match on implicit
> masked fields.
This is a new capability that only exists for templates.
We can think about adding it to the old rte_flow_create() API when
we are allowed to break ABI again.
Hi Andrew,
> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Monday, February 21, 2022 5:06 PM
> Subject: Re: [PATCH v8 02/11] ethdev: add flow item/action templates
>
> On 2/21/22 16:12, Ori Kam wrote:
> > Hi Andrew,
> >
> >> -----Original Message-----
> >> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >> Sent: Monday, February 21, 2022 12:57 PM
> >> Subject: Re: [PATCH v8 02/11] ethdev: add flow item/action templates
> >>
> >> On 2/20/22 06:44, 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
[Snip]
> >> Anyway, the feature looks hidden in the patch.
> >>
> > No this is not hidden feature.
> > In current API application must specify all the preciding items,
> > For example application wants to match on udp source port.
> > The rte flow will look something like eth / ipv4/ udp sport = xxx ..
> > When PMD gets this pattern it must enforce the after the eth
> > there will be IPv4 and then UDP and then add the match for the
> > sport.
> > This means that the PMD addes extra matching.
> > If the application already validated that there is udp in the packet
> > in group 0 and then jump to group 1 it can save the HW those extra matching
> > by enabling this bit which means that the HW should only match on implicit
> > masked fields.
>
> Old API allows to insert rule to non-0 table as well.
> So, similar logic could be applicable. Do we want to
> have the same feature in old API?
>
Maybe but in any case this should be done when we can break API.
In general I'm not sure that any new capabilities that will be added to this
API will be implemented in the current one.
> >
> >>> + /** Pattern valid for rules applied to ingress traffic. */
> >>> + uint32_t ingress:1;
> >>> + /** Pattern valid for rules applied to egress traffic. */
> >>> + uint32_t egress:1;
> >>> + /** Pattern valid for rules applied to transfer traffic. */
> >>> + uint32_t transfer:1;
> >>> +};
> >>> +
[Snip]
> >>> +/**
> >>> + * @warning
> >>> + * @b EXPERIMENTAL: this API may change without prior notice.
> >>> + *
> >>> + * Create flow actions template.
> >>> + *
> >>> + * The actions 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 number and order of actions in the template must be the same
> >>> + * at the rule creation.
> >>> + *
> >>> + * @param port_id
> >>> + * Port identifier of Ethernet device.
> >>> + * @param[in] template_attr
> >>> + * Template attributes.
> >>> + * @param[in] actions
> >>> + * Associated actions (list terminated by the END action).
> >>> + * The spec member is only used if @p masks spec is non-zero.
> >>> + * @param[in] masks
> >>> + * List of actions that marks which of the action's member is constant.
> >>> + * A mask has the same format as the corresponding action.
> >>> + * If the action field in @p masks is not 0,
> >>
> >> Comparison with zero makes sense for integers only.
> >>
> >
> > Why? It can also be with pointers enums.
>
> It should be NULL for pointers and enum-specific member of
> enum.
>
Since NULL is zero, I think it is much better to have the same logic
and compare to 0 in all cases.
Adding dedicated enum member will break current API, in addition
if we will have more complex structures like arrays it is much easier
to compare the first element with 0.
You can look at it as true/false for each field.
> >
> >>> + * 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_actions_template *
> >>> +rte_flow_actions_template_create(uint16_t port_id,
> >>> + const struct rte_flow_actions_template_attr *template_attr,
> >>> + const struct rte_flow_action actions[],
> >>> + const struct rte_flow_action masks[],
> >>> + struct rte_flow_error *error);
> >>
> >> [snip]
> >
> > Best,
> > Ori
> >
Best,
Ori
@@ -3642,6 +3642,141 @@ Information about the number of available resources can be retrieved via
struct rte_flow_port_info *port_info,
struct rte_flow_error *error);
+Flow templates
+~~~~~~~~~~~~~~
+
+Oftentimes in an application, many flow rules share a common structure
+(the same pattern 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 flow rule creation is done by selecting a table, a pattern template
+and an actions template (which are bound to the table), and setting unique
+values for the items and actions. This API is not thread-safe.
+
+Pattern templates
+^^^^^^^^^^^^^^^^^
+
+The pattern template defines a common pattern (the item mask) without values.
+The mask value is used to select a field to match on, spec/last are ignored.
+The pattern template may be used by multiple tables and must not be destroyed
+until all these tables are destroyed first.
+
+.. code-block:: c
+
+ struct rte_flow_pattern_template *
+ rte_flow_pattern_template_create(uint16_t port_id,
+ const struct rte_flow_pattern_template_attr *template_attr,
+ const struct rte_flow_item pattern[],
+ struct rte_flow_error *error);
+
+For example, to create a pattern template to match on the destination MAC:
+
+.. code-block:: c
+
+ const struct rte_flow_pattern_template_attr attr = {.ingress = 1};
+ struct rte_flow_item_eth eth_m = {
+ .dst.addr_bytes = "\xff\xff\xff\xff\xff\xff";
+ };
+ struct rte_flow_item pattern[] = {
+ [0] = {.type = RTE_FLOW_ITEM_TYPE_ETH,
+ .mask = ð_m},
+ [1] = {.type = RTE_FLOW_ITEM_TYPE_END,},
+ };
+ struct rte_flow_error err;
+
+ struct rte_flow_pattern_template *pattern_template =
+ rte_flow_pattern_template_create(port, &attr, &pattern, &err);
+
+The concrete value to match on will be provided at the rule creation.
+
+Actions templates
+^^^^^^^^^^^^^^^^^
+
+The actions template holds a list of action types to be used in flow rules.
+The mask parameter allows specifying a shared constant value for every rule.
+The actions template may be used by multiple tables and must not be destroyed
+until all these tables are destroyed first.
+
+.. code-block:: c
+
+ struct rte_flow_actions_template *
+ rte_flow_actions_template_create(uint16_t port_id,
+ const struct rte_flow_actions_template_attr *template_attr,
+ const struct rte_flow_action actions[],
+ const struct rte_flow_action masks[],
+ struct rte_flow_error *error);
+
+For example, to create an actions template with the same Mark ID
+but different Queue Index for every rule:
+
+.. code-block:: c
+
+ rte_flow_actions_template_attr attr = {.ingress = 1};
+ struct rte_flow_action act[] = {
+ /* Mark ID is 4 for every rule, Queue Index is unique */
+ [0] = {.type = RTE_FLOW_ACTION_TYPE_MARK,
+ .conf = &(struct rte_flow_action_mark){.id = 4}},
+ [1] = {.type = RTE_FLOW_ACTION_TYPE_QUEUE},
+ [2] = {.type = RTE_FLOW_ACTION_TYPE_END,},
+ };
+ struct rte_flow_action msk[] = {
+ /* Assign to MARK mask any non-zero value to make it constant */
+ [0] = {.type = RTE_FLOW_ACTION_TYPE_MARK,
+ .conf = &(struct rte_flow_action_mark){.id = 1}},
+ [1] = {.type = RTE_FLOW_ACTION_TYPE_QUEUE},
+ [2] = {.type = RTE_FLOW_ACTION_TYPE_END,},
+ };
+ struct rte_flow_error err;
+
+ struct rte_flow_actions_template *actions_template =
+ rte_flow_actions_template_create(port, &attr, &act, &msk, &err);
+
+The concrete value for Queue Index will be provided at the rule creation.
+
+Template table
+^^^^^^^^^^^^^^
+
+A template table combines a number of pattern and actions 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.
+Any flow rule creation beyond the maximum table size is rejected.
+Application may create another table to accommodate more rules in this case.
+
+.. code-block:: c
+
+ struct rte_flow_template_table *
+ rte_flow_template_table_create(uint16_t port_id,
+ const struct rte_flow_template_table_attr *table_attr,
+ struct rte_flow_pattern_template *pattern_templates[],
+ uint8_t nb_pattern_templates,
+ struct rte_flow_actions_template *actions_templates[],
+ uint8_t nb_actions_templates,
+ struct rte_flow_error *error);
+
+A table can be created only after the Flow Rules management is configured
+and pattern and actions templates are created.
+
+.. code-block:: c
+
+ rte_flow_template_table_attr table_attr = {
+ .flow_attr.ingress = 1,
+ .nb_flows = 10000;
+ };
+ uint8_t nb_pattern_templ = 1;
+ struct rte_flow_pattern_template *pattern_templates[nb_pattern_templ];
+ pattern_templates[0] = pattern_template;
+ uint8_t nb_actions_templ = 1;
+ struct rte_flow_actions_template *actions_templates[nb_actions_templ];
+ actions_templates[0] = actions_template;
+ struct rte_flow_error error;
+
+ struct rte_flow_template_table *table =
+ rte_flow_template_table_create(port, &table_attr,
+ &pattern_templates, nb_pattern_templ,
+ &actions_templates, nb_actions_templ,
+ &error);
+
.. _flow_isolated_mode:
Flow isolated mode
@@ -105,6 +105,14 @@ New Features
engine, allowing to pre-allocate some resources for better performance.
Added ``rte_flow_info_get`` API to retrieve available resources.
+ * ethdev: Added ``rte_flow_template_table_create`` API to group flow rules
+ with the same flow attributes and common matching patterns and actions
+ defined by ``rte_flow_pattern_template_create`` and
+ ``rte_flow_actions_template_create`` respectively.
+ Corresponding functions to destroy these entities are:
+ ``rte_flow_template_table_destroy``, ``rte_flow_pattern_template_destroy``
+ and ``rte_flow_actions_template_destroy``.
+
* **Updated AF_XDP PMD**
* Added support for libxdp >=v1.2.2.
@@ -1461,3 +1461,255 @@ rte_flow_configure(uint16_t port_id,
RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
NULL, rte_strerror(ENOTSUP));
}
+
+struct rte_flow_pattern_template *
+rte_flow_pattern_template_create(uint16_t port_id,
+ const struct rte_flow_pattern_template_attr *template_attr,
+ const struct rte_flow_item pattern[],
+ 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_pattern_template *template;
+
+ if (template_attr == NULL) {
+ RTE_FLOW_LOG(ERR,
+ "Port %"PRIu16" template attr is NULL.\n",
+ port_id);
+ rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ATTR,
+ NULL, rte_strerror(EINVAL));
+ return NULL;
+ }
+ if (pattern == NULL) {
+ RTE_FLOW_LOG(ERR,
+ "Port %"PRIu16" pattern is NULL.\n",
+ port_id);
+ rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ATTR,
+ NULL, rte_strerror(EINVAL));
+ return NULL;
+ }
+ if (dev->data->flow_configured == 0) {
+ RTE_FLOW_LOG(INFO,
+ "Flow engine on port_id=%"PRIu16" is not configured.\n",
+ port_id);
+ rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_STATE,
+ NULL, rte_strerror(EINVAL));
+ return NULL;
+ }
+ if (unlikely(!ops))
+ return NULL;
+ if (likely(!!ops->pattern_template_create)) {
+ template = ops->pattern_template_create(dev, template_attr,
+ pattern, error);
+ if (template == NULL)
+ flow_err(port_id, -rte_errno, error);
+ return template;
+ }
+ rte_flow_error_set(error, ENOTSUP,
+ RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+ NULL, rte_strerror(ENOTSUP));
+ return NULL;
+}
+
+int
+rte_flow_pattern_template_destroy(uint16_t port_id,
+ struct rte_flow_pattern_template *pattern_template,
+ 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);
+
+ if (unlikely(pattern_template == NULL))
+ return 0;
+ if (unlikely(!ops))
+ return -rte_errno;
+ if (likely(!!ops->pattern_template_destroy)) {
+ return flow_err(port_id,
+ ops->pattern_template_destroy(dev,
+ pattern_template,
+ error),
+ error);
+ }
+ return rte_flow_error_set(error, ENOTSUP,
+ RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+ NULL, rte_strerror(ENOTSUP));
+}
+
+struct rte_flow_actions_template *
+rte_flow_actions_template_create(uint16_t port_id,
+ const struct rte_flow_actions_template_attr *template_attr,
+ const struct rte_flow_action actions[],
+ const struct rte_flow_action masks[],
+ 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_actions_template *template;
+
+ if (template_attr == NULL) {
+ RTE_FLOW_LOG(ERR,
+ "Port %"PRIu16" template attr is NULL.\n",
+ port_id);
+ rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ATTR,
+ NULL, rte_strerror(EINVAL));
+ return NULL;
+ }
+ if (actions == NULL) {
+ RTE_FLOW_LOG(ERR,
+ "Port %"PRIu16" actions is NULL.\n",
+ port_id);
+ rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ATTR,
+ NULL, rte_strerror(EINVAL));
+ return NULL;
+ }
+ if (masks == NULL) {
+ RTE_FLOW_LOG(ERR,
+ "Port %"PRIu16" masks is NULL.\n",
+ port_id);
+ rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ATTR,
+ NULL, rte_strerror(EINVAL));
+
+ }
+ if (dev->data->flow_configured == 0) {
+ RTE_FLOW_LOG(INFO,
+ "Flow engine on port_id=%"PRIu16" is not configured.\n",
+ port_id);
+ rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_STATE,
+ NULL, rte_strerror(EINVAL));
+ return NULL;
+ }
+ if (unlikely(!ops))
+ return NULL;
+ if (likely(!!ops->actions_template_create)) {
+ template = ops->actions_template_create(dev, template_attr,
+ actions, masks, error);
+ if (template == NULL)
+ flow_err(port_id, -rte_errno, error);
+ return template;
+ }
+ rte_flow_error_set(error, ENOTSUP,
+ RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+ NULL, rte_strerror(ENOTSUP));
+ return NULL;
+}
+
+int
+rte_flow_actions_template_destroy(uint16_t port_id,
+ struct rte_flow_actions_template *actions_template,
+ 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);
+
+ if (unlikely(actions_template == NULL))
+ return 0;
+ if (unlikely(!ops))
+ return -rte_errno;
+ if (likely(!!ops->actions_template_destroy)) {
+ return flow_err(port_id,
+ ops->actions_template_destroy(dev,
+ actions_template,
+ error),
+ error);
+ }
+ return rte_flow_error_set(error, ENOTSUP,
+ RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+ NULL, rte_strerror(ENOTSUP));
+}
+
+struct rte_flow_template_table *
+rte_flow_template_table_create(uint16_t port_id,
+ const struct rte_flow_template_table_attr *table_attr,
+ struct rte_flow_pattern_template *pattern_templates[],
+ uint8_t nb_pattern_templates,
+ struct rte_flow_actions_template *actions_templates[],
+ uint8_t nb_actions_templates,
+ 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_template_table *table;
+
+ if (table_attr == NULL) {
+ RTE_FLOW_LOG(ERR,
+ "Port %"PRIu16" table attr is NULL.\n",
+ port_id);
+ rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ATTR,
+ NULL, rte_strerror(EINVAL));
+ return NULL;
+ }
+ if (pattern_templates == NULL) {
+ RTE_FLOW_LOG(ERR,
+ "Port %"PRIu16" pattern templates is NULL.\n",
+ port_id);
+ rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ATTR,
+ NULL, rte_strerror(EINVAL));
+ return NULL;
+ }
+ if (actions_templates == NULL) {
+ RTE_FLOW_LOG(ERR,
+ "Port %"PRIu16" actions templates is NULL.\n",
+ port_id);
+ rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ATTR,
+ NULL, rte_strerror(EINVAL));
+ return NULL;
+ }
+ if (dev->data->flow_configured == 0) {
+ RTE_FLOW_LOG(INFO,
+ "Flow engine on port_id=%"PRIu16" is not configured.\n",
+ port_id);
+ rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_STATE,
+ NULL, rte_strerror(EINVAL));
+ return NULL;
+ }
+ if (unlikely(!ops))
+ return NULL;
+ if (likely(!!ops->template_table_create)) {
+ table = ops->template_table_create(dev, table_attr,
+ pattern_templates, nb_pattern_templates,
+ actions_templates, nb_actions_templates,
+ error);
+ if (table == NULL)
+ flow_err(port_id, -rte_errno, error);
+ return table;
+ }
+ rte_flow_error_set(error, ENOTSUP,
+ RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+ NULL, rte_strerror(ENOTSUP));
+ return NULL;
+}
+
+int
+rte_flow_template_table_destroy(uint16_t port_id,
+ struct rte_flow_template_table *template_table,
+ 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);
+
+ if (unlikely(template_table == NULL))
+ return 0;
+ if (unlikely(!ops))
+ return -rte_errno;
+ if (likely(!!ops->template_table_destroy)) {
+ return flow_err(port_id,
+ ops->template_table_destroy(dev,
+ template_table,
+ error),
+ error);
+ }
+ return rte_flow_error_set(error, ENOTSUP,
+ RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+ NULL, rte_strerror(ENOTSUP));
+}
@@ -4983,6 +4983,280 @@ rte_flow_configure(uint16_t port_id,
const struct rte_flow_port_attr *port_attr,
struct rte_flow_error *error);
+/**
+ * Opaque type returned after successful creation of pattern template.
+ * This handle can be used to manage the created pattern template.
+ */
+struct rte_flow_pattern_template;
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Flow pattern template attributes.
+ */
+__extension__
+struct rte_flow_pattern_template_attr {
+ /**
+ * Relaxed matching policy.
+ * - PMD may match only on items with mask member set and skip
+ * matching on protocol layers specified without any masks.
+ * - If not set, PMD will match on protocol layers
+ * specified without any masks as well.
+ * - Packet data must be stacked in the same order as the
+ * protocol layers to match inside packets, starting from the lowest.
+ */
+ uint32_t relaxed_matching:1;
+ /** Pattern valid for rules applied to ingress traffic. */
+ uint32_t ingress:1;
+ /** Pattern valid for rules applied to egress traffic. */
+ uint32_t egress:1;
+ /** Pattern valid for rules applied to transfer traffic. */
+ uint32_t transfer:1;
+};
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Create flow pattern template.
+ *
+ * The pattern template defines common matching fields 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 number and order of items in the template must be the same
+ * at the rule creation.
+ *
+ * @param port_id
+ * Port identifier of Ethernet device.
+ * @param[in] template_attr
+ * Pattern template attributes.
+ * @param[in] pattern
+ * 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_pattern_template *
+rte_flow_pattern_template_create(uint16_t port_id,
+ const struct rte_flow_pattern_template_attr *template_attr,
+ const struct rte_flow_item pattern[],
+ struct rte_flow_error *error);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Destroy flow pattern 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] pattern_template
+ * Handle of 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_pattern_template_destroy(uint16_t port_id,
+ struct rte_flow_pattern_template *pattern_template,
+ struct rte_flow_error *error);
+
+/**
+ * Opaque type returned after successful creation of actions template.
+ * This handle can be used to manage the created actions template.
+ */
+struct rte_flow_actions_template;
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Flow actions template attributes.
+ */
+__extension__
+struct rte_flow_actions_template_attr {
+ /** Action valid for rules applied to ingress traffic. */
+ uint32_t ingress:1;
+ /** Action valid for rules applied to egress traffic. */
+ uint32_t egress:1;
+ /** Action valid for rules applied to transfer traffic. */
+ uint32_t transfer:1;
+};
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Create flow actions template.
+ *
+ * The actions 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 number and order of actions in the template must be the same
+ * at the rule creation.
+ *
+ * @param port_id
+ * Port identifier of Ethernet device.
+ * @param[in] template_attr
+ * Template attributes.
+ * @param[in] actions
+ * Associated actions (list terminated by the END action).
+ * The spec member is only used if @p masks spec is non-zero.
+ * @param[in] masks
+ * List of actions that marks which of the action's member is constant.
+ * 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_actions_template *
+rte_flow_actions_template_create(uint16_t port_id,
+ const struct rte_flow_actions_template_attr *template_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 flow actions 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] actions_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_actions_template_destroy(uint16_t port_id,
+ struct rte_flow_actions_template *actions_template,
+ struct rte_flow_error *error);
+
+/**
+ * Opaque type returned after successful creation of a template table.
+ * This handle can be used to manage the created template table.
+ */
+struct rte_flow_template_table;
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Table attributes.
+ */
+struct rte_flow_template_table_attr {
+ /**
+ * Flow attributes to be used in each rule generated from this table.
+ */
+ struct rte_flow_attr flow_attr;
+ /**
+ * Maximum number of flow rules that this table holds.
+ */
+ uint32_t nb_flows;
+};
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Create flow template table.
+ *
+ * A template table consists of multiple pattern templates and actions
+ * templates associated with a single set of rule attributes (group ID,
+ * priority and traffic direction).
+ *
+ * Each rule is free to use any combination of pattern and actions templates
+ * and specify particular values for items and actions it would like to change.
+ *
+ * @param port_id
+ * Port identifier of Ethernet device.
+ * @param[in] table_attr
+ * Template table attributes.
+ * @param[in] pattern_templates
+ * Array of pattern templates to be used in this table.
+ * @param[in] nb_pattern_templates
+ * The number of pattern templates in the pattern_templates array.
+ * @param[in] actions_templates
+ * Array of actions templates to be used in this table.
+ * @param[in] nb_actions_templates
+ * The number of actions templates in the actions_templates array.
+ * @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_template_table *
+rte_flow_template_table_create(uint16_t port_id,
+ const struct rte_flow_template_table_attr *table_attr,
+ struct rte_flow_pattern_template *pattern_templates[],
+ uint8_t nb_pattern_templates,
+ struct rte_flow_actions_template *actions_templates[],
+ uint8_t nb_actions_templates,
+ struct rte_flow_error *error);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Destroy flow template table.
+ *
+ * This function may be called only when
+ * there are no more flow rules referencing this table.
+ *
+ * @param port_id
+ * Port identifier of Ethernet device.
+ * @param[in] template_table
+ * Handle to the table 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_template_table_destroy(uint16_t port_id,
+ struct rte_flow_template_table *template_table,
+ struct rte_flow_error *error);
+
#ifdef __cplusplus
}
#endif
@@ -162,6 +162,43 @@ struct rte_flow_ops {
(struct rte_eth_dev *dev,
const struct rte_flow_port_attr *port_attr,
struct rte_flow_error *err);
+ /** See rte_flow_pattern_template_create() */
+ struct rte_flow_pattern_template *(*pattern_template_create)
+ (struct rte_eth_dev *dev,
+ const struct rte_flow_pattern_template_attr *template_attr,
+ const struct rte_flow_item pattern[],
+ struct rte_flow_error *err);
+ /** See rte_flow_pattern_template_destroy() */
+ int (*pattern_template_destroy)
+ (struct rte_eth_dev *dev,
+ struct rte_flow_pattern_template *pattern_template,
+ struct rte_flow_error *err);
+ /** See rte_flow_actions_template_create() */
+ struct rte_flow_actions_template *(*actions_template_create)
+ (struct rte_eth_dev *dev,
+ const struct rte_flow_actions_template_attr *template_attr,
+ const struct rte_flow_action actions[],
+ const struct rte_flow_action masks[],
+ struct rte_flow_error *err);
+ /** See rte_flow_actions_template_destroy() */
+ int (*actions_template_destroy)
+ (struct rte_eth_dev *dev,
+ struct rte_flow_actions_template *actions_template,
+ struct rte_flow_error *err);
+ /** See rte_flow_template_table_create() */
+ struct rte_flow_template_table *(*template_table_create)
+ (struct rte_eth_dev *dev,
+ const struct rte_flow_template_table_attr *table_attr,
+ struct rte_flow_pattern_template *pattern_templates[],
+ uint8_t nb_pattern_templates,
+ struct rte_flow_actions_template *actions_templates[],
+ uint8_t nb_actions_templates,
+ struct rte_flow_error *err);
+ /** See rte_flow_template_table_destroy() */
+ int (*template_table_destroy)
+ (struct rte_eth_dev *dev,
+ struct rte_flow_template_table *template_table,
+ struct rte_flow_error *err);
};
/**
@@ -266,6 +266,12 @@ EXPERIMENTAL {
rte_eth_ip_reassembly_conf_set;
rte_flow_info_get;
rte_flow_configure;
+ rte_flow_pattern_template_create;
+ rte_flow_pattern_template_destroy;
+ rte_flow_actions_template_create;
+ rte_flow_actions_template_destroy;
+ rte_flow_template_table_create;
+ rte_flow_template_table_destroy;
};
INTERNAL {