[dpdk-stable] [PATCH 17.11 1/6] net/failsafe: fix crash on slave queue release

[dpdk-stable] [PATCH 17.11 1/6] net/failsafe: fix crash on slave queue release

[Andrew Rybchenko]

From: Igor Romanov <igor.romanov@oktetlabs.ru>

[ backported from upstream commit 6b35f4d88b40645425b4b8e156423982471eccf5 ]

Releasing a queue that is already released by slave may cause a
segmentation fault. For example, after a successfull device
configuration a queue is set up. Afterwards the device is reconfigured
with an invalid argument, forcing slaves to release the queues
(e.g. rte_eth_dev.data.tx_queues). Finally the failsafe's queues
are released. The queue release functions also try to release slaves'
queues using ETH(sdev)->data->tx_queues which is NULL at the time.

Add checks for NULL slaves' Tx and Rx queues before releasing them.

Fixes: a46f8d584eb8 ("net/failsafe: add fail-safe PMD")

Signed-off-by: Igor Romanov <igor.romanov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 drivers/net/failsafe/failsafe_ops.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c
index 9a5d873..885d63c 100644
--- a/drivers/net/failsafe/failsafe_ops.c
+++ b/drivers/net/failsafe/failsafe_ops.c
@@ -260,9 +260,13 @@
 		return;
 	rxq = queue;
 	dev = rxq->priv->dev;
-	FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE)
-		SUBOPS(sdev, rx_queue_release)
-			(ETH(sdev)->data->rx_queues[rxq->qid]);
+	FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
+		if (ETH(sdev)->data->rx_queues != NULL &&
+		    ETH(sdev)->data->rx_queues[rxq->qid] != NULL) {
+			SUBOPS(sdev, rx_queue_release)
+				(ETH(sdev)->data->rx_queues[rxq->qid]);
+		}
+	}
 	dev->data->rx_queues[rxq->qid] = NULL;
 	rte_free(rxq);
 }
@@ -328,9 +332,13 @@
 		return;
 	txq = queue;
 	dev = txq->priv->dev;
-	FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE)
-		SUBOPS(sdev, tx_queue_release)
-			(ETH(sdev)->data->tx_queues[txq->qid]);
+	FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
+		if (ETH(sdev)->data->tx_queues != NULL &&
+		    ETH(sdev)->data->tx_queues[txq->qid] != NULL) {
+			SUBOPS(sdev, tx_queue_release)
+				(ETH(sdev)->data->tx_queues[txq->qid]);
+		}
+	}
 	dev->data->tx_queues[txq->qid] = NULL;
 	rte_free(txq);
 }
-- 
1.8.3.1

[dpdk-stable] [PATCH 17.11 2/6] net/sfc/base: fix ID retrieval in v3 licensing

[Andrew Rybchenko]

From: Andy Moreton <amoreton@solarflare.com>

[ backported from upstream commit 416aa7f1c98acfaf996a54f63fd2a02dc5ef3a73 ]

Fixes: 05fce2ce8451 ("net/sfc/base: import libefx licensing")
Fixes: f67e4719147d ("net/sfc/base: fix coding style")

Signed-off-by: Andy Moreton <amoreton@solarflare.com>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 drivers/net/sfc/base/efx_lic.c | 38 +++++++++-----------------------------
 1 file changed, 9 insertions(+), 29 deletions(-)

diff --git a/drivers/net/sfc/base/efx_lic.c b/drivers/net/sfc/base/efx_lic.c
index 2cd05cc..26f38ec 100644
--- a/drivers/net/sfc/base/efx_lic.c
+++ b/drivers/net/sfc/base/efx_lic.c
@@ -1035,26 +1035,14 @@
 {
 	efx_mcdi_req_t req;
 	uint8_t payload[MAX(MC_CMD_LICENSING_GET_ID_V3_IN_LEN,
-			    MC_CMD_LICENSING_GET_ID_V3_OUT_LENMIN)];
+			    MC_CMD_LICENSING_GET_ID_V3_OUT_LENMAX)];
 	efx_rc_t rc;
 
 	req.emr_cmd = MC_CMD_LICENSING_GET_ID_V3;
