From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f44.google.com (mail-wm0-f44.google.com [74.125.82.44]) by dpdk.org (Postfix) with ESMTP id B9F628D37 for ; Wed, 25 Apr 2018 17:28:01 +0200 (CEST) Received: by mail-wm0-f44.google.com with SMTP id j4so7881792wme.1 for ; Wed, 25 Apr 2018 08:28:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=cX5zfDbWKzEErISRh1iQbSWcCnbeGnOsL4EBVVduzs0=; b=jcpXsgpj6fUcipKQOnfH18y2aWUdfe1eerDW++AC7UKX1gEzb0I+30Pa0sHkIdWQsl vU+szhCQ078+W8Zq740BhX27H22smbMvW04VXBBfab5Lpo5JCUaGjpI5AH3jF2+HR/0O XFFYG5+0xrXV2tZfNVBGJoqiQWTIBP18zTo0I3PWAmu/kQd9xYV58Po5xe5Fs42yCrl0 2uoFSt2DDl15ZAjcBZdRKS+ha+6WH/VtuG4aErcolbin4E9IWi+qO27pdN10OL0e13Z+ 39c2Td3Ms19zUI/j5nClI7YEXRjgvm7BCfnStNE6Wc0GTIvErI8+vpEN3776yoBRvY53 evdA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=cX5zfDbWKzEErISRh1iQbSWcCnbeGnOsL4EBVVduzs0=; b=rfYAAdL2japVmW87P+5/7IFTtXzDig9ok4V7ACttwZUy9QzXEsdmCprXix40vSxU6C PmR5e3oQ+n06RlJYKh9hyOa01lNTqZR+ly3IlBEQR+G9/fLLVKNW7UY5kY7DL78Tq8r5 m8TmDu4V7Zc1zaySuFTTPh4EcQoScGfMqSKeiciWmq7dbErjYlU8bbg2t68VF/jNfH1a tzpAYiuwQRl49PslwmQJcGLH7wL5dnbTAlFP5Kz4TaOgdBSw1pFE+Ue5niLYYjWf+IqM y8gy0TsngU7FYRyYHfEo7x0nT0z7tI43bdSL1Hqnt94Og17JSgWonM2fDA3Q72arjU3O qrSQ== X-Gm-Message-State: ALQs6tBLbFW85xpvAoU8zV+bFuMtQhkKRNHTpGQtnaIl8RrfkGrdi2op 0W7wBXlZPMxG0ih2ejGfYbwjkA== X-Google-Smtp-Source: AB8JxZo6R7np41uzAdnhrCINqAyGHgXCRMGafGlKTE5z/JT5JHKuUVe9naqjsOvGwhQONlNaW09L+Q== X-Received: by 10.28.137.129 with SMTP id l123mr16343835wmd.160.1524670081246; Wed, 25 Apr 2018 08:28:01 -0700 (PDT) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id 42-v6sm3158883wrx.24.2018.04.25.08.28.00 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 25 Apr 2018 08:28:00 -0700 (PDT) Date: Wed, 25 Apr 2018 17:27:46 +0200 From: Adrien Mazarguil To: Thomas Monjalon , Ferruh Yigit , dev@dpdk.org Cc: Ajit Khaparde , Wenzhuo Lu , John Daley , Gaetan Rivet , Beilei Xing , Konstantin Ananyev , Nelio Laranjeiro , Andrew Rybchenko , Pascal Mazon Message-ID: <20180425151852.7676-6-adrien.mazarguil@6wind.com> References: <20180419100848.6178-1-adrien.mazarguil@6wind.com> <20180425151852.7676-1-adrien.mazarguil@6wind.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180425151852.7676-1-adrien.mazarguil@6wind.com> X-Mailer: git-send-email 2.11.0 Subject: [dpdk-dev] [PATCH v6 05/16] ethdev: alter behavior of flow API actions X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 25 Apr 2018 15:28:02 -0000 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 Reviewed-by: Andrew Rybchenko Cc: Ajit Khaparde Cc: Wenzhuo Lu Cc: John Daley Cc: Gaetan Rivet Cc: Beilei Xing Cc: Konstantin Ananyev Cc: Nelio Laranjeiro Cc: Andrew Rybchenko Cc: Pascal Mazon -- v6 changes: Updated API and ABI changes sections in release notes. v5 changes: Fixed issues raised by GCC and Clang with overlap checks in both enic and mlx5, as reported by Andrew [1]. [1] http://dpdk.org/ml/archives/dev/2018-April/097864.html --- doc/guides/prog_guide/rte_flow.rst | 67 +++++++++++----------------- doc/guides/rel_notes/release_18_05.rst | 15 +++++-- drivers/net/enic/enic_flow.c | 25 +++++++++++ drivers/net/mlx4/mlx4_flow.c | 21 ++++++--- drivers/net/mlx5/mlx5_flow.c | 69 +++++++++++++---------------- drivers/net/sfc/sfc_flow.c | 22 ++++++--- drivers/net/tap/tap_flow.c | 11 +++++ lib/librte_ether/rte_flow.h | 54 +++++++--------------- 8 files changed, 150 insertions(+), 134 deletions(-) diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst index a237e4fd2..80360d068 100644 --- a/doc/guides/prog_guide/rte_flow.rst +++ b/doc/guides/prog_guide/rte_flow.rst @@ -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. + +Note that such a list may be thought as ambiguous and rejected on that +basis. -.. _table_rte_flow_redirect_queue_3: +.. _table_rte_flow_redirect_queue_5_3: -.. table:: Redirect to queue 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 diff --git a/doc/guides/rel_notes/release_18_05.rst b/doc/guides/rel_notes/release_18_05.rst index d3b619216..1db6c5257 100644 --- a/doc/guides/rel_notes/release_18_05.rst +++ b/doc/guides/rel_notes/release_18_05.rst @@ -243,7 +243,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 @@ -290,8 +298,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 diff --git a/drivers/net/enic/enic_flow.c b/drivers/net/enic/enic_flow.c index b9f36587c..c34ae84d1 100644 --- a/drivers/net/enic/enic_flow.c +++ b/drivers/net/enic/enic_flow.c @@ -3,6 +3,7 @@ */ #include +#include #include #include #include @@ -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; } diff --git a/drivers/net/mlx4/mlx4_flow.c b/drivers/net/mlx4/mlx4_flow.c index 67fd568bc..15cdf07b7 100644 --- a/drivers/net/mlx4/mlx4_flow.c +++ b/drivers/net/mlx4/mlx4_flow.c @@ -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) diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c index 1807e3f5b..586e780c4 100644 --- a/drivers/net/mlx5/mlx5_flow.c +++ b/drivers/net/mlx5/mlx5_flow.c @@ -4,6 +4,7 @@ */ #include +#include #include /* 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; } /** diff --git a/drivers/net/sfc/sfc_flow.c b/drivers/net/sfc/sfc_flow.c index fe4c0b0c5..056405515 100644 --- a/drivers/net/sfc/sfc_flow.c +++ b/drivers/net/sfc/sfc_flow.c @@ -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; diff --git a/drivers/net/tap/tap_flow.c b/drivers/net/tap/tap_flow.c index 3b7a960b0..fe2f94010 100644 --- a/drivers/net/tap/tap_flow.c +++ b/drivers/net/tap/tap_flow.c @@ -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 */ diff --git a/lib/librte_ether/rte_flow.h b/lib/librte_ether/rte_flow.h index 6ace24ff4..96184f030 100644 --- a/lib/librte_ether/rte_flow.h +++ b/lib/librte_ether/rte_flow.h @@ -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. */ -- 2.11.0