From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 12E9DA0093 for ; Tue, 19 May 2020 15:11:34 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id ECCAD1D702; Tue, 19 May 2020 15:11:33 +0200 (CEST) Received: from mail-wm1-f68.google.com (mail-wm1-f68.google.com [209.85.128.68]) by dpdk.org (Postfix) with ESMTP id 61DAC1D702 for ; Tue, 19 May 2020 15:11:32 +0200 (CEST) Received: by mail-wm1-f68.google.com with SMTP id z4so3009643wmi.2 for ; Tue, 19 May 2020 06:11:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=rilkvi9qybl9hLb0FQm10oLYCWc0MUGViHngCyQcBbU=; b=BYCpYON6qlF29zGi7SbGOctlWHalLGdTuCeFSllUxJi1Eix4mCIZomOrwOqD5EWm/m YD0Hr9RTslp4uKCVwTFY5g35KEh6gRb22d6zdC4xM8jdumL/BVlBg6m7Qoh9mNlUSAZm En7B33NpS4+SXPKUuQIklLCHFtNL4+r+Q6oHuTJKl1tPPTC7pi1DQC2UinIfRXDHUgxA hVjrOLSjtYSCAn3jFyXxip5yW/anBY4gYEHT30VxqFvDkoowo5OYkKqXlATg9mRZza3Z P0ZLRglPKjbJG+BC8UQfflFZUXNbKs3+kQySEQFPcLLWS0lBo/l1KQdgHXcNY72HbAUD HGAg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=rilkvi9qybl9hLb0FQm10oLYCWc0MUGViHngCyQcBbU=; b=sbOqtBJDuwHIaWDoN2V139fMlvty8ck4Z3eimDn/o+KYsGjOAAUsQO2rxWYYg1jTfp BprtkJgWAyHVWWyT5+XWNf/wTl4kL3BnvjIb1E7+LeCbkvLSY1azhrk7eC6QtP+XUidx uP/85b2jQXF4j13BvH6YNHNxzZeNxuHTuzF7l5SAp4WQF5RGPlcWiMju95JArJpxuJEe D74gilM4cuX3vq1QV7is6hWjQTYgZ1D3lr0zBId2Rrp3MLJ0lIJIhmnxxdnsY9fNnwtT LN7wSvmZT9FKJ4Ep6mayBFMJV6MAqRkL0t6G4FqZTkHwElU8WjtsZ9VOCiK9Z9xKv7OM poOA== X-Gm-Message-State: AOAM532GJzkht2Vtw4/P8jZUzqyQr6a+hC/HB4KwlLkmSn3RFJg42tTl 3IQQRLht2smowtjsxKnSdws= X-Google-Smtp-Source: ABdhPJx8niFR03rn7/oC+iRA3vESVTSio0plhMydvILndSIUS5BGpw+09DirA8rGtYFQHxCWBtW2Kw== X-Received: by 2002:a1c:6884:: with SMTP id d126mr5391803wmc.179.1589893891911; Tue, 19 May 2020 06:11:31 -0700 (PDT) Received: from localhost ([88.98.246.218]) by smtp.gmail.com with ESMTPSA id r9sm3809046wmg.47.2020.05.19.06.11.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 19 May 2020 06:11:31 -0700 (PDT) From: luca.boccassi@gmail.com To: Bing Zhao Cc: Viacheslav Ovsiienko , dpdk stable Date: Tue, 19 May 2020 14:04:01 +0100 Message-Id: <20200519130549.112823-106-luca.boccassi@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200519130549.112823-1-luca.boccassi@gmail.com> References: <20200519125804.104349-1-luca.boccassi@gmail.com> <20200519130549.112823-1-luca.boccassi@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [dpdk-stable] patch 'net/mlx5: fix header modify action validation' has been queued to stable release 19.11.3 X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Sender: "stable" Hi, FYI, your patch has been queued to stable release 19.11.3 Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet. It will be pushed if I get no objections before 05/21/20. So please shout if anyone has objections. Also note that after the patch there's a diff of the upstream commit vs the patch applied to the branch. This will indicate if there was any rebasing needed to apply to the stable branch. If there were code changes for rebasing (ie: not only metadata diffs), please double check that the rebase was correctly done. Thanks. Luca Boccassi --- >From bf10f18c64b7ab27ec38682f4eefe4702a7ef9b2 Mon Sep 17 00:00:00 2001 From: Bing Zhao Date: Tue, 21 Apr 2020 22:03:34 +0800 Subject: [PATCH] net/mlx5: fix header modify action validation [ upstream commit 72a944dba1639fff11f74c5ae1ab7d622bcc39fd ] The header modify actions number supported now has some limitation, and it is decided by both driver and hardware. If the configuration is different or the table to insert the flow is different, the result might be different if the flow contains header modify actions. Currently, the actual action number could only be calculated in the later stage called translate, from user specified value to the driver format. And the action numbers checking is missed in the flow validation. So PMD will return incorrect result to indicate the flow actions are valid by rte_flow_validate but then it will fail when calling rte_flow_create. Adding some simple checking in the validation will help to get rid of this incorrect checking. Most of the actions will only consume 1 SW action field except the MAC address and IPv6 address. And from SW POV, the maximal action fields for these will be consumed even if only part of such field will be modified because that there is no mask in the flow actions and the mask will always be all ONEs. The metering or extra metadata supports will cost one more action. Fixes: 9597330c6844 ("net/mlx5: update modify header action translator") Signed-off-by: Bing Zhao Acked-by: Viacheslav Ovsiienko --- drivers/net/mlx5/mlx5_flow.c | 58 +++++++++++++++------------- drivers/net/mlx5/mlx5_flow.h | 18 +++++++++ drivers/net/mlx5/mlx5_flow_dv.c | 61 ++++++++++++++++++++++++++---- drivers/net/mlx5/mlx5_flow_verbs.c | 3 ++ 4 files changed, 106 insertions(+), 34 deletions(-) diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c index 55f514af30..05e2d4c82a 100644 --- a/drivers/net/mlx5/mlx5_flow.c +++ b/drivers/net/mlx5/mlx5_flow.c @@ -2302,6 +2302,7 @@ flow_null_validate(struct rte_eth_dev *dev __rte_unused, const struct rte_flow_item items[] __rte_unused, const struct rte_flow_action actions[] __rte_unused, bool external __rte_unused, + int hairpin __rte_unused, struct rte_flow_error *error) { return rte_flow_error_set(error, ENOTSUP, @@ -2416,6 +2417,8 @@ flow_get_drv_type(struct rte_eth_dev *dev, const struct rte_flow_attr *attr) * Pointer to the list of actions. * @param[in] external * This flow rule is created by request external to PMD. + * @param[in] hairpin + * Number of hairpin TX actions, 0 means classic flow. * @param[out] error * Pointer to the error structure. * @@ -2427,13 +2430,14 @@ flow_drv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, const struct rte_flow_item items[], const struct rte_flow_action actions[], - bool external, struct rte_flow_error *error) + bool external, int hairpin, struct rte_flow_error *error) { const struct mlx5_flow_driver_ops *fops; enum mlx5_flow_drv_type type = flow_get_drv_type(dev, attr); fops = flow_get_drv_ops(type); - return fops->validate(dev, attr, items, actions, external, error); + return fops->validate(dev, attr, items, actions, external, + hairpin, error); } /** @@ -2591,27 +2595,6 @@ flow_drv_destroy(struct rte_eth_dev *dev, struct rte_flow *flow) fops->destroy(dev, flow); } -/** - * Validate a flow supported by the NIC. - * - * @see rte_flow_validate() - * @see rte_flow_ops - */ -int -mlx5_flow_validate(struct rte_eth_dev *dev, - const struct rte_flow_attr *attr, - const struct rte_flow_item items[], - const struct rte_flow_action actions[], - struct rte_flow_error *error) -{ - int ret; - - ret = flow_drv_validate(dev, attr, items, actions, true, error); - if (ret < 0) - return ret; - return 0; -} - /** * Get RSS action from the action list. * @@ -4184,15 +4167,16 @@ flow_list_create(struct rte_eth_dev *dev, struct mlx5_flows *list, const struct rte_flow_action *p_actions_rx = actions; uint32_t i; uint32_t flow_size; - int hairpin_flow = 0; + int hairpin_flow; uint32_t hairpin_id = 0; struct rte_flow_attr attr_tx = { .priority = 0 }; - int ret = flow_drv_validate(dev, attr, items, p_actions_rx, external, - error); + int ret; + hairpin_flow = flow_check_hairpin_split(dev, attr, actions); + ret = flow_drv_validate(dev, attr, items, p_actions_rx, + external, hairpin_flow, error); if (ret < 0) return NULL; - hairpin_flow = flow_check_hairpin_split(dev, attr, actions); if (hairpin_flow > 0) { if (hairpin_flow > MLX5_MAX_SPLIT_ACTIONS) { rte_errno = EINVAL; @@ -4369,6 +4353,26 @@ mlx5_flow_create_esw_table_zero_flow(struct rte_eth_dev *dev) actions, false, &error); } +/** + * Validate a flow supported by the NIC. + * + * @see rte_flow_validate() + * @see rte_flow_ops + */ +int +mlx5_flow_validate(struct rte_eth_dev *dev, + const struct rte_flow_attr *attr, + const struct rte_flow_item items[], + const struct rte_flow_action actions[], + struct rte_flow_error *error) +{ + int hairpin_flow; + + hairpin_flow = flow_check_hairpin_split(dev, attr, actions); + return flow_drv_validate(dev, attr, items, actions, + true, hairpin_flow, error); +} + /** * Create a flow. * diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h index 6d278c63d8..4e38d3fa5a 100644 --- a/drivers/net/mlx5/mlx5_flow.h +++ b/drivers/net/mlx5/mlx5_flow.h @@ -330,6 +330,23 @@ enum mlx5_feature_name { #define MLX5_ENCAPSULATION_DECISION_SIZE (sizeof(struct rte_flow_item_eth) + \ sizeof(struct rte_flow_item_ipv4)) +/* Software header modify action numbers of a flow. */ +#define MLX5_ACT_NUM_MDF_IPV4 1 +#define MLX5_ACT_NUM_MDF_IPV6 4 +#define MLX5_ACT_NUM_MDF_MAC 2 +#define MLX5_ACT_NUM_MDF_VID 1 +#define MLX5_ACT_NUM_MDF_PORT 2 +#define MLX5_ACT_NUM_MDF_TTL 1 +#define MLX5_ACT_NUM_DEC_TTL MLX5_ACT_NUM_MDF_TTL +#define MLX5_ACT_NUM_MDF_TCPSEQ 1 +#define MLX5_ACT_NUM_MDF_TCPACK 1 +#define MLX5_ACT_NUM_SET_REG 1 +#define MLX5_ACT_NUM_SET_TAG 1 +#define MLX5_ACT_NUM_CPY_MREG MLX5_ACT_NUM_SET_TAG +#define MLX5_ACT_NUM_SET_MARK MLX5_ACT_NUM_SET_TAG +#define MLX5_ACT_NUM_SET_META MLX5_ACT_NUM_SET_TAG +#define MLX5_ACT_NUM_SET_DSCP 1 + enum mlx5_flow_drv_type { MLX5_FLOW_TYPE_MIN, MLX5_FLOW_TYPE_DV, @@ -677,6 +694,7 @@ typedef int (*mlx5_flow_validate_t)(struct rte_eth_dev *dev, const struct rte_flow_item items[], const struct rte_flow_action actions[], bool external, + int hairpin, struct rte_flow_error *error); typedef struct mlx5_flow *(*mlx5_flow_prepare_t) (const struct rte_flow_attr *attr, const struct rte_flow_item items[], diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c index acc0fd3271..8d326f0229 100644 --- a/drivers/net/mlx5/mlx5_flow_dv.c +++ b/drivers/net/mlx5/mlx5_flow_dv.c @@ -427,6 +427,7 @@ flow_dv_convert_modify_action(struct rte_flow_item *item, /* Fetch variable byte size mask from the array. */ mask = flow_dv_fetch_field((const uint8_t *)item->mask + field->offset, field->size); + assert(mask); if (!mask) { ++field; continue; @@ -4285,7 +4286,9 @@ flow_dv_counter_release(struct rte_eth_dev *dev, * Pointer to error structure. * * @return - * 0 on success, a negative errno value otherwise and rte_errno is set. + * - 0 on success and non root table. + * - 1 on success and root table. + * - a negative errno value otherwise and rte_errno is set. */ static int flow_dv_validate_attributes(struct rte_eth_dev *dev, @@ -4295,6 +4298,7 @@ flow_dv_validate_attributes(struct rte_eth_dev *dev, { struct mlx5_priv *priv = dev->data->dev_private; uint32_t priority_max = priv->config.flow_prio - 1; + int ret = 0; #ifndef HAVE_MLX5DV_DR if (attributes->group) @@ -4303,14 +4307,15 @@ flow_dv_validate_attributes(struct rte_eth_dev *dev, NULL, "groups are not supported"); #else - uint32_t table; - int ret; + uint32_t table = 0; ret = mlx5_flow_group_to_table(attributes, external, attributes->group, !!priv->fdb_def_rule, &table, error); if (ret) return ret; + if (!table) + ret = MLX5DV_DR_ACTION_FLAGS_ROOT_LEVEL; #endif if (attributes->priority != MLX5_FLOW_PRIO_RSVD && attributes->priority >= priority_max) @@ -4340,7 +4345,7 @@ flow_dv_validate_attributes(struct rte_eth_dev *dev, RTE_FLOW_ERROR_TYPE_ATTR, NULL, "must specify exactly one of " "ingress or egress"); - return 0; + return ret; } /** @@ -4356,6 +4361,8 @@ flow_dv_validate_attributes(struct rte_eth_dev *dev, * Pointer to the list of actions. * @param[in] external * This flow rule is created by request external to PMD. + * @param[in] hairpin + * Number of hairpin TX actions, 0 means classic flow. * @param[out] error * Pointer to the error structure. * @@ -4366,7 +4373,7 @@ static int flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, const struct rte_flow_item items[], const struct rte_flow_action actions[], - bool external, struct rte_flow_error *error) + bool external, int hairpin, struct rte_flow_error *error) { int ret; uint64_t action_flags = 0; @@ -4391,12 +4398,15 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, struct mlx5_dev_config *dev_conf = &priv->config; uint16_t queue_index = 0xFFFF; const struct rte_flow_item_vlan *vlan_m = NULL; + int16_t rw_act_num = 0; + uint64_t is_root; if (items == NULL) return -1; ret = flow_dv_validate_attributes(dev, attr, external, error); if (ret < 0) return ret; + is_root = (uint64_t)ret; for (; items->type != RTE_FLOW_ITEM_TYPE_END; items++) { int tunnel = !!(item_flags & MLX5_FLOW_LAYER_TUNNEL); int type = items->type; @@ -4664,6 +4674,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, action_flags |= MLX5_FLOW_ACTION_FLAG; ++actions_n; } + rw_act_num += MLX5_ACT_NUM_SET_MARK; break; case RTE_FLOW_ACTION_TYPE_MARK: ret = flow_dv_validate_action_mark(dev, actions, @@ -4682,6 +4693,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, action_flags |= MLX5_FLOW_ACTION_MARK; ++actions_n; } + rw_act_num += MLX5_ACT_NUM_SET_MARK; break; case RTE_FLOW_ACTION_TYPE_SET_META: ret = flow_dv_validate_action_set_meta(dev, actions, @@ -4693,6 +4705,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, if (!(action_flags & MLX5_FLOW_MODIFY_HDR_ACTIONS)) ++actions_n; action_flags |= MLX5_FLOW_ACTION_SET_META; + rw_act_num += MLX5_ACT_NUM_SET_META; break; case RTE_FLOW_ACTION_TYPE_SET_TAG: ret = flow_dv_validate_action_set_tag(dev, actions, @@ -4704,6 +4717,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, if (!(action_flags & MLX5_FLOW_MODIFY_HDR_ACTIONS)) ++actions_n; action_flags |= MLX5_FLOW_ACTION_SET_TAG; + rw_act_num += MLX5_ACT_NUM_SET_TAG; break; case RTE_FLOW_ACTION_TYPE_DROP: ret = mlx5_flow_validate_action_drop(action_flags, @@ -4781,6 +4795,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, return ret; /* Count VID with push_vlan command. */ action_flags |= MLX5_FLOW_ACTION_OF_SET_VLAN_VID; + rw_act_num += MLX5_ACT_NUM_MDF_VID; break; case RTE_FLOW_ACTION_TYPE_VXLAN_ENCAP: case RTE_FLOW_ACTION_TYPE_NVGRE_ENCAP: @@ -4842,8 +4857,15 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, RTE_FLOW_ACTION_TYPE_SET_MAC_SRC ? MLX5_FLOW_ACTION_SET_MAC_SRC : MLX5_FLOW_ACTION_SET_MAC_DST; + /* + * Even if the source and destination MAC addresses have + * overlap in the header with 4B alignment, the convert + * function will handle them separately and 4 SW actions + * will be created. And 2 actions will be added each + * time no matter how many bytes of address will be set. + */ + rw_act_num += MLX5_ACT_NUM_MDF_MAC; break; - case RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC: case RTE_FLOW_ACTION_TYPE_SET_IPV4_DST: ret = flow_dv_validate_action_modify_ipv4(action_flags, @@ -4859,6 +4881,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC ? MLX5_FLOW_ACTION_SET_IPV4_SRC : MLX5_FLOW_ACTION_SET_IPV4_DST; + rw_act_num += MLX5_ACT_NUM_MDF_IPV4; break; case RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC: case RTE_FLOW_ACTION_TYPE_SET_IPV6_DST: @@ -4881,6 +4904,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC ? MLX5_FLOW_ACTION_SET_IPV6_SRC : MLX5_FLOW_ACTION_SET_IPV6_DST; + rw_act_num += MLX5_ACT_NUM_MDF_IPV6; break; case RTE_FLOW_ACTION_TYPE_SET_TP_SRC: case RTE_FLOW_ACTION_TYPE_SET_TP_DST: @@ -4897,6 +4921,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, RTE_FLOW_ACTION_TYPE_SET_TP_SRC ? MLX5_FLOW_ACTION_SET_TP_SRC : MLX5_FLOW_ACTION_SET_TP_DST; + rw_act_num += MLX5_ACT_NUM_MDF_PORT; break; case RTE_FLOW_ACTION_TYPE_DEC_TTL: case RTE_FLOW_ACTION_TYPE_SET_TTL: @@ -4913,6 +4938,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, RTE_FLOW_ACTION_TYPE_SET_TTL ? MLX5_FLOW_ACTION_SET_TTL : MLX5_FLOW_ACTION_DEC_TTL; + rw_act_num += MLX5_ACT_NUM_MDF_TTL; break; case RTE_FLOW_ACTION_TYPE_JUMP: ret = flow_dv_validate_action_jump(actions, @@ -4940,6 +4966,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, RTE_FLOW_ACTION_TYPE_INC_TCP_SEQ ? MLX5_FLOW_ACTION_INC_TCP_SEQ : MLX5_FLOW_ACTION_DEC_TCP_SEQ; + rw_act_num += MLX5_ACT_NUM_MDF_TCPSEQ; break; case RTE_FLOW_ACTION_TYPE_INC_TCP_ACK: case RTE_FLOW_ACTION_TYPE_DEC_TCP_ACK: @@ -4957,10 +4984,13 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, RTE_FLOW_ACTION_TYPE_INC_TCP_ACK ? MLX5_FLOW_ACTION_INC_TCP_ACK : MLX5_FLOW_ACTION_DEC_TCP_ACK; + rw_act_num += MLX5_ACT_NUM_MDF_TCPACK; break; - case MLX5_RTE_FLOW_ACTION_TYPE_TAG: case MLX5_RTE_FLOW_ACTION_TYPE_MARK: + break; + case MLX5_RTE_FLOW_ACTION_TYPE_TAG: case MLX5_RTE_FLOW_ACTION_TYPE_COPY_MREG: + rw_act_num += MLX5_ACT_NUM_SET_TAG; break; case RTE_FLOW_ACTION_TYPE_METER: ret = mlx5_flow_validate_action_meter(dev, @@ -4971,6 +5001,8 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, return ret; action_flags |= MLX5_FLOW_ACTION_METER; ++actions_n; + /* Meter action will add one more TAG action. */ + rw_act_num += MLX5_ACT_NUM_SET_TAG; break; default: return rte_flow_error_set(error, ENOTSUP, @@ -5043,6 +5075,21 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, NULL, "encap is not supported" " for ingress traffic"); } + /* Hairpin flow will add one more TAG action. */ + if (hairpin > 0) + rw_act_num += MLX5_ACT_NUM_SET_TAG; + /* extra metadata enabled: one more TAG action will be add. */ + if (dev_conf->dv_flow_en && + dev_conf->dv_xmeta_en != MLX5_XMETA_MODE_LEGACY && + mlx5_flow_ext_mreg_supported(dev)) + rw_act_num += MLX5_ACT_NUM_SET_TAG; + if ((uint32_t)rw_act_num >= + flow_dv_modify_hdr_action_max(dev, is_root)) { + return rte_flow_error_set(error, ENOTSUP, + RTE_FLOW_ERROR_TYPE_ACTION, + NULL, "too many header modify" + " actions to support"); + } return 0; } diff --git a/drivers/net/mlx5/mlx5_flow_verbs.c b/drivers/net/mlx5/mlx5_flow_verbs.c index c162527a5e..f66489c791 100644 --- a/drivers/net/mlx5/mlx5_flow_verbs.c +++ b/drivers/net/mlx5/mlx5_flow_verbs.c @@ -1016,6 +1016,8 @@ flow_verbs_translate_action_count(struct mlx5_flow *dev_flow, * Pointer to the list of actions. * @param[in] external * This flow rule is created by request external to PMD. + * @param[in] hairpin + * Number of hairpin TX actions, 0 means classic flow. * @param[out] error * Pointer to the error structure. * @@ -1028,6 +1030,7 @@ flow_verbs_validate(struct rte_eth_dev *dev, const struct rte_flow_item items[], const struct rte_flow_action actions[], bool external __rte_unused, + int hairpin __rte_unused, struct rte_flow_error *error) { int ret; -- 2.20.1 --- Diff of the applied patch vs upstream commit (please double-check if non-empty: --- --- - 2020-05-19 14:04:48.772446780 +0100 +++ 0106-net-mlx5-fix-header-modify-action-validation.patch 2020-05-19 14:04:44.340650620 +0100 @@ -1,8 +1,10 @@ -From 72a944dba1639fff11f74c5ae1ab7d622bcc39fd Mon Sep 17 00:00:00 2001 +From bf10f18c64b7ab27ec38682f4eefe4702a7ef9b2 Mon Sep 17 00:00:00 2001 From: Bing Zhao Date: Tue, 21 Apr 2020 22:03:34 +0800 Subject: [PATCH] net/mlx5: fix header modify action validation +[ upstream commit 72a944dba1639fff11f74c5ae1ab7d622bcc39fd ] + The header modify actions number supported now has some limitation, and it is decided by both driver and hardware. If the configuration is different or the table to insert the flow is different, the result @@ -24,22 +26,21 @@ The metering or extra metadata supports will cost one more action. Fixes: 9597330c6844 ("net/mlx5: update modify header action translator") -Cc: stable@dpdk.org Signed-off-by: Bing Zhao Acked-by: Viacheslav Ovsiienko --- - drivers/net/mlx5/mlx5_flow.c | 58 ++++++++++++++------------- + drivers/net/mlx5/mlx5_flow.c | 58 +++++++++++++++------------- drivers/net/mlx5/mlx5_flow.h | 18 +++++++++ - drivers/net/mlx5/mlx5_flow_dv.c | 63 ++++++++++++++++++++++++++---- + drivers/net/mlx5/mlx5_flow_dv.c | 61 ++++++++++++++++++++++++++---- drivers/net/mlx5/mlx5_flow_verbs.c | 3 ++ - 4 files changed, 108 insertions(+), 34 deletions(-) + 4 files changed, 106 insertions(+), 34 deletions(-) diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c -index 109d71e923..84aa069db4 100644 +index 55f514af30..05e2d4c82a 100644 --- a/drivers/net/mlx5/mlx5_flow.c +++ b/drivers/net/mlx5/mlx5_flow.c -@@ -2342,6 +2342,7 @@ flow_null_validate(struct rte_eth_dev *dev __rte_unused, +@@ -2302,6 +2302,7 @@ flow_null_validate(struct rte_eth_dev *dev __rte_unused, const struct rte_flow_item items[] __rte_unused, const struct rte_flow_action actions[] __rte_unused, bool external __rte_unused, @@ -47,7 +48,7 @@ struct rte_flow_error *error) { return rte_flow_error_set(error, ENOTSUP, -@@ -2457,6 +2458,8 @@ flow_get_drv_type(struct rte_eth_dev *dev, const struct rte_flow_attr *attr) +@@ -2416,6 +2417,8 @@ flow_get_drv_type(struct rte_eth_dev *dev, const struct rte_flow_attr *attr) * Pointer to the list of actions. * @param[in] external * This flow rule is created by request external to PMD. @@ -56,7 +57,7 @@ * @param[out] error * Pointer to the error structure. * -@@ -2468,13 +2471,14 @@ flow_drv_validate(struct rte_eth_dev *dev, +@@ -2427,13 +2430,14 @@ flow_drv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, const struct rte_flow_item items[], const struct rte_flow_action actions[], @@ -73,7 +74,7 @@ } /** -@@ -2635,27 +2639,6 @@ flow_drv_destroy(struct rte_eth_dev *dev, struct rte_flow *flow) +@@ -2591,27 +2595,6 @@ flow_drv_destroy(struct rte_eth_dev *dev, struct rte_flow *flow) fops->destroy(dev, flow); } @@ -101,10 +102,10 @@ /** * Get RSS action from the action list. * -@@ -4271,15 +4254,16 @@ flow_list_create(struct rte_eth_dev *dev, uint32_t *list, +@@ -4184,15 +4167,16 @@ flow_list_create(struct rte_eth_dev *dev, struct mlx5_flows *list, const struct rte_flow_action *p_actions_rx = actions; uint32_t i; - uint32_t idx = 0; + uint32_t flow_size; - int hairpin_flow = 0; + int hairpin_flow; uint32_t hairpin_id = 0; @@ -117,13 +118,13 @@ + ret = flow_drv_validate(dev, attr, items, p_actions_rx, + external, hairpin_flow, error); if (ret < 0) - return 0; + return NULL; - hairpin_flow = flow_check_hairpin_split(dev, attr, actions); if (hairpin_flow > 0) { if (hairpin_flow > MLX5_MAX_SPLIT_ACTIONS) { rte_errno = EINVAL; -@@ -4470,6 +4454,26 @@ mlx5_flow_create_esw_table_zero_flow(struct rte_eth_dev *dev) - actions, false, &error); +@@ -4369,6 +4353,26 @@ mlx5_flow_create_esw_table_zero_flow(struct rte_eth_dev *dev) + actions, false, &error); } +/** @@ -150,10 +151,10 @@ * Create a flow. * diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h -index 062bcf1f57..2a1f59698c 100644 +index 6d278c63d8..4e38d3fa5a 100644 --- a/drivers/net/mlx5/mlx5_flow.h +++ b/drivers/net/mlx5/mlx5_flow.h -@@ -332,6 +332,23 @@ enum mlx5_feature_name { +@@ -330,6 +330,23 @@ enum mlx5_feature_name { #define MLX5_ENCAPSULATION_DECISION_SIZE (sizeof(struct rte_flow_item_eth) + \ sizeof(struct rte_flow_item_ipv4)) @@ -177,27 +178,27 @@ enum mlx5_flow_drv_type { MLX5_FLOW_TYPE_MIN, MLX5_FLOW_TYPE_DV, -@@ -812,6 +829,7 @@ typedef int (*mlx5_flow_validate_t)(struct rte_eth_dev *dev, +@@ -677,6 +694,7 @@ typedef int (*mlx5_flow_validate_t)(struct rte_eth_dev *dev, const struct rte_flow_item items[], const struct rte_flow_action actions[], bool external, + int hairpin, struct rte_flow_error *error); typedef struct mlx5_flow *(*mlx5_flow_prepare_t) - (struct rte_eth_dev *dev, const struct rte_flow_attr *attr, + (const struct rte_flow_attr *attr, const struct rte_flow_item items[], diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c -index c6d132c6b7..6263ecc731 100644 +index acc0fd3271..8d326f0229 100644 --- a/drivers/net/mlx5/mlx5_flow_dv.c +++ b/drivers/net/mlx5/mlx5_flow_dv.c -@@ -434,6 +434,7 @@ flow_dv_convert_modify_action(struct rte_flow_item *item, +@@ -427,6 +427,7 @@ flow_dv_convert_modify_action(struct rte_flow_item *item, /* Fetch variable byte size mask from the array. */ mask = flow_dv_fetch_field((const uint8_t *)item->mask + field->offset, field->size); -+ MLX5_ASSERT(mask); ++ assert(mask); if (!mask) { ++field; continue; -@@ -4490,7 +4491,9 @@ flow_dv_counter_release(struct rte_eth_dev *dev, uint32_t counter) +@@ -4285,7 +4286,9 @@ flow_dv_counter_release(struct rte_eth_dev *dev, * Pointer to error structure. * * @return @@ -208,7 +209,7 @@ */ static int flow_dv_validate_attributes(struct rte_eth_dev *dev, -@@ -4500,6 +4503,7 @@ flow_dv_validate_attributes(struct rte_eth_dev *dev, +@@ -4295,6 +4298,7 @@ flow_dv_validate_attributes(struct rte_eth_dev *dev, { struct mlx5_priv *priv = dev->data->dev_private; uint32_t priority_max = priv->config.flow_prio - 1; @@ -216,7 +217,7 @@ #ifndef HAVE_MLX5DV_DR if (attributes->group) -@@ -4508,14 +4512,15 @@ flow_dv_validate_attributes(struct rte_eth_dev *dev, +@@ -4303,14 +4307,15 @@ flow_dv_validate_attributes(struct rte_eth_dev *dev, NULL, "groups are not supported"); #else @@ -234,7 +235,7 @@ #endif if (attributes->priority != MLX5_FLOW_PRIO_RSVD && attributes->priority >= priority_max) -@@ -4545,7 +4550,7 @@ flow_dv_validate_attributes(struct rte_eth_dev *dev, +@@ -4340,7 +4345,7 @@ flow_dv_validate_attributes(struct rte_eth_dev *dev, RTE_FLOW_ERROR_TYPE_ATTR, NULL, "must specify exactly one of " "ingress or egress"); @@ -243,7 +244,7 @@ } /** -@@ -4561,6 +4566,8 @@ flow_dv_validate_attributes(struct rte_eth_dev *dev, +@@ -4356,6 +4361,8 @@ flow_dv_validate_attributes(struct rte_eth_dev *dev, * Pointer to the list of actions. * @param[in] external * This flow rule is created by request external to PMD. @@ -252,7 +253,7 @@ * @param[out] error * Pointer to the error structure. * -@@ -4571,7 +4578,7 @@ static int +@@ -4366,7 +4373,7 @@ static int flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, const struct rte_flow_item items[], const struct rte_flow_action actions[], @@ -261,7 +262,7 @@ { int ret; uint64_t action_flags = 0; -@@ -4618,12 +4625,15 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, +@@ -4391,12 +4398,15 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, struct mlx5_dev_config *dev_conf = &priv->config; uint16_t queue_index = 0xFFFF; const struct rte_flow_item_vlan *vlan_m = NULL; @@ -277,7 +278,7 @@ for (; items->type != RTE_FLOW_ITEM_TYPE_END; items++) { int tunnel = !!(item_flags & MLX5_FLOW_LAYER_TUNNEL); int type = items->type; -@@ -4900,6 +4910,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, +@@ -4664,6 +4674,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, action_flags |= MLX5_FLOW_ACTION_FLAG; ++actions_n; } @@ -285,7 +286,7 @@ break; case RTE_FLOW_ACTION_TYPE_MARK: ret = flow_dv_validate_action_mark(dev, actions, -@@ -4918,6 +4929,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, +@@ -4682,6 +4693,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, action_flags |= MLX5_FLOW_ACTION_MARK; ++actions_n; } @@ -293,7 +294,7 @@ break; case RTE_FLOW_ACTION_TYPE_SET_META: ret = flow_dv_validate_action_set_meta(dev, actions, -@@ -4929,6 +4941,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, +@@ -4693,6 +4705,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, if (!(action_flags & MLX5_FLOW_MODIFY_HDR_ACTIONS)) ++actions_n; action_flags |= MLX5_FLOW_ACTION_SET_META; @@ -301,7 +302,7 @@ break; case RTE_FLOW_ACTION_TYPE_SET_TAG: ret = flow_dv_validate_action_set_tag(dev, actions, -@@ -4940,6 +4953,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, +@@ -4704,6 +4717,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, if (!(action_flags & MLX5_FLOW_MODIFY_HDR_ACTIONS)) ++actions_n; action_flags |= MLX5_FLOW_ACTION_SET_TAG; @@ -309,7 +310,7 @@ break; case RTE_FLOW_ACTION_TYPE_DROP: ret = mlx5_flow_validate_action_drop(action_flags, -@@ -5017,6 +5031,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, +@@ -4781,6 +4795,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, return ret; /* Count VID with push_vlan command. */ action_flags |= MLX5_FLOW_ACTION_OF_SET_VLAN_VID; @@ -317,7 +318,7 @@ break; case RTE_FLOW_ACTION_TYPE_VXLAN_ENCAP: case RTE_FLOW_ACTION_TYPE_NVGRE_ENCAP: -@@ -5078,8 +5093,15 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, +@@ -4842,8 +4857,15 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, RTE_FLOW_ACTION_TYPE_SET_MAC_SRC ? MLX5_FLOW_ACTION_SET_MAC_SRC : MLX5_FLOW_ACTION_SET_MAC_DST; @@ -334,7 +335,7 @@ case RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC: case RTE_FLOW_ACTION_TYPE_SET_IPV4_DST: ret = flow_dv_validate_action_modify_ipv4(action_flags, -@@ -5095,6 +5117,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, +@@ -4859,6 +4881,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC ? MLX5_FLOW_ACTION_SET_IPV4_SRC : MLX5_FLOW_ACTION_SET_IPV4_DST; @@ -342,7 +343,7 @@ break; case RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC: case RTE_FLOW_ACTION_TYPE_SET_IPV6_DST: -@@ -5117,6 +5140,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, +@@ -4881,6 +4904,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC ? MLX5_FLOW_ACTION_SET_IPV6_SRC : MLX5_FLOW_ACTION_SET_IPV6_DST; @@ -350,7 +351,7 @@ break; case RTE_FLOW_ACTION_TYPE_SET_TP_SRC: case RTE_FLOW_ACTION_TYPE_SET_TP_DST: -@@ -5133,6 +5157,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, +@@ -4897,6 +4921,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, RTE_FLOW_ACTION_TYPE_SET_TP_SRC ? MLX5_FLOW_ACTION_SET_TP_SRC : MLX5_FLOW_ACTION_SET_TP_DST; @@ -358,7 +359,7 @@ break; case RTE_FLOW_ACTION_TYPE_DEC_TTL: case RTE_FLOW_ACTION_TYPE_SET_TTL: -@@ -5149,6 +5174,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, +@@ -4913,6 +4938,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, RTE_FLOW_ACTION_TYPE_SET_TTL ? MLX5_FLOW_ACTION_SET_TTL : MLX5_FLOW_ACTION_DEC_TTL; @@ -366,7 +367,7 @@ break; case RTE_FLOW_ACTION_TYPE_JUMP: ret = flow_dv_validate_action_jump(actions, -@@ -5176,6 +5202,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, +@@ -4940,6 +4966,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, RTE_FLOW_ACTION_TYPE_INC_TCP_SEQ ? MLX5_FLOW_ACTION_INC_TCP_SEQ : MLX5_FLOW_ACTION_DEC_TCP_SEQ; @@ -374,7 +375,7 @@ break; case RTE_FLOW_ACTION_TYPE_INC_TCP_ACK: case RTE_FLOW_ACTION_TYPE_DEC_TCP_ACK: -@@ -5193,10 +5220,13 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, +@@ -4957,10 +4984,13 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, RTE_FLOW_ACTION_TYPE_INC_TCP_ACK ? MLX5_FLOW_ACTION_INC_TCP_ACK : MLX5_FLOW_ACTION_DEC_TCP_ACK; @@ -389,32 +390,16 @@ break; case RTE_FLOW_ACTION_TYPE_METER: ret = mlx5_flow_validate_action_meter(dev, -@@ -5207,6 +5237,8 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, +@@ -4971,6 +5001,8 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, return ret; action_flags |= MLX5_FLOW_ACTION_METER; ++actions_n; + /* Meter action will add one more TAG action. */ + rw_act_num += MLX5_ACT_NUM_SET_TAG; break; - case RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP: - ret = flow_dv_validate_action_modify_ipv4_dscp -@@ -5220,6 +5252,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, - if (!(action_flags & MLX5_FLOW_MODIFY_HDR_ACTIONS)) - ++actions_n; - action_flags |= MLX5_FLOW_ACTION_SET_IPV4_DSCP; -+ rw_act_num += MLX5_ACT_NUM_SET_DSCP; - break; - case RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP: - ret = flow_dv_validate_action_modify_ipv6_dscp -@@ -5233,6 +5266,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, - if (!(action_flags & MLX5_FLOW_MODIFY_HDR_ACTIONS)) - ++actions_n; - action_flags |= MLX5_FLOW_ACTION_SET_IPV6_DSCP; -+ rw_act_num += MLX5_ACT_NUM_SET_DSCP; - break; default: return rte_flow_error_set(error, ENOTSUP, -@@ -5305,6 +5339,21 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, +@@ -5043,6 +5075,21 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr, NULL, "encap is not supported" " for ingress traffic"); } @@ -437,10 +422,10 @@ } diff --git a/drivers/net/mlx5/mlx5_flow_verbs.c b/drivers/net/mlx5/mlx5_flow_verbs.c -index 722220c3d3..d20098ce45 100644 +index c162527a5e..f66489c791 100644 --- a/drivers/net/mlx5/mlx5_flow_verbs.c +++ b/drivers/net/mlx5/mlx5_flow_verbs.c -@@ -1108,6 +1108,8 @@ flow_verbs_translate_action_count(struct mlx5_flow *dev_flow, +@@ -1016,6 +1016,8 @@ flow_verbs_translate_action_count(struct mlx5_flow *dev_flow, * Pointer to the list of actions. * @param[in] external * This flow rule is created by request external to PMD. @@ -449,7 +434,7 @@ * @param[out] error * Pointer to the error structure. * -@@ -1120,6 +1122,7 @@ flow_verbs_validate(struct rte_eth_dev *dev, +@@ -1028,6 +1030,7 @@ flow_verbs_validate(struct rte_eth_dev *dev, const struct rte_flow_item items[], const struct rte_flow_action actions[], bool external __rte_unused,