DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 0/6] net/mlx5: fixes
@ 2017-10-19 12:51 Nelio Laranjeiro
  2017-10-19 12:51 ` [dpdk-dev] [PATCH 1/6] net/mlx5: fix segfault on flow creation Nelio Laranjeiro
                   ` (14 more replies)
  0 siblings, 15 replies; 32+ messages in thread
From: Nelio Laranjeiro @ 2017-10-19 12:51 UTC (permalink / raw)
  To: dev; +Cc: Yongseok Koh, Adrien Mazarguil

Some fixes on bugs and wrong behaviors.

This series applies on top of:
http://dpdk.org/patch/30351

Nelio Laranjeiro (6):
  net/mlx5: fix segfault on flow creation
  net/mlx5: fix work queue array size
  net/mlx5: fix drop flows when port is stopped
  net/mlx5: fix flow director drop action
  net/mlx5: fix mark action with drop action
  net/mlx5: fix allmulti mode

 drivers/net/mlx5/mlx5_flow.c    |  31 +++++++---
 drivers/net/mlx5/mlx5_rxq.c     |   2 +-
 drivers/net/mlx5/mlx5_trigger.c | 131 ++++++++++++++++++++--------------------
 3 files changed, 88 insertions(+), 76 deletions(-)

-- 
2.11.0

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [dpdk-dev] [PATCH 1/6] net/mlx5: fix segfault on flow creation
  2017-10-19 12:51 [dpdk-dev] [PATCH 0/6] net/mlx5: fixes Nelio Laranjeiro
@ 2017-10-19 12:51 ` Nelio Laranjeiro
  2017-10-19 12:51 ` [dpdk-dev] [PATCH 2/6] net/mlx5: fix work queue array size Nelio Laranjeiro
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 32+ messages in thread
From: Nelio Laranjeiro @ 2017-10-19 12:51 UTC (permalink / raw)
  To: dev; +Cc: Yongseok Koh, Adrien Mazarguil

When ports are stopped, the hash Rx queue should not be created.

