From patchwork Wed Oct 24 12:36:14 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Shahaf Shuler X-Patchwork-Id: 47323 X-Patchwork-Delegate: shahafs@mellanox.com Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 898DF1B212; Wed, 24 Oct 2018 14:38:55 +0200 (CEST) Received: from mellanox.co.il (mail-il-dmz.mellanox.com [193.47.165.129]) by dpdk.org (Postfix) with ESMTP id F0D0D1B1FF for ; Wed, 24 Oct 2018 14:38:49 +0200 (CEST) Received: from Internal Mail-Server by MTLPINE1 (envelope-from shahafs@mellanox.com) with ESMTPS (AES256-SHA encrypted); 24 Oct 2018 14:43:54 +0200 Received: from unicorn01.mtl.labs.mlnx. (unicorn01.mtl.labs.mlnx [10.7.12.62]) by labmailer.mlnx (8.13.8/8.13.8) with ESMTP id w9OCaLcu001458; Wed, 24 Oct 2018 15:38:46 +0300 From: Shahaf Shuler To: shahafs@mellanox.com Cc: dev@dpdk.org, yskoh@mellanox.com, orika@mellanox.com Date: Wed, 24 Oct 2018 15:36:14 +0300 Message-Id: <8493774f50d4044312dfda70f0a096d135e6e3ea.1540384475.git.shahafs@mellanox.com> X-Mailer: git-send-email 2.12.0 In-Reply-To: References: Subject: [dpdk-dev] [PATCH 2/3] net/mlx5: fix flow tunnel handling 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" From: Yongseok Koh Both rte_flow and mlx5_flow redundantly have item flags. And it is not properly set in the code. This causes wrong tunnel flag handling. A rte_flow can have multiple expanded device flows if the flow has an RSS action. Therefore, mlx5_flow should have the layers field. Fixes: c4d9b9f7f382 ("net/mlx5: add Direct Verbs final functions") Fixes: fc2c498ccb94 ("net/mlx5: add Direct Verbs translate items") Fixes: 3d69434113d1 ("net/mlx5: add Direct Verbs validation function") Fixes: 84c406e74524 ("net/mlx5: add flow translate function") Fixes: 4e05a229c5da ("net/mlx5: add flow prepare function") Fixes: 23c1d42c7138 ("net/mlx5: split flow validation to dedicated function") Cc: orika@mellanox.com Signed-off-by: Yongseok Koh --- drivers/net/mlx5/mlx5_flow.c | 69 +++++++++++++++++++++++++++------ drivers/net/mlx5/mlx5_flow.h | 8 ++-- drivers/net/mlx5/mlx5_flow_dv.c | 12 +++--- drivers/net/mlx5/mlx5_flow_verbs.c | 15 ++++--- 4 files changed, 77 insertions(+), 27 deletions(-) diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c index 68eb7da3f6..0d9a03b632 100644 --- a/drivers/net/mlx5/mlx5_flow.c +++ b/drivers/net/mlx5/mlx5_flow.c @@ -525,20 +525,22 @@ flow_rxq_tunnel_ptype_update(struct mlx5_rxq_ctrl *rxq_ctrl) } /** - * Set the Rx queue flags (Mark/Flag and Tunnel Ptypes) according to the flow. + * Set the Rx queue flags (Mark/Flag and Tunnel Ptypes) according to the devive + * flow. * * @param[in] dev * Pointer to the Ethernet device structure. - * @param[in] flow - * Pointer to flow structure. + * @param[in] dev_flow + * Pointer to device flow structure. */ static void -flow_rxq_flags_set(struct rte_eth_dev *dev, struct rte_flow *flow) +flow_drv_rxq_flags_set(struct rte_eth_dev *dev, struct mlx5_flow *dev_flow) { struct priv *priv = dev->data->dev_private; + struct rte_flow *flow = dev_flow->flow; const int mark = !!(flow->actions & (MLX5_FLOW_ACTION_FLAG | MLX5_FLOW_ACTION_MARK)); - const int tunnel = !!(flow->layers & MLX5_FLOW_LAYER_TUNNEL); + const int tunnel = !!(dev_flow->layers & MLX5_FLOW_LAYER_TUNNEL); unsigned int i; for (i = 0; i != flow->rss.queue_num; ++i) { @@ -556,7 +558,8 @@ flow_rxq_flags_set(struct rte_eth_dev *dev, struct rte_flow *flow) /* Increase the counter matching the flow. */ for (j = 0; j != MLX5_FLOW_TUNNEL; ++j) { - if ((tunnels_info[j].tunnel & flow->layers) == + if ((tunnels_info[j].tunnel & + dev_flow->layers) == tunnels_info[j].tunnel) { rxq_ctrl->flow_tunnels_n[j]++; break; @@ -568,21 +571,39 @@ flow_rxq_flags_set(struct rte_eth_dev *dev, struct rte_flow *flow) } /** + * Set the Rx queue flags (Mark/Flag and Tunnel Ptypes) for a flow + * + * @param[in] dev + * Pointer to the Ethernet device structure. + * @param[in] flow + * Pointer to flow structure. + */ +static void +flow_rxq_flags_set(struct rte_eth_dev *dev, struct rte_flow *flow) +{ + struct mlx5_flow *dev_flow; + + LIST_FOREACH(dev_flow, &flow->dev_flows, next) + flow_drv_rxq_flags_set(dev, dev_flow); +} + +/** * Clear the Rx queue flags (Mark/Flag and Tunnel Ptype) associated with the - * @p flow if no other flow uses it with the same kind of request. + * device flow if no other flow uses it with the same kind of request. * * @param dev * Pointer to Ethernet device. - * @param[in] flow - * Pointer to the flow. + * @param[in] dev_flow + * Pointer to the device flow. */ static void -flow_rxq_flags_trim(struct rte_eth_dev *dev, struct rte_flow *flow) +flow_drv_rxq_flags_trim(struct rte_eth_dev *dev, struct mlx5_flow *dev_flow) { struct priv *priv = dev->data->dev_private; + struct rte_flow *flow = dev_flow->flow; const int mark = !!(flow->actions & (MLX5_FLOW_ACTION_FLAG | MLX5_FLOW_ACTION_MARK)); - const int tunnel = !!(flow->layers & MLX5_FLOW_LAYER_TUNNEL); + const int tunnel = !!(dev_flow->layers & MLX5_FLOW_LAYER_TUNNEL); unsigned int i; assert(dev->data->dev_started); @@ -601,7 +622,8 @@ flow_rxq_flags_trim(struct rte_eth_dev *dev, struct rte_flow *flow) /* Decrease the counter matching the flow. */ for (j = 0; j != MLX5_FLOW_TUNNEL; ++j) { - if ((tunnels_info[j].tunnel & flow->layers) == + if ((tunnels_info[j].tunnel & + dev_flow->layers) == tunnels_info[j].tunnel) { rxq_ctrl->flow_tunnels_n[j]--; break; @@ -613,6 +635,24 @@ flow_rxq_flags_trim(struct rte_eth_dev *dev, struct rte_flow *flow) } /** + * Clear the Rx queue flags (Mark/Flag and Tunnel Ptype) associated with the + * @p flow if no other flow uses it with the same kind of request. + * + * @param dev + * Pointer to Ethernet device. + * @param[in] flow + * Pointer to the flow. + */ +static void +flow_rxq_flags_trim(struct rte_eth_dev *dev, struct rte_flow *flow) +{ + struct mlx5_flow *dev_flow; + + LIST_FOREACH(dev_flow, &flow->dev_flows, next) + flow_drv_rxq_flags_trim(dev, dev_flow); +} + +/** * Clear the Mark/Flag and Tunnel ptype information in all Rx queues. * * @param dev @@ -2024,6 +2064,11 @@ flow_list_create(struct rte_eth_dev *dev, struct mlx5_flows *list, if (!dev_flow) goto error; dev_flow->flow = flow; + dev_flow->layers = item_flags; + /* Store actions once as expanded flows have same actions. */ + if (i == 0) + flow->actions = action_flags; + assert(flow->actions == action_flags); LIST_INSERT_HEAD(&flow->dev_flows, dev_flow, next); ret = flow_drv_translate(dev, dev_flow, attr, buf->entry[i].pattern, diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h index 38635c9fdb..1cc989ae91 100644 --- a/drivers/net/mlx5/mlx5_flow.h +++ b/drivers/net/mlx5/mlx5_flow.h @@ -215,7 +215,8 @@ struct mlx5_flow_verbs { struct mlx5_flow { LIST_ENTRY(mlx5_flow) next; struct rte_flow *flow; /**< Pointer to the main flow. */ - uint32_t layers; /**< Bit-fields that holds the detected layers. */ + uint32_t layers; + /**< Bit-fields of present layers, see MLX5_FLOW_LAYER_*. */ union { #ifdef HAVE_IBV_FLOW_DV_SUPPORT struct mlx5_flow_dv dv; @@ -240,15 +241,14 @@ struct mlx5_flow_counter { struct rte_flow { TAILQ_ENTRY(rte_flow) next; /**< Pointer to the next flow structure. */ enum mlx5_flow_drv_type drv_type; /**< Drvier type. */ - uint32_t layers; - /**< Bit-fields of present layers see MLX5_FLOW_LAYER_*. */ struct mlx5_flow_counter *counter; /**< Holds flow counter. */ struct rte_flow_action_rss rss;/**< RSS context. */ uint8_t key[MLX5_RSS_HASH_KEY_LEN]; /**< RSS hash key. */ uint16_t (*queue)[]; /**< Destination queues to redirect traffic to. */ LIST_HEAD(dev_flows, mlx5_flow) dev_flows; /**< Device flows that are part of the flow. */ - uint32_t actions; /**< Bit-fields which mark all detected actions. */ + uint32_t actions; + /**< Bit-fields of detected actions, see MLX5_FLOW_ACTION_*. */ }; typedef int (*mlx5_flow_validate_t)(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c index 071f31d0fd..53e9a170c3 100644 --- a/drivers/net/mlx5/mlx5_flow_dv.c +++ b/drivers/net/mlx5/mlx5_flow_dv.c @@ -177,6 +177,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, if (ret < 0) return ret; for (; items->type != RTE_FLOW_ITEM_TYPE_END; items++) { + tunnel = !!(item_flags & MLX5_FLOW_LAYER_TUNNEL); switch (items->type) { case RTE_FLOW_ITEM_TYPE_VOID: break; @@ -285,7 +286,6 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, return rte_flow_error_set(error, ENOTSUP, RTE_FLOW_ERROR_TYPE_ACTION, actions, "too many actions"); - tunnel = !!(item_flags & MLX5_FLOW_LAYER_TUNNEL); switch (actions->type) { case RTE_FLOW_ACTION_TYPE_VOID: break; @@ -1253,13 +1253,15 @@ flow_dv_translate(struct rte_eth_dev *dev, }, }; void *match_value = dev_flow->dv.value.buf; - uint8_t inner = 0; + int tunnel = 0; if (priority == MLX5_FLOW_PRIO_RSVD) priority = priv->config.flow_prio - 1; - for (; items->type != RTE_FLOW_ITEM_TYPE_END; items++) + for (; items->type != RTE_FLOW_ITEM_TYPE_END; items++) { + tunnel = !!(dev_flow->layers & MLX5_FLOW_LAYER_TUNNEL); flow_dv_create_item(&matcher, match_value, items, dev_flow, - inner); + tunnel); + } matcher.crc = rte_raw_cksum((const void *)matcher.mask.buf, matcher.mask.size); if (priority == MLX5_FLOW_PRIO_RSVD) @@ -1324,7 +1326,7 @@ flow_dv_apply(struct rte_eth_dev *dev, struct rte_flow *flow, (dev, flow->key, MLX5_RSS_HASH_KEY_LEN, dv->hash_fields, (*flow->queue), flow->rss.queue_num, - !!(flow->layers & + !!(dev_flow->layers & MLX5_FLOW_LAYER_TUNNEL)); if (!hrxq) { rte_flow_error_set diff --git a/drivers/net/mlx5/mlx5_flow_verbs.c b/drivers/net/mlx5/mlx5_flow_verbs.c index 4ae974b072..75b950e16f 100644 --- a/drivers/net/mlx5/mlx5_flow_verbs.c +++ b/drivers/net/mlx5/mlx5_flow_verbs.c @@ -343,7 +343,7 @@ flow_verbs_translate_item_ipv6(const struct rte_flow_item *item, { const struct rte_flow_item_ipv6 *spec = item->spec; const struct rte_flow_item_ipv6 *mask = item->mask; - const int tunnel = !!(dev_flow->flow->layers & MLX5_FLOW_LAYER_TUNNEL); + const int tunnel = !!(dev_flow->layers & MLX5_FLOW_LAYER_TUNNEL); unsigned int size = sizeof(struct ibv_flow_spec_ipv6); struct ibv_flow_spec_ipv6 ipv6 = { .type = IBV_FLOW_SPEC_IPV6 | (tunnel ? IBV_FLOW_SPEC_INNER : 0), @@ -467,7 +467,7 @@ flow_verbs_translate_item_tcp(const struct rte_flow_item *item, { const struct rte_flow_item_tcp *spec = item->spec; const struct rte_flow_item_tcp *mask = item->mask; - const int tunnel = !!(dev_flow->flow->layers & MLX5_FLOW_LAYER_TUNNEL); + const int tunnel = !!(dev_flow->layers & MLX5_FLOW_LAYER_TUNNEL); unsigned int size = sizeof(struct ibv_flow_spec_tcp_udp); struct ibv_flow_spec_tcp_udp tcp = { .type = IBV_FLOW_SPEC_TCP | (tunnel ? IBV_FLOW_SPEC_INNER : 0), @@ -999,6 +999,8 @@ flow_verbs_validate(struct rte_eth_dev *dev, return ret; for (; items->type != RTE_FLOW_ITEM_TYPE_END; items++) { int ret = 0; + + tunnel = !!(item_flags & MLX5_FLOW_LAYER_TUNNEL); switch (items->type) { case RTE_FLOW_ITEM_TYPE_VOID: break; @@ -1110,7 +1112,6 @@ flow_verbs_validate(struct rte_eth_dev *dev, } } for (; actions->type != RTE_FLOW_ACTION_TYPE_END; actions++) { - tunnel = !!(item_flags & MLX5_FLOW_LAYER_TUNNEL); switch (actions->type) { case RTE_FLOW_ACTION_TYPE_VOID: break; @@ -1252,9 +1253,10 @@ flow_verbs_get_items_and_size(const struct rte_flow_item items[], { int size = 0; uint64_t detected_items = 0; - const int tunnel = !!(*item_flags & MLX5_FLOW_LAYER_TUNNEL); for (; items->type != RTE_FLOW_ITEM_TYPE_END; items++) { + int tunnel = !!(detected_items & MLX5_FLOW_LAYER_TUNNEL); + switch (items->type) { case RTE_FLOW_ITEM_TYPE_VOID: break; @@ -1450,7 +1452,8 @@ flow_verbs_translate(struct rte_eth_dev *dev, "action not supported"); } } - dev_flow->flow->actions |= action_flags; + /* Device flow should have action flags by flow_drv_prepare(). */ + assert(dev_flow->flow->actions == action_flags); for (; items->type != RTE_FLOW_ITEM_TYPE_END; items++) { switch (items->type) { case RTE_FLOW_ITEM_TYPE_VOID: @@ -1613,7 +1616,7 @@ flow_verbs_apply(struct rte_eth_dev *dev, struct rte_flow *flow, verbs->hash_fields, (*flow->queue), flow->rss.queue_num, - !!(flow->layers & + !!(dev_flow->layers & MLX5_FLOW_LAYER_TUNNEL)); if (!hrxq) { rte_flow_error_set