[dpdk-dev] [PATCH 0/6] net/mlx5: fixes

[dpdk-dev] [PATCH 0/6] net/mlx5: fixes

[Nelio Laranjeiro]

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

[dpdk-dev] [PATCH 1/6] net/mlx5: fix segfault on flow creation

[Nelio Laranjeiro]

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

[dpdk-dev] [PATCH 2/6] net/mlx5: fix work queue array size

[Nelio Laranjeiro]

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

[dpdk-dev] [PATCH 3/6] net/mlx5: fix drop flows when port is stopped

[Nelio Laranjeiro]

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

[dpdk-dev] [PATCH 4/6] net/mlx5: fix flow director drop action

[Nelio Laranjeiro]

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

[dpdk-dev] [PATCH 5/6] net/mlx5: fix mark action with drop action

[Nelio Laranjeiro]

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

[dpdk-dev] [PATCH 6/6] net/mlx5: fix allmulti mode

[Nelio Laranjeiro]

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

Re: [dpdk-dev] [PATCH 0/6] net/mlx5: fixes

[Yongseok Koh]

> 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

[dpdk-dev] [PATCH v2 0/7] net/mlx5: fixes

[Nelio Laranjeiro]

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

[dpdk-dev] [PATCH v3 0/7] net/mlx5: fixes

[Nelio Laranjeiro]

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

Re: [dpdk-dev] [PATCH v3 0/7] net/mlx5: fixes

[Ferruh Yigit]

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.

[dpdk-dev] [PATCH v3 1/7] net/mlx5: fix segfault on flow creation

[Nelio Laranjeiro]

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

[dpdk-dev] [PATCH v3 2/7] net/mlx5: fix work queue array size

[Nelio Laranjeiro]

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

[dpdk-dev] [PATCH v3 3/7] net/mlx5: fix drop flows when port is stopped

[Nelio Laranjeiro]

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

[dpdk-dev] [PATCH v3 4/7] net/mlx5: fix flow director drop action

[Nelio Laranjeiro]

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

[dpdk-dev] [PATCH v3 5/7] net/mlx5: fix mark action with drop action

[Nelio Laranjeiro]

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

[dpdk-dev] [PATCH v3 6/7] net/mlx5: fix reception when VLAN is added

[Nelio Laranjeiro]

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

Re: [dpdk-dev] [PATCH v3 6/7] net/mlx5: fix reception when VLAN is added

[Yongseok Koh]

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

[dpdk-dev] [PATCH v3 7/7] net/mlx5: fix flow director flow add

[Nelio Laranjeiro]

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

[dpdk-dev] [PATCH v2 1/7] net/mlx5: fix segfault on flow creation

[Nelio Laranjeiro]

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

[dpdk-dev] [PATCH v2 2/7] net/mlx5: fix work queue array size

[Nelio Laranjeiro]

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

[dpdk-dev] [PATCH v2 3/7] net/mlx5: fix drop flows when port is stopped

[Nelio Laranjeiro]

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

[dpdk-dev] [PATCH v2 4/7] net/mlx5: fix flow director drop action

[Nelio Laranjeiro]

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

[dpdk-dev] [PATCH v2 5/7] net/mlx5: fix mark action with drop action

[Nelio Laranjeiro]

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

[dpdk-dev] [PATCH v2 6/7] net/mlx5: fix reception when VLAN is added

[Nelio Laranjeiro]

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

Re: [dpdk-dev] [PATCH v2 6/7] net/mlx5: fix reception when VLAN is added

[Yongseok Koh]

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

Re: [dpdk-dev] [PATCH v2 6/7] net/mlx5: fix reception when VLAN is added

[Nélio Laranjeiro]

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

Re: [dpdk-dev] [PATCH v2 6/7] net/mlx5: fix reception when VLAN is added

[Nélio Laranjeiro]

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

Re: [dpdk-dev] [PATCH v2 6/7] net/mlx5: fix reception when VLAN is added

[Yongseok Koh]

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

Re: [dpdk-dev] [PATCH v2 6/7] net/mlx5: fix reception when VLAN is added

[Nélio Laranjeiro]

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

[dpdk-dev] [PATCH v2 7/7] net/mlx5: fix flow director flow add

[Nelio Laranjeiro]

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

Re: [dpdk-dev] [PATCH v2 7/7] net/mlx5: fix flow director flow add

[Yongseok Koh]

> 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