[dpdk-stable] [PATCH v2 07/30] net/mlx5: fix reta update can segfault

[dpdk-stable] [PATCH v2 07/30] net/mlx5: fix reta update can segfault

[Nelio Laranjeiro]

Reta update needs to stop/start the port but stopping the port does not
disable the polling functions which may end in a segfault if a core is
polling the queue while the control thread is modifying it.

This patch changes the sequences to an order where such situation cannot
happen.

Fixes: aa13338faf5e ("net/mlx5: rebuild flows on updating RETA")
Cc: yskoh@mellanox.com
Cc: stable@dpdk.org

Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
---
 drivers/net/mlx5/mlx5_rss.c     | 9 +++++----
 drivers/net/mlx5/mlx5_trigger.c | 7 +++++++
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_rss.c b/drivers/net/mlx5/mlx5_rss.c
index d3d2603..8942879 100644
--- a/drivers/net/mlx5/mlx5_rss.c
+++ b/drivers/net/mlx5/mlx5_rss.c
@@ -351,11 +351,12 @@ mlx5_dev_rss_reta_update(struct rte_eth_dev *dev,
 	struct priv *priv = dev->data->dev_private;
 
 	assert(!mlx5_is_secondary());
-	mlx5_dev_stop(dev);
 	priv_lock(priv);
 	ret = priv_dev_rss_reta_update(priv, reta_conf, reta_size);
 	priv_unlock(priv);
-	if (ret)
-		return -ret;
-	return mlx5_dev_start(dev);
+	if (dev->data->dev_started) {
+		mlx5_dev_stop(dev);
+		mlx5_dev_start(dev);
+	}
+	return -ret;
 }
diff --git a/drivers/net/mlx5/mlx5_trigger.c b/drivers/net/mlx5/mlx5_trigger.c
index 212b4df..eeb9585 100644
--- a/drivers/net/mlx5/mlx5_trigger.c
+++ b/drivers/net/mlx5/mlx5_trigger.c
@@ -30,6 +30,7 @@
  *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
  *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
+#include <unistd.h>
 
 #include <rte_ether.h>
 #include <rte_ethdev.h>
@@ -118,6 +119,12 @@ mlx5_dev_stop(struct rte_eth_dev *dev)
 		return;
 
 	priv_lock(priv);
+	dev->data->dev_started = 0;
+	/* Prevent crashes when queues are still in use. */
+	dev->rx_pkt_burst = removed_rx_burst;
+	dev->tx_pkt_burst = removed_tx_burst;
+	rte_wmb();
+	usleep(1000 * priv->rxqs_n);
 	DEBUG("%p: cleaning up and destroying hash RX queues", (void *)dev);
 	priv_special_flow_disable_all(priv);
 	priv_mac_addrs_disable(priv);
-- 
2.1.4

Re: [dpdk-stable] [PATCH v2 07/30] net/mlx5: fix reta update can segfault

[Yongseok Koh]

> On Oct 5, 2017, at 5:49 AM, Nelio Laranjeiro <nelio.laranjeiro@6wind.com> wrote:
> 
> Reta update needs to stop/start the port but stopping the port does not
> disable the polling functions which may end in a segfault if a core is
> polling the queue while the control thread is modifying it.
> 
> This patch changes the sequences to an order where such situation cannot
> happen.
> 
> Fixes: aa13338faf5e ("net/mlx5: rebuild flows on updating RETA")
> Cc: yskoh@mellanox.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> ---
Acked-by: Yongseok Koh <yskoh@mellanox.com>
 
Thanks

[dpdk-stable] [PATCH v2 08/30] net/mlx5: fix rxqs vector support verification

[Nelio Laranjeiro]

The number of queues in DPDK does not means that the array of queue will be
totally filled, those information are uncorrelated.  The number of queues
is provided in the port configuration whereas the array is filled by
calling tx/rx_queue_setup().  As this number of queue is not increased or
decrease according to tx/rx_queue_setup() or tx/rx_queue_release(), PMD
must consider a queue may not be initialised in some position of the array.

Fixes: 6cb559d67b83 ("net/mlx5: add vectorized Rx/Tx burst for x86")
Cc: yskoh@mellanox.com
Cc: stable@dpdk.org

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

diff --git a/drivers/net/mlx5/mlx5_rxtx_vec_sse.c b/drivers/net/mlx5/mlx5_rxtx_vec_sse.c
index 2750eac..20ea38e 100644
--- a/drivers/net/mlx5/mlx5_rxtx_vec_sse.c
+++ b/drivers/net/mlx5/mlx5_rxtx_vec_sse.c
@@ -1367,6 +1367,8 @@ priv_check_vec_rx_support(struct priv *priv)
 	for (i = 0; i < priv->rxqs_n; ++i) {
 		struct mlx5_rxq_data *rxq = (*priv->rxqs)[i];
 
+		if (!rxq)
+			continue;
 		if (rxq_check_vec_support(rxq) < 0)
 			break;
 	}