-
-	if (bufferp == NULL) {
-		/* Request id type and length only */
-		req.emr_in_buf = bufferp;
-		req.emr_in_length = MC_CMD_LICENSING_GET_ID_V3_IN_LEN;
-		req.emr_out_buf = bufferp;
-		req.emr_out_length = MC_CMD_LICENSING_GET_ID_V3_OUT_LENMIN;
-		(void) memset(payload, 0, sizeof (payload));
-	} else {
-		/* Request full buffer */
-		req.emr_in_buf = bufferp;
-		req.emr_in_length = MC_CMD_LICENSING_GET_ID_V3_IN_LEN;
-		req.emr_out_buf = bufferp;
-		req.emr_out_length = MIN(buffer_size, MC_CMD_LICENSING_GET_ID_V3_OUT_LENMAX);
-		(void) memset(bufferp, 0, req.emr_out_length);
-	}
+	req.emr_in_buf = payload;
+	req.emr_in_length = MC_CMD_LICENSING_GET_ID_V3_IN_LEN;
+	req.emr_out_buf = payload;
+	req.emr_out_length = MC_CMD_LICENSING_GET_ID_V3_OUT_LENMAX;
 
 	efx_mcdi_execute_quiet(enp, &req);
 
@@ -1071,18 +1059,10 @@
 	*typep = MCDI_OUT_DWORD(req, LICENSING_GET_ID_V3_OUT_LICENSE_TYPE);
 	*lengthp = MCDI_OUT_DWORD(req, LICENSING_GET_ID_V3_OUT_LICENSE_ID_LENGTH);
 