Fixes: 8086cf08b2f0 ("net/mlx5: handle RSS hash configuration in RSS flow")

Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
Acked-by: Yongseok Koh <yskoh@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 440bda9a1..452fde588 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -1743,6 +1743,8 @@ priv_flow_create_action_queue_rss(struct priv *priv,
 		flow->frxq[i].ibv_attr = parser->queue[i].ibv_attr;
 		parser->queue[i].ibv_attr = NULL;
 		hash_fields = hash_rxq_init[i].hash_fields;
+		if (!priv->dev->data->dev_started)
+			continue;
 		flow->frxq[i].hrxq =
 			mlx5_priv_hrxq_get(priv,
 					   parser->rss_conf.rss_key,
-- 
2.11.0

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [dpdk-dev] [PATCH 2/6] net/mlx5: fix work queue array size
  2017-10-19 12:51 [dpdk-dev] [PATCH 0/6] net/mlx5: fixes Nelio Laranjeiro
  2017-10-19 12:51 ` [dpdk-dev] [PATCH 1/6] net/mlx5: fix segfault on flow creation Nelio Laranjeiro
@ 2017-10-19 12:51 ` Nelio Laranjeiro
  2017-10-19 12:51 ` [dpdk-dev] [PATCH 3/6] net/mlx5: fix drop flows when port is stopped Nelio Laranjeiro
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 32+ messages in thread
From: Nelio Laranjeiro @ 2017-10-19 12:51 UTC (permalink / raw)
  To: dev; +Cc: Yongseok Koh, Adrien Mazarguil

Indirection table size must be in log to communicate with verbs when the
number of queue is not a power of two, the maximum indirection table size
is use, but not converted to log2.  This makes a memory corruption.

Fixes: 4c7a0f5ff876 ("net/mlx5: make indirection tables shareable")

Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
Acked-by: Yongseok Koh <yskoh@mellanox.com>
---
 drivers/net/mlx5/mlx5_rxq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
index ad741ef44..fce9fb116 100644
--- a/drivers/net/mlx5/mlx5_rxq.c
+++ b/drivers/net/mlx5/mlx5_rxq.c
@@ -1113,7 +1113,7 @@ mlx5_priv_ind_table_ibv_new(struct priv *priv, uint16_t queues[],
 	struct mlx5_ind_table_ibv *ind_tbl;
 	const unsigned int wq_n = rte_is_power_of_2(queues_n) ?
 		log2above(queues_n) :
-		priv->ind_table_max_size;
+		log2above(priv->ind_table_max_size);
 	struct ibv_wq *wq[1 << wq_n];
 	unsigned int i;
 	unsigned int j;
-- 
2.11.0

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [dpdk-dev] [PATCH 3/6] net/mlx5: fix drop flows when port is stopped
  2017-10-19 12:51 [dpdk-dev] [PATCH 0/6] net/mlx5: fixes Nelio Laranjeiro
  2017-10-19 12:51 ` [dpdk-dev] [PATCH 1/6] net/mlx5: fix segfault on flow creation Nelio Laranjeiro
  2017-10-19 12:51 ` [dpdk-dev] [PATCH 2/6] net/mlx5: fix work queue array size Nelio Laranjeiro
@ 2017-10-19 12:51 ` Nelio Laranjeiro
  2017-10-19 12:51 ` [dpdk-dev] [PATCH 4/6] net/mlx5: fix flow director drop action Nelio Laranjeiro
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 32+ messages in thread
From: Nelio Laranjeiro @ 2017-10-19 12:51 UTC (permalink / raw)
  To: dev; +Cc: Yongseok Koh, Adrien Mazarguil

Fix the drop queue rule creation when the port is stopped.

Fixes: 8086cf08b2f0 ("net/mlx5: handle RSS hash configuration in RSS flow")

Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
Acked-by: Yongseok Koh <yskoh@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 452fde588..6f458f44a 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -1679,9 +1679,9 @@ priv_flow_create_action_queue_drop(struct priv *priv,
 	};
 	++parser->drop_q.ibv_attr->num_of_specs;
 	parser->drop_q.offset += size;
+	flow->drxq.ibv_attr = parser->drop_q.ibv_attr;
 	if (!priv->dev->data->dev_started)
 		return 0;
-	flow->drxq.ibv_attr = parser->drop_q.ibv_attr;
 	parser->drop_q.ibv_attr = NULL;
 	flow->drxq.ibv_flow = ibv_create_flow(priv->flow_drop_queue->qp,
 					      flow->drxq.ibv_attr);
-- 
2.11.0

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [dpdk-dev] [PATCH 4/6] net/mlx5: fix flow director drop action
  2017-10-19 12:51 [dpdk-dev] [PATCH 0/6] net/mlx5: fixes Nelio Laranjeiro
                   ` (2 preceding siblings ...)
  2017-10-19 12:51 ` [dpdk-dev] [PATCH 3/6] net/mlx5: fix drop flows when port is stopped Nelio Laranjeiro
@ 2017-10-19 12:51 ` Nelio Laranjeiro
  2017-10-19 12:51 ` [dpdk-dev] [PATCH 5/6] net/mlx5: fix mark action with " Nelio Laranjeiro
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 32+ messages in thread
From: Nelio Laranjeiro @ 2017-10-19 12:51 UTC (permalink / raw)
  To: dev; +Cc: Yongseok Koh, Adrien Mazarguil

Flow director drop action as not been brought back with the new
implementation on top of rte flow.

Fixes: 4c3e9bcdd52e ("net/mlx5: support flow director")

Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
Acked-by: Yongseok Koh <yskoh@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 6f458f44a..96a753e8d 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -2601,20 +2601,27 @@ priv_fdir_filter_convert(struct priv *priv,
 		ERROR("invalid queue number %d", fdir_filter->action.rx_queue);
 		return EINVAL;
 	}
-	/* Validate the behavior. */
-	if (fdir_filter->action.behavior != RTE_ETH_FDIR_ACCEPT) {
-		ERROR("invalid behavior %d", fdir_filter->action.behavior);
-		return ENOTSUP;
-	}
 	attributes->attr.ingress = 1;
 	attributes->items[0] = (struct rte_flow_item) {
 		.type = RTE_FLOW_ITEM_TYPE_ETH,
 		.spec = &attributes->l2,
 	};
-	attributes->actions[0] = (struct rte_flow_action){
-		.type = RTE_FLOW_ACTION_TYPE_QUEUE,
-		.conf = &attributes->queue,
-	};
+	switch (fdir_filter->action.behavior) {
+	case RTE_ETH_FDIR_ACCEPT:
+		attributes->actions[0] = (struct rte_flow_action){
+			.type = RTE_FLOW_ACTION_TYPE_QUEUE,
+			.conf = &attributes->queue,
+		};
+		break;
+	case RTE_ETH_FDIR_REJECT:
+		attributes->actions[0] = (struct rte_flow_action){
+			.type = RTE_FLOW_ACTION_TYPE_DROP,
+		};
+		break;
+	default:
+		ERROR("invalid behavior %d", fdir_filter->action.behavior);
+		return ENOTSUP;
+	}
 	attributes->queue.index = fdir_filter->action.rx_queue;
 	switch (fdir_filter->input.flow_type) {
 	case RTE_ETH_FLOW_NONFRAG_IPV4_UDP:
-- 
2.11.0

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [dpdk-dev] [PATCH 5/6] net/mlx5: fix mark action with drop action
  2017-10-19 12:51 [dpdk-dev] [PATCH 0/6] net/mlx5: fixes Nelio Laranjeiro
                   ` (3 preceding siblings ...)
  2017-10-19 12:51 ` [dpdk-dev] [PATCH 4/6] net/mlx5: fix flow director drop action Nelio Laranjeiro
@ 2017-10-19 12:51 ` Nelio Laranjeiro
  2017-10-19 12:51 ` [dpdk-dev] [PATCH 6/6] net/mlx5: fix allmulti mode Nelio Laranjeiro
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 32+ messages in thread
From: Nelio Laranjeiro @ 2017-10-19 12:51 UTC (permalink / raw)
  To: dev; +Cc: Yongseok Koh, Adrien Mazarguil

Marking a packet which will not be received by the NIC is useless, even if
this action remains possible, it blocks the creation of the flow counter
which embed a mark action to a drop queue to be created.

Fixes: 31ba9997f11a ("net/mlx5: fully convert a flow to verbs in validate")

Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
Acked-by: Yongseok Koh <yskoh@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 96a753e8d..13b78ce9b 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -786,6 +786,8 @@ priv_flow_convert_actions(struct priv *priv,
 			goto exit_action_not_supported;
 		}
 	}
+	if (parser->drop && parser->mark)
+		parser->mark = 0;
 	if (!parser->queues_n && !parser->drop) {
 		rte_flow_error_set(error, ENOTSUP, RTE_FLOW_ERROR_TYPE_HANDLE,
 				   NULL, "no valid action");
-- 
2.11.0

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [dpdk-dev] [PATCH 6/6] net/mlx5: fix allmulti mode
  2017-10-19 12:51 [dpdk-dev] [PATCH 0/6] net/mlx5: fixes Nelio Laranjeiro
                   ` (4 preceding siblings ...)
  2017-10-19 12:51 ` [dpdk-dev] [PATCH 5/6] net/mlx5: fix mark action with " Nelio Laranjeiro
@ 2017-10-19 12:51 ` Nelio Laranjeiro
  2017-10-19 19:36 ` [dpdk-dev] [PATCH 0/6] net/mlx5: fixes Yongseok Koh
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 32+ messages in thread
From: Nelio Laranjeiro @ 2017-10-19 12:51 UTC (permalink / raw)
  To: dev; +Cc: Yongseok Koh, Adrien Mazarguil

All multi is not adding unicast flows which cause unicast packets to be
dropped by the NIC.

Fixes: 6a6b6828fe6a ("net/mlx5: use flow to enable all multi mode")

Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
---
 drivers/net/mlx5/mlx5_trigger.c | 131 ++++++++++++++++++++--------------------
 1 file changed, 66 insertions(+), 65 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_trigger.c b/drivers/net/mlx5/mlx5_trigger.c
index 29167badd..edbed2f99 100644
--- a/drivers/net/mlx5/mlx5_trigger.c
+++ b/drivers/net/mlx5/mlx5_trigger.c
@@ -251,6 +251,30 @@ mlx5_dev_stop(struct rte_eth_dev *dev)
 int
 priv_dev_traffic_enable(struct priv *priv, struct rte_eth_dev *dev)
 {
+	struct rte_flow_item_eth bcast = {
+		.dst.addr_bytes = "\xff\xff\xff\xff\xff\xff",
+	};
+	struct rte_flow_item_eth ipv6_multi_spec = {
+		.dst.addr_bytes = "\x33\x33\x00\x00\x00\x00",
+	};
+	struct rte_flow_item_eth ipv6_multi_mask = {
+		.dst.addr_bytes = "\xff\xff\x00\x00\x00\x00",
+	};
+	struct rte_flow_item_eth unicast = {
+		.src.addr_bytes = "\x00\x00\x00\x00\x00\x00",
+	};
+	struct rte_flow_item_eth unicast_mask = {
+		.dst.addr_bytes = "\xff\xff\xff\xff\xff\xff",
+	};
+	const unsigned int vlan_filter_n = priv->vlan_filter_n;
+	const struct ether_addr cmp = {
+		.addr_bytes = "\x00\x00\x00\x00\x00\x00",
+	};
+	unsigned int i;
+	unsigned int j;
+	unsigned int unicast_flow = 0;
+	int ret;
+
 	if (priv->isolated)
 		return 0;
 	if (dev->data->promiscuous) {
@@ -261,82 +285,59 @@ priv_dev_traffic_enable(struct priv *priv, struct rte_eth_dev *dev)
 		};
 
 		claim_zero(mlx5_ctrl_flow(dev, &promisc, &promisc));
-	} else if (dev->data->all_multicast) {
+		return 0;
+	}
+	if (dev->data->all_multicast) {
 		struct rte_flow_item_eth multicast = {
 			.dst.addr_bytes = "\x01\x00\x00\x00\x00\x00",
-			.src.addr_bytes = "\x01\x00\x00\x00\x00\x00",
+			.src.addr_bytes = "\x00\x00\x00\x00\x00\x00",
 			.type = 0,
 		};
 
 		claim_zero(mlx5_ctrl_flow(dev, &multicast, &multicast));
-	} else {
-		struct rte_flow_item_eth bcast = {
-			.dst.addr_bytes = "\xff\xff\xff\xff\xff\xff",
-		};
-		struct rte_flow_item_eth ipv6_multi_spec = {
-			.dst.addr_bytes = "\x33\x33\x00\x00\x00\x00",
-		};
-		struct rte_flow_item_eth ipv6_multi_mask = {
-			.dst.addr_bytes = "\xff\xff\x00\x00\x00\x00",
-		};
-		struct rte_flow_item_eth unicast = {
-			.src.addr_bytes = "\x00\x00\x00\x00\x00\x00",
-		};
-		struct rte_flow_item_eth unicast_mask = {
-			.dst.addr_bytes = "\xff\xff\xff\xff\xff\xff",
-		};
-		const unsigned int vlan_filter_n = priv->vlan_filter_n;
-		const struct ether_addr cmp = {
-			.addr_bytes = "\x00\x00\x00\x00\x00\x00",
-		};
-		unsigned int i;
-		unsigned int j;
-		unsigned int unicast_flow = 0;
-		int ret;
-
-		for (i = 0; i != MLX5_MAX_MAC_ADDRESSES; ++i) {
-			struct ether_addr *mac = &dev->data->mac_addrs[i];
+	}
+	for (i = 0; i != MLX5_MAX_MAC_ADDRESSES; ++i) {
+		struct ether_addr *mac = &dev->data->mac_addrs[i];
 
-			if (!memcmp(mac, &cmp, sizeof(*mac)))
-				continue;
-			memcpy(&unicast.dst.addr_bytes,
-			       mac->addr_bytes,
-			       ETHER_ADDR_LEN);
-			for (j = 0; j != vlan_filter_n; ++j) {
-				uint16_t vlan = priv->vlan_filter[j];
+		if (!memcmp(mac, &cmp, sizeof(*mac)))
+			continue;
+		memcpy(&unicast.dst.addr_bytes,
+		       mac->addr_bytes,
+		       ETHER_ADDR_LEN);
+		for (j = 0; j != vlan_filter_n; ++j) {
+			uint16_t vlan = priv->vlan_filter[j];
 
-				struct rte_flow_item_vlan vlan_spec = {
-					.tci = rte_cpu_to_be_16(vlan),
-				};
-				struct rte_flow_item_vlan vlan_mask = {
-					.tci = 0xffff,
-				};
+			struct rte_flow_item_vlan vlan_spec = {
+				.tci = rte_cpu_to_be_16(vlan),
+			};
+			struct rte_flow_item_vlan vlan_mask = {
+				.tci = 0xffff,
+			};
 
-				ret = mlx5_ctrl_flow_vlan(dev, &unicast,
-							  &unicast_mask,
-							  &vlan_spec,
-							  &vlan_mask);
-				if (ret)
-					goto error;
-				unicast_flow = 1;
-			}
-			if (!vlan_filter_n) {
-				ret = mlx5_ctrl_flow(dev, &unicast,
-						     &unicast_mask);
-				if (ret)
-					goto error;
-				unicast_flow = 1;
-			}
+			ret = mlx5_ctrl_flow_vlan(dev, &unicast,
+						  &unicast_mask,
+						  &vlan_spec,
+						  &vlan_mask);
+			if (ret)
+				goto error;
+			unicast_flow = 1;
+		}
+		if (!vlan_filter_n) {
+			ret = mlx5_ctrl_flow(dev, &unicast,
+					     &unicast_mask);
+			if (ret)
+				goto error;
+			unicast_flow = 1;
 		}
-		if (!unicast_flow)
-			return 0;
-		ret = mlx5_ctrl_flow(dev, &bcast, &bcast);
-		if (ret)
-			goto error;
-		ret = mlx5_ctrl_flow(dev, &ipv6_multi_spec, &ipv6_multi_mask);
-		if (ret)
-			goto error;
 	}
+	if (!unicast_flow)
+		return 0;
+	ret = mlx5_ctrl_flow(dev, &bcast, &bcast);
+	if (ret)
+		goto error;
+	ret = mlx5_ctrl_flow(dev, &ipv6_multi_spec, &ipv6_multi_mask);
+	if (ret)
+		goto error;
 	return 0;
 error:
 	return rte_errno;
-- 
2.11.0

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [dpdk-dev] [PATCH 0/6] net/mlx5: fixes
  2017-10-19 12:51 [dpdk-dev] [PATCH 0/6] net/mlx5: fixes Nelio Laranjeiro
                   ` (5 preceding siblings ...)
  2017-10-19 12:51 ` [dpdk-dev] [PATCH 6/6] net/mlx5: fix allmulti mode Nelio Laranjeiro
@ 2017-10-19 19:36 ` Yongseok Koh
  2017-10-23 14:49 ` [dpdk-dev] [PATCH v2 0/7] " Nelio Laranjeiro
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 32+ messages in thread
From: Yongseok Koh @ 2017-10-19 19:36 UTC (permalink / raw)
  To: Nélio Laranjeiro; +Cc: dev, Adrien Mazarguil


> On Oct 19, 2017, at 5:51 AM, Nelio Laranjeiro <nelio.laranjeiro@6wind.com> wrote:
> 
> Some fixes on bugs and wrong behaviors.
> 
> This series applies on top of:
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdpdk.org%2Fpatch%2F30351&data=02%7C01%7Cyskoh%40mellanox.com%7C2a536ed7e66541c8c0d308d516f02b00%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636440143132580697&sdata=GHl9tXTXl5BLmyhEfnNKfvf15LIRjx6nsrd5ImFLSgQ%3D&reserved=0
> 
> Nelio Laranjeiro (6):
>  net/mlx5: fix segfault on flow creation
>  net/mlx5: fix work queue array size
>  net/mlx5: fix drop flows when port is stopped
>  net/mlx5: fix flow director drop action
>  net/mlx5: fix mark action with drop action
>  net/mlx5: fix allmulti mode
> 
> drivers/net/mlx5/mlx5_flow.c    |  31 +++++++---
> drivers/net/mlx5/mlx5_rxq.c     |   2 +-
> drivers/net/mlx5/mlx5_trigger.c | 131 ++++++++++++++++++++--------------------
> 3 files changed, 88 insertions(+), 76 deletions(-)
> 
> -- 

For all series,
Acked-by: Yongseok Koh <yskoh@mellanox.com>
 
Thanks

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [dpdk-dev] [PATCH v2 0/7] net/mlx5: fixes
  2017-10-19 12:51 [dpdk-dev] [PATCH 0/6] net/mlx5: fixes Nelio Laranjeiro
                   ` (6 preceding siblings ...)
  2017-10-19 19:36 ` [dpdk-dev] [PATCH 0/6] net/mlx5: fixes Yongseok Koh
@ 2017-10-23 14:49 ` Nelio Laranjeiro
  2017-10-24 15:18   ` [dpdk-dev] [PATCH v3 " Nelio Laranjeiro
                     ` (7 more replies)
  2017-10-23 14:49 ` [dpdk-dev] [PATCH v2 1/7] net/mlx5: fix segfault on flow creation Nelio Laranjeiro
                   ` (6 subsequent siblings)
  14 siblings, 8 replies; 32+ messages in thread
From: Nelio Laranjeiro @ 2017-10-23 14:49 UTC (permalink / raw)
  To: dev; +Cc: Yongseok Koh, Adrien Mazarguil

Some fixes on bugs and wrong behaviors.

This series applies on top of:
http://dpdk.org/patch/30707

Changes in v2:

 * Fix packets reception in allmulti mode and when VLAN are present.
 * Fix a double flow insertion causing an infinite loop.

Nelio Laranjeiro (7):
  net/mlx5: fix segfault on flow creation
  net/mlx5: fix work queue array size
  net/mlx5: fix drop flows when port is stopped
  net/mlx5: fix flow director drop action
  net/mlx5: fix mark action with drop action
  net/mlx5: fix reception when VLAN is added
  net/mlx5: fix flow director flow add

 drivers/net/mlx5/mlx5_flow.c    |  32 ++++++----
 drivers/net/mlx5/mlx5_rxq.c     |   2 +-
 drivers/net/mlx5/mlx5_trigger.c | 125 +++++++++++++++++++++-------------------
 3 files changed, 88 insertions(+), 71 deletions(-)

-- 
2.11.0

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [dpdk-dev] [PATCH v2 1/7] net/mlx5: fix segfault on flow creation
  2017-10-19 12:51 [dpdk-dev] [PATCH 0/6] net/mlx5: fixes Nelio Laranjeiro
                   ` (7 preceding siblings ...)
  2017-10-23 14:49 ` [dpdk-dev] [PATCH v2 0/7] " Nelio Laranjeiro
@ 2017-10-23 14:49 ` Nelio Laranjeiro
  2017-10-23 14:49 ` [dpdk-dev] [PATCH v2 2/7] net/mlx5: fix work queue array size Nelio Laranjeiro
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 32+ messages in thread
From: Nelio Laranjeiro @ 2017-10-23 14:49 UTC (permalink / raw)
  To: dev; +Cc: Yongseok Koh, Adrien Mazarguil

When ports are stopped, the hash Rx queue should not be created.

Fixes: 8086cf08b2f0 ("net/mlx5: handle RSS hash configuration in RSS flow")

Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
Acked-by: Yongseok Koh <yskoh@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 440bda9a1..452fde588 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -1743,6 +1743,8 @@ priv_flow_create_action_queue_rss(struct priv *priv,
 		flow->frxq[i].ibv_attr = parser->queue[i].ibv_attr;
 		parser->queue[i].ibv_attr = NULL;
 		hash_fields = hash_rxq_init[i].hash_fields;
+		if (!priv->dev->data->dev_started)
+			continue;
 		flow->frxq[i].hrxq =
 			mlx5_priv_hrxq_get(priv,
 					   parser->rss_conf.rss_key,
-- 
2.11.0

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [dpdk-dev] [PATCH v2 2/7] net/mlx5: fix work queue array size
  2017-10-19 12:51 [dpdk-dev] [PATCH 0/6] net/mlx5: fixes Nelio Laranjeiro
                   ` (8 preceding siblings ...)
  2017-10-23 14:49 ` [dpdk-dev] [PATCH v2 1/7] net/mlx5: fix segfault on flow creation Nelio Laranjeiro
@ 2017-10-23 14:49 ` Nelio Laranjeiro
  2017-10-23 14:49 ` [dpdk-dev] [PATCH v2 3/7] net/mlx5: fix drop flows when port is stopped Nelio Laranjeiro
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 32+ messages in thread
From: Nelio Laranjeiro @ 2017-10-23 14:49 UTC (permalink / raw)
  To: dev; +Cc: Yongseok Koh, Adrien Mazarguil

Indirection table size must be in log to communicate with verbs when the
number of queue is not a power of two, the maximum indirection table size
is use, but not converted to log2.  This makes a memory corruption.

Fixes: 4c7a0f5ff876 ("net/mlx5: make indirection tables shareable")

Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
Acked-by: Yongseok Koh <yskoh@mellanox.com>
---
 drivers/net/mlx5/mlx5_rxq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
index ad741ef44..fce9fb116 100644
--- a/drivers/net/mlx5/mlx5_rxq.c
+++ b/drivers/net/mlx5/mlx5_rxq.c
@@ -1113,7 +1113,7 @@ mlx5_priv_ind_table_ibv_new(struct priv *priv, uint16_t queues[],
 	struct mlx5_ind_table_ibv *ind_tbl;
 	const unsigned int wq_n = rte_is_power_of_2(queues_n) ?
 		log2above(queues_n) :
-		priv->ind_table_max_size;
+		log2above(priv->ind_table_max_size);
 	struct ibv_wq *wq[1 << wq_n];
 	unsigned int i;
 	unsigned int j;
-- 
2.11.0

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [dpdk-dev] [PATCH v2 3/7] net/mlx5: fix drop flows when port is stopped
  2017-10-19 12:51 [dpdk-dev] [PATCH 0/6] net/mlx5: fixes Nelio Laranjeiro
                   ` (9 preceding siblings ...)
  2017-10-23 14:49 ` [dpdk-dev] [PATCH v2 2/7] net/mlx5: fix work queue array size Nelio Laranjeiro
@ 2017-10-23 14:49 ` Nelio Laranjeiro
  2017-10-23 14:49 ` [dpdk-dev] [PATCH v2 4/7] net/mlx5: fix flow director drop action Nelio Laranjeiro
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 32+ messages in thread
From: Nelio Laranjeiro @ 2017-10-23 14:49 UTC (permalink / raw)
  To: dev; +Cc: Yongseok Koh, Adrien Mazarguil

Fix the drop queue rule creation when the port is stopped.

Fixes: 8086cf08b2f0 ("net/mlx5: handle RSS hash configuration in RSS flow")

Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
Acked-by: Yongseok Koh <yskoh@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 452fde588..6f458f44a 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -1679,9 +1679,9 @@ priv_flow_create_action_queue_drop(struct priv *priv,
 	};
 	++parser->drop_q.ibv_attr->num_of_specs;
 	parser->drop_q.offset += size;
+	flow->drxq.ibv_attr = parser->drop_q.ibv_attr;
 	if (!priv->dev->data->dev_started)
 		return 0;
-	flow->drxq.ibv_attr = parser->drop_q.ibv_attr;
 	parser->drop_q.ibv_attr = NULL;
 	flow->drxq.ibv_flow = ibv_create_flow(priv->flow_drop_queue->qp,
 					      flow->drxq.ibv_attr);
-- 
2.11.0

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [dpdk-dev] [PATCH v2 4/7] net/mlx5: fix flow director drop action
  2017-10-19 12:51 [dpdk-dev] [PATCH 0/6] net/mlx5: fixes Nelio Laranjeiro
                   ` (10 preceding siblings ...)
  2017-10-23 14:49 ` [dpdk-dev] [PATCH v2 3/7] net/mlx5: fix drop flows when port is stopped Nelio Laranjeiro
@ 2017-10-23 14:49 ` Nelio Laranjeiro
  2017-10-23 14:49 ` [dpdk-dev] [PATCH v2 5/7] net/mlx5: fix mark action with " Nelio Laranjeiro
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 32+ messages in thread
From: Nelio Laranjeiro @ 2017-10-23 14:49 UTC (permalink / raw)
  To: dev; +Cc: Yongseok Koh, Adrien Mazarguil

Flow director drop action as not been brought back with the new
implementation on top of rte flow.

Fixes: 4c3e9bcdd52e ("net/mlx5: support flow director")

Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
Acked-by: Yongseok Koh <yskoh@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 6f458f44a..96a753e8d 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -2601,20 +2601,27 @@ priv_fdir_filter_convert(struct priv *priv,
 		ERROR("invalid queue number %d", fdir_filter->action.rx_queue);
 		return EINVAL;
 	}
-	/* Validate the behavior. */
-	if (fdir_filter->action.behavior != RTE_ETH_FDIR_ACCEPT) {
-		ERROR("invalid behavior %d", fdir_filter->action.behavior);
-		return ENOTSUP;
-	}
 	attributes->attr.ingress = 1;
 	attributes->items[0] = (struct rte_flow_item) {
 		.type = RTE_FLOW_ITEM_TYPE_ETH,
 		.spec = &attributes->l2,
 	};
-	attributes->actions[0] = (struct rte_flow_action){
-		.type = RTE_FLOW_ACTION_TYPE_QUEUE,
-		.conf = &attributes->queue,
-	};
+	switch (fdir_filter->action.behavior) {
+	case RTE_ETH_FDIR_ACCEPT:
+		attributes->actions[0] = (struct rte_flow_action){
+			.type = RTE_FLOW_ACTION_TYPE_QUEUE,
+			.conf = &attributes->queue,
+		};
+		break;
+	case RTE_ETH_FDIR_REJECT:
+		attributes->actions[0] = (struct rte_flow_action){
+			.type = RTE_FLOW_ACTION_TYPE_DROP,
+		};
+		break;
+	default:
+		ERROR("invalid behavior %d", fdir_filter->action.behavior);
+		return ENOTSUP;
+	}
 	attributes->queue.index = fdir_filter->action.rx_queue;
 	switch (fdir_filter->input.flow_type) {
 	case RTE_ETH_FLOW_NONFRAG_IPV4_UDP:
-- 
2.11.0

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [dpdk-dev] [PATCH v2 5/7] net/mlx5: fix mark action with drop action
  2017-10-19 12:51 [dpdk-dev] [PATCH 0/6] net/mlx5: fixes Nelio Laranjeiro
                   ` (11 preceding siblings ...)
  2017-10-23 14:49 ` [dpdk-dev] [PATCH v2 4/7] net/mlx5: fix flow director drop action Nelio Laranjeiro
@ 2017-10-23 14:49 ` Nelio Laranjeiro
  2017-10-23 14:49 ` [dpdk-dev] [PATCH v2 6/7] net/mlx5: fix reception when VLAN is added Nelio Laranjeiro
  2017-10-23 14:49 ` [dpdk-dev] [PATCH v2 7/7] net/mlx5: fix flow director flow add Nelio Laranjeiro
  14 siblings, 0 replies; 32+ messages in thread
From: Nelio Laranjeiro @ 2017-10-23 14:49 UTC (permalink / raw)
  To: dev; +Cc: Yongseok Koh, Adrien Mazarguil

Marking a packet which will not be received by the NIC is useless, even if
this action remains possible, it blocks the creation of the flow counter
which embed a mark action to a drop queue to be created.

Fixes: 31ba9997f11a ("net/mlx5: fully convert a flow to verbs in validate")

Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
Acked-by: Yongseok Koh <yskoh@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 96a753e8d..13b78ce9b 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -786,6 +786,8 @@ priv_flow_convert_actions(struct priv *priv,
 			goto exit_action_not_supported;
 		}
 	}
+	if (parser->drop && parser->mark)
+		parser->mark = 0;
 	if (!parser->queues_n && !parser->drop) {
 		rte_flow_error_set(error, ENOTSUP, RTE_FLOW_ERROR_TYPE_HANDLE,
 				   NULL, "no valid action");
-- 
2.11.0

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [dpdk-dev] [PATCH v2 6/7] net/mlx5: fix reception when VLAN is added
  2017-10-19 12:51 [dpdk-dev] [PATCH 0/6] net/mlx5: fixes Nelio Laranjeiro
                   ` (12 preceding siblings ...)
  2017-10-23 14:49 ` [dpdk-dev] [PATCH v2 5/7] net/mlx5: fix mark action with " Nelio Laranjeiro
@ 2017-10-23 14:49 ` Nelio Laranjeiro
  2017-10-23 19:25   ` Yongseok Koh
  2017-10-23 14:49 ` [dpdk-dev] [PATCH v2 7/7] net/mlx5: fix flow director flow add Nelio Laranjeiro
  14 siblings, 1 reply; 32+ messages in thread
From: Nelio Laranjeiro @ 2017-10-23 14:49 UTC (permalink / raw)
  To: dev; +Cc: Yongseok Koh, Adrien Mazarguil

When VLAN is enabled in the Rx side, only packets matching this VLAN are
expected, this also includes the broadcast and all multicast packets.

Fixes: 272733b5ebfd ("net/mlx5: use flow to enable unicast traffic")
Fixes: 6a6b6828fe6a ("net/mlx5: use flow to enable all multi mode")

Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
---
 drivers/net/mlx5/mlx5_trigger.c | 125 +++++++++++++++++++++-------------------
 1 file changed, 66 insertions(+), 59 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_trigger.c b/drivers/net/mlx5/mlx5_trigger.c
index 29167badd..9b62c0e6a 100644
--- a/drivers/net/mlx5/mlx5_trigger.c
+++ b/drivers/net/mlx5/mlx5_trigger.c
@@ -251,6 +251,29 @@ mlx5_dev_stop(struct rte_eth_dev *dev)
 int
 priv_dev_traffic_enable(struct priv *priv, struct rte_eth_dev *dev)
 {
+	struct rte_flow_item_eth bcast = {
+		.dst.addr_bytes = "\xff\xff\xff\xff\xff\xff",
+	};
+	struct rte_flow_item_eth ipv6_multi_spec = {
+		.dst.addr_bytes = "\x33\x33\x00\x00\x00\x00",
+	};
+	struct rte_flow_item_eth ipv6_multi_mask = {
+		.dst.addr_bytes = "\xff\xff\x00\x00\x00\x00",
+	};
+	struct rte_flow_item_eth unicast = {
+		.src.addr_bytes = "\x00\x00\x00\x00\x00\x00",
+	};
+	struct rte_flow_item_eth unicast_mask = {
+		.dst.addr_bytes = "\xff\xff\xff\xff\xff\xff",
+	};
+	const unsigned int vlan_filter_n = priv->vlan_filter_n;
+	const struct ether_addr cmp = {
+		.addr_bytes = "\x00\x00\x00\x00\x00\x00",
+	};
+	unsigned int i;
+	unsigned int j;
+	int ret;
+
 	if (priv->isolated)
 		return 0;
 	if (dev->data->promiscuous) {
@@ -261,75 +284,59 @@ priv_dev_traffic_enable(struct priv *priv, struct rte_eth_dev *dev)
 		};
 
 		claim_zero(mlx5_ctrl_flow(dev, &promisc, &promisc));
-	} else if (dev->data->all_multicast) {
+		return 0;
+	}
+	if (dev->data->all_multicast) {
 		struct rte_flow_item_eth multicast = {
 			.dst.addr_bytes = "\x01\x00\x00\x00\x00\x00",
-			.src.addr_bytes = "\x01\x00\x00\x00\x00\x00",
+			.src.addr_bytes = "\x00\x00\x00\x00\x00\x00",
 			.type = 0,
 		};
 
 		claim_zero(mlx5_ctrl_flow(dev, &multicast, &multicast));
-	} else {
-		struct rte_flow_item_eth bcast = {
-			.dst.addr_bytes = "\xff\xff\xff\xff\xff\xff",
-		};
-		struct rte_flow_item_eth ipv6_multi_spec = {
-			.dst.addr_bytes = "\x33\x33\x00\x00\x00\x00",
-		};
-		struct rte_flow_item_eth ipv6_multi_mask = {
-			.dst.addr_bytes = "\xff\xff\x00\x00\x00\x00",
-		};
-		struct rte_flow_item_eth unicast = {
-			.src.addr_bytes = "\x00\x00\x00\x00\x00\x00",
-		};
-		struct rte_flow_item_eth unicast_mask = {
-			.dst.addr_bytes = "\xff\xff\xff\xff\xff\xff",
-		};
-		const unsigned int vlan_filter_n = priv->vlan_filter_n;
-		const struct ether_addr cmp = {
-			.addr_bytes = "\x00\x00\x00\x00\x00\x00",
-		};
-		unsigned int i;
-		unsigned int j;
-		unsigned int unicast_flow = 0;
-		int ret;
-
-		for (i = 0; i != MLX5_MAX_MAC_ADDRESSES; ++i) {
-			struct ether_addr *mac = &dev->data->mac_addrs[i];
+	}
+	for (i = 0; i != MLX5_MAX_MAC_ADDRESSES; ++i) {
+		struct ether_addr *mac = &dev->data->mac_addrs[i];
 
-			if (!memcmp(mac, &cmp, sizeof(*mac)))
-				continue;
-			memcpy(&unicast.dst.addr_bytes,
-			       mac->addr_bytes,
-			       ETHER_ADDR_LEN);
-			for (j = 0; j != vlan_filter_n; ++j) {
-				uint16_t vlan = priv->vlan_filter[j];
+		if (!memcmp(mac, &cmp, sizeof(*mac)))
+			continue;
+		memcpy(&unicast.dst.addr_bytes,
+		       mac->addr_bytes,
+		       ETHER_ADDR_LEN);
+		for (j = 0; j != vlan_filter_n; ++j) {
+			uint16_t vlan = priv->vlan_filter[j];
 
-				struct rte_flow_item_vlan vlan_spec = {
-					.tci = rte_cpu_to_be_16(vlan),
-				};
-				struct rte_flow_item_vlan vlan_mask = {
-					.tci = 0xffff,
-				};
+			struct rte_flow_item_vlan vlan_spec = {
+				.tci = rte_cpu_to_be_16(vlan),
+			};
+			struct rte_flow_item_vlan vlan_mask = {
+				.tci = 0xffff,
+			};
 
-				ret = mlx5_ctrl_flow_vlan(dev, &unicast,
-							  &unicast_mask,
-							  &vlan_spec,
-							  &vlan_mask);
-				if (ret)
-					goto error;
-				unicast_flow = 1;
-			}
-			if (!vlan_filter_n) {
-				ret = mlx5_ctrl_flow(dev, &unicast,
-						     &unicast_mask);
-				if (ret)
-					goto error;
-				unicast_flow = 1;
-			}
+			ret = mlx5_ctrl_flow_vlan(dev, &unicast,
+						  &unicast_mask,
+						  &vlan_spec,
+						  &vlan_mask);
+			if (ret)
+				goto error;
+			ret = mlx5_ctrl_flow_vlan(dev, &bcast, &bcast,
+						  &vlan_spec, &vlan_mask);
+			if (ret)
+				goto error;
+			ret = mlx5_ctrl_flow_vlan(dev, &ipv6_multi_spec,
+						  &ipv6_multi_mask,
+						  &vlan_spec, &vlan_mask);
+			if (ret)
+				goto error;
 		}
-		if (!unicast_flow)
-			return 0;
+		if (!vlan_filter_n) {
+			ret = mlx5_ctrl_flow(dev, &unicast,
+					     &unicast_mask);
+			if (ret)
+				goto error;
+		}
+	}
+	if (!dev->data->all_multicast && !vlan_filter_n) {
 		ret = mlx5_ctrl_flow(dev, &bcast, &bcast);
 		if (ret)
 			goto error;
-- 
2.11.0

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [dpdk-dev] [PATCH v2 7/7] net/mlx5: fix flow director flow add
  2017-10-19 12:51 [dpdk-dev] [PATCH 0/6] net/mlx5: fixes Nelio Laranjeiro
                   ` (13 preceding siblings ...)
  2017-10-23 14:49 ` [dpdk-dev] [PATCH v2 6/7] net/mlx5: fix reception when VLAN is added Nelio Laranjeiro
@ 2017-10-23 14:49 ` Nelio Laranjeiro
  2017-10-23 20:48   ` Yongseok Koh
  14 siblings, 1 reply; 32+ messages in thread
From: Nelio Laranjeiro @ 2017-10-23 14:49 UTC (permalink / raw)
  To: dev; +Cc: Yongseok Koh, Adrien Mazarguil

Flows are added by priv_flow_create() in the associated list, adding them a
second time corrupts the list causing an infinite loop when parsing it.

Fixes: 4c3e9bcdd52e ("net/mlx5: support flow director")

Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
---
 drivers/net/mlx5/mlx5_flow.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 13b78ce9b..26cf593af 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -2792,7 +2792,6 @@ priv_fdir_filter_add(struct priv *priv,
 				attributes.actions,
 				&error);
 	if (flow) {
-		TAILQ_INSERT_TAIL(&priv->flows, flow, next);
 		DEBUG("FDIR created %p", (void *)flow);
 		return 0;
 	}
-- 
2.11.0

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [dpdk-dev] [PATCH v2 6/7] net/mlx5: fix reception when VLAN is added
  2017-10-23 14:49 ` [dpdk-dev] [PATCH v2 6/7] net/mlx5: fix reception when VLAN is added Nelio Laranjeiro
@ 2017-10-23 19:25   ` Yongseok Koh
  2017-10-24  7:11     ` Nélio Laranjeiro
  0 siblings, 1 reply; 32+ messages in thread
From: Yongseok Koh @ 2017-10-23 19:25 UTC (permalink / raw)
  To: Nelio Laranjeiro; +Cc: dev, Adrien Mazarguil

On Mon, Oct 23, 2017 at 04:49:56PM +0200, Nelio Laranjeiro wrote:
> @@ -261,75 +284,59 @@ priv_dev_traffic_enable(struct priv *priv, struct rte_eth_dev *dev)
>  		};
>  
>  		claim_zero(mlx5_ctrl_flow(dev, &promisc, &promisc));
> -	} else if (dev->data->all_multicast) {
> +		return 0;
> +	}
> +	if (dev->data->all_multicast) {
>  		struct rte_flow_item_eth multicast = {
>  			.dst.addr_bytes = "\x01\x00\x00\x00\x00\x00",
> -			.src.addr_bytes = "\x01\x00\x00\x00\x00\x00",
> +			.src.addr_bytes = "\x00\x00\x00\x00\x00\x00",
>  			.type = 0,
>  		};
>  
>  		claim_zero(mlx5_ctrl_flow(dev, &multicast, &multicast));

Just curious. No need to consider VLAN for multicast here?

> -	} else {
> -		struct rte_flow_item_eth bcast = {
> -			.dst.addr_bytes = "\xff\xff\xff\xff\xff\xff",
> -		};
> -		struct rte_flow_item_eth ipv6_multi_spec = {
> -			.dst.addr_bytes = "\x33\x33\x00\x00\x00\x00",
> -		};
> -		struct rte_flow_item_eth ipv6_multi_mask = {
> -			.dst.addr_bytes = "\xff\xff\x00\x00\x00\x00",
> -		};
> -		struct rte_flow_item_eth unicast = {
> -			.src.addr_bytes = "\x00\x00\x00\x00\x00\x00",
> -		};
> -		struct rte_flow_item_eth unicast_mask = {
> -			.dst.addr_bytes = "\xff\xff\xff\xff\xff\xff",
> -		};
> -		const unsigned int vlan_filter_n = priv->vlan_filter_n;
> -		const struct ether_addr cmp = {
> -			.addr_bytes = "\x00\x00\x00\x00\x00\x00",
> -		};
> -		unsigned int i;
> -		unsigned int j;
> -		unsigned int unicast_flow = 0;
> -		int ret;
> -
> -		for (i = 0; i != MLX5_MAX_MAC_ADDRESSES; ++i) {
> -			struct ether_addr *mac = &dev->data->mac_addrs[i];
> +	}
> +	for (i = 0; i != MLX5_MAX_MAC_ADDRESSES; ++i) {
> +		struct ether_addr *mac = &dev->data->mac_addrs[i];
>  
> -			if (!memcmp(mac, &cmp, sizeof(*mac)))
> -				continue;
> -			memcpy(&unicast.dst.addr_bytes,
> -			       mac->addr_bytes,
> -			       ETHER_ADDR_LEN);
> -			for (j = 0; j != vlan_filter_n; ++j) {
> -				uint16_t vlan = priv->vlan_filter[j];
> +		if (!memcmp(mac, &cmp, sizeof(*mac)))
> +			continue;
> +		memcpy(&unicast.dst.addr_bytes,
> +		       mac->addr_bytes,
> +		       ETHER_ADDR_LEN);
> +		for (j = 0; j != vlan_filter_n; ++j) {
> +			uint16_t vlan = priv->vlan_filter[j];
>  
> -				struct rte_flow_item_vlan vlan_spec = {
> -					.tci = rte_cpu_to_be_16(vlan),
> -				};
> -				struct rte_flow_item_vlan vlan_mask = {
> -					.tci = 0xffff,
> -				};
> +			struct rte_flow_item_vlan vlan_spec = {
> +				.tci = rte_cpu_to_be_16(vlan),
> +			};
> +			struct rte_flow_item_vlan vlan_mask = {
> +				.tci = 0xffff,
> +			};
>  
> -				ret = mlx5_ctrl_flow_vlan(dev, &unicast,
> -							  &unicast_mask,
> -							  &vlan_spec,
> -							  &vlan_mask);
> -				if (ret)
> -					goto error;
> -				unicast_flow = 1;
> -			}
> -			if (!vlan_filter_n) {
> -				ret = mlx5_ctrl_flow(dev, &unicast,
> -						     &unicast_mask);
> -				if (ret)
> -					goto error;
> -				unicast_flow = 1;
> -			}
> +			ret = mlx5_ctrl_flow_vlan(dev, &unicast,
> +						  &unicast_mask,
> +						  &vlan_spec,
> +						  &vlan_mask);
> +			if (ret)
> +				goto error;
> +			ret = mlx5_ctrl_flow_vlan(dev, &bcast, &bcast,
> +						  &vlan_spec, &vlan_mask);
> +			if (ret)
> +				goto error;
> +			ret = mlx5_ctrl_flow_vlan(dev, &ipv6_multi_spec,
> +						  &ipv6_multi_mask,
> +						  &vlan_spec, &vlan_mask);

These (bcast and ipv6_multi_mask) can be duplicated multiple times if there are
multiple MAC addrs, is that intended?


Thanks,
Yongseok

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [dpdk-dev] [PATCH v2 7/7] net/mlx5: fix flow director flow add
  2017-10-23 14:49 ` [dpdk-dev] [PATCH v2 7/7] net/mlx5: fix flow director flow add Nelio Laranjeiro
@ 2017-10-23 20:48   ` Yongseok Koh
  0 siblings, 0 replies; 32+ messages in thread
From: Yongseok Koh @ 2017-10-23 20:48 UTC (permalink / raw)
  To: Nélio Laranjeiro; +Cc: dev, Adrien Mazarguil


> On Oct 23, 2017, at 7:49 AM, Nelio Laranjeiro <nelio.laranjeiro@6wind.com> wrote:
> 
> Flows are added by priv_flow_create() in the associated list, adding them a
> second time corrupts the list causing an infinite loop when parsing it.
> 
> Fixes: 4c3e9bcdd52e ("net/mlx5: support flow director")
> 
> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> ---
Acked-by: Yongseok Koh <yskoh@mellanox.com>
 
Thanks

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [dpdk-dev] [PATCH v2 6/7] net/mlx5: fix reception when VLAN is added
  2017-10-23 19:25   ` Yongseok Koh
@ 2017-10-24  7:11     ` Nélio Laranjeiro
  2017-10-24  7:34       ` Nélio Laranjeiro
  0 siblings, 1 reply; 32+ messages in thread
From: Nélio Laranjeiro @ 2017-10-24  7:11 UTC (permalink / raw)
  To: Yongseok Koh; +Cc: Nelio Laranjeiro, dev, Adrien Mazarguil

On Mon, Oct 23, 2017 at 12:25:45PM -0700, Yongseok Koh wrote:
> On Mon, Oct 23, 2017 at 04:49:56PM +0200, Nelio Laranjeiro wrote:
> > @@ -261,75 +284,59 @@ priv_dev_traffic_enable(struct priv *priv, struct rte_eth_dev *dev)
> >  		};
> >  
> >  		claim_zero(mlx5_ctrl_flow(dev, &promisc, &promisc));
> > -	} else if (dev->data->all_multicast) {
> > +		return 0;
> > +	}
> > +	if (dev->data->all_multicast) {
> >  		struct rte_flow_item_eth multicast = {
> >  			.dst.addr_bytes = "\x01\x00\x00\x00\x00\x00",
> > -			.src.addr_bytes = "\x01\x00\x00\x00\x00\x00",
> > +			.src.addr_bytes = "\x00\x00\x00\x00\x00\x00",
> >  			.type = 0,
> >  		};
> >  
> >  		claim_zero(mlx5_ctrl_flow(dev, &multicast, &multicast));
> 
> Just curious. No need to consider VLAN for multicast here?

According to the lib documentation no [1]

 "Enable the receipt of any multicast frame by an Ethernet device"

> [...]
> These (bcast and ipv6_multi_mask) can be duplicated multiple times if there are
> multiple MAC addrs, is that intended?

There is in fact an issue in this series, it does not match my final code.

I'll send a v3.

[1] http://dpdk.org/browse/dpdk/tree/lib/librte_ether/rte_ethdev.h#n2304

-- 
Nélio Laranjeiro
6WIND

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [dpdk-dev] [PATCH v2 6/7] net/mlx5: fix reception when VLAN is added
  2017-10-24  7:11     ` Nélio Laranjeiro
@ 2017-10-24  7:34       ` Nélio Laranjeiro
  2017-10-24 13:41         ` Yongseok Koh
  0 siblings, 1 reply; 32+ messages in thread
From: Nélio Laranjeiro @ 2017-10-24  7:34 UTC (permalink / raw)
  To: Yongseok Koh, dev; +Cc: Adrien Mazarguil

On Tue, Oct 24, 2017 at 09:11:42AM +0200, Nélio Laranjeiro wrote:
> On Mon, Oct 23, 2017 at 12:25:45PM -0700, Yongseok Koh wrote:
> > On Mon, Oct 23, 2017 at 04:49:56PM +0200, Nelio Laranjeiro wrote:
> > > @@ -261,75 +284,59 @@ priv_dev_traffic_enable(struct priv *priv, struct rte_eth_dev *dev)
> > >  		};
> > >  
> > >  		claim_zero(mlx5_ctrl_flow(dev, &promisc, &promisc));
> > > -	} else if (dev->data->all_multicast) {
> > > +		return 0;
> > > +	}
> > > +	if (dev->data->all_multicast) {
> > >  		struct rte_flow_item_eth multicast = {
> > >  			.dst.addr_bytes = "\x01\x00\x00\x00\x00\x00",
> > > -			.src.addr_bytes = "\x01\x00\x00\x00\x00\x00",
> > > +			.src.addr_bytes = "\x00\x00\x00\x00\x00\x00",
> > >  			.type = 0,
> > >  		};
> > >  
> > >  		claim_zero(mlx5_ctrl_flow(dev, &multicast, &multicast));
> > 
> > Just curious. No need to consider VLAN for multicast here?
> 
> According to the lib documentation no [1]
> 
>  "Enable the receipt of any multicast frame by an Ethernet device"
> 
> > [...]
> > These (bcast and ipv6_multi_mask) can be duplicated multiple times if there are
> > multiple MAC addrs, is that intended?
> 
> There is in fact an issue in this series, it does not match my final code.
> 
> I'll send a v3.
> 
> [1] http://dpdk.org/browse/dpdk/tree/lib/librte_ether/rte_ethdev.h#n2304

I've wrongly read your last comment, the patch is correct, it won't add
multiple time the broadcast multicast, it will add one per expected
VLAN.

Example:

 testpmd> set promisc all off
 testpmd> set allmulti all off
 testpmd> rx_vlan add 0 1330
 testpmd> rx_vlan add 0 1331

Will cause this code to add a broadcast flow with VLAN TCI 1330 and
another broadcast flow with VLAN TCI 1331, others won't be received.

The user will only receive broadcast packets with VLAN TCI 1330 and
1331.  It is what he expects.

In case not VLAN is configured, the broadcast and muilticast flow
insertion are under the condition:

 "if (!dev->data->all_multicast && !vlan_filter_n)"

Thanks,

-- 
Nélio Laranjeiro
6WIND

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [dpdk-dev] [PATCH v2 6/7] net/mlx5: fix reception when VLAN is added
  2017-10-24  7:34       ` Nélio Laranjeiro
@ 2017-10-24 13:41         ` Yongseok Koh
  2017-10-24 14:19           ` Nélio Laranjeiro
  0 siblings, 1 reply; 32+ messages in thread
From: Yongseok Koh @ 2017-10-24 13:41 UTC (permalink / raw)
  To: Nélio Laranjeiro; +Cc: dev, Adrien Mazarguil

On Tue, Oct 24, 2017 at 09:34:34AM +0200, Nélio Laranjeiro wrote:
> On Tue, Oct 24, 2017 at 09:11:42AM +0200, Nélio Laranjeiro wrote:
> > On Mon, Oct 23, 2017 at 12:25:45PM -0700, Yongseok Koh wrote:
> > > On Mon, Oct 23, 2017 at 04:49:56PM +0200, Nelio Laranjeiro wrote:
> > > > @@ -261,75 +284,59 @@ priv_dev_traffic_enable(struct priv *priv, struct rte_eth_dev *dev)
[...]
> I've wrongly read your last comment, the patch is correct, it won't add
> multiple time the broadcast multicast, it will add one per expected
> VLAN.
> 
> Example:
> 
>  testpmd> set promisc all off
>  testpmd> set allmulti all off
>  testpmd> rx_vlan add 0 1330
>  testpmd> rx_vlan add 0 1331
> 
> Will cause this code to add a broadcast flow with VLAN TCI 1330 and
> another broadcast flow with VLAN TCI 1331, others won't be received.
> 
> The user will only receive broadcast packets with VLAN TCI 1330 and
> 1331.  It is what he expects.

What I meant was, if there are multiple MAC addresses on a port, the bcast/mcast
flows will be repeated. For example, if there are 3 valid addrs in
dev->data->mac_addrs

    testpmd> mac_addr add 0 <addr1>
    testpmd> mac_addr add 0 <addr2>
    testpmd> mac_addr add 0 <addr3>

and 2 vlan filters are configured like your example above, then 6 ucast flows
(2 per an addr) will be added along with 6 bcast flows and 6 mcast flows. But it
only needs 2 bcast flows and 2 mcast flows - one per vlan.

Thanks,
Yongseok

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [dpdk-dev] [PATCH v2 6/7] net/mlx5: fix reception when VLAN is added
  2017-10-24 13:41         ` Yongseok Koh
@ 2017-10-24 14:19           ` Nélio Laranjeiro
  0 siblings, 0 replies; 32+ messages in thread
From: Nélio Laranjeiro @ 2017-10-24 14:19 UTC (permalink / raw)
  To: Yongseok Koh; +Cc: dev, Adrien Mazarguil

On Tue, Oct 24, 2017 at 06:41:07AM -0700, Yongseok Koh wrote:
> On Tue, Oct 24, 2017 at 09:34:34AM +0200, Nélio Laranjeiro wrote:
> > On Tue, Oct 24, 2017 at 09:11:42AM +0200, Nélio Laranjeiro wrote:
> > > On Mon, Oct 23, 2017 at 12:25:45PM -0700, Yongseok Koh wrote:
> > > > On Mon, Oct 23, 2017 at 04:49:56PM +0200, Nelio Laranjeiro wrote:
> > > > > @@ -261,75 +284,59 @@ priv_dev_traffic_enable(struct priv *priv, struct rte_eth_dev *dev)
> [...]
> > I've wrongly read your last comment, the patch is correct, it won't add
> > multiple time the broadcast multicast, it will add one per expected
> > VLAN.
> > 
> > Example:
> > 
> >  testpmd> set promisc all off
> >  testpmd> set allmulti all off
> >  testpmd> rx_vlan add 0 1330
> >  testpmd> rx_vlan add 0 1331
> > 
> > Will cause this code to add a broadcast flow with VLAN TCI 1330 and
> > another broadcast flow with VLAN TCI 1331, others won't be received.
> > 
> > The user will only receive broadcast packets with VLAN TCI 1330 and
> > 1331.  It is what he expects.
> 
> What I meant was, if there are multiple MAC addresses on a port, the bcast/mcast
> flows will be repeated. For example, if there are 3 valid addrs in
> dev->data->mac_addrs
> 
>     testpmd> mac_addr add 0 <addr1>
>     testpmd> mac_addr add 0 <addr2>
>     testpmd> mac_addr add 0 <addr3>
> 
> and 2 vlan filters are configured like your example above, then 6 ucast flows
> (2 per an addr) will be added along with 6 bcast flows and 6 mcast flows. But it
> only needs 2 bcast flows and 2 mcast flows - one per vlan.

Right, I missed this case.

I'll fix it in a v3.

Thanks,

-- 
Nélio Laranjeiro
6WIND

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [dpdk-dev] [PATCH v3 0/7] net/mlx5: fixes
  2017-10-23 14:49 ` [dpdk-dev] [PATCH v2 0/7] " Nelio Laranjeiro
@ 2017-10-24 15:18   ` Nelio Laranjeiro
  2017-10-24 19:07     ` Ferruh Yigit
  2017-10-24 15:18   ` [dpdk-dev] [PATCH v3 1/7] net/mlx5: fix segfault on flow creation Nelio Laranjeiro
                     ` (6 subsequent siblings)
  7 siblings, 1 reply; 32+ messages in thread
From: Nelio Laranjeiro @ 2017-10-24 15:18 UTC (permalink / raw)
  To: dev; +Cc: Yongseok Koh, Adrien Mazarguil

Changes in v3:

 * Avoid to duplicate bcast/multicast flows when multiple MAC addressed are
   configured.

Changes in v2:

 * Fix packets reception in allmulti mode and when VLAN are present.
 * Fix a double flow insertion causing an infinite loop.

Nelio Laranjeiro (7):
  net/mlx5: fix segfault on flow creation
  net/mlx5: fix work queue array size
  net/mlx5: fix drop flows when port is stopped
  net/mlx5: fix flow director drop action
  net/mlx5: fix mark action with drop action
  net/mlx5: fix reception when VLAN is added
  net/mlx5: fix flow director flow add

 drivers/net/mlx5/mlx5_flow.c    |  32 ++++++---
 drivers/net/mlx5/mlx5_rxq.c     |   2 +-
 drivers/net/mlx5/mlx5_trigger.c | 156 +++++++++++++++++++++++-----------------
 3 files changed, 111 insertions(+), 79 deletions(-)

-- 
2.11.0

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [dpdk-dev] [PATCH v3 1/7] net/mlx5: fix segfault on flow creation
  2017-10-23 14:49 ` [dpdk-dev] [PATCH v2 0/7] " Nelio Laranjeiro
  2017-10-24 15:18   ` [dpdk-dev] [PATCH v3 " Nelio Laranjeiro
@ 2017-10-24 15:18   ` Nelio Laranjeiro
  2017-10-24 15:18   ` [dpdk-dev] [PATCH v3 2/7] net/mlx5: fix work queue array size Nelio Laranjeiro
                     ` (5 subsequent siblings)
  7 siblings, 0 replies; 32+ messages in thread
From: Nelio Laranjeiro @ 2017-10-24 15:18 UTC (permalink / raw)
  To: dev; +Cc: Yongseok Koh, Adrien Mazarguil

When ports are stopped, the hash Rx queue should not be created.

Fixes: 8086cf08b2f0 ("net/mlx5: handle RSS hash configuration in RSS flow")

Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
Acked-by: Yongseok Koh <yskoh@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 440bda9a1..452fde588 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -1743,6 +1743,8 @@ priv_flow_create_action_queue_rss(struct priv *priv,
 		flow->frxq[i].ibv_attr = parser->queue[i].ibv_attr;
 		parser->queue[i].ibv_attr = NULL;
 		hash_fields = hash_rxq_init[i].hash_fields;
+		if (!priv->dev->data->dev_started)
+			continue;
 		flow->frxq[i].hrxq =
 			mlx5_priv_hrxq_get(priv,
 					   parser->rss_conf.rss_key,
-- 
2.11.0

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [dpdk-dev] [PATCH v3 2/7] net/mlx5: fix work queue array size
  2017-10-23 14:49 ` [dpdk-dev] [PATCH v2 0/7] " Nelio Laranjeiro
  2017-10-24 15:18   ` [dpdk-dev] [PATCH v3 " Nelio Laranjeiro
  2017-10-24 15:18   ` [dpdk-dev] [PATCH v3 1/7] net/mlx5: fix segfault on flow creation Nelio Laranjeiro
@ 2017-10-24 15:18   ` Nelio Laranjeiro
  2017-10-24 15:18   ` [dpdk-dev] [PATCH v3 3/7] net/mlx5: fix drop flows when port is stopped Nelio Laranjeiro
                     ` (4 subsequent siblings)
  7 siblings, 0 replies; 32+ messages in thread
From: Nelio Laranjeiro @ 2017-10-24 15:18 UTC (permalink / raw)
  To: dev; +Cc: Yongseok Koh, Adrien Mazarguil

Indirection table size must be in log to communicate with verbs when the
number of queue is not a power of two, the maximum indirection table size
is use, but not converted to log2.  This makes a memory corruption.

Fixes: 4c7a0f5ff876 ("net/mlx5: make indirection tables shareable")

Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
Acked-by: Yongseok Koh <yskoh@mellanox.com>
---
 drivers/net/mlx5/mlx5_rxq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
index daf05cb09..a1f382b1f 100644
--- a/drivers/net/mlx5/mlx5_rxq.c
+++ b/drivers/net/mlx5/mlx5_rxq.c
@@ -1120,7 +1120,7 @@ mlx5_priv_ind_table_ibv_new(struct priv *priv, uint16_t queues[],
 	struct mlx5_ind_table_ibv *ind_tbl;
 	const unsigned int wq_n = rte_is_power_of_2(queues_n) ?
 		log2above(queues_n) :
-		priv->ind_table_max_size;
+		log2above(priv->ind_table_max_size);
 	struct ibv_wq *wq[1 << wq_n];
 	unsigned int i;
 	unsigned int j;
-- 
2.11.0

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [dpdk-dev] [PATCH v3 3/7] net/mlx5: fix drop flows when port is stopped
  2017-10-23 14:49 ` [dpdk-dev] [PATCH v2 0/7] " Nelio Laranjeiro
                     ` (2 preceding siblings ...)
  2017-10-24 15:18   ` [dpdk-dev] [PATCH v3 2/7] net/mlx5: fix work queue array size Nelio Laranjeiro
@ 2017-10-24 15:18   ` Nelio Laranjeiro
  2017-10-24 15:18   ` [dpdk-dev] [PATCH v3 4/7] net/mlx5: fix flow director drop action Nelio Laranjeiro
                     ` (3 subsequent siblings)
  7 siblings, 0 replies; 32+ messages in thread
From: Nelio Laranjeiro @ 2017-10-24 15:18 UTC (permalink / raw)
  To: dev; +Cc: Yongseok Koh, Adrien Mazarguil

Fix the drop queue rule creation when the port is stopped.

Fixes: 8086cf08b2f0 ("net/mlx5: handle RSS hash configuration in RSS flow")

Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
Acked-by: Yongseok Koh <yskoh@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 452fde588..6f458f44a 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -1679,9 +1679,9 @@ priv_flow_create_action_queue_drop(struct priv *priv,
 	};
 	++parser->drop_q.ibv_attr->num_of_specs;
 	parser->drop_q.offset += size;
+	flow->drxq.ibv_attr = parser->drop_q.ibv_attr;
 	if (!priv->dev->data->dev_started)
 		return 0;
-	flow->drxq.ibv_attr = parser->drop_q.ibv_attr;
 	parser->drop_q.ibv_attr = NULL;
 	flow->drxq.ibv_flow = ibv_create_flow(priv->flow_drop_queue->qp,
 					      flow->drxq.ibv_attr);
-- 
2.11.0

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [dpdk-dev] [PATCH v3 4/7] net/mlx5: fix flow director drop action
  2017-10-23 14:49 ` [dpdk-dev] [PATCH v2 0/7] " Nelio Laranjeiro
                     ` (3 preceding siblings ...)
  2017-10-24 15:18   ` [dpdk-dev] [PATCH v3 3/7] net/mlx5: fix drop flows when port is stopped Nelio Laranjeiro
@ 2017-10-24 15:18   ` Nelio Laranjeiro
  2017-10-24 15:18   ` [dpdk-dev] [PATCH v3 5/7] net/mlx5: fix mark action with " Nelio Laranjeiro
                     ` (2 subsequent siblings)
  7 siblings, 0 replies; 32+ messages in thread
From: Nelio Laranjeiro @ 2017-10-24 15:18 UTC (permalink / raw)
  To: dev; +Cc: Yongseok Koh, Adrien Mazarguil

Flow director drop action as not been brought back with the new
implementation on top of rte flow.

Fixes: 4c3e9bcdd52e ("net/mlx5: support flow director")

Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
Acked-by: Yongseok Koh <yskoh@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 6f458f44a..96a753e8d 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -2601,20 +2601,27 @@ priv_fdir_filter_convert(struct priv *priv,
 		ERROR("invalid queue number %d", fdir_filter->action.rx_queue);
 		return EINVAL;
 	}
-	/* Validate the behavior. */
-	if (fdir_filter->action.behavior != RTE_ETH_FDIR_ACCEPT) {
-		ERROR("invalid behavior %d", fdir_filter->action.behavior);
-		return ENOTSUP;
-	}
 	attributes->attr.ingress = 1;
 	attributes->items[0] = (struct rte_flow_item) {
 		.type = RTE_FLOW_ITEM_TYPE_ETH,
 		.spec = &attributes->l2,
 	};
-	attributes->actions[0] = (struct rte_flow_action){
-		.type = RTE_FLOW_ACTION_TYPE_QUEUE,
-		.conf = &attributes->queue,
-	};
+	switch (fdir_filter->action.behavior) {
+	case RTE_ETH_FDIR_ACCEPT:
+		attributes->actions[0] = (struct rte_flow_action){
+			.type = RTE_FLOW_ACTION_TYPE_QUEUE,
+			.conf = &attributes->queue,
+		};
+		break;
+	case RTE_ETH_FDIR_REJECT:
+		attributes->actions[0] = (struct rte_flow_action){
+			.type = RTE_FLOW_ACTION_TYPE_DROP,
+		};
+		break;
+	default:
+		ERROR("invalid behavior %d", fdir_filter->action.behavior);
+		return ENOTSUP;
+	}
 	attributes->queue.index = fdir_filter->action.rx_queue;
 	switch (fdir_filter->input.flow_type) {
 	case RTE_ETH_FLOW_NONFRAG_IPV4_UDP:
-- 
2.11.0

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [dpdk-dev] [PATCH v3 5/7] net/mlx5: fix mark action with drop action
  2017-10-23 14:49 ` [dpdk-dev] [PATCH v2 0/7] " Nelio Laranjeiro
                     ` (4 preceding siblings ...)
  2017-10-24 15:18   ` [dpdk-dev] [PATCH v3 4/7] net/mlx5: fix flow director drop action Nelio Laranjeiro
@ 2017-10-24 15:18   ` Nelio Laranjeiro
  2017-10-24 15:18   ` [dpdk-dev] [PATCH v3 6/7] net/mlx5: fix reception when VLAN is added Nelio Laranjeiro
  2017-10-24 15:18   ` [dpdk-dev] [PATCH v3 7/7] net/mlx5: fix flow director flow add Nelio Laranjeiro
  7 siblings, 0 replies; 32+ messages in thread
From: Nelio Laranjeiro @ 2017-10-24 15:18 UTC (permalink / raw)
  To: dev; +Cc: Yongseok Koh, Adrien Mazarguil

Marking a packet which will not be received by the NIC is useless, even if
this action remains possible, it blocks the creation of the flow counter
which embed a mark action to a drop queue to be created.

Fixes: 31ba9997f11a ("net/mlx5: fully convert a flow to verbs in validate")

Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
Acked-by: Yongseok Koh <yskoh@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 96a753e8d..13b78ce9b 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -786,6 +786,8 @@ priv_flow_convert_actions(struct priv *priv,
 			goto exit_action_not_supported;
 		}
 	}
+	if (parser->drop && parser->mark)
+		parser->mark = 0;
 	if (!parser->queues_n && !parser->drop) {
 		rte_flow_error_set(error, ENOTSUP, RTE_FLOW_ERROR_TYPE_HANDLE,
 				   NULL, "no valid action");
-- 
2.11.0

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [dpdk-dev] [PATCH v3 6/7] net/mlx5: fix reception when VLAN is added
  2017-10-23 14:49 ` [dpdk-dev] [PATCH v2 0/7] " Nelio Laranjeiro
                     ` (5 preceding siblings ...)
  2017-10-24 15:18   ` [dpdk-dev] [PATCH v3 5/7] net/mlx5: fix mark action with " Nelio Laranjeiro
@ 2017-10-24 15:18   ` Nelio Laranjeiro
  2017-10-24 17:34     ` Yongseok Koh
  2017-10-24 15:18   ` [dpdk-dev] [PATCH v3 7/7] net/mlx5: fix flow director flow add Nelio Laranjeiro
  7 siblings, 1 reply; 32+ messages in thread
From: Nelio Laranjeiro @ 2017-10-24 15:18 UTC (permalink / raw)
  To: dev; +Cc: Yongseok Koh, Adrien Mazarguil

When VLAN is enabled in the Rx side, only packets matching this VLAN are
expected, this also includes the broadcast and all multicast packets.

Fixes: 272733b5ebfd ("net/mlx5: use flow to enable unicast traffic")
Fixes: 6a6b6828fe6a ("net/mlx5: use flow to enable all multi mode")

Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
---
 drivers/net/mlx5/mlx5_trigger.c | 156 +++++++++++++++++++++++-----------------
 1 file changed, 89 insertions(+), 67 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_trigger.c b/drivers/net/mlx5/mlx5_trigger.c
index 29167badd..982d0a24d 100644
--- a/drivers/net/mlx5/mlx5_trigger.c
+++ b/drivers/net/mlx5/mlx5_trigger.c
@@ -251,6 +251,29 @@ mlx5_dev_stop(struct rte_eth_dev *dev)
 int
 priv_dev_traffic_enable(struct priv *priv, struct rte_eth_dev *dev)
 {
+	struct rte_flow_item_eth bcast = {
+		.dst.addr_bytes = "\xff\xff\xff\xff\xff\xff",
+	};
+	struct rte_flow_item_eth ipv6_multi_spec = {
+		.dst.addr_bytes = "\x33\x33\x00\x00\x00\x00",
+	};
+	struct rte_flow_item_eth ipv6_multi_mask = {
+		.dst.addr_bytes = "\xff\xff\x00\x00\x00\x00",
+	};
+	struct rte_flow_item_eth unicast = {
+		.src.addr_bytes = "\x00\x00\x00\x00\x00\x00",
+	};
+	struct rte_flow_item_eth unicast_mask = {
+		.dst.addr_bytes = "\xff\xff\xff\xff\xff\xff",
+	};
+	const unsigned int vlan_filter_n = priv->vlan_filter_n;
+	const struct ether_addr cmp = {
+		.addr_bytes = "\x00\x00\x00\x00\x00\x00",
+	};
+	unsigned int i;
+	unsigned int j;
+	int ret;
+
 	if (priv->isolated)
 		return 0;
 	if (dev->data->promiscuous) {
@@ -261,81 +284,80 @@ priv_dev_traffic_enable(struct priv *priv, struct rte_eth_dev *dev)
 		};
 
 		claim_zero(mlx5_ctrl_flow(dev, &promisc, &promisc));
-	} else if (dev->data->all_multicast) {
+		return 0;
+	}
+	if (dev->data->all_multicast) {
 		struct rte_flow_item_eth multicast = {
 			.dst.addr_bytes = "\x01\x00\x00\x00\x00\x00",
-			.src.addr_bytes = "\x01\x00\x00\x00\x00\x00",
+			.src.addr_bytes = "\x00\x00\x00\x00\x00\x00",
 			.type = 0,
 		};
 
 		claim_zero(mlx5_ctrl_flow(dev, &multicast, &multicast));
 	} else {
-		struct rte_flow_item_eth bcast = {
-			.dst.addr_bytes = "\xff\xff\xff\xff\xff\xff",
-		};
-		struct rte_flow_item_eth ipv6_multi_spec = {
-			.dst.addr_bytes = "\x33\x33\x00\x00\x00\x00",
-		};
-		struct rte_flow_item_eth ipv6_multi_mask = {
-			.dst.addr_bytes = "\xff\xff\x00\x00\x00\x00",
-		};
-		struct rte_flow_item_eth unicast = {
-			.src.addr_bytes = "\x00\x00\x00\x00\x00\x00",
-		};
-		struct rte_flow_item_eth unicast_mask = {
-			.dst.addr_bytes = "\xff\xff\xff\xff\xff\xff",
-		};
-		const unsigned int vlan_filter_n = priv->vlan_filter_n;
-		const struct ether_addr cmp = {
-			.addr_bytes = "\x00\x00\x00\x00\x00\x00",
-		};
-		unsigned int i;
-		unsigned int j;
-		unsigned int unicast_flow = 0;
-		int ret;
-
-		for (i = 0; i != MLX5_MAX_MAC_ADDRESSES; ++i) {
-			struct ether_addr *mac = &dev->data->mac_addrs[i];
-
-			if (!memcmp(mac, &cmp, sizeof(*mac)))
-				continue;
-			memcpy(&unicast.dst.addr_bytes,
-			       mac->addr_bytes,
-			       ETHER_ADDR_LEN);
-			for (j = 0; j != vlan_filter_n; ++j) {
-				uint16_t vlan = priv->vlan_filter[j];
-
-				struct rte_flow_item_vlan vlan_spec = {
-					.tci = rte_cpu_to_be_16(vlan),
-				};
-				struct rte_flow_item_vlan vlan_mask = {
-					.tci = 0xffff,
-				};
-
-				ret = mlx5_ctrl_flow_vlan(dev, &unicast,
-							  &unicast_mask,
-							  &vlan_spec,
-							  &vlan_mask);
-				if (ret)
-					goto error;
-				unicast_flow = 1;
-			}
-			if (!vlan_filter_n) {
-				ret = mlx5_ctrl_flow(dev, &unicast,
-						     &unicast_mask);
-				if (ret)
-					goto error;
-				unicast_flow = 1;
-			}
+		/* Add broadcast/multicast flows. */
+		for (i = 0; i != vlan_filter_n; ++i) {
+			uint16_t vlan = priv->vlan_filter[i];
+
+			struct rte_flow_item_vlan vlan_spec = {
+				.tci = rte_cpu_to_be_16(vlan),
+			};
+			struct rte_flow_item_vlan vlan_mask = {
+				.tci = 0xffff,
+			};
+
+			ret = mlx5_ctrl_flow_vlan(dev, &bcast, &bcast,
+						  &vlan_spec, &vlan_mask);
+			if (ret)
+				goto error;
+			ret = mlx5_ctrl_flow_vlan(dev, &ipv6_multi_spec,
+						  &ipv6_multi_mask,
+						  &vlan_spec, &vlan_mask);
+			if (ret)
+				goto error;
+		}
+		if (!vlan_filter_n) {
+			ret = mlx5_ctrl_flow(dev, &bcast, &bcast);
+			if (ret)
+				goto error;
+			ret = mlx5_ctrl_flow(dev, &ipv6_multi_spec,
+					     &ipv6_multi_mask);
+			if (ret)
+				goto error;
+		}
+	}
+	/* Add MAC address flows. */
+	for (i = 0; i != MLX5_MAX_MAC_ADDRESSES; ++i) {
+		struct ether_addr *mac = &dev->data->mac_addrs[i];
+
+		if (!memcmp(mac, &cmp, sizeof(*mac)))
+			continue;
+		memcpy(&unicast.dst.addr_bytes,
+		       mac->addr_bytes,
+		       ETHER_ADDR_LEN);
+		for (j = 0; j != vlan_filter_n; ++j) {
+			uint16_t vlan = priv->vlan_filter[j];
+
+			struct rte_flow_item_vlan vlan_spec = {
+				.tci = rte_cpu_to_be_16(vlan),
+			};
+			struct rte_flow_item_vlan vlan_mask = {
+				.tci = 0xffff,
+			};
+
+			ret = mlx5_ctrl_flow_vlan(dev, &unicast,
+						  &unicast_mask,
+						  &vlan_spec,
+						  &vlan_mask);
+			if (ret)
+				goto error;
+		}
+		if (!vlan_filter_n) {
+			ret = mlx5_ctrl_flow(dev, &unicast,
+					     &unicast_mask);
+			if (ret)
+				goto error;
 		}
-		if (!unicast_flow)
-			return 0;
-		ret = mlx5_ctrl_flow(dev, &bcast, &bcast);
-		if (ret)
-			goto error;
-		ret = mlx5_ctrl_flow(dev, &ipv6_multi_spec, &ipv6_multi_mask);
-		if (ret)
-			goto error;
 	}
 	return 0;
 error:
-- 
2.11.0

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [dpdk-dev] [PATCH v3 7/7] net/mlx5: fix flow director flow add
  2017-10-23 14:49 ` [dpdk-dev] [PATCH v2 0/7] " Nelio Laranjeiro
                     ` (6 preceding siblings ...)
  2017-10-24 15:18   ` [dpdk-dev] [PATCH v3 6/7] net/mlx5: fix reception when VLAN is added Nelio Laranjeiro
@ 2017-10-24 15:18   ` Nelio Laranjeiro
  7 siblings, 0 replies; 32+ messages in thread
From: Nelio Laranjeiro @ 2017-10-24 15:18 UTC (permalink / raw)
  To: dev; +Cc: Yongseok Koh, Adrien Mazarguil

Flows are added by priv_flow_create() in the associated list, adding them a
second time corrupts the list causing an infinite loop when parsing it.

Fixes: 4c3e9bcdd52e ("net/mlx5: support flow director")

Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
Acked-by: Yongseok Koh <yskoh@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 13b78ce9b..26cf593af 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -2792,7 +2792,6 @@ priv_fdir_filter_add(struct priv *priv,
 				attributes.actions,
 				&error);
 	if (flow) {
-		TAILQ_INSERT_TAIL(&priv->flows, flow, next);
 		DEBUG("FDIR created %p", (void *)flow);
 		return 0;
 	}
-- 
2.11.0

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [dpdk-dev] [PATCH v3 6/7] net/mlx5: fix reception when VLAN is added
  2017-10-24 15:18   ` [dpdk-dev] [PATCH v3 6/7] net/mlx5: fix reception when VLAN is added Nelio Laranjeiro
@ 2017-10-24 17:34     ` Yongseok Koh
  0 siblings, 0 replies; 32+ messages in thread
From: Yongseok Koh @ 2017-10-24 17:34 UTC (permalink / raw)
  To: Nelio Laranjeiro; +Cc: dev, Adrien Mazarguil

On Tue, Oct 24, 2017 at 05:18:17PM +0200, Nelio Laranjeiro wrote:
> When VLAN is enabled in the Rx side, only packets matching this VLAN are
> expected, this also includes the broadcast and all multicast packets.
> 
> Fixes: 272733b5ebfd ("net/mlx5: use flow to enable unicast traffic")
> Fixes: 6a6b6828fe6a ("net/mlx5: use flow to enable all multi mode")
> 
> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> ---
Acked-by: Yongseok Koh <yskoh@mellanox.com>

Thanks

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [dpdk-dev] [PATCH v3 0/7] net/mlx5: fixes
  2017-10-24 15:18   ` [dpdk-dev] [PATCH v3 " Nelio Laranjeiro
@ 2017-10-24 19:07     ` Ferruh Yigit
  0 siblings, 0 replies; 32+ messages in thread
From: Ferruh Yigit @ 2017-10-24 19:07 UTC (permalink / raw)
  To: Nelio Laranjeiro, dev; +Cc: Yongseok Koh, Adrien Mazarguil

On 10/24/2017 8:18 AM, Nelio Laranjeiro wrote:
> Changes in v3:
> 
>  * Avoid to duplicate bcast/multicast flows when multiple MAC addressed are
>    configured.
> 
> Changes in v2:
> 
>  * Fix packets reception in allmulti mode and when VLAN are present.
>  * Fix a double flow insertion causing an infinite loop.
> 
> Nelio Laranjeiro (7):
>   net/mlx5: fix segfault on flow creation
>   net/mlx5: fix work queue array size
>   net/mlx5: fix drop flows when port is stopped
>   net/mlx5: fix flow director drop action
>   net/mlx5: fix mark action with drop action
>   net/mlx5: fix reception when VLAN is added
>   net/mlx5: fix flow director flow add

Series applied to dpdk-next-net/master, thanks.

^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2017-10-24 19:07 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-19 12:51 [dpdk-dev] [PATCH 0/6] net/mlx5: fixes Nelio Laranjeiro
2017-10-19 12:51 ` [dpdk-dev] [PATCH 1/6] net/mlx5: fix segfault on flow creation Nelio Laranjeiro
2017-10-19 12:51 ` [dpdk-dev] [PATCH 2/6] net/mlx5: fix work queue array size Nelio Laranjeiro
2017-10-19 12:51 ` [dpdk-dev] [PATCH 3/6] net/mlx5: fix drop flows when port is stopped Nelio Laranjeiro
2017-10-19 12:51 ` [dpdk-dev] [PATCH 4/6] net/mlx5: fix flow director drop action Nelio Laranjeiro
2017-10-19 12:51 ` [dpdk-dev] [PATCH 5/6] net/mlx5: fix mark action with " Nelio Laranjeiro
2017-10-19 12:51 ` [dpdk-dev] [PATCH 6/6] net/mlx5: fix allmulti mode Nelio Laranjeiro
2017-10-19 19:36 ` [dpdk-dev] [PATCH 0/6] net/mlx5: fixes Yongseok Koh
2017-10-23 14:49 ` [dpdk-dev] [PATCH v2 0/7] " Nelio Laranjeiro
2017-10-24 15:18   ` [dpdk-dev] [PATCH v3 " Nelio Laranjeiro
2017-10-24 19:07     ` Ferruh Yigit
2017-10-24 15:18   ` [dpdk-dev] [PATCH v3 1/7] net/mlx5: fix segfault on flow creation Nelio Laranjeiro
2017-10-24 15:18   ` [dpdk-dev] [PATCH v3 2/7] net/mlx5: fix work queue array size Nelio Laranjeiro
2017-10-24 15:18   ` [dpdk-dev] [PATCH v3 3/7] net/mlx5: fix drop flows when port is stopped Nelio Laranjeiro
2017-10-24 15:18   ` [dpdk-dev] [PATCH v3 4/7] net/mlx5: fix flow director drop action Nelio Laranjeiro
2017-10-24 15:18   ` [dpdk-dev] [PATCH v3 5/7] net/mlx5: fix mark action with " Nelio Laranjeiro
2017-10-24 15:18   ` [dpdk-dev] [PATCH v3 6/7] net/mlx5: fix reception when VLAN is added Nelio Laranjeiro
2017-10-24 17:34     ` Yongseok Koh
2017-10-24 15:18   ` [dpdk-dev] [PATCH v3 7/7] net/mlx5: fix flow director flow add Nelio Laranjeiro
2017-10-23 14:49 ` [dpdk-dev] [PATCH v2 1/7] net/mlx5: fix segfault on flow creation Nelio Laranjeiro
2017-10-23 14:49 ` [dpdk-dev] [PATCH v2 2/7] net/mlx5: fix work queue array size Nelio Laranjeiro
2017-10-23 14:49 ` [dpdk-dev] [PATCH v2 3/7] net/mlx5: fix drop flows when port is stopped Nelio Laranjeiro
2017-10-23 14:49 ` [dpdk-dev] [PATCH v2 4/7] net/mlx5: fix flow director drop action Nelio Laranjeiro
2017-10-23 14:49 ` [dpdk-dev] [PATCH v2 5/7] net/mlx5: fix mark action with " Nelio Laranjeiro
2017-10-23 14:49 ` [dpdk-dev] [PATCH v2 6/7] net/mlx5: fix reception when VLAN is added Nelio Laranjeiro
2017-10-23 19:25   ` Yongseok Koh
2017-10-24  7:11     ` Nélio Laranjeiro
2017-10-24  7:34       ` Nélio Laranjeiro
2017-10-24 13:41         ` Yongseok Koh
2017-10-24 14:19           ` Nélio Laranjeiro
2017-10-23 14:49 ` [dpdk-dev] [PATCH v2 7/7] net/mlx5: fix flow director flow add Nelio Laranjeiro
2017-10-23 20:48   ` Yongseok Koh

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).