-- 
2.1.4

Re: [dpdk-stable] [PATCH v2 08/30] net/mlx5: fix rxqs vector support verification

[Yongseok Koh]

> On Oct 5, 2017, at 5:49 AM, Nelio Laranjeiro <nelio.laranjeiro@6wind.com> wrote:
> 
> The number of queues in DPDK does not means that the array of queue will be
> totally filled, those information are uncorrelated.  The number of queues
> is provided in the port configuration whereas the array is filled by
> calling tx/rx_queue_setup().  As this number of queue is not increased or
> decrease according to tx/rx_queue_setup() or tx/rx_queue_release(), PMD
> must consider a queue may not be initialised in some position of the array.
> 
> Fixes: 6cb559d67b83 ("net/mlx5: add vectorized Rx/Tx burst for x86")
> Cc: yskoh@mellanox.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> ---
Acked-by: Yongseok Koh <yskoh@mellanox.com>
 
Thanks

[dpdk-stable] [PATCH v2 16/30] net/mlx5: fix clang compilation error

[Nelio Laranjeiro]

drivers/net/mlx5/mlx5_rxq.c:606:6: error: comparison of constant 4
      with expression of type 'enum hash_rxq_flow_type' is always true
      [-Werror,-Wtautological-constant-out-of-range-compare]
                        i != (int)RTE_DIM((*priv->hash_rxqs)[0].special_flow);
                        ~ ^  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Clang expects to have an index going upto special_flow size which is
defined by MLX5_MAX_SPECIAL_FLOWS and value is 4.  Comparing to an
unrelated enum where index my be lower cause this compilation issue.

Fixes: 36351ea34b92 ("net/mlx: fix build with icc")
Cc: ferruh.yigit@intel.com
Cc: stable@dpdk.org

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

diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
index b240c16..81e9eb5 100644
--- a/drivers/net/mlx5/mlx5_rxq.c
+++ b/drivers/net/mlx5/mlx5_rxq.c
@@ -606,11 +606,9 @@ priv_allow_flow_type(struct priv *priv, enum hash_rxq_flow_type type)
 int
 priv_rehash_flows(struct priv *priv)
 {
-	enum hash_rxq_flow_type i;
+	size_t i;
 
-	for (i = HASH_RXQ_FLOW_TYPE_PROMISC;
-			i != RTE_DIM((*priv->hash_rxqs)[0].special_flow);
-			++i)
+	for (i = 0; i != RTE_DIM((*priv->hash_rxqs)[0].special_flow); ++i)
 		if (!priv_allow_flow_type(priv, i)) {
 			priv_special_flow_disable(priv, i);
 		} else {
-- 
2.1.4

Re: [dpdk-stable] [PATCH v2 16/30] net/mlx5: fix clang compilation error

[Yongseok Koh]

On Thu, Oct 05, 2017 at 02:49:48PM +0200, Nelio Laranjeiro wrote:
> drivers/net/mlx5/mlx5_rxq.c:606:6: error: comparison of constant 4
>       with expression of type 'enum hash_rxq_flow_type' is always true
>       [-Werror,-Wtautological-constant-out-of-range-compare]
>                         i != (int)RTE_DIM((*priv->hash_rxqs)[0].special_flow);
>                         ~ ^  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Clang expects to have an index going upto special_flow size which is
> defined by MLX5_MAX_SPECIAL_FLOWS and value is 4.  Comparing to an
> unrelated enum where index my be lower cause this compilation issue.
> 
> Fixes: 36351ea34b92 ("net/mlx: fix build with icc")
> Cc: ferruh.yigit@intel.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> ---
Acked-by: Yongseok Koh <yskoh@mellanox.com>
 
Thanks

[dpdk-stable] [PATCH v3 07/30] net/mlx5: fix reta update can segfault

[Nelio Laranjeiro]

Reta update needs to stop/start the port but stopping the port does not
disable the polling functions which may end in a segfault if a core is
polling the queue while the control thread is modifying it.

This patch changes the sequences to an order where such situation cannot
happen.

Fixes: aa13338faf5e ("net/mlx5: rebuild flows on updating RETA")
Cc: yskoh@mellanox.com
Cc: stable@dpdk.org

Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
Acked-by: Yongseok Koh <yskoh@mellanox.com>
---
 drivers/net/mlx5/mlx5_rss.c     | 9 +++++----
 drivers/net/mlx5/mlx5_trigger.c | 7 +++++++
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_rss.c b/drivers/net/mlx5/mlx5_rss.c
index d3d2603..8942879 100644
--- a/drivers/net/mlx5/mlx5_rss.c
+++ b/drivers/net/mlx5/mlx5_rss.c
@@ -351,11 +351,12 @@ mlx5_dev_rss_reta_update(struct rte_eth_dev *dev,
 	struct priv *priv = dev->data->dev_private;
 
 	assert(!mlx5_is_secondary());
-	mlx5_dev_stop(dev);
 	priv_lock(priv);
 	ret = priv_dev_rss_reta_update(priv, reta_conf, reta_size);
 	priv_unlock(priv);
-	if (ret)
-		return -ret;
-	return mlx5_dev_start(dev);
+	if (dev->data->dev_started) {
+		mlx5_dev_stop(dev);
+		mlx5_dev_start(dev);
+	}
+	return -ret;
 }
diff --git a/drivers/net/mlx5/mlx5_trigger.c b/drivers/net/mlx5/mlx5_trigger.c
index 212b4df..eeb9585 100644
--- a/drivers/net/mlx5/mlx5_trigger.c
+++ b/drivers/net/mlx5/mlx5_trigger.c
@@ -30,6 +30,7 @@
  *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
  *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
+#include <unistd.h>
 
 #include <rte_ether.h>
 #include <rte_ethdev.h>
@@ -118,6 +119,12 @@ mlx5_dev_stop(struct rte_eth_dev *dev)
 		return;
 
 	priv_lock(priv);
+	dev->data->dev_started = 0;
+	/* Prevent crashes when queues are still in use. */
+	dev->rx_pkt_burst = removed_rx_burst;
+	dev->tx_pkt_burst = removed_tx_burst;
+	rte_wmb();
+	usleep(1000 * priv->rxqs_n);
 	DEBUG("%p: cleaning up and destroying hash RX queues", (void *)dev);
 	priv_special_flow_disable_all(priv);
 	priv_mac_addrs_disable(priv);
-- 
2.1.4

[dpdk-stable] [PATCH v3 08/30] net/mlx5: fix rxqs vector support verification

[Nelio Laranjeiro]

The number of queues in DPDK does not means that the array of queue will be
totally filled, those information are uncorrelated.  The number of queues
is provided in the port configuration whereas the array is filled by
calling tx/rx_queue_setup().  As this number of queue is not increased or
decrease according to tx/rx_queue_setup() or tx/rx_queue_release(), PMD
must consider a queue may not be initialised in some position of the array.

Fixes: 6cb559d67b83 ("net/mlx5: add vectorized Rx/Tx burst for x86")
Cc: yskoh@mellanox.com
Cc: stable@dpdk.org

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

diff --git a/drivers/net/mlx5/mlx5_rxtx_vec_sse.c b/drivers/net/mlx5/mlx5_rxtx_vec_sse.c
index 7e5ce6d..6d337ec 100644
--- a/drivers/net/mlx5/mlx5_rxtx_vec_sse.c
+++ b/drivers/net/mlx5/mlx5_rxtx_vec_sse.c
@@ -1376,6 +1376,8 @@ priv_check_vec_rx_support(struct priv *priv)
 	for (i = 0; i < priv->rxqs_n; ++i) {
 		struct mlx5_rxq_data *rxq = (*priv->rxqs)[i];
 
+		if (!rxq)
+			continue;
 		if (rxq_check_vec_support(rxq) < 0)
 			break;
 	}
-- 
2.1.4

[dpdk-stable] [PATCH v3 16/30] net/mlx5: fix clang compilation error

[Nelio Laranjeiro]

drivers/net/mlx5/mlx5_rxq.c:606:6: error: comparison of constant 4
      with expression of type 'enum hash_rxq_flow_type' is always true
      [-Werror,-Wtautological-constant-out-of-range-compare]
                        i != (int)RTE_DIM((*priv->hash_rxqs)[0].special_flow);
                        ~ ^  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Clang expects to have an index going upto special_flow size which is
defined by MLX5_MAX_SPECIAL_FLOWS and value is 4.  Comparing to an
unrelated enum where index my be lower cause this compilation issue.

Fixes: 36351ea34b92 ("net/mlx: fix build with icc")
Cc: ferruh.yigit@intel.com
Cc: stable@dpdk.org

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

diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
index b240c16..81e9eb5 100644
--- a/drivers/net/mlx5/mlx5_rxq.c
+++ b/drivers/net/mlx5/mlx5_rxq.c
@@ -606,11 +606,9 @@ priv_allow_flow_type(struct priv *priv, enum hash_rxq_flow_type type)
 int
 priv_rehash_flows(struct priv *priv)
 {
-	enum hash_rxq_flow_type i;
+	size_t i;
 
-	for (i = HASH_RXQ_FLOW_TYPE_PROMISC;
-			i != RTE_DIM((*priv->hash_rxqs)[0].special_flow);
-			++i)
+	for (i = 0; i != RTE_DIM((*priv->hash_rxqs)[0].special_flow); ++i)
 		if (!priv_allow_flow_type(priv, i)) {
 			priv_special_flow_disable(priv, i);
 		} else {
-- 
2.1.4