-	if (bufferp == NULL) {
-		/* modify length requirements to indicate to caller the extra buffering
-		** needed to read the complete output.
-		*/
-		*lengthp += MC_CMD_LICENSING_GET_ID_V3_OUT_LENMIN;
-	} else {
-		/* Shift ID down to start of buffer */
-		memmove(bufferp,
-		    bufferp + MC_CMD_LICENSING_GET_ID_V3_OUT_LICENSE_ID_OFST,
-		    *lengthp);
-		memset(bufferp + (*lengthp), 0,
-		    MC_CMD_LICENSING_GET_ID_V3_OUT_LICENSE_ID_OFST);
+	if (bufferp != NULL) {
+		memcpy(bufferp,
+		    payload + MC_CMD_LICENSING_GET_ID_V3_OUT_LICENSE_ID_OFST,
+		    MIN(buffer_size, *lengthp));
 	}
 
 	return (0);
-- 
1.8.3.1

[dpdk-stable] [PATCH 17.11 3/6] net/failsafe: add checks for deferred queue setup

[Andrew Rybchenko]

From: Ian Dolzhansky <ian.dolzhansky@oktetlabs.ru>

[ backported from upstream commit c3a210a264bda441e36109af1624ecc79ff4655f ]

Fixes: a46f8d584eb8 ("net/failsafe: add fail-safe PMD")

Signed-off-by: Ian Dolzhansky <ian.dolzhansky@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 drivers/net/failsafe/failsafe_ops.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c
index 885d63c..7eb6f71 100644
--- a/drivers/net/failsafe/failsafe_ops.c
+++ b/drivers/net/failsafe/failsafe_ops.c
@@ -284,6 +284,11 @@
 	uint8_t i;
 	int ret;
 
+	if (rx_conf->rx_deferred_start) {
+		ERROR("Rx queue deferred start is not supported");
+		return -EINVAL;
+	}
+
 	rxq = dev->data->rx_queues[rx_queue_id];
 	if (rxq != NULL) {
 		fs_rx_queue_release(rxq);
@@ -355,6 +360,11 @@
 	uint8_t i;
 	int ret;
 
+	if (tx_conf->tx_deferred_start) {
+		ERROR("Tx queue deferred start is not supported");
+		return -EINVAL;
+	}
+
 	txq = dev->data->tx_queues[tx_queue_id];
 	if (txq != NULL) {
 		fs_tx_queue_release(txq);
-- 
1.8.3.1

[dpdk-stable] [PATCH 17.11 4/6] net/sfc: do not skip RSS configuration step on reconfigure

[Andrew Rybchenko]

From: Ivan Malov <ivan.malov@oktetlabs.ru>

[ backported from upstream commit 5205c436a33e060d06f7f230e051b94be31d6d91 ]

Earlier a patch was made to support change of Rx queue
number. That patch added goto label in wrong place
because reconfiguration with the same number of queues
results in skipping not only queue init but also RSS
settings. If a user configures device with RSS multiqueue
mode and then wants to stop it and reconfigure without RSS,
this change will be ignored and RSS will continue working.

Move the label in the right place and rename it to describe it.

Fixes: 55a539003f1a ("net/sfc: support changing the number of receive queues")

Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 drivers/net/sfc/sfc_rx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/sfc/sfc_rx.c b/drivers/net/sfc/sfc_rx.c
index 4497d5f..238aa5e 100644
--- a/drivers/net/sfc/sfc_rx.c
+++ b/drivers/net/sfc/sfc_rx.c
@@ -1341,7 +1341,7 @@ struct sfc_dp_rx sfc_efx_rx = {
 		goto fail_check_mode;
 
 	if (nb_rx_queues == sa->rxq_count)
-		goto done;
+		goto configure_rss;
 
 	if (sa->rxq_info == NULL) {
 		rc = ENOMEM;
@@ -1378,6 +1378,7 @@ struct sfc_dp_rx sfc_efx_rx = {
 		sa->rxq_count++;
 	}
 
+configure_rss:
 #if EFSYS_OPT_RX_SCALE
 	sa->rss_channels = (dev_conf->rxmode.mq_mode == ETH_MQ_RX_RSS) ?
 			   MIN(sa->rxq_count, EFX_MAXRSS) : 0;
@@ -1396,7 +1397,6 @@ struct sfc_dp_rx sfc_efx_rx = {
 	}
 #endif
 
-done:
 	return 0;
 
 fail_rx_process_adv_conf_rss:
-- 
1.8.3.1

[dpdk-stable] [PATCH 17.11 5/6] net/sfc: allow to query RSS key and HF in isolated mode

[Andrew Rybchenko]

From: Ivan Malov <ivan.malov@oktetlabs.ru>

[ backported from upstream commit 453e1d4baad9262391eaef15812ade9af1f15a3f ]

Isolated mode prevents global RSS from being enabled and configured.
However, an application may need to query default RSS key and hash
functions when a flow rule with RSS action is added which does not
contain custom RSS key or hash function choice. In this case
global RSS key and hash functions will be used to handle the rule,
and there should be some way for the application to query these
global default settings to clarify expectations on the traffic
distribution.

Fixes: 84a9b48128c1 ("net/sfc: support flow API isolated mode")

Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 drivers/net/sfc/sfc_ethdev.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index 94e7c79..f4e880d 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -1225,9 +1225,8 @@
 			  struct rte_eth_rss_conf *rss_conf)
 {
 	struct sfc_adapter *sa = dev->data->dev_private;
-	struct sfc_port *port = &sa->port;
 
-	if ((sa->rss_support != EFX_RX_SCALE_EXCLUSIVE) || port->isolated)
+	if (sa->rss_support != EFX_RX_SCALE_EXCLUSIVE)
 		return -ENOTSUP;
 
 	if (sa->rss_channels == 0)
-- 
1.8.3.1

[dpdk-stable] [PATCH 17.11 6/6] net/sfc: allow to query RSS key and HF when RSS is disabled

[Andrew Rybchenko]

From: Ivan Malov <ivan.malov@oktetlabs.ru>

[ backported from upstream commit 662e382244fb29d8bf03113524a139541e10123f ]

If global RSS is not enabled in the multiqueue mode setting,
it will not be possible to change RSS configuration. However,
querying default RSS settings should be possible in any case since
it may be needed by RTE flow API users to find out what RSS settings
will be used by default for a flow rule with RSS action if custom
RSS key and hash function choice are not specified.

Fixes: 63ab5e0c8fda ("net/sfc: use zero RSS channels as disabled RSS indicator")

Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 drivers/net/sfc/sfc_ethdev.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index f4e880d..819177a 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -1229,9 +1229,6 @@
 	if (sa->rss_support != EFX_RX_SCALE_EXCLUSIVE)
 		return -ENOTSUP;
 
-	if (sa->rss_channels == 0)
-		return -EINVAL;
-
 	sfc_adapter_lock(sa);
 
 	/*
-- 
1.8.3.1

Re: [dpdk-stable] [PATCH 17.11 1/6] net/failsafe: fix crash on slave queue release

[Yongseok Koh]

> On Dec 10, 2018, at 7:10 AM, Andrew Rybchenko <arybchenko@solarflare.com> wrote:
> 
> From: Igor Romanov <igor.romanov@oktetlabs.ru>
> 
> [ backported from upstream commit 6b35f4d88b40645425b4b8e156423982471eccf5 ]
> 
> Releasing a queue that is already released by slave may cause a
> segmentation fault. For example, after a successfull device
> configuration a queue is set up. Afterwards the device is reconfigured
> with an invalid argument, forcing slaves to release the queues
> (e.g. rte_eth_dev.data.tx_queues). Finally the failsafe's queues
> are released. The queue release functions also try to release slaves'
> queues using ETH(sdev)->data->tx_queues which is NULL at the time.
> 
> Add checks for NULL slaves' Tx and Rx queues before releasing them.
> 
> Fixes: a46f8d584eb8 ("net/failsafe: add fail-safe PMD")
> 
> Signed-off-by: Igor Romanov <igor.romanov@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> ---

Series applied to stable/17.11

Thanks,
Yongseok

> drivers/net/failsafe/failsafe_ops.c | 20 ++++++++++++++------
> 1 file changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c
> index 9a5d873..885d63c 100644
> --- a/drivers/net/failsafe/failsafe_ops.c
> +++ b/drivers/net/failsafe/failsafe_ops.c
> @@ -260,9 +260,13 @@
> 		return;
> 	rxq = queue;
> 	dev = rxq->priv->dev;
> -	FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE)
> -		SUBOPS(sdev, rx_queue_release)
> -			(ETH(sdev)->data->rx_queues[rxq->qid]);
> +	FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
> +		if (ETH(sdev)->data->rx_queues != NULL &&
> +		    ETH(sdev)->data->rx_queues[rxq->qid] != NULL) {
> +			SUBOPS(sdev, rx_queue_release)
> +				(ETH(sdev)->data->rx_queues[rxq->qid]);
> +		}
> +	}
> 	dev->data->rx_queues[rxq->qid] = NULL;
> 	rte_free(rxq);
> }
> @@ -328,9 +332,13 @@
> 		return;
> 	txq = queue;
> 	dev = txq->priv->dev;
> -	FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE)
> -		SUBOPS(sdev, tx_queue_release)
> -			(ETH(sdev)->data->tx_queues[txq->qid]);
> +	FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
> +		if (ETH(sdev)->data->tx_queues != NULL &&
> +		    ETH(sdev)->data->tx_queues[txq->qid] != NULL) {
> +			SUBOPS(sdev, tx_queue_release)
> +				(ETH(sdev)->data->tx_queues[txq->qid]);
> +		}
> +	}
> 	dev->data->tx_queues[txq->qid] = NULL;
> 	rte_free(txq);
> }
> -- 
> 1.8.3.1
>