* Re: [dpdk-dev] [PATCH] net/mlx5: change tunnel flow priority
2018-02-15 9:23 [dpdk-dev] [PATCH] net/mlx5: change tunnel flow priority Nelio Laranjeiro
@ 2018-02-16 11:17 ` Adrien Mazarguil
2018-03-13 14:17 ` [dpdk-dev] [PATCH 0/4] net/mlx5: multiple fix and improvements in flows Nelio Laranjeiro
` (4 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Adrien Mazarguil @ 2018-02-16 11:17 UTC (permalink / raw)
To: Nelio Laranjeiro; +Cc: dev, Yongseok Koh
A few commit log rewording suggestions for clarity and a couple of
additional comments below, otherwise patch looks fine.
On Thu, Feb 15, 2018 at 10:23:29AM +0100, Nelio Laranjeiro wrote:
> Packet matching inner and outer flow rules are catch by the first one
catch => caught
> added in the device as both flows are configured with the same priority.
> To avoid such situation, the inner flow can have an higher priority than
in the device => to the device
can => must?
> the outer ones as their pattern matching will not collide.
not => otherwise
>
> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> ---
> drivers/net/mlx5/mlx5_flow.c | 72 +++++++++++++++++++++++++++-----------------
> 1 file changed, 45 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
> index 26002c4b9..6223f1f94 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -118,7 +118,7 @@ const struct hash_rxq_init hash_rxq_init[] = {
> IBV_RX_HASH_SRC_PORT_TCP |
> IBV_RX_HASH_DST_PORT_TCP),
> .dpdk_rss_hf = ETH_RSS_NONFRAG_IPV4_TCP,
> - .flow_priority = 0,
> + .flow_priority = 1,
> .ip_version = MLX5_IPV4,
> },
> [HASH_RXQ_UDPV4] = {
> @@ -127,7 +127,7 @@ const struct hash_rxq_init hash_rxq_init[] = {
> IBV_RX_HASH_SRC_PORT_UDP |
> IBV_RX_HASH_DST_PORT_UDP),
> .dpdk_rss_hf = ETH_RSS_NONFRAG_IPV4_UDP,
> - .flow_priority = 0,
> + .flow_priority = 1,
> .ip_version = MLX5_IPV4,
> },
> [HASH_RXQ_IPV4] = {
> @@ -135,7 +135,7 @@ const struct hash_rxq_init hash_rxq_init[] = {
> IBV_RX_HASH_DST_IPV4),
> .dpdk_rss_hf = (ETH_RSS_IPV4 |
> ETH_RSS_FRAG_IPV4),
> - .flow_priority = 1,
> + .flow_priority = 2,
> .ip_version = MLX5_IPV4,
> },
> [HASH_RXQ_TCPV6] = {
> @@ -144,7 +144,7 @@ const struct hash_rxq_init hash_rxq_init[] = {
> IBV_RX_HASH_SRC_PORT_TCP |
> IBV_RX_HASH_DST_PORT_TCP),
> .dpdk_rss_hf = ETH_RSS_NONFRAG_IPV6_TCP,
> - .flow_priority = 0,
> + .flow_priority = 1,
> .ip_version = MLX5_IPV6,
> },
> [HASH_RXQ_UDPV6] = {
> @@ -153,7 +153,7 @@ const struct hash_rxq_init hash_rxq_init[] = {
> IBV_RX_HASH_SRC_PORT_UDP |
> IBV_RX_HASH_DST_PORT_UDP),
> .dpdk_rss_hf = ETH_RSS_NONFRAG_IPV6_UDP,
> - .flow_priority = 0,
> + .flow_priority = 1,
> .ip_version = MLX5_IPV6,
> },
> [HASH_RXQ_IPV6] = {
> @@ -161,13 +161,13 @@ const struct hash_rxq_init hash_rxq_init[] = {
> IBV_RX_HASH_DST_IPV6),
> .dpdk_rss_hf = (ETH_RSS_IPV6 |
> ETH_RSS_FRAG_IPV6),
> - .flow_priority = 1,
> + .flow_priority = 2,
> .ip_version = MLX5_IPV6,
> },
> [HASH_RXQ_ETH] = {
> .hash_fields = 0,
> .dpdk_rss_hf = 0,
> - .flow_priority = 2,
> + .flow_priority = 3,
> },
> };
>
> @@ -860,8 +860,6 @@ priv_flow_convert_items_validate(struct priv *priv,
> *
> * @param priv
> * Pointer to private structure.
> - * @param[in] priority
> - * Flow priority.
> * @param[in] size
> * Amount of byte to allocate.
> * @param[out] error
> @@ -872,7 +870,6 @@ priv_flow_convert_items_validate(struct priv *priv,
> */
> static struct ibv_flow_attr*
> priv_flow_convert_allocate(struct priv *priv,
> - unsigned int priority,
> unsigned int size,
> struct rte_flow_error *error)
> {
> @@ -887,11 +884,44 @@ priv_flow_convert_allocate(struct priv *priv,
> "cannot allocate verbs spec attributes.");
> return NULL;
> }
> - ibv_attr->priority = priority;
> return ibv_attr;
> }
>
> /**
> + * Make Inner packet matching with an higher priority from the non Inner
> + * matching.
Inner => inner
> + *
> + * @param priv
> + * Pointer to private structure.
> + * @param[in, out] parser
> + * Internal parser structure.
> + * @param attr
> + * User flow attribute.
> + */
> +static void
> +priv_flow_update_priority(struct priv *priv, struct mlx5_flow_parse *parser,
> + const struct rte_flow_attr *attr)
> +{
> + unsigned int i;
> +
> + (void)priv;
I suggest dropping "priv" from everywhere in this function including its
name. It's a static helper not even exposed, no need to be consistent with
other priv* functions for the sake of consistency if the parameter is
unnecessary.
> + if (parser->drop) {
> + parser->queue[HASH_RXQ_ETH].ibv_attr->priority =
> + attr->priority +
> + hash_rxq_init[HASH_RXQ_ETH].flow_priority;
> + return;
> + }
> + for (i = 0; i != hash_rxq_init_n; ++i) {
> + if (parser->queue[i].ibv_attr) {
> + parser->queue[i].ibv_attr->priority =
> + attr->priority +
> + hash_rxq_init[i].flow_priority -
> + (parser->inner ? 1 : 0);
> + }
> + }
> +}
On a personal note, I noticed how complex (convoluted?) mlx5 flow rule
handling code turned lately due to all the added features, unforeseen issues
and the resulting workarounds, partly caused by the limited number of
available priorities on the Verbs side. I think we're nearing another major
rewrite.
> +
> +/**
> * Finalise verbs flow attributes.
> *
> * @param priv
> @@ -1059,23 +1089,16 @@ priv_flow_convert(struct priv *priv,
> * Allocate the memory space to store verbs specifications.
> */
> if (parser->drop) {
> - unsigned int priority =
> - attr->priority +
> - hash_rxq_init[HASH_RXQ_ETH].flow_priority;
> unsigned int offset = parser->queue[HASH_RXQ_ETH].offset;
>
> parser->queue[HASH_RXQ_ETH].ibv_attr =
> - priv_flow_convert_allocate(priv, priority,
> - offset, error);
> + priv_flow_convert_allocate(priv, offset, error);
> if (!parser->queue[HASH_RXQ_ETH].ibv_attr)
> return ENOMEM;
> parser->queue[HASH_RXQ_ETH].offset =
> sizeof(struct ibv_flow_attr);
> } else {
> for (i = 0; i != hash_rxq_init_n; ++i) {
> - unsigned int priority =
> - attr->priority +
> - hash_rxq_init[i].flow_priority;
> unsigned int offset;
>
> if (!(parser->rss_conf.rss_hf &
> @@ -1084,8 +1107,7 @@ priv_flow_convert(struct priv *priv,
> continue;
> offset = parser->queue[i].offset;
> parser->queue[i].ibv_attr =
> - priv_flow_convert_allocate(priv, priority,
> - offset, error);
> + priv_flow_convert_allocate(priv, offset, error);
> if (!parser->queue[i].ibv_attr)
> goto exit_enomem;
> parser->queue[i].offset = sizeof(struct ibv_flow_attr);
> @@ -1120,13 +1142,9 @@ priv_flow_convert(struct priv *priv,
> * Last step. Complete missing specification to reach the RSS
> * configuration.
> */
> - if (!parser->drop) {
> + if (!parser->drop)
> priv_flow_convert_finalise(priv, parser);
> - } else {
> - parser->queue[HASH_RXQ_ETH].ibv_attr->priority =
> - attr->priority +
> - hash_rxq_init[parser->layer].flow_priority;
> - }
> + priv_flow_update_priority(priv, parser, attr);
> exit_free:
> /* Only verification is expected, all resources should be released. */
> if (!parser->create) {
> --
> 2.11.0
>
--
Adrien Mazarguil
6WIND
^ permalink raw reply [flat|nested] 15+ messages in thread* [dpdk-dev] [PATCH 0/4] net/mlx5: multiple fix and improvements in flows
2018-02-15 9:23 [dpdk-dev] [PATCH] net/mlx5: change tunnel flow priority Nelio Laranjeiro
2018-02-16 11:17 ` Adrien Mazarguil
@ 2018-03-13 14:17 ` Nelio Laranjeiro
2018-03-21 14:53 ` Shahaf Shuler
2018-03-13 14:17 ` [dpdk-dev] [PATCH 1/4] net/mlx5: change tunnel flow priority Nelio Laranjeiro
` (3 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Nelio Laranjeiro @ 2018-03-13 14:17 UTC (permalink / raw)
To: Adrien Mazarguil, Yongseok Koh; +Cc: dev
This series apply on top of [1]
Changes in v2:
- rebase on top of series [2] which drops priv locks.
[1] https://dpdk.org/dev/patchwork/patch/36058/
[2] https://dpdk.org/dev/patchwork/patch/35650/
Nelio Laranjeiro (4):
net/mlx5: change tunnel flow priority
net/mlx5: fix flow single queue
net/mlx5: improve flow error explanation
net/mlx5: refuse empty VLAN flow specification
drivers/net/mlx5/mlx5_flow.c | 164 +++++++++++++++++++++++++++----------------
1 file changed, 102 insertions(+), 62 deletions(-)
--
2.11.0
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [dpdk-dev] [PATCH 0/4] net/mlx5: multiple fix and improvements in flows
2018-03-13 14:17 ` [dpdk-dev] [PATCH 0/4] net/mlx5: multiple fix and improvements in flows Nelio Laranjeiro
@ 2018-03-21 14:53 ` Shahaf Shuler
0 siblings, 0 replies; 15+ messages in thread
From: Shahaf Shuler @ 2018-03-21 14:53 UTC (permalink / raw)
To: Nélio Laranjeiro, Adrien Mazarguil, Yongseok Koh; +Cc: dev
Tuesday, March 13, 2018 4:18 PM, Nelio Laranjeiro:
> This series apply on top of [1]
>
> Changes in v2:
> - rebase on top of series [2] which drops priv locks.
>
> [1]
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdp
> dk.org%2Fdev%2Fpatchwork%2Fpatch%2F36058%2F&data=02%7C01%7Csha
> hafs%40mellanox.com%7C6258651c419f49d449e808d588ed5ce6%7Ca652971c
> 7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636565475410509385&sdata=Y4sR
> XTP1eW42hNx75KcZ1eA8hG4MC4vwIKbvWrE1whs%3D&reserved=0
> [2]
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdp
> dk.org%2Fdev%2Fpatchwork%2Fpatch%2F35650%2F&data=02%7C01%7Csha
> hafs%40mellanox.com%7C6258651c419f49d449e808d588ed5ce6%7Ca652971c
> 7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636565475410509385&sdata=4IT
> MZRuizi25TN3f6EFv970RgZj5CzKN5CcvY21PsOE%3D&reserved=0
>
> Nelio Laranjeiro (4):
> net/mlx5: change tunnel flow priority
> net/mlx5: fix flow single queue
> net/mlx5: improve flow error explanation
> net/mlx5: refuse empty VLAN flow specification
>
> drivers/net/mlx5/mlx5_flow.c | 164 +++++++++++++++++++++++++++------
> ----------
> 1 file changed, 102 insertions(+), 62 deletions(-)
Series applied to next-net-mlx, thanks.
>
> --
> 2.11.0
^ permalink raw reply [flat|nested] 15+ messages in thread
* [dpdk-dev] [PATCH 1/4] net/mlx5: change tunnel flow priority
2018-02-15 9:23 [dpdk-dev] [PATCH] net/mlx5: change tunnel flow priority Nelio Laranjeiro
2018-02-16 11:17 ` Adrien Mazarguil
2018-03-13 14:17 ` [dpdk-dev] [PATCH 0/4] net/mlx5: multiple fix and improvements in flows Nelio Laranjeiro
@ 2018-03-13 14:17 ` Nelio Laranjeiro
2018-03-13 14:17 ` [dpdk-dev] [PATCH 2/4] net/mlx5: fix flow single queue Nelio Laranjeiro
` (2 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Nelio Laranjeiro @ 2018-03-13 14:17 UTC (permalink / raw)
To: Adrien Mazarguil, Yongseok Koh; +Cc: dev
Packet matching inner and outer flow rules are caught by the first one
added in the device as both flows are configured with the same priority.
To avoid such situation, the inner flow can have an higher priority than
the outer ones as their pattern matching will otherwise collide.
Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
---
drivers/net/mlx5/mlx5_flow.c | 71 +++++++++++++++++++++++++++-----------------
1 file changed, 43 insertions(+), 28 deletions(-)
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index e5e6be6cc..e1ffb38a5 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -118,7 +118,7 @@ const struct hash_rxq_init hash_rxq_init[] = {
IBV_RX_HASH_SRC_PORT_TCP |
IBV_RX_HASH_DST_PORT_TCP),
.dpdk_rss_hf = ETH_RSS_NONFRAG_IPV4_TCP,
- .flow_priority = 0,
+ .flow_priority = 1,
.ip_version = MLX5_IPV4,
},
[HASH_RXQ_UDPV4] = {
@@ -127,7 +127,7 @@ const struct hash_rxq_init hash_rxq_init[] = {
IBV_RX_HASH_SRC_PORT_UDP |
IBV_RX_HASH_DST_PORT_UDP),
.dpdk_rss_hf = ETH_RSS_NONFRAG_IPV4_UDP,
- .flow_priority = 0,
+ .flow_priority = 1,
.ip_version = MLX5_IPV4,
},
[HASH_RXQ_IPV4] = {
@@ -135,7 +135,7 @@ const struct hash_rxq_init hash_rxq_init[] = {
IBV_RX_HASH_DST_IPV4),
.dpdk_rss_hf = (ETH_RSS_IPV4 |
ETH_RSS_FRAG_IPV4),
- .flow_priority = 1,
+ .flow_priority = 2,
.ip_version = MLX5_IPV4,
},
[HASH_RXQ_TCPV6] = {
@@ -144,7 +144,7 @@ const struct hash_rxq_init hash_rxq_init[] = {
IBV_RX_HASH_SRC_PORT_TCP |
IBV_RX_HASH_DST_PORT_TCP),
.dpdk_rss_hf = ETH_RSS_NONFRAG_IPV6_TCP,
- .flow_priority = 0,
+ .flow_priority = 1,
.ip_version = MLX5_IPV6,
},
[HASH_RXQ_UDPV6] = {
@@ -153,7 +153,7 @@ const struct hash_rxq_init hash_rxq_init[] = {
IBV_RX_HASH_SRC_PORT_UDP |
IBV_RX_HASH_DST_PORT_UDP),
.dpdk_rss_hf = ETH_RSS_NONFRAG_IPV6_UDP,
- .flow_priority = 0,
+ .flow_priority = 1,
.ip_version = MLX5_IPV6,
},
[HASH_RXQ_IPV6] = {
@@ -161,13 +161,13 @@ const struct hash_rxq_init hash_rxq_init[] = {
IBV_RX_HASH_DST_IPV6),
.dpdk_rss_hf = (ETH_RSS_IPV6 |
ETH_RSS_FRAG_IPV6),
- .flow_priority = 1,
+ .flow_priority = 2,
.ip_version = MLX5_IPV6,
},
[HASH_RXQ_ETH] = {
.hash_fields = 0,
.dpdk_rss_hf = 0,
- .flow_priority = 2,
+ .flow_priority = 3,
},
};
@@ -861,8 +861,6 @@ mlx5_flow_convert_items_validate(const struct rte_flow_item items[],
/**
* Allocate memory space to store verbs flow attributes.
*
- * @param[in] priority
- * Flow priority.
* @param[in] size
* Amount of byte to allocate.
* @param[out] error
@@ -872,9 +870,7 @@ mlx5_flow_convert_items_validate(const struct rte_flow_item items[],
* A verbs flow attribute on success, NULL otherwise and rte_errno is set.
*/
static struct ibv_flow_attr *
-mlx5_flow_convert_allocate(unsigned int priority,
- unsigned int size,
- struct rte_flow_error *error)
+mlx5_flow_convert_allocate(unsigned int size, struct rte_flow_error *error)
{
struct ibv_flow_attr *ibv_attr;
@@ -886,11 +882,41 @@ mlx5_flow_convert_allocate(unsigned int priority,
"cannot allocate verbs spec attributes");
return NULL;
}
- ibv_attr->priority = priority;
return ibv_attr;
}
/**
+ * Make inner packet matching with an higher priority from the non Inner
+ * matching.
+ *
+ * @param[in, out] parser
+ * Internal parser structure.
+ * @param attr
+ * User flow attribute.
+ */
+static void
+mlx5_flow_update_priority(struct mlx5_flow_parse *parser,
+ const struct rte_flow_attr *attr)
+{
+ unsigned int i;
+
+ if (parser->drop) {
+ parser->queue[HASH_RXQ_ETH].ibv_attr->priority =
+ attr->priority +
+ hash_rxq_init[HASH_RXQ_ETH].flow_priority;
+ return;
+ }
+ for (i = 0; i != hash_rxq_init_n; ++i) {
+ if (parser->queue[i].ibv_attr) {
+ parser->queue[i].ibv_attr->priority =
+ attr->priority +
+ hash_rxq_init[i].flow_priority -
+ (parser->inner ? 1 : 0);
+ }
+ }
+}
+
+/**
* Finalise verbs flow attributes.
*
* @param[in, out] parser
@@ -1064,22 +1090,16 @@ mlx5_flow_convert(struct rte_eth_dev *dev,
* Allocate the memory space to store verbs specifications.
*/
if (parser->drop) {
- unsigned int priority =
- attr->priority +
- hash_rxq_init[HASH_RXQ_ETH].flow_priority;
unsigned int offset = parser->queue[HASH_RXQ_ETH].offset;
parser->queue[HASH_RXQ_ETH].ibv_attr =
- mlx5_flow_convert_allocate(priority, offset, error);
+ mlx5_flow_convert_allocate(offset, error);
if (!parser->queue[HASH_RXQ_ETH].ibv_attr)
goto exit_enomem;
parser->queue[HASH_RXQ_ETH].offset =
sizeof(struct ibv_flow_attr);
} else {
for (i = 0; i != hash_rxq_init_n; ++i) {
- unsigned int priority =
- attr->priority +
- hash_rxq_init[i].flow_priority;
unsigned int offset;
if (!(parser->rss_conf.rss_hf &
@@ -1088,8 +1108,7 @@ mlx5_flow_convert(struct rte_eth_dev *dev,
continue;
offset = parser->queue[i].offset;
parser->queue[i].ibv_attr =
- mlx5_flow_convert_allocate(priority,
- offset, error);
+ mlx5_flow_convert_allocate(offset, error);
if (!parser->queue[i].ibv_attr)
goto exit_enomem;
parser->queue[i].offset = sizeof(struct ibv_flow_attr);
@@ -1124,13 +1143,9 @@ mlx5_flow_convert(struct rte_eth_dev *dev,
* Last step. Complete missing specification to reach the RSS
* configuration.
*/
- if (!parser->drop) {
+ if (!parser->drop)
mlx5_flow_convert_finalise(parser);
- } else {
- parser->queue[HASH_RXQ_ETH].ibv_attr->priority =
- attr->priority +
- hash_rxq_init[parser->layer].flow_priority;
- }
+ mlx5_flow_update_priority(parser, attr);
exit_free:
/* Only verification is expected, all resources should be released. */
if (!parser->create) {
--
2.11.0
^ permalink raw reply [flat|nested] 15+ messages in thread* [dpdk-dev] [PATCH 2/4] net/mlx5: fix flow single queue
2018-02-15 9:23 [dpdk-dev] [PATCH] net/mlx5: change tunnel flow priority Nelio Laranjeiro
` (2 preceding siblings ...)
2018-03-13 14:17 ` [dpdk-dev] [PATCH 1/4] net/mlx5: change tunnel flow priority Nelio Laranjeiro
@ 2018-03-13 14:17 ` Nelio Laranjeiro
2018-03-21 18:45 ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit
2018-03-13 14:17 ` [dpdk-dev] [PATCH 3/4] net/mlx5: improve flow error explanation Nelio Laranjeiro
2018-03-13 14:17 ` [dpdk-dev] [PATCH 4/4] net/mlx5: refuse empty VLAN flow specification Nelio Laranjeiro
5 siblings, 1 reply; 15+ messages in thread
From: Nelio Laranjeiro @ 2018-03-13 14:17 UTC (permalink / raw)
To: Adrien Mazarguil, Yongseok Koh; +Cc: dev, stable
The flow is created with any steering being applied in the NIC when the
device is handling a single Rx queue.
Fixes: cede123a158f ("net/mlx5: fix flow creation with a single target queue")
Fixes: 8086cf08b2f0 ("net/mlx5: handle RSS hash configuration in RSS flow")
Cc: stable@dpdk.org
Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
---
drivers/net/mlx5/mlx5_flow.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index e1ffb38a5..e7040daad 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -937,11 +937,12 @@ mlx5_flow_convert_finalise(struct mlx5_flow_parse *parser)
/* Remove any other flow not matching the pattern. */
if (parser->queues_n == 1) {
for (i = 0; i != hash_rxq_init_n; ++i) {
- if (i == parser->layer || !parser->queue[i].ibv_attr)
+ if (i == HASH_RXQ_ETH)
continue;
rte_free(parser->queue[i].ibv_attr);
parser->queue[i].ibv_attr = NULL;
}
+ return;
}
if (parser->layer == HASH_RXQ_ETH) {
goto fill;
@@ -1807,6 +1808,7 @@ mlx5_flow_create_action_queue(struct rte_eth_dev *dev,
struct priv *priv = dev->data->dev_private;
int ret;
unsigned int i;
+ unsigned int flows_n = 0;
assert(priv->pd);
assert(priv->ctx);
@@ -1830,12 +1832,18 @@ mlx5_flow_create_action_queue(struct rte_eth_dev *dev,
NULL, "flow rule creation failure");
goto error;
}
+ ++flows_n;
DRV_LOG(DEBUG, "port %u %p type %d QP %p ibv_flow %p",
dev->data->port_id,
(void *)flow, i,
(void *)flow->frxq[i].hrxq,
(void *)flow->frxq[i].ibv_flow);
}
+ if (!flows_n) {
+ rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_HANDLE,
+ NULL, "internal error in flow creation");
+ goto error;
+ }
for (i = 0; i != parser->queues_n; ++i) {
struct mlx5_rxq_data *q =
(*priv->rxqs)[parser->queues[i]];
--
2.11.0
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [dpdk-dev] [dpdk-stable] [PATCH 2/4] net/mlx5: fix flow single queue
2018-03-13 14:17 ` [dpdk-dev] [PATCH 2/4] net/mlx5: fix flow single queue Nelio Laranjeiro
@ 2018-03-21 18:45 ` Ferruh Yigit
2018-03-28 9:04 ` Ferruh Yigit
0 siblings, 1 reply; 15+ messages in thread
From: Ferruh Yigit @ 2018-03-21 18:45 UTC (permalink / raw)
To: Nelio Laranjeiro, Adrien Mazarguil, Yongseok Koh; +Cc: dev, stable
On 3/13/2018 2:17 PM, Nelio Laranjeiro wrote:
> The flow is created with any steering being applied in the NIC when the
> device is handling a single Rx queue.
>
> Fixes: cede123a158f ("net/mlx5: fix flow creation with a single target queue")
This patch is from current release and still in next-net. If it is possible to
split this patch to fix issue in this particular commit it can be squashed into
original fix.
If not commit id for fixes will be wrong and needs to be fixed when merged into
main repo.
> Fixes: 8086cf08b2f0 ("net/mlx5: handle RSS hash configuration in RSS flow")
> Cc: stable@dpdk.org
>
> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
<...>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [dpdk-dev] [dpdk-stable] [PATCH 2/4] net/mlx5: fix flow single queue
2018-03-21 18:45 ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit
@ 2018-03-28 9:04 ` Ferruh Yigit
2018-03-28 11:13 ` Shahaf Shuler
0 siblings, 1 reply; 15+ messages in thread
From: Ferruh Yigit @ 2018-03-28 9:04 UTC (permalink / raw)
To: Nelio Laranjeiro, Adrien Mazarguil, Yongseok Koh, Shahaf Shuler
Cc: dev, stable
On 3/21/2018 6:45 PM, Ferruh Yigit wrote:
> On 3/13/2018 2:17 PM, Nelio Laranjeiro wrote:
>> The flow is created with any steering being applied in the NIC when the
>> device is handling a single Rx queue.
>>
>> Fixes: cede123a158f ("net/mlx5: fix flow creation with a single target queue")
>
> This patch is from current release and still in next-net. If it is possible to
> split this patch to fix issue in this particular commit it can be squashed into
> original fix.
Cc'ed Shahaf.
>
> If not commit id for fixes will be wrong and needs to be fixed when merged into
> main repo.
>
>> Fixes: 8086cf08b2f0 ("net/mlx5: handle RSS hash configuration in RSS flow")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
>
> <...>
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [dpdk-dev] [dpdk-stable] [PATCH 2/4] net/mlx5: fix flow single queue
2018-03-28 9:04 ` Ferruh Yigit
@ 2018-03-28 11:13 ` Shahaf Shuler
2018-03-28 11:34 ` Nélio Laranjeiro
0 siblings, 1 reply; 15+ messages in thread
From: Shahaf Shuler @ 2018-03-28 11:13 UTC (permalink / raw)
To: Ferruh Yigit, Nélio Laranjeiro, Adrien Mazarguil, Yongseok Koh
Cc: dev, stable
Wednesday, March 28, 2018 12:05 PM, Ferruh Yigit:
> On 3/21/2018 6:45 PM, Ferruh Yigit wrote:
> > On 3/13/2018 2:17 PM, Nelio Laranjeiro wrote:
> >> The flow is created with any steering being applied in the NIC when
> >> the device is handling a single Rx queue.
> >>
> >> Fixes: cede123a158f ("net/mlx5: fix flow creation with a single
> >> target queue")
> >
> > This patch is from current release and still in next-net. If it is
> > possible to split this patch to fix issue in this particular commit it
> > can be squashed into original fix.
>
We can say it only fixes the above and squash the commits.
> Cc'ed Shahaf.
>
> >
> > If not commit id for fixes will be wrong and needs to be fixed when
> > merged into main repo.
> >
> >> Fixes: 8086cf08b2f0 ("net/mlx5: handle RSS hash configuration in RSS
> >> flow")
> >> Cc: stable@dpdk.org
> >>
> >> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> >
> > <...>
> >
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [dpdk-dev] [dpdk-stable] [PATCH 2/4] net/mlx5: fix flow single queue
2018-03-28 11:13 ` Shahaf Shuler
@ 2018-03-28 11:34 ` Nélio Laranjeiro
2018-03-28 13:16 ` Ferruh Yigit
0 siblings, 1 reply; 15+ messages in thread
From: Nélio Laranjeiro @ 2018-03-28 11:34 UTC (permalink / raw)
To: Shahaf Shuler; +Cc: Ferruh Yigit, Adrien Mazarguil, Yongseok Koh, dev, stable
On Wed, Mar 28, 2018 at 11:13:42AM +0000, Shahaf Shuler wrote:
> Wednesday, March 28, 2018 12:05 PM, Ferruh Yigit:
> > On 3/21/2018 6:45 PM, Ferruh Yigit wrote:
> > > On 3/13/2018 2:17 PM, Nelio Laranjeiro wrote:
> > >> The flow is created with any steering being applied in the NIC when
> > >> the device is handling a single Rx queue.
> > >>
> > >> Fixes: cede123a158f ("net/mlx5: fix flow creation with a single
> > >> target queue")
> > >
> > > This patch is from current release and still in next-net. If it is
> > > possible to split this patch to fix issue in this particular commit it
> > > can be squashed into original fix.
> >
>
> We can say it only fixes the above and squash the commits.
Indeed they are fixing the same issue.
> > Cc'ed Shahaf.
> >
> > >
> > > If not commit id for fixes will be wrong and needs to be fixed when
> > > merged into main repo.
> > >
> > >> Fixes: 8086cf08b2f0 ("net/mlx5: handle RSS hash configuration in RSS
> > >> flow")
> > >> Cc: stable@dpdk.org
> > >>
> > >> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> > >
> > > <...>
> > >
>
--
Nélio Laranjeiro
6WIND
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [dpdk-dev] [dpdk-stable] [PATCH 2/4] net/mlx5: fix flow single queue
2018-03-28 11:34 ` Nélio Laranjeiro
@ 2018-03-28 13:16 ` Ferruh Yigit
2018-03-28 15:09 ` Nélio Laranjeiro
0 siblings, 1 reply; 15+ messages in thread
From: Ferruh Yigit @ 2018-03-28 13:16 UTC (permalink / raw)
To: Nélio Laranjeiro, Shahaf Shuler
Cc: Adrien Mazarguil, Yongseok Koh, dev, stable
On 3/28/2018 12:34 PM, Nélio Laranjeiro wrote:
> On Wed, Mar 28, 2018 at 11:13:42AM +0000, Shahaf Shuler wrote:
>> Wednesday, March 28, 2018 12:05 PM, Ferruh Yigit:
>>> On 3/21/2018 6:45 PM, Ferruh Yigit wrote:
>>>> On 3/13/2018 2:17 PM, Nelio Laranjeiro wrote:
>>>>> The flow is created with any steering being applied in the NIC when
>>>>> the device is handling a single Rx queue.
>>>>>
>>>>> Fixes: cede123a158f ("net/mlx5: fix flow creation with a single
>>>>> target queue")
>>>>
>>>> This patch is from current release and still in next-net. If it is
>>>> possible to split this patch to fix issue in this particular commit it
>>>> can be squashed into original fix.
>>>
>>
>> We can say it only fixes the above and squash the commits.
>
> Indeed they are fixing the same issue.
Can you please confirm it is safe to squash this commit [1] into [2]?
[1]: ebfe9a38cb47 ("net/mlx5: fix flow single queue")
[2]: cede123a158f ("net/mlx5: fix flow creation with a single target queue")
>
>>> Cc'ed Shahaf.
>>>
>>>>
>>>> If not commit id for fixes will be wrong and needs to be fixed when
>>>> merged into main repo.
>>>>
>>>>> Fixes: 8086cf08b2f0 ("net/mlx5: handle RSS hash configuration in RSS
>>>>> flow")
>>>>> Cc: stable@dpdk.org
>>>>>
>>>>> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
>>>>
>>>> <...>
>>>>
>>
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [dpdk-dev] [dpdk-stable] [PATCH 2/4] net/mlx5: fix flow single queue
2018-03-28 13:16 ` Ferruh Yigit
@ 2018-03-28 15:09 ` Nélio Laranjeiro
2018-03-28 16:38 ` Ferruh Yigit
0 siblings, 1 reply; 15+ messages in thread
From: Nélio Laranjeiro @ 2018-03-28 15:09 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: Shahaf Shuler, Adrien Mazarguil, Yongseok Koh, dev, stable
On Wed, Mar 28, 2018 at 02:16:44PM +0100, Ferruh Yigit wrote:
> On 3/28/2018 12:34 PM, Nélio Laranjeiro wrote:
> > On Wed, Mar 28, 2018 at 11:13:42AM +0000, Shahaf Shuler wrote:
> >> Wednesday, March 28, 2018 12:05 PM, Ferruh Yigit:
> >>> On 3/21/2018 6:45 PM, Ferruh Yigit wrote:
> >>>> On 3/13/2018 2:17 PM, Nelio Laranjeiro wrote:
> >>>>> The flow is created with any steering being applied in the NIC when
> >>>>> the device is handling a single Rx queue.
> >>>>>
> >>>>> Fixes: cede123a158f ("net/mlx5: fix flow creation with a single
> >>>>> target queue")
> >>>>
> >>>> This patch is from current release and still in next-net. If it is
> >>>> possible to split this patch to fix issue in this particular commit it
> >>>> can be squashed into original fix.
> >>>
> >>
> >> We can say it only fixes the above and squash the commits.
> >
> > Indeed they are fixing the same issue.
>
> Can you please confirm it is safe to squash this commit [1] into [2]?
>
> [1]: ebfe9a38cb47 ("net/mlx5: fix flow single queue")
> [2]: cede123a158f ("net/mlx5: fix flow creation with a single target queue")
I confirm, [1] is a fix fixing a bug introduced in [2]. Both are needed
to fix the same bug, they should be squash in a single commit.
The confusion came from the fact commit [2] was already integrated in
shshaf's branch, when the issue was raised using another scenario.
Sorry for the confusion.
> >>> Cc'ed Shahaf.
> >>>
> >>>>
> >>>> If not commit id for fixes will be wrong and needs to be fixed when
> >>>> merged into main repo.
> >>>>
> >>>>> Fixes: 8086cf08b2f0 ("net/mlx5: handle RSS hash configuration in RSS
> >>>>> flow")
> >>>>> Cc: stable@dpdk.org
> >>>>>
> >>>>> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> >>>>
> >>>> <...>
> >>>>
> >>
> >
>
Regards,
--
Nélio Laranjeiro
6WIND
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [dpdk-dev] [dpdk-stable] [PATCH 2/4] net/mlx5: fix flow single queue
2018-03-28 15:09 ` Nélio Laranjeiro
@ 2018-03-28 16:38 ` Ferruh Yigit
0 siblings, 0 replies; 15+ messages in thread
From: Ferruh Yigit @ 2018-03-28 16:38 UTC (permalink / raw)
To: Nélio Laranjeiro
Cc: Shahaf Shuler, Adrien Mazarguil, Yongseok Koh, dev, stable
On 3/28/2018 4:09 PM, Nélio Laranjeiro wrote:
> On Wed, Mar 28, 2018 at 02:16:44PM +0100, Ferruh Yigit wrote:
>> On 3/28/2018 12:34 PM, Nélio Laranjeiro wrote:
>>> On Wed, Mar 28, 2018 at 11:13:42AM +0000, Shahaf Shuler wrote:
>>>> Wednesday, March 28, 2018 12:05 PM, Ferruh Yigit:
>>>>> On 3/21/2018 6:45 PM, Ferruh Yigit wrote:
>>>>>> On 3/13/2018 2:17 PM, Nelio Laranjeiro wrote:
>>>>>>> The flow is created with any steering being applied in the NIC when
>>>>>>> the device is handling a single Rx queue.
>>>>>>>
>>>>>>> Fixes: cede123a158f ("net/mlx5: fix flow creation with a single
>>>>>>> target queue")
>>>>>>
>>>>>> This patch is from current release and still in next-net. If it is
>>>>>> possible to split this patch to fix issue in this particular commit it
>>>>>> can be squashed into original fix.
>>>>>
>>>>
>>>> We can say it only fixes the above and squash the commits.
>>>
>>> Indeed they are fixing the same issue.
>>
>> Can you please confirm it is safe to squash this commit [1] into [2]?
>>
>> [1]: ebfe9a38cb47 ("net/mlx5: fix flow single queue")
>> [2]: cede123a158f ("net/mlx5: fix flow creation with a single target queue")
>
> I confirm, [1] is a fix fixing a bug introduced in [2]. Both are needed
> to fix the same bug, they should be squash in a single commit.
[1] Squashed into relevant commit [2] in next-net, thanks.
(There were a few conflicts, please check latest next-net.)
^ permalink raw reply [flat|nested] 15+ messages in thread
* [dpdk-dev] [PATCH 3/4] net/mlx5: improve flow error explanation
2018-02-15 9:23 [dpdk-dev] [PATCH] net/mlx5: change tunnel flow priority Nelio Laranjeiro
` (3 preceding siblings ...)
2018-03-13 14:17 ` [dpdk-dev] [PATCH 2/4] net/mlx5: fix flow single queue Nelio Laranjeiro
@ 2018-03-13 14:17 ` Nelio Laranjeiro
2018-03-13 14:17 ` [dpdk-dev] [PATCH 4/4] net/mlx5: refuse empty VLAN flow specification Nelio Laranjeiro
5 siblings, 0 replies; 15+ messages in thread
From: Nelio Laranjeiro @ 2018-03-13 14:17 UTC (permalink / raw)
To: Adrien Mazarguil, Yongseok Koh; +Cc: dev
Fill the error context in conversion function to provide a better reason on
why it cannot be done to the user.
Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
---
drivers/net/mlx5/mlx5_flow.c | 72 ++++++++++++++++++++++++--------------------
1 file changed, 40 insertions(+), 32 deletions(-)
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index e7040daad..5c096ca9c 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -44,40 +44,46 @@ struct ibv_flow_spec_counter_action {
extern const struct eth_dev_ops mlx5_dev_ops;
extern const struct eth_dev_ops mlx5_dev_ops_isolate;
+/** Structure give to the conversion functions. */
+struct mlx5_flow_data {
+ struct mlx5_flow_parse *parser; /** Parser context. */
+ struct rte_flow_error *error; /** Error context. */
+};
+
static int
mlx5_flow_create_eth(const struct rte_flow_item *item,
const void *default_mask,
- void *data);
+ struct mlx5_flow_data *data);
static int
mlx5_flow_create_vlan(const struct rte_flow_item *item,
const void *default_mask,
- void *data);
+ struct mlx5_flow_data *data);
static int
mlx5_flow_create_ipv4(const struct rte_flow_item *item,
const void *default_mask,
- void *data);
+ struct mlx5_flow_data *data);
static int
mlx5_flow_create_ipv6(const struct rte_flow_item *item,
const void *default_mask,
- void *data);
+ struct mlx5_flow_data *data);
static int
mlx5_flow_create_udp(const struct rte_flow_item *item,
const void *default_mask,
- void *data);
+ struct mlx5_flow_data *data);
static int
mlx5_flow_create_tcp(const struct rte_flow_item *item,
const void *default_mask,
- void *data);
+ struct mlx5_flow_data *data);
static int
mlx5_flow_create_vxlan(const struct rte_flow_item *item,
const void *default_mask,
- void *data);
+ struct mlx5_flow_data *data);
struct mlx5_flow_parse;
@@ -252,7 +258,7 @@ struct mlx5_flow_items {
*/
int (*convert)(const struct rte_flow_item *item,
const void *default_mask,
- void *data);
+ struct mlx5_flow_data *data);
/** Size in bytes of the destination structure. */
const unsigned int dst_sz;
/** List of possible following items. */
@@ -1118,6 +1124,11 @@ mlx5_flow_convert(struct rte_eth_dev *dev,
/* Third step. Conversion parse, fill the specifications. */
parser->inner = 0;
for (; items->type != RTE_FLOW_ITEM_TYPE_END; ++items) {
+ struct mlx5_flow_data data = {
+ .parser = parser,
+ .error = error,
+ };
+
if (items->type == RTE_FLOW_ITEM_TYPE_VOID)
continue;
cur_item = &mlx5_flow_items[items->type];
@@ -1125,13 +1136,9 @@ mlx5_flow_convert(struct rte_eth_dev *dev,
(cur_item->default_mask ?
cur_item->default_mask :
cur_item->mask),
- parser);
- if (ret) {
- rte_flow_error_set(error, rte_errno,
- RTE_FLOW_ERROR_TYPE_ITEM,
- items, "item not supported");
+ &data);
+ if (ret)
goto exit_free;
- }
}
if (parser->mark)
mlx5_flow_create_flag_mark(parser, parser->mark_id);
@@ -1224,11 +1231,11 @@ mlx5_flow_create_copy(struct mlx5_flow_parse *parser, void *src,
static int
mlx5_flow_create_eth(const struct rte_flow_item *item,
const void *default_mask,
- void *data)
+ struct mlx5_flow_data *data)
{
const struct rte_flow_item_eth *spec = item->spec;
const struct rte_flow_item_eth *mask = item->mask;
- struct mlx5_flow_parse *parser = (struct mlx5_flow_parse *)data;
+ struct mlx5_flow_parse *parser = data->parser;
const unsigned int eth_size = sizeof(struct ibv_flow_spec_eth);
struct ibv_flow_spec_eth eth = {
.type = parser->inner | IBV_FLOW_SPEC_ETH,
@@ -1276,11 +1283,11 @@ mlx5_flow_create_eth(const struct rte_flow_item *item,
static int
mlx5_flow_create_vlan(const struct rte_flow_item *item,
const void *default_mask,
- void *data)
+ struct mlx5_flow_data *data)
{
const struct rte_flow_item_vlan *spec = item->spec;
const struct rte_flow_item_vlan *mask = item->mask;
- struct mlx5_flow_parse *parser = (struct mlx5_flow_parse *)data;
+ struct mlx5_flow_parse *parser = data->parser;
struct ibv_flow_spec_eth *eth;
const unsigned int eth_size = sizeof(struct ibv_flow_spec_eth);
@@ -1319,11 +1326,11 @@ mlx5_flow_create_vlan(const struct rte_flow_item *item,
static int
mlx5_flow_create_ipv4(const struct rte_flow_item *item,
const void *default_mask,
- void *data)
+ struct mlx5_flow_data *data)
{
const struct rte_flow_item_ipv4 *spec = item->spec;
const struct rte_flow_item_ipv4 *mask = item->mask;
- struct mlx5_flow_parse *parser = (struct mlx5_flow_parse *)data;
+ struct mlx5_flow_parse *parser = data->parser;
unsigned int ipv4_size = sizeof(struct ibv_flow_spec_ipv4_ext);
struct ibv_flow_spec_ipv4_ext ipv4 = {
.type = parser->inner | IBV_FLOW_SPEC_IPV4_EXT,
@@ -1374,11 +1381,11 @@ mlx5_flow_create_ipv4(const struct rte_flow_item *item,
static int
mlx5_flow_create_ipv6(const struct rte_flow_item *item,
const void *default_mask,
- void *data)
+ struct mlx5_flow_data *data)
{
const struct rte_flow_item_ipv6 *spec = item->spec;
const struct rte_flow_item_ipv6 *mask = item->mask;
- struct mlx5_flow_parse *parser = (struct mlx5_flow_parse *)data;
+ struct mlx5_flow_parse *parser = data->parser;
unsigned int ipv6_size = sizeof(struct ibv_flow_spec_ipv6);
struct ibv_flow_spec_ipv6 ipv6 = {
.type = parser->inner | IBV_FLOW_SPEC_IPV6,
@@ -1449,11 +1456,11 @@ mlx5_flow_create_ipv6(const struct rte_flow_item *item,
static int
mlx5_flow_create_udp(const struct rte_flow_item *item,
const void *default_mask,
- void *data)
+ struct mlx5_flow_data *data)
{
const struct rte_flow_item_udp *spec = item->spec;
const struct rte_flow_item_udp *mask = item->mask;
- struct mlx5_flow_parse *parser = (struct mlx5_flow_parse *)data;
+ struct mlx5_flow_parse *parser = data->parser;
unsigned int udp_size = sizeof(struct ibv_flow_spec_tcp_udp);
struct ibv_flow_spec_tcp_udp udp = {
.type = parser->inner | IBV_FLOW_SPEC_UDP,
@@ -1498,11 +1505,11 @@ mlx5_flow_create_udp(const struct rte_flow_item *item,
static int
mlx5_flow_create_tcp(const struct rte_flow_item *item,
const void *default_mask,
- void *data)
+ struct mlx5_flow_data *data)
{
const struct rte_flow_item_tcp *spec = item->spec;
const struct rte_flow_item_tcp *mask = item->mask;
- struct mlx5_flow_parse *parser = (struct mlx5_flow_parse *)data;
+ struct mlx5_flow_parse *parser = data->parser;
unsigned int tcp_size = sizeof(struct ibv_flow_spec_tcp_udp);
struct ibv_flow_spec_tcp_udp tcp = {
.type = parser->inner | IBV_FLOW_SPEC_TCP,
@@ -1547,11 +1554,11 @@ mlx5_flow_create_tcp(const struct rte_flow_item *item,
static int
mlx5_flow_create_vxlan(const struct rte_flow_item *item,
const void *default_mask,
- void *data)
+ struct mlx5_flow_data *data)
{
const struct rte_flow_item_vxlan *spec = item->spec;
const struct rte_flow_item_vxlan *mask = item->mask;
- struct mlx5_flow_parse *parser = (struct mlx5_flow_parse *)data;
+ struct mlx5_flow_parse *parser = data->parser;
unsigned int size = sizeof(struct ibv_flow_spec_tunnel);
struct ibv_flow_spec_tunnel vxlan = {
.type = parser->inner | IBV_FLOW_SPEC_VXLAN_TUNNEL,
@@ -1582,10 +1589,11 @@ mlx5_flow_create_vxlan(const struct rte_flow_item *item,
* before will also match this rule.
* To avoid such situation, VNI 0 is currently refused.
*/
- if (!vxlan.val.tunnel_id) {
- rte_errno = EINVAL;
- return -rte_errno;
- }
+ if (!vxlan.val.tunnel_id)
+ return rte_flow_error_set(data->error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ITEM,
+ item,
+ "VxLAN vni cannot be 0");
mlx5_flow_create_copy(parser, &vxlan, size);
return 0;
}
--
2.11.0
^ permalink raw reply [flat|nested] 15+ messages in thread* [dpdk-dev] [PATCH 4/4] net/mlx5: refuse empty VLAN flow specification
2018-02-15 9:23 [dpdk-dev] [PATCH] net/mlx5: change tunnel flow priority Nelio Laranjeiro
` (4 preceding siblings ...)
2018-03-13 14:17 ` [dpdk-dev] [PATCH 3/4] net/mlx5: improve flow error explanation Nelio Laranjeiro
@ 2018-03-13 14:17 ` Nelio Laranjeiro
5 siblings, 0 replies; 15+ messages in thread
From: Nelio Laranjeiro @ 2018-03-13 14:17 UTC (permalink / raw)
To: Adrien Mazarguil, Yongseok Koh; +Cc: dev
Verbs specification does help to distinguish between packets having an VLAN
and those which do not have, this ends by having flow rule which does not
react as the user expects e.g.
flow create 0 ingress pattern eth / vlan / end action queue index 0 / end
flow create 0 ingress pattern eth / end action queue index 1 / end
are colliding in Verbs definition as in both rule are matching packets with
or without VLAN.
For this reason, the VLAN specification must not be empty, otherwise the
PMD has to refuse it.
Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
---
drivers/net/mlx5/mlx5_flow.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 5c096ca9c..875a5028d 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -1305,9 +1305,18 @@ mlx5_flow_create_vlan(const struct rte_flow_item *item,
eth->val.vlan_tag = spec->tci;
eth->mask.vlan_tag = mask->tci;
eth->val.vlan_tag &= eth->mask.vlan_tag;
+ /*
+ * From verbs perspective an empty VLAN is equivalent
+ * to a packet without VLAN layer.
+ */
+ if (!eth->mask.vlan_tag)
+ goto error;
}
+ return 0;
}
- return 0;
+error:
+ return rte_flow_error_set(data->error, EINVAL, RTE_FLOW_ERROR_TYPE_ITEM,
+ item, "VLAN cannot be empty");
}
/**
--
2.11.0
^ permalink raw reply [flat|nested] 15+ messages in thread