ethdev: alter behavior of flow API actions

This patch makes the following changes to flow rule actions:

- List order now matters, they are redefined as performed first to last
  instead of "all simultaneously".

- Repeated actions are now supported (e.g. specifying QUEUE multiple times
  now duplicates traffic among them). Previously only the last action of
  any given kind was taken into account.

- No more distinction between terminating/non-terminating/meta actions.
  Flow rules themselves are now defined as always terminating unless a
  PASSTHRU action is specified.

These changes alter the behavior of flow rules in corner cases in order to
prepare the flow API for actions that modify traffic contents or properties
(e.g. encapsulation, compression) and for which order matter when combined.

Previously one would have to do so through multiple flow rules by combining
PASSTRHU with priority levels, however this proved overly complex to
implement at the PMD level, hence this simpler approach.

This breaks ABI compatibility for the following public functions:

- rte_flow_create()
- rte_flow_validate()

PMDs with rte_flow support are modified accordingly:

- bnxt: no change, implementation already forbids multiple actions and does
  not support PASSTHRU.

- e1000: no change, same as bnxt.

- enic: modified to forbid redundant actions, no support for default drop.

- failsafe: no change needed.

- i40e: no change, implementation already forbids multiple actions.

- ixgbe: same as i40e.

- mlx4: modified to forbid multiple fate-deciding actions and drop when
  unspecified.

- mlx5: same as mlx4, with other redundant actions also forbidden.

- sfc: same as mlx4.

- tap: implementation already complies with the new behavior except for
  the default pass-through modified as a default drop.

Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
This commit is contained in:
Adrien Mazarguil 2018-04-25 17:27:46 +02:00 committed by Ferruh Yigit
parent 2e6e75679a
commit cc17feb904
8 changed files with 150 additions and 134 deletions

View File

@ -995,28 +995,27 @@ Actions
Each possible action is represented by a type. Some have associated
configuration structures. Several actions combined in a list can be assigned
to a flow rule. That list is not ordered.
to a flow rule and are performed in order.
They fall in three categories:
- Terminating actions that prevent processing matched packets by subsequent
flow rules, unless overridden with PASSTHRU.
- Actions that modify the fate of matching traffic, for instance by dropping
or assigning it a specific destination.
- Non-terminating actions that leave matched packets up for additional
processing by subsequent flow rules.
- Actions that modify matching traffic contents or its properties. This
includes adding/removing encapsulation, encryption, compression and marks.
- Other non-terminating meta actions that do not affect the fate of packets.
- Actions related to the flow rule itself, such as updating counters or
making it non-terminating.
When several actions are combined in a flow rule, they should all have
different types (e.g. dropping a packet twice is not possible).
Flow rules being terminating by default, not specifying any action of the
fate kind results in undefined behavior. This applies to both ingress and
egress.
Only the last action of a given type is taken into account. PMDs still
perform error checking on the entire list.
PASSTHRU, when supported, makes a flow rule non-terminating.
Like matching patterns, action lists are terminated by END items.
*Note that PASSTHRU is the only action able to override a terminating rule.*
Example of action that redirects packets to queue index 10:
.. _table_rte_flow_action_example:
@ -1029,12 +1028,11 @@ Example of action that redirects packets to queue index 10:
| ``index`` | 10 |
+-----------+-------+
Action lists examples, their order is not significant, applications must
consider all actions to be performed simultaneously:
Actions are performed in list order:
.. _table_rte_flow_count_and_drop:
.. _table_rte_flow_count_then_drop:
.. table:: Count and drop
.. table:: Count then drop
+-------+--------+
| Index | Action |
@ -1050,7 +1048,7 @@ consider all actions to be performed simultaneously:
.. _table_rte_flow_mark_count_redirect:
.. table:: Mark, count and redirect
.. table:: Mark, count then redirect
+-------+--------+-----------+-------+
| Index | Action | Field | Value |
@ -1080,12 +1078,15 @@ consider all actions to be performed simultaneously:
| 2 | END |
+-------+----------------------------+
In the above example, considering both actions are performed simultaneously,
the end result is that only QUEUE has any effect.
In the above example, while DROP and QUEUE must be performed in order, both
have to happen before reaching END. Only QUEUE has a visible effect.
.. _table_rte_flow_redirect_queue_3:
Note that such a list may be thought as ambiguous and rejected on that
basis.
.. table:: Redirect to queue 3
.. _table_rte_flow_redirect_queue_5_3:
.. table:: Redirect to queues 5 and 3
+-------+--------+-----------+-------+
| Index | Action | Field | Value |
@ -1099,9 +1100,9 @@ the end result is that only QUEUE has any effect.
| 3 | END |
+-------+----------------------------+
As previously described, only the last action of a given type found in the
list is taken into account. The above example also shows that VOID is
ignored.
As previously described, all actions must be taken into account. This
effectively duplicates traffic to both queues. The above example also shows
that VOID is ignored.
Action types
~~~~~~~~~~~~
@ -1151,9 +1152,8 @@ PMDs.
Action: ``PASSTHRU``
^^^^^^^^^^^^^^^^^^^^
Leaves packets up for additional processing by subsequent flow rules. This
is the default when a rule does not contain a terminating action, but can be
specified to force a rule to become non-terminating.
Leaves traffic up for additional processing by subsequent flow rules; makes
a flow rule non-terminating.
- No configurable properties.
@ -1227,8 +1227,6 @@ Action: ``QUEUE``
Assigns packets to a given queue index.
- Terminating by default.
.. _table_rte_flow_action_queue:
.. table:: QUEUE
@ -1245,8 +1243,6 @@ Action: ``DROP``
Drop packets.
- No configurable properties.
- Terminating by default.
- PASSTHRU overrides this action if both are specified.
.. _table_rte_flow_action_drop:
@ -1309,8 +1305,6 @@ Note: RSS hash result is stored in the ``hash.rss`` mbuf field which
overlaps ``hash.fdir.lo``. Since `Action: MARK`_ sets the ``hash.fdir.hi``
field only, both can be requested simultaneously.
- Terminating by default.
.. _table_rte_flow_action_rss:
.. table:: RSS
@ -1331,7 +1325,6 @@ Action: ``PF``
Redirects packets to the physical function (PF) of the current device.
- No configurable properties.
- Terminating by default.
.. _table_rte_flow_action_pf:
@ -1353,8 +1346,6 @@ ID instead of the specified one. This parameter may not be available and is
not guaranteed to work properly if the VF part is matched by a prior flow
rule or if packets are not addressed to a VF in the first place.
- Terminating by default.
.. _table_rte_flow_action_vf:
.. table:: VF
@ -1378,8 +1369,6 @@ action parameter. More than one flow can use the same MTR object through
the meter action. The MTR object can be further updated or queried using
the rte_mtr* API.
- Non-terminating by default.
.. _table_rte_flow_action_meter:
.. table:: METER
@ -1415,8 +1404,6 @@ direction.
Multiple flows can be configured to use the same security session.
- Non-terminating by default.
.. _table_rte_flow_action_security:
.. table:: SECURITY

View File

@ -258,7 +258,15 @@ API Changes
fall-back value. Previously, setting ``nb_tx_desc`` to zero would have
resulted in an error.
* ethdev: unused DUP action was removed from the flow API.
* ethdev: several changes were made to the flow API.
* Unused DUP action was removed.
* Actions semantics in flow rules: list order now matters ("first
to last" instead of "all simultaneously"), repeated actions are now
all performed, and they do not individually have (non-)terminating
properties anymore.
* Flow rules are now always terminating unless a PASSTHRU action is
present.
ABI Changes
@ -305,8 +313,9 @@ ABI Changes
This includes functions ``rte_flow_copy``, ``rte_flow_create``,
``rte_flow_destroy``, ``rte_flow_error_set``, ``rte_flow_flush``,
``rte_flow_isolate``, ``rte_flow_query`` and ``rte_flow_validate``, due to
changes in error type definitions (``enum rte_flow_error_type``) and
removal of the unused DUP action (``enum rte_flow_action_type``).
changes in error type definitions (``enum rte_flow_error_type``), removal
of the unused DUP action (``enum rte_flow_action_type``) and modified
behavior for flow rule actions (see API changes).
Removed Items

View File

@ -3,6 +3,7 @@
*/
#include <errno.h>
#include <stdint.h>
#include <rte_log.h>
#include <rte_ethdev_driver.h>
#include <rte_flow_driver.h>
@ -964,6 +965,9 @@ static int
enic_copy_action_v1(const struct rte_flow_action actions[],
struct filter_action_v2 *enic_action)
{
enum { FATE = 1, };
uint32_t overlap = 0;
FLOW_TRACE();
for (; actions->type != RTE_FLOW_ACTION_TYPE_END; actions++) {
@ -975,6 +979,10 @@ enic_copy_action_v1(const struct rte_flow_action actions[],
const struct rte_flow_action_queue *queue =
(const struct rte_flow_action_queue *)
actions->conf;
if (overlap & FATE)
return ENOTSUP;
overlap |= FATE;
enic_action->rq_idx =
enic_rte_rq_idx_to_sop_idx(queue->index);
break;
@ -984,6 +992,8 @@ enic_copy_action_v1(const struct rte_flow_action actions[],
break;
}
}
if (!(overlap & FATE))
return ENOTSUP;
enic_action->type = FILTER_ACTION_RQ_STEERING;
return 0;
}
@ -1001,6 +1011,9 @@ static int
enic_copy_action_v2(const struct rte_flow_action actions[],
struct filter_action_v2 *enic_action)
{
enum { FATE = 1, MARK = 2, };
uint32_t overlap = 0;
FLOW_TRACE();
for (; actions->type != RTE_FLOW_ACTION_TYPE_END; actions++) {
@ -1009,6 +1022,10 @@ enic_copy_action_v2(const struct rte_flow_action actions[],
const struct rte_flow_action_queue *queue =
(const struct rte_flow_action_queue *)
actions->conf;
if (overlap & FATE)
return ENOTSUP;
overlap |= FATE;
enic_action->rq_idx =
enic_rte_rq_idx_to_sop_idx(queue->index);
enic_action->flags |= FILTER_ACTION_RQ_STEERING_FLAG;
@ -1019,6 +1036,9 @@ enic_copy_action_v2(const struct rte_flow_action actions[],
(const struct rte_flow_action_mark *)
actions->conf;
if (overlap & MARK)
return ENOTSUP;
overlap |= MARK;
/* ENIC_MAGIC_FILTER_ID is reserved and is the highest
* in the range of allows mark ids.
*/
@ -1029,6 +1049,9 @@ enic_copy_action_v2(const struct rte_flow_action actions[],
break;
}
case RTE_FLOW_ACTION_TYPE_FLAG: {
if (overlap & MARK)
return ENOTSUP;
overlap |= MARK;
enic_action->filter_id = ENIC_MAGIC_FILTER_ID;
enic_action->flags |= FILTER_ACTION_FILTER_ID_FLAG;
break;
@ -1044,6 +1067,8 @@ enic_copy_action_v2(const struct rte_flow_action actions[],
break;
}
}
if (!(overlap & FATE))
return ENOTSUP;
enic_action->type = FILTER_ACTION_V2;
return 0;
}

View File

@ -637,6 +637,7 @@ mlx4_flow_prepare(struct priv *priv,
struct rte_flow temp = { .ibv_attr_size = sizeof(*temp.ibv_attr) };
struct rte_flow *flow = &temp;
const char *msg = NULL;
int overlap;
if (attr->group)
return rte_flow_error_set
@ -656,6 +657,7 @@ mlx4_flow_prepare(struct priv *priv,
(error, ENOTSUP, RTE_FLOW_ERROR_TYPE_ATTR_INGRESS,
NULL, "only ingress is supported");
fill:
overlap = 0;
proc = mlx4_flow_proc_item_list;
/* Go over pattern. */
for (item = pattern; item->type; ++item) {
@ -702,6 +704,16 @@ mlx4_flow_prepare(struct priv *priv,
}
/* Go over actions list. */
for (action = actions; action->type; ++action) {
/* This one may appear anywhere multiple times. */
if (action->type == RTE_FLOW_ACTION_TYPE_VOID)
continue;
/* Fate-deciding actions may appear exactly once. */
if (overlap) {
msg = "cannot combine several fate-deciding actions,"
" choose between DROP, QUEUE or RSS";
goto exit_action_not_supported;
}
overlap = 1;
switch (action->type) {
const struct rte_flow_action_queue *queue;
const struct rte_flow_action_rss *rss;
@ -709,8 +721,6 @@ mlx4_flow_prepare(struct priv *priv,
uint64_t fields;
unsigned int i;
case RTE_FLOW_ACTION_TYPE_VOID:
continue;
case RTE_FLOW_ACTION_TYPE_DROP:
flow->drop = 1;
break;
@ -801,10 +811,9 @@ mlx4_flow_prepare(struct priv *priv,
goto exit_action_not_supported;
}
}
if (!flow->rss && !flow->drop)
return rte_flow_error_set
(error, ENOTSUP, RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
NULL, "no valid action");
/* When fate is unknown, drop traffic. */
if (!overlap)
flow->drop = 1;
/* Validation ends here. */
if (!addr) {
if (flow->rss)

View File

@ -4,6 +4,7 @@
*/
#include <sys/queue.h>
#include <stdint.h>
#include <string.h>
/* Verbs header. */
@ -646,6 +647,8 @@ mlx5_flow_convert_actions(struct rte_eth_dev *dev,
struct rte_flow_error *error,
struct mlx5_flow_parse *parser)
{
enum { FATE = 1, MARK = 2, COUNT = 4, };
uint32_t overlap = 0;
struct priv *priv = dev->data->dev_private;
int ret;
@ -662,39 +665,31 @@ mlx5_flow_convert_actions(struct rte_eth_dev *dev,
if (actions->type == RTE_FLOW_ACTION_TYPE_VOID) {
continue;
} else if (actions->type == RTE_FLOW_ACTION_TYPE_DROP) {
if (overlap & FATE)
goto exit_action_overlap;
overlap |= FATE;
parser->drop = 1;
} else if (actions->type == RTE_FLOW_ACTION_TYPE_QUEUE) {
const struct rte_flow_action_queue *queue =
(const struct rte_flow_action_queue *)
actions->conf;
uint16_t n;
uint16_t found = 0;
if (overlap & FATE)
goto exit_action_overlap;
overlap |= FATE;
if (!queue || (queue->index > (priv->rxqs_n - 1)))
goto exit_action_not_supported;
for (n = 0; n < parser->queues_n; ++n) {
if (parser->queues[n] == queue->index) {
found = 1;
break;
}
}
if (parser->queues_n > 1 && !found) {
rte_flow_error_set(error, ENOTSUP,
RTE_FLOW_ERROR_TYPE_ACTION,
actions,
"queue action not in RSS queues");
return -rte_errno;
}
if (!found) {
parser->queues_n = 1;
parser->queues[0] = queue->index;
}
parser->queues_n = 1;
parser->queues[0] = queue->index;
} else if (actions->type == RTE_FLOW_ACTION_TYPE_RSS) {
const struct rte_flow_action_rss *rss =
(const struct rte_flow_action_rss *)
actions->conf;
uint16_t n;
if (overlap & FATE)
goto exit_action_overlap;
overlap |= FATE;
if (!rss || !rss->num) {
rte_flow_error_set(error, EINVAL,
RTE_FLOW_ERROR_TYPE_ACTION,
@ -702,26 +697,6 @@ mlx5_flow_convert_actions(struct rte_eth_dev *dev,
"no valid queues");
return -rte_errno;
}
if (parser->queues_n == 1) {
uint16_t found = 0;
assert(parser->queues_n);
for (n = 0; n < rss->num; ++n) {
if (parser->queues[0] ==
rss->queue[n]) {
found = 1;
break;
}
}
if (!found) {
rte_flow_error_set(error, ENOTSUP,
RTE_FLOW_ERROR_TYPE_ACTION,
actions,
"queue action not in RSS"
" queues");
return -rte_errno;
}
}
if (rss->num > RTE_DIM(parser->queues)) {
rte_flow_error_set(error, EINVAL,
RTE_FLOW_ERROR_TYPE_ACTION,
@ -755,6 +730,9 @@ mlx5_flow_convert_actions(struct rte_eth_dev *dev,
(const struct rte_flow_action_mark *)
actions->conf;
if (overlap & MARK)
goto exit_action_overlap;
overlap |= MARK;
if (!mark) {
rte_flow_error_set(error, EINVAL,
RTE_FLOW_ERROR_TYPE_ACTION,
@ -772,14 +750,23 @@ mlx5_flow_convert_actions(struct rte_eth_dev *dev,
parser->mark = 1;
parser->mark_id = mark->id;
} else if (actions->type == RTE_FLOW_ACTION_TYPE_FLAG) {
if (overlap & MARK)
goto exit_action_overlap;
overlap |= MARK;
parser->mark = 1;
} else if (actions->type == RTE_FLOW_ACTION_TYPE_COUNT &&
priv->config.flow_counter_en) {
if (overlap & COUNT)
goto exit_action_overlap;
overlap |= COUNT;
parser->count = 1;
} else {
goto exit_action_not_supported;
}
}
/* When fate is unknown, drop traffic. */
if (!(overlap & FATE))
parser->drop = 1;
if (parser->drop && parser->mark)
parser->mark = 0;
if (!parser->queues_n && !parser->drop) {
@ -792,6 +779,10 @@ mlx5_flow_convert_actions(struct rte_eth_dev *dev,
rte_flow_error_set(error, ENOTSUP, RTE_FLOW_ERROR_TYPE_ACTION,
actions, "action not supported");
return -rte_errno;
exit_action_overlap:
rte_flow_error_set(error, ENOTSUP, RTE_FLOW_ERROR_TYPE_ACTION,
actions, "overlapping actions are not supported");
return -rte_errno;
}
/**

View File

@ -1467,10 +1467,19 @@ sfc_flow_parse_actions(struct sfc_adapter *sa,
}
for (; actions->type != RTE_FLOW_ACTION_TYPE_END; actions++) {
/* This one may appear anywhere multiple times. */
if (actions->type == RTE_FLOW_ACTION_TYPE_VOID)
continue;
/* Fate-deciding actions may appear exactly once. */
if (is_specified) {
rte_flow_error_set
(error, ENOTSUP, RTE_FLOW_ERROR_TYPE_ACTION,
actions,
"Cannot combine several fate-deciding actions,"
"choose between QUEUE, RSS or DROP");
return -rte_errno;
}
switch (actions->type) {
case RTE_FLOW_ACTION_TYPE_VOID:
break;
case RTE_FLOW_ACTION_TYPE_QUEUE:
rc = sfc_flow_parse_queue(sa, actions->conf, flow);
if (rc != 0) {
@ -1512,11 +1521,10 @@ sfc_flow_parse_actions(struct sfc_adapter *sa,
}
}
/* When fate is unknown, drop traffic. */
if (!is_specified) {
rte_flow_error_set(error, EINVAL,
RTE_FLOW_ERROR_TYPE_ACTION_NUM, actions,
"Action is unspecified");
return -rte_errno;
flow->spec.template.efs_dmaq_id =
EFX_FILTER_SPEC_RX_DMAQ_ID_DROP;
}
return 0;

View File

@ -1140,6 +1140,7 @@ priv_flow_process(struct pmd_internals *pmd,
else
goto end;
}
actions:
for (; actions->type != RTE_FLOW_ACTION_TYPE_END; ++actions) {
int err = 0;
@ -1222,6 +1223,16 @@ priv_flow_process(struct pmd_internals *pmd,
if (err)
goto exit_action_not_supported;
}
/* When fate is unknown, drop traffic. */
if (!action) {
static const struct rte_flow_action drop[] = {
{ .type = RTE_FLOW_ACTION_TYPE_DROP, },
{ .type = RTE_FLOW_ACTION_TYPE_END, },
};
actions = drop;
goto actions;
}
end:
if (flow)
tap_nlattr_nested_finish(&flow->msg); /* nested TCA_OPTIONS */

View File

@ -859,32 +859,28 @@ struct rte_flow_item {
*
* Each possible action is represented by a type. Some have associated
* configuration structures. Several actions combined in a list can be
* affected to a flow rule. That list is not ordered.
* assigned to a flow rule and are performed in order.
*
* They fall in three categories:
*
* - Terminating actions that prevent processing matched packets by
* subsequent flow rules, unless overridden with PASSTHRU.
* - Actions that modify the fate of matching traffic, for instance by
* dropping or assigning it a specific destination.
*
* - Non terminating actions that leave matched packets up for additional
* processing by subsequent flow rules.
* - Actions that modify matching traffic contents or its properties. This
* includes adding/removing encapsulation, encryption, compression and
* marks.
*
* - Other non terminating meta actions that do not affect the fate of
* packets.
* - Actions related to the flow rule itself, such as updating counters or
* making it non-terminating.
*
* When several actions are combined in a flow rule, they should all have
* different types (e.g. dropping a packet twice is not possible).
* Flow rules being terminating by default, not specifying any action of the
* fate kind results in undefined behavior. This applies to both ingress and
* egress.
*
* Only the last action of a given type is taken into account. PMDs still
* perform error checking on the entire list.
*
* Note that PASSTHRU is the only action able to override a terminating
* rule.
* PASSTHRU, when supported, makes a flow rule non-terminating.
*/
enum rte_flow_action_type {
/**
* [META]
*
* End marker for action lists. Prevents further processing of
* actions, thereby ending the list.
*
@ -893,8 +889,6 @@ enum rte_flow_action_type {
RTE_FLOW_ACTION_TYPE_END,
/**
* [META]
*
* Used as a placeholder for convenience. It is ignored and simply
* discarded by PMDs.
*
@ -903,18 +897,14 @@ enum rte_flow_action_type {
RTE_FLOW_ACTION_TYPE_VOID,
/**
* Leaves packets up for additional processing by subsequent flow
* rules. This is the default when a rule does not contain a
* terminating action, but can be specified to force a rule to
* become non-terminating.
* Leaves traffic up for additional processing by subsequent flow
* rules; makes a flow rule non-terminating.
*
* No associated configuration structure.
*/
RTE_FLOW_ACTION_TYPE_PASSTHRU,
/**
* [META]
*
* Attaches an integer value to packets and sets PKT_RX_FDIR and
* PKT_RX_FDIR_ID mbuf flags.
*
@ -923,8 +913,6 @@ enum rte_flow_action_type {
RTE_FLOW_ACTION_TYPE_MARK,
/**
* [META]
*
* Flags packets. Similar to MARK without a specific value; only
* sets the PKT_RX_FDIR mbuf flag.
*
@ -949,9 +937,7 @@ enum rte_flow_action_type {
RTE_FLOW_ACTION_TYPE_DROP,
/**
* [META]
*
* Enables counters for this rule.
* Enables counters for this flow rule.
*
* These counters can be retrieved and reset through rte_flow_query(),
* see struct rte_flow_query_count.
@ -1020,8 +1006,6 @@ struct rte_flow_action_mark {
* RTE_FLOW_ACTION_TYPE_QUEUE
*
* Assign packets to a given queue index.
*
* Terminating by default.
*/
struct rte_flow_action_queue {
uint16_t index; /**< Queue index to use. */
@ -1050,8 +1034,6 @@ struct rte_flow_query_count {
* Note: RSS hash result is stored in the hash.rss mbuf field which overlaps
* hash.fdir.lo. Since the MARK action sets the hash.fdir.hi field only,
* both can be requested simultaneously.
*
* Terminating by default.
*/
struct rte_flow_action_rss {
const struct rte_eth_rss_conf *rss_conf; /**< RSS parameters. */
@ -1069,8 +1051,6 @@ struct rte_flow_action_rss {
* and is not guaranteed to work properly if the VF part is matched by a
* prior flow rule or if packets are not addressed to a VF in the first
* place.
*
* Terminating by default.
*/
struct rte_flow_action_vf {
uint32_t original:1; /**< Use original VF ID if possible. */
@ -1085,8 +1065,6 @@ struct rte_flow_action_vf {
*
* Packets matched by items of this type can be either dropped or passed to the
* next item with their color set by the MTR object.
*
* Non-terminating by default.
*/
struct rte_flow_action_meter {
uint32_t mtr_id; /**< MTR object ID created with rte_mtr_create(). */
@ -1116,8 +1094,6 @@ struct rte_flow_action_meter {
* direction.
*
* Multiple flows can be configured to use the same security session.
*
* Non-terminating by default.
*/
struct rte_flow_action_security {
void *security_session; /**< Pointer to security session structure. */