DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 1/2] net/qede: fix assignment of Rx/Tx handlers
@ 2020-05-05  3:09 Rasesh Mody
  2020-05-05  3:09 ` [dpdk-dev] [PATCH 2/2] net/qede: restore Tx queue setup Rasesh Mody
                   ` (5 more replies)
  0 siblings, 6 replies; 22+ messages in thread
From: Rasesh Mody @ 2020-05-05  3:09 UTC (permalink / raw)
  To: dev, jerinj, ferruh.yigit
  Cc: Rasesh Mody, GR-Everest-DPDK-Dev, stable, Igor Russkikh

Fix to assign dummy Rx/Tx handlers in dev_stop.
For MTU set, assignment of the appropriate Rx/Tx handlers will be
handled by dev_start/dev_stop.

Fixes: 81f8804992c9 ("net/qede: enhance Rx CPU utilization")
Fixes: 8de0c4201926 ("net/qede: fix odd number of queues usage in 100G mode")
Cc: stable@dpdk.org

Signed-off-by: Rasesh Mody <rmody@marvell.com>
Signed-off-by: Igor Russkikh <irusskikh@marvell.com>
---
 drivers/net/qede/qede_ethdev.c | 33 ++++++++++++++++-----------------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
index e71fa1e6a..726daa3e3 100644
--- a/drivers/net/qede/qede_ethdev.c
+++ b/drivers/net/qede/qede_ethdev.c
@@ -320,13 +320,19 @@ qede_interrupt_handler(void *param)
 }
 
 static void
-qede_assign_rxtx_handlers(struct rte_eth_dev *dev)
+qede_assign_rxtx_handlers(struct rte_eth_dev *dev, bool is_dummy)
 {
 	uint64_t tx_offloads = dev->data->dev_conf.txmode.offloads;
 	struct qede_dev *qdev = dev->data->dev_private;
 	struct ecore_dev *edev = &qdev->edev;
 	bool use_tx_offload = false;
 
+	if (is_dummy) {
+		dev->rx_pkt_burst = qede_rxtx_pkts_dummy;
+		dev->tx_pkt_burst = qede_rxtx_pkts_dummy;
+		return;
+	}
+
 	if (ECORE_IS_CMT(edev)) {
 		dev->rx_pkt_burst = qede_recv_pkts_cmt;
 		dev->tx_pkt_burst = qede_xmit_pkts_cmt;
@@ -1150,7 +1156,9 @@ static int qede_dev_start(struct rte_eth_dev *eth_dev)
 	/* Start/resume traffic */
 	qede_fastpath_start(edev);
 
-	qede_assign_rxtx_handlers(eth_dev);
+	/* Assign I/O handlers */
+	qede_assign_rxtx_handlers(eth_dev, false);
+
 	DP_INFO(edev, "Device started\n");
 
 	return 0;
@@ -1166,6 +1174,11 @@ static void qede_dev_stop(struct rte_eth_dev *eth_dev)
 
 	PMD_INIT_FUNC_TRACE(edev);
 
+	/* Replace I/O functions with dummy ones. It cannot
+	 * be set to NULL because rte_eth_rx_burst() doesn't check for NULL.
+	 */
+	qede_assign_rxtx_handlers(eth_dev, true);
+
 	/* Disable vport */
 	if (qede_activate_vport(eth_dev, false))
 		return;
@@ -2316,11 +2329,6 @@ static int qede_set_mtu(struct rte_eth_dev *dev, uint16_t mtu)
 			dev->data->min_rx_buf_size);
 		return -EINVAL;
 	}
-	/* Temporarily replace I/O functions with dummy ones. It cannot
-	 * be set to NULL because rte_eth_rx_burst() doesn't check for NULL.
-	 */
-	dev->rx_pkt_burst = qede_rxtx_pkts_dummy;
-	dev->tx_pkt_burst = qede_rxtx_pkts_dummy;
 	if (dev->data->dev_started) {
 		dev->data->dev_started = 0;
 		qede_dev_stop(dev);
@@ -2359,15 +2367,6 @@ static int qede_set_mtu(struct rte_eth_dev *dev, uint16_t mtu)
 	/* update max frame size */
 	dev->data->dev_conf.rxmode.max_rx_pkt_len = max_rx_pkt_len;
 
-	/* Reassign back */
-	qede_assign_rxtx_handlers(dev);
-	if (ECORE_IS_CMT(edev)) {
-		dev->rx_pkt_burst = qede_recv_pkts_cmt;
-		dev->tx_pkt_burst = qede_xmit_pkts_cmt;
-	} else {
-		dev->rx_pkt_burst = qede_recv_pkts;
-		dev->tx_pkt_burst = qede_xmit_pkts;
-	}
 	return 0;
 }
 
@@ -2570,7 +2569,7 @@ static int qede_common_dev_init(struct rte_eth_dev *eth_dev, bool is_vf)
 	strncpy((char *)params.name, QEDE_PMD_VER_PREFIX,
 		QEDE_PMD_DRV_VER_STR_SIZE);
 
-	qede_assign_rxtx_handlers(eth_dev);
+	qede_assign_rxtx_handlers(eth_dev, true);
 	eth_dev->tx_pkt_prepare = qede_xmit_prep_pkts;
 
 	/* For CMT mode device do periodic polling for slowpath events.
-- 
2.18.0


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

* [dpdk-dev] [PATCH 2/2] net/qede: restore Tx queue setup
  2020-05-05  3:09 [dpdk-dev] [PATCH 1/2] net/qede: fix assignment of Rx/Tx handlers Rasesh Mody
@ 2020-05-05  3:09 ` Rasesh Mody
  2020-05-05  6:44   ` Jerin Jacob
  2020-05-05  9:01 ` [dpdk-dev] [PATCH 1/2] net/qede: fix assignment of Rx/Tx handlers Ferruh Yigit
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 22+ messages in thread
From: Rasesh Mody @ 2020-05-05  3:09 UTC (permalink / raw)
  To: dev, jerinj, ferruh.yigit; +Cc: Rasesh Mody, GR-Everest-DPDK-Dev, Igor Russkikh

Some applications do not explicitly restore Tx queues setup during
port re-configuration. This patch adds changes to check for
released Tx queues and restore the setup if application doesn't
explicitly does that.

Signed-off-by: Rasesh Mody <rmody@marvell.com>
Signed-off-by: Igor Russkikh <irusskikh@marvell.com>
---
 drivers/net/qede/qede_ethdev.h |  3 +++
 drivers/net/qede/qede_rxtx.c   | 25 ++++++++++++++++++++++++-
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/net/qede/qede_ethdev.h b/drivers/net/qede/qede_ethdev.h
index b988a73f2..2e8e5febc 100644
--- a/drivers/net/qede/qede_ethdev.h
+++ b/drivers/net/qede/qede_ethdev.h
@@ -235,6 +235,9 @@ struct qede_dev {
 	bool enable_lro;
 	uint8_t num_rx_queues;
 	uint8_t num_tx_queues;
+	uint16_t num_rx_desc;
+	uint16_t num_tx_desc;
+	const struct rte_eth_txconf *tx_conf;
 	SLIST_HEAD(vlan_list_head, qede_vlan_entry)vlan_list_head;
 	uint16_t configured_vlans;
 	bool accept_any_vlan;
diff --git a/drivers/net/qede/qede_rxtx.c b/drivers/net/qede/qede_rxtx.c
index b81788ca4..1b212a4fb 100644
--- a/drivers/net/qede/qede_rxtx.c
+++ b/drivers/net/qede/qede_rxtx.c
@@ -151,6 +151,7 @@ qede_alloc_rx_queue_mem(struct rte_eth_dev *dev,
 	rxq->qdev = qdev;
 	rxq->mb_pool = mp;
 	rxq->nb_rx_desc = nb_desc;
+	qdev->num_rx_desc = rxq->nb_rx_desc;
 	rxq->queue_id = queue_idx;
 	rxq->port_id = dev->data->port_id;
 
@@ -405,6 +406,7 @@ qede_alloc_tx_queue_mem(struct rte_eth_dev *dev,
 	}
 
 	txq->nb_tx_desc = nb_desc;
+	qdev->num_tx_desc = txq->nb_tx_desc;
 	txq->qdev = qdev;
 	txq->port_id = dev->data->port_id;
 
@@ -443,6 +445,7 @@ qede_alloc_tx_queue_mem(struct rte_eth_dev *dev,
 
 	txq->nb_tx_avail = txq->nb_tx_desc;
 
+	qdev->tx_conf = tx_conf;
 	txq->tx_free_thresh =
 	    tx_conf->tx_free_thresh ? tx_conf->tx_free_thresh :
 	    (txq->nb_tx_desc - QEDE_DEFAULT_TX_FREE_THRESH);
@@ -593,7 +596,7 @@ qede_alloc_mem_sb(struct qede_dev *qdev, struct ecore_sb_info *sb_info,
 
 int qede_alloc_fp_resc(struct qede_dev *qdev)
 {
-	struct ecore_dev *edev = &qdev->edev;
+	struct ecore_dev *edev = QEDE_INIT_EDEV(qdev);
 	struct qede_fastpath *fp;
 	uint32_t num_sbs;
 	uint16_t sb_idx;
@@ -1005,9 +1008,29 @@ static int qede_tx_queue_stop(struct rte_eth_dev *eth_dev, uint16_t tx_queue_id)
 int qede_start_queues(struct rte_eth_dev *eth_dev)
 {
 	struct qede_dev *qdev = QEDE_INIT_QDEV(eth_dev);
+	struct ecore_dev *edev = QEDE_INIT_EDEV(qdev);
+	struct qede_tx_queue *txq;
+	struct qede_fastpath *fp;
 	uint8_t id;
 	int rc = -1;
 
+	/* Restore setup of Tx queues */
+	for (id = 0; id < qdev->num_tx_queues; id++) {
+		fp = &qdev->fp_array[id];
+		txq = fp->txq;
+
+		if (!txq) {
+			rc = qede_tx_queue_setup(eth_dev, id, qdev->num_tx_desc,
+						 eth_dev->data->numa_node,
+						 qdev->tx_conf);
+			if (rc != ECORE_SUCCESS) {
+				DP_ERR(edev, "TX queue %u not setup rc=%d\n",
+				       id, rc);
+				return -1;
+			}
+		}
+	}
+
 	for (id = 0; id < qdev->num_rx_queues; id++) {
 		rc = qede_rx_queue_start(eth_dev, id);
 		if (rc != ECORE_SUCCESS)
-- 
2.18.0


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

* Re: [dpdk-dev] [PATCH 2/2] net/qede: restore Tx queue setup
  2020-05-05  3:09 ` [dpdk-dev] [PATCH 2/2] net/qede: restore Tx queue setup Rasesh Mody
@ 2020-05-05  6:44   ` Jerin Jacob
  2020-05-05  8:59     ` Ferruh Yigit
  0 siblings, 1 reply; 22+ messages in thread
From: Jerin Jacob @ 2020-05-05  6:44 UTC (permalink / raw)
  To: Rasesh Mody
  Cc: dpdk-dev, Jerin Jacob, Ferruh Yigit, GR-Everest-DPDK-Dev,
	Igor Russkikh, Thomas Monjalon, Andrew Rybchenko

On Tue, May 5, 2020 at 8:39 AM Rasesh Mody <rmody@marvell.com> wrote:
>
> Some applications do not explicitly restore Tx queues setup during
> port re-configuration. This patch adds changes to check for
> released Tx queues and restore the setup if application doesn't
> explicitly does that.

+ethdev maintainers.

I think, Ideally, the fix should be in common code if we need to
support such applications.

This would avoid the following case
- The application works only on a specific PMD.
- every PMD duplicating such restoration code.


>
> Signed-off-by: Rasesh Mody <rmody@marvell.com>
> Signed-off-by: Igor Russkikh <irusskikh@marvell.com>
> ---
>  drivers/net/qede/qede_ethdev.h |  3 +++
>  drivers/net/qede/qede_rxtx.c   | 25 ++++++++++++++++++++++++-
>  2 files changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/qede/qede_ethdev.h b/drivers/net/qede/qede_ethdev.h
> index b988a73f2..2e8e5febc 100644
> --- a/drivers/net/qede/qede_ethdev.h
> +++ b/drivers/net/qede/qede_ethdev.h
> @@ -235,6 +235,9 @@ struct qede_dev {
>         bool enable_lro;
>         uint8_t num_rx_queues;
>         uint8_t num_tx_queues;
> +       uint16_t num_rx_desc;
> +       uint16_t num_tx_desc;
> +       const struct rte_eth_txconf *tx_conf;
>         SLIST_HEAD(vlan_list_head, qede_vlan_entry)vlan_list_head;
>         uint16_t configured_vlans;
>         bool accept_any_vlan;
> diff --git a/drivers/net/qede/qede_rxtx.c b/drivers/net/qede/qede_rxtx.c
> index b81788ca4..1b212a4fb 100644
> --- a/drivers/net/qede/qede_rxtx.c
> +++ b/drivers/net/qede/qede_rxtx.c
> @@ -151,6 +151,7 @@ qede_alloc_rx_queue_mem(struct rte_eth_dev *dev,
>         rxq->qdev = qdev;
>         rxq->mb_pool = mp;
>         rxq->nb_rx_desc = nb_desc;
> +       qdev->num_rx_desc = rxq->nb_rx_desc;
>         rxq->queue_id = queue_idx;
>         rxq->port_id = dev->data->port_id;
>
> @@ -405,6 +406,7 @@ qede_alloc_tx_queue_mem(struct rte_eth_dev *dev,
>         }
>
>         txq->nb_tx_desc = nb_desc;
> +       qdev->num_tx_desc = txq->nb_tx_desc;
>         txq->qdev = qdev;
>         txq->port_id = dev->data->port_id;
>
> @@ -443,6 +445,7 @@ qede_alloc_tx_queue_mem(struct rte_eth_dev *dev,
>
>         txq->nb_tx_avail = txq->nb_tx_desc;
>
> +       qdev->tx_conf = tx_conf;
>         txq->tx_free_thresh =
>             tx_conf->tx_free_thresh ? tx_conf->tx_free_thresh :
>             (txq->nb_tx_desc - QEDE_DEFAULT_TX_FREE_THRESH);
> @@ -593,7 +596,7 @@ qede_alloc_mem_sb(struct qede_dev *qdev, struct ecore_sb_info *sb_info,
>
>  int qede_alloc_fp_resc(struct qede_dev *qdev)
>  {
> -       struct ecore_dev *edev = &qdev->edev;
> +       struct ecore_dev *edev = QEDE_INIT_EDEV(qdev);
>         struct qede_fastpath *fp;
>         uint32_t num_sbs;
>         uint16_t sb_idx;
> @@ -1005,9 +1008,29 @@ static int qede_tx_queue_stop(struct rte_eth_dev *eth_dev, uint16_t tx_queue_id)
>  int qede_start_queues(struct rte_eth_dev *eth_dev)
>  {
>         struct qede_dev *qdev = QEDE_INIT_QDEV(eth_dev);
> +       struct ecore_dev *edev = QEDE_INIT_EDEV(qdev);
> +       struct qede_tx_queue *txq;
> +       struct qede_fastpath *fp;
>         uint8_t id;
>         int rc = -1;
>
> +       /* Restore setup of Tx queues */
> +       for (id = 0; id < qdev->num_tx_queues; id++) {
> +               fp = &qdev->fp_array[id];
> +               txq = fp->txq;
> +
> +               if (!txq) {
> +                       rc = qede_tx_queue_setup(eth_dev, id, qdev->num_tx_desc,
> +                                                eth_dev->data->numa_node,
> +                                                qdev->tx_conf);
> +                       if (rc != ECORE_SUCCESS) {
> +                               DP_ERR(edev, "TX queue %u not setup rc=%d\n",
> +                                      id, rc);
> +                               return -1;
> +                       }
> +               }
> +       }
> +
>         for (id = 0; id < qdev->num_rx_queues; id++) {
>                 rc = qede_rx_queue_start(eth_dev, id);
>                 if (rc != ECORE_SUCCESS)
> --
> 2.18.0
>

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

* Re: [dpdk-dev] [PATCH 2/2] net/qede: restore Tx queue setup
  2020-05-05  6:44   ` Jerin Jacob
@ 2020-05-05  8:59     ` Ferruh Yigit
  2020-05-05  9:15       ` Thomas Monjalon
  0 siblings, 1 reply; 22+ messages in thread
From: Ferruh Yigit @ 2020-05-05  8:59 UTC (permalink / raw)
  To: Jerin Jacob, Rasesh Mody
  Cc: dpdk-dev, Jerin Jacob, GR-Everest-DPDK-Dev, Igor Russkikh,
	Thomas Monjalon, Andrew Rybchenko

On 5/5/2020 7:44 AM, Jerin Jacob wrote:
> On Tue, May 5, 2020 at 8:39 AM Rasesh Mody <rmody@marvell.com> wrote:
>>
>> Some applications do not explicitly restore Tx queues setup during
>> port re-configuration. This patch adds changes to check for
>> released Tx queues and restore the setup if application doesn't
>> explicitly does that.
> 
> +ethdev maintainers.
> 
> I think, Ideally, the fix should be in common code if we need to
> support such applications.

Is this a case application re-configures to increase the number of queues but
doesn't setup new queues?
If so this looks like application error and application should be fixed instead
of recover this in the ethdev.

> 
> This would avoid the following case
> - The application works only on a specific PMD.
> - every PMD duplicating such restoration code.
> 
> 
>>
>> Signed-off-by: Rasesh Mody <rmody@marvell.com>
>> Signed-off-by: Igor Russkikh <irusskikh@marvell.com>
>> ---
>>  drivers/net/qede/qede_ethdev.h |  3 +++
>>  drivers/net/qede/qede_rxtx.c   | 25 ++++++++++++++++++++++++-
>>  2 files changed, 27 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/qede/qede_ethdev.h b/drivers/net/qede/qede_ethdev.h
>> index b988a73f2..2e8e5febc 100644
>> --- a/drivers/net/qede/qede_ethdev.h
>> +++ b/drivers/net/qede/qede_ethdev.h
>> @@ -235,6 +235,9 @@ struct qede_dev {
>>         bool enable_lro;
>>         uint8_t num_rx_queues;
>>         uint8_t num_tx_queues;
>> +       uint16_t num_rx_desc;
>> +       uint16_t num_tx_desc;
>> +       const struct rte_eth_txconf *tx_conf;
>>         SLIST_HEAD(vlan_list_head, qede_vlan_entry)vlan_list_head;
>>         uint16_t configured_vlans;
>>         bool accept_any_vlan;
>> diff --git a/drivers/net/qede/qede_rxtx.c b/drivers/net/qede/qede_rxtx.c
>> index b81788ca4..1b212a4fb 100644
>> --- a/drivers/net/qede/qede_rxtx.c
>> +++ b/drivers/net/qede/qede_rxtx.c
>> @@ -151,6 +151,7 @@ qede_alloc_rx_queue_mem(struct rte_eth_dev *dev,
>>         rxq->qdev = qdev;
>>         rxq->mb_pool = mp;
>>         rxq->nb_rx_desc = nb_desc;
>> +       qdev->num_rx_desc = rxq->nb_rx_desc;
>>         rxq->queue_id = queue_idx;
>>         rxq->port_id = dev->data->port_id;
>>
>> @@ -405,6 +406,7 @@ qede_alloc_tx_queue_mem(struct rte_eth_dev *dev,
>>         }
>>
>>         txq->nb_tx_desc = nb_desc;
>> +       qdev->num_tx_desc = txq->nb_tx_desc;
>>         txq->qdev = qdev;
>>         txq->port_id = dev->data->port_id;
>>
>> @@ -443,6 +445,7 @@ qede_alloc_tx_queue_mem(struct rte_eth_dev *dev,
>>
>>         txq->nb_tx_avail = txq->nb_tx_desc;
>>
>> +       qdev->tx_conf = tx_conf;
>>         txq->tx_free_thresh =
>>             tx_conf->tx_free_thresh ? tx_conf->tx_free_thresh :
>>             (txq->nb_tx_desc - QEDE_DEFAULT_TX_FREE_THRESH);
>> @@ -593,7 +596,7 @@ qede_alloc_mem_sb(struct qede_dev *qdev, struct ecore_sb_info *sb_info,
>>
>>  int qede_alloc_fp_resc(struct qede_dev *qdev)
>>  {
>> -       struct ecore_dev *edev = &qdev->edev;
>> +       struct ecore_dev *edev = QEDE_INIT_EDEV(qdev);
>>         struct qede_fastpath *fp;
>>         uint32_t num_sbs;
>>         uint16_t sb_idx;
>> @@ -1005,9 +1008,29 @@ static int qede_tx_queue_stop(struct rte_eth_dev *eth_dev, uint16_t tx_queue_id)
>>  int qede_start_queues(struct rte_eth_dev *eth_dev)
>>  {
>>         struct qede_dev *qdev = QEDE_INIT_QDEV(eth_dev);
>> +       struct ecore_dev *edev = QEDE_INIT_EDEV(qdev);
>> +       struct qede_tx_queue *txq;
>> +       struct qede_fastpath *fp;
>>         uint8_t id;
>>         int rc = -1;
>>
>> +       /* Restore setup of Tx queues */
>> +       for (id = 0; id < qdev->num_tx_queues; id++) {
>> +               fp = &qdev->fp_array[id];
>> +               txq = fp->txq;
>> +
>> +               if (!txq) {
>> +                       rc = qede_tx_queue_setup(eth_dev, id, qdev->num_tx_desc,
>> +                                                eth_dev->data->numa_node,
>> +                                                qdev->tx_conf);
>> +                       if (rc != ECORE_SUCCESS) {
>> +                               DP_ERR(edev, "TX queue %u not setup rc=%d\n",
>> +                                      id, rc);
>> +                               return -1;
>> +                       }
>> +               }
>> +       }
>> +
>>         for (id = 0; id < qdev->num_rx_queues; id++) {
>>                 rc = qede_rx_queue_start(eth_dev, id);
>>                 if (rc != ECORE_SUCCESS)
>> --
>> 2.18.0
>>


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

* Re: [dpdk-dev] [PATCH 1/2] net/qede: fix assignment of Rx/Tx handlers
  2020-05-05  3:09 [dpdk-dev] [PATCH 1/2] net/qede: fix assignment of Rx/Tx handlers Rasesh Mody
  2020-05-05  3:09 ` [dpdk-dev] [PATCH 2/2] net/qede: restore Tx queue setup Rasesh Mody
@ 2020-05-05  9:01 ` Ferruh Yigit
  2020-05-06  2:34   ` [dpdk-dev] [EXT] " Rasesh Mody
  2020-05-14  4:09 ` [dpdk-dev] [PATCH v2 " Rasesh Mody
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 22+ messages in thread
From: Ferruh Yigit @ 2020-05-05  9:01 UTC (permalink / raw)
  To: Rasesh Mody, dev, jerinj; +Cc: GR-Everest-DPDK-Dev, stable, Igor Russkikh

On 5/5/2020 4:09 AM, Rasesh Mody wrote:
> Fix to assign dummy Rx/Tx handlers in dev_stop.
> For MTU set, assignment of the appropriate Rx/Tx handlers will be
> handled by dev_start/dev_stop.
> 
> Fixes: 81f8804992c9 ("net/qede: enhance Rx CPU utilization")
> Fixes: 8de0c4201926 ("net/qede: fix odd number of queues usage in 100G mode")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Rasesh Mody <rmody@marvell.com>
> Signed-off-by: Igor Russkikh <irusskikh@marvell.com>
> ---
>  drivers/net/qede/qede_ethdev.c | 33 ++++++++++++++++-----------------
>  1 file changed, 16 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
> index e71fa1e6a..726daa3e3 100644
> --- a/drivers/net/qede/qede_ethdev.c
> +++ b/drivers/net/qede/qede_ethdev.c
> @@ -320,13 +320,19 @@ qede_interrupt_handler(void *param)
>  }
>  
>  static void
> -qede_assign_rxtx_handlers(struct rte_eth_dev *dev)
> +qede_assign_rxtx_handlers(struct rte_eth_dev *dev, bool is_dummy)
>  {
>  	uint64_t tx_offloads = dev->data->dev_conf.txmode.offloads;
>  	struct qede_dev *qdev = dev->data->dev_private;
>  	struct ecore_dev *edev = &qdev->edev;
>  	bool use_tx_offload = false;
>  
> +	if (is_dummy) {
> +		dev->rx_pkt_burst = qede_rxtx_pkts_dummy;
> +		dev->tx_pkt_burst = qede_rxtx_pkts_dummy;
> +		return;
> +	}
> +
>  	if (ECORE_IS_CMT(edev)) {
>  		dev->rx_pkt_burst = qede_recv_pkts_cmt;
>  		dev->tx_pkt_burst = qede_xmit_pkts_cmt;
> @@ -1150,7 +1156,9 @@ static int qede_dev_start(struct rte_eth_dev *eth_dev)
>  	/* Start/resume traffic */
>  	qede_fastpath_start(edev);
>  
> -	qede_assign_rxtx_handlers(eth_dev);
> +	/* Assign I/O handlers */
> +	qede_assign_rxtx_handlers(eth_dev, false);
> +
>  	DP_INFO(edev, "Device started\n");
>  
>  	return 0;
> @@ -1166,6 +1174,11 @@ static void qede_dev_stop(struct rte_eth_dev *eth_dev)
>  
>  	PMD_INIT_FUNC_TRACE(edev);
>  
> +	/* Replace I/O functions with dummy ones. It cannot
> +	 * be set to NULL because rte_eth_rx_burst() doesn't check for NULL.
> +	 */
> +	qede_assign_rxtx_handlers(eth_dev, true);
> +

Why need to assign dummy handlers on stop(), what happens if you keep them as
they are as many PMD does?

>  	/* Disable vport */
>  	if (qede_activate_vport(eth_dev, false))
>  		return;
> @@ -2316,11 +2329,6 @@ static int qede_set_mtu(struct rte_eth_dev *dev, uint16_t mtu)
>  			dev->data->min_rx_buf_size);
>  		return -EINVAL;
>  	}
> -	/* Temporarily replace I/O functions with dummy ones. It cannot
> -	 * be set to NULL because rte_eth_rx_burst() doesn't check for NULL.
> -	 */
> -	dev->rx_pkt_burst = qede_rxtx_pkts_dummy;
> -	dev->tx_pkt_burst = qede_rxtx_pkts_dummy;
>  	if (dev->data->dev_started) {
>  		dev->data->dev_started = 0;
>  		qede_dev_stop(dev);
> @@ -2359,15 +2367,6 @@ static int qede_set_mtu(struct rte_eth_dev *dev, uint16_t mtu)
>  	/* update max frame size */
>  	dev->data->dev_conf.rxmode.max_rx_pkt_len = max_rx_pkt_len;
>  
> -	/* Reassign back */
> -	qede_assign_rxtx_handlers(dev);
> -	if (ECORE_IS_CMT(edev)) {
> -		dev->rx_pkt_burst = qede_recv_pkts_cmt;
> -		dev->tx_pkt_burst = qede_xmit_pkts_cmt;
> -	} else {
> -		dev->rx_pkt_burst = qede_recv_pkts;
> -		dev->tx_pkt_burst = qede_xmit_pkts;
> -	}
>  	return 0;
>  }
>  
> @@ -2570,7 +2569,7 @@ static int qede_common_dev_init(struct rte_eth_dev *eth_dev, bool is_vf)
>  	strncpy((char *)params.name, QEDE_PMD_VER_PREFIX,
>  		QEDE_PMD_DRV_VER_STR_SIZE);
>  
> -	qede_assign_rxtx_handlers(eth_dev);
> +	qede_assign_rxtx_handlers(eth_dev, true);
>  	eth_dev->tx_pkt_prepare = qede_xmit_prep_pkts;
>  
>  	/* For CMT mode device do periodic polling for slowpath events.
> 


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

* Re: [dpdk-dev] [PATCH 2/2] net/qede: restore Tx queue setup
  2020-05-05  8:59     ` Ferruh Yigit
@ 2020-05-05  9:15       ` Thomas Monjalon
  2020-05-06  2:43         ` [dpdk-dev] [EXT] " Rasesh Mody
  0 siblings, 1 reply; 22+ messages in thread
From: Thomas Monjalon @ 2020-05-05  9:15 UTC (permalink / raw)
  To: Jerin Jacob, Rasesh Mody, Ferruh Yigit
  Cc: dpdk-dev, Jerin Jacob, GR-Everest-DPDK-Dev, Igor Russkikh,
	Andrew Rybchenko

05/05/2020 10:59, Ferruh Yigit:
> On 5/5/2020 7:44 AM, Jerin Jacob wrote:
> > On Tue, May 5, 2020 at 8:39 AM Rasesh Mody <rmody@marvell.com> wrote:
> >>
> >> Some applications do not explicitly restore Tx queues setup during
> >> port re-configuration. This patch adds changes to check for
> >> released Tx queues and restore the setup if application doesn't
> >> explicitly does that.
> > 
> > +ethdev maintainers.
> > 
> > I think, Ideally, the fix should be in common code if we need to
> > support such applications.
> 
> Is this a case application re-configures to increase the number of queues but
> doesn't setup new queues?
> If so this looks like application error and application should be fixed instead
> of recover this in the ethdev.

+1



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

* Re: [dpdk-dev] [EXT] Re: [PATCH 1/2] net/qede: fix assignment of Rx/Tx handlers
  2020-05-05  9:01 ` [dpdk-dev] [PATCH 1/2] net/qede: fix assignment of Rx/Tx handlers Ferruh Yigit
@ 2020-05-06  2:34   ` Rasesh Mody
  0 siblings, 0 replies; 22+ messages in thread
From: Rasesh Mody @ 2020-05-06  2:34 UTC (permalink / raw)
  To: Ferruh Yigit, dev, Jerin Jacob Kollanukkaran
  Cc: GR-Everest-DPDK-Dev, stable, Igor Russkikh

Hi Ferruh,

>From: Ferruh Yigit <ferruh.yigit@intel.com>
>Sent: Tuesday, May 05, 2020 2:01 AM
>
>On 5/5/2020 4:09 AM, Rasesh Mody wrote:
>> Fix to assign dummy Rx/Tx handlers in dev_stop.
>> For MTU set, assignment of the appropriate Rx/Tx handlers will be
>> handled by dev_start/dev_stop.
>>
>> Fixes: 81f8804992c9 ("net/qede: enhance Rx CPU utilization")
>> Fixes: 8de0c4201926 ("net/qede: fix odd number of queues usage in 100G
>> mode")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Rasesh Mody <rmody@marvell.com>
>> Signed-off-by: Igor Russkikh <irusskikh@marvell.com>
>> ---
>>  drivers/net/qede/qede_ethdev.c | 33 ++++++++++++++++-----------------
>>  1 file changed, 16 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/net/qede/qede_ethdev.c
>> b/drivers/net/qede/qede_ethdev.c index e71fa1e6a..726daa3e3 100644
>> --- a/drivers/net/qede/qede_ethdev.c
>> +++ b/drivers/net/qede/qede_ethdev.c
>> @@ -320,13 +320,19 @@ qede_interrupt_handler(void *param)  }
>>
>>  static void
>> -qede_assign_rxtx_handlers(struct rte_eth_dev *dev)
>> +qede_assign_rxtx_handlers(struct rte_eth_dev *dev, bool is_dummy)
>>  {
>>  	uint64_t tx_offloads = dev->data->dev_conf.txmode.offloads;
>>  	struct qede_dev *qdev = dev->data->dev_private;
>>  	struct ecore_dev *edev = &qdev->edev;
>>  	bool use_tx_offload = false;
>>
>> +	if (is_dummy) {
>> +		dev->rx_pkt_burst = qede_rxtx_pkts_dummy;
>> +		dev->tx_pkt_burst = qede_rxtx_pkts_dummy;
>> +		return;
>> +	}
>> +
>>  	if (ECORE_IS_CMT(edev)) {
>>  		dev->rx_pkt_burst = qede_recv_pkts_cmt;
>>  		dev->tx_pkt_burst = qede_xmit_pkts_cmt; @@ -1150,7
>+1156,9 @@
>> static int qede_dev_start(struct rte_eth_dev *eth_dev)
>>  	/* Start/resume traffic */
>>  	qede_fastpath_start(edev);
>>
>> -	qede_assign_rxtx_handlers(eth_dev);
>> +	/* Assign I/O handlers */
>> +	qede_assign_rxtx_handlers(eth_dev, false);
>> +
>>  	DP_INFO(edev, "Device started\n");
>>
>>  	return 0;
>> @@ -1166,6 +1174,11 @@ static void qede_dev_stop(struct rte_eth_dev
>> *eth_dev)
>>
>>  	PMD_INIT_FUNC_TRACE(edev);
>>
>> +	/* Replace I/O functions with dummy ones. It cannot
>> +	 * be set to NULL because rte_eth_rx_burst() doesn't check for NULL.
>> +	 */
>> +	qede_assign_rxtx_handlers(eth_dev, true);
>> +
>
>Why need to assign dummy handlers on stop(), what happens if you keep
>them as they are as many PMD does?

It helps in preventing crash when queues are still in use.

Thanks!
-Rasesh

>
>>  	/* Disable vport */
>>  	if (qede_activate_vport(eth_dev, false))
>>  		return;
>> @@ -2316,11 +2329,6 @@ static int qede_set_mtu(struct rte_eth_dev
>*dev, uint16_t mtu)
>>  			dev->data->min_rx_buf_size);
>>  		return -EINVAL;
>>  	}
>> -	/* Temporarily replace I/O functions with dummy ones. It cannot
>> -	 * be set to NULL because rte_eth_rx_burst() doesn't check for NULL.
>> -	 */
>> -	dev->rx_pkt_burst = qede_rxtx_pkts_dummy;
>> -	dev->tx_pkt_burst = qede_rxtx_pkts_dummy;
>>  	if (dev->data->dev_started) {
>>  		dev->data->dev_started = 0;
>>  		qede_dev_stop(dev);
>> @@ -2359,15 +2367,6 @@ static int qede_set_mtu(struct rte_eth_dev
>*dev, uint16_t mtu)
>>  	/* update max frame size */
>>  	dev->data->dev_conf.rxmode.max_rx_pkt_len = max_rx_pkt_len;
>>
>> -	/* Reassign back */
>> -	qede_assign_rxtx_handlers(dev);
>> -	if (ECORE_IS_CMT(edev)) {
>> -		dev->rx_pkt_burst = qede_recv_pkts_cmt;
>> -		dev->tx_pkt_burst = qede_xmit_pkts_cmt;
>> -	} else {
>> -		dev->rx_pkt_burst = qede_recv_pkts;
>> -		dev->tx_pkt_burst = qede_xmit_pkts;
>> -	}
>>  	return 0;
>>  }
>>
>> @@ -2570,7 +2569,7 @@ static int qede_common_dev_init(struct
>rte_eth_dev *eth_dev, bool is_vf)
>>  	strncpy((char *)params.name, QEDE_PMD_VER_PREFIX,
>>  		QEDE_PMD_DRV_VER_STR_SIZE);
>>
>> -	qede_assign_rxtx_handlers(eth_dev);
>> +	qede_assign_rxtx_handlers(eth_dev, true);
>>  	eth_dev->tx_pkt_prepare = qede_xmit_prep_pkts;
>>
>>  	/* For CMT mode device do periodic polling for slowpath events.
>>



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

* Re: [dpdk-dev] [EXT] Re: [PATCH 2/2] net/qede: restore Tx queue setup
  2020-05-05  9:15       ` Thomas Monjalon
@ 2020-05-06  2:43         ` Rasesh Mody
  2020-05-10  7:04           ` Jerin Jacob
  2020-05-14 14:32           ` Ferruh Yigit
  0 siblings, 2 replies; 22+ messages in thread
From: Rasesh Mody @ 2020-05-06  2:43 UTC (permalink / raw)
  To: Thomas Monjalon, Jerin Jacob, Ferruh Yigit
  Cc: dpdk-dev, Jerin Jacob Kollanukkaran, GR-Everest-DPDK-Dev,
	Igor Russkikh, Andrew Rybchenko

Hi,

>From: Thomas Monjalon <thomas@monjalon.net>
>Sent: Tuesday, May 05, 2020 2:15 AM
>
>05/05/2020 10:59, Ferruh Yigit:
>> On 5/5/2020 7:44 AM, Jerin Jacob wrote:
>> > On Tue, May 5, 2020 at 8:39 AM Rasesh Mody <rmody@marvell.com>
>wrote:
>> >>
>> >> Some applications do not explicitly restore Tx queues setup during
>> >> port re-configuration. This patch adds changes to check for
>> >> released Tx queues and restore the setup if application doesn't
>> >> explicitly does that.
>> >
>> > +ethdev maintainers.
>> >
>> > I think, Ideally, the fix should be in common code if we need to
>> > support such applications.
>>
>> Is this a case application re-configures to increase the number of
>> queues but doesn't setup new queues?
>> If so this looks like application error and application should be
>> fixed instead of recover this in the ethdev.
>
>+1
>

This is a case of KNI application performing device re-configuration to change MTU. The application explicitly calls Rx queue setup, however doesn't call Tx queue setup. When MTU for KNI interface is changed it runs into a segfault when trying to start Tx queues.
Some other applications make use of rte_eth_dev_set_mtu() ethdev op, which looks to be cleaner approach.

Thanks!
-Rasesh


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

* Re: [dpdk-dev] [EXT] Re: [PATCH 2/2] net/qede: restore Tx queue setup
  2020-05-06  2:43         ` [dpdk-dev] [EXT] " Rasesh Mody
@ 2020-05-10  7:04           ` Jerin Jacob
  2020-05-14  4:10             ` Rasesh Mody
  2020-05-14 14:32           ` Ferruh Yigit
  1 sibling, 1 reply; 22+ messages in thread
From: Jerin Jacob @ 2020-05-10  7:04 UTC (permalink / raw)
  To: Rasesh Mody
  Cc: Thomas Monjalon, Ferruh Yigit, dpdk-dev,
	Jerin Jacob Kollanukkaran, GR-Everest-DPDK-Dev, Igor Russkikh,
	Andrew Rybchenko

On Wed, May 6, 2020 at 8:13 AM Rasesh Mody <rmody@marvell.com> wrote:
>
> Hi,
>
> >From: Thomas Monjalon <thomas@monjalon.net>
> >Sent: Tuesday, May 05, 2020 2:15 AM
> >
> >05/05/2020 10:59, Ferruh Yigit:
> >> On 5/5/2020 7:44 AM, Jerin Jacob wrote:
> >> > On Tue, May 5, 2020 at 8:39 AM Rasesh Mody <rmody@marvell.com>
> >wrote:
> >> >>
> >> >> Some applications do not explicitly restore Tx queues setup during
> >> >> port re-configuration. This patch adds changes to check for
> >> >> released Tx queues and restore the setup if application doesn't
> >> >> explicitly does that.
> >> >
> >> > +ethdev maintainers.
> >> >
> >> > I think, Ideally, the fix should be in common code if we need to
> >> > support such applications.
> >>
> >> Is this a case application re-configures to increase the number of
> >> queues but doesn't setup new queues?
> >> If so this looks like application error and application should be
> >> fixed instead of recover this in the ethdev.
> >
> >+1
> >
>
> This is a case of KNI application performing device re-configuration to change MTU. The application explicitly calls Rx queue setup, however doesn't call Tx queue setup. When MTU for KNI interface is changed it runs into a segfault when trying to start Tx queues.

Can we fix the KNI application then?

> Some other applications make use of rte_eth_dev_set_mtu() ethdev op, which looks to be cleaner approach.
>
> Thanks!
> -Rasesh
>

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

* [dpdk-dev] [PATCH v2 1/2] net/qede: fix assignment of Rx/Tx handlers
  2020-05-05  3:09 [dpdk-dev] [PATCH 1/2] net/qede: fix assignment of Rx/Tx handlers Rasesh Mody
  2020-05-05  3:09 ` [dpdk-dev] [PATCH 2/2] net/qede: restore Tx queue setup Rasesh Mody
  2020-05-05  9:01 ` [dpdk-dev] [PATCH 1/2] net/qede: fix assignment of Rx/Tx handlers Ferruh Yigit
@ 2020-05-14  4:09 ` Rasesh Mody
  2020-05-14  4:09 ` [dpdk-dev] [PATCH v2 2/2] examples/kni: fix MTU change to setup Tx queue Rasesh Mody
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 22+ messages in thread
From: Rasesh Mody @ 2020-05-14  4:09 UTC (permalink / raw)
  To: dev, jerinj, ferruh.yigit
  Cc: Rasesh Mody, GR-Everest-DPDK-Dev, stable, Igor Russkikh

Fix to assign dummy Rx/Tx handlers in dev_stop.
For MTU set, assignment of the appropriate Rx/Tx handlers will be
handled by dev_start/dev_stop.

Fixes: 81f8804992c9 ("net/qede: enhance Rx CPU utilization")
Fixes: 8de0c4201926 ("net/qede: fix odd number of queues usage in 100G mode")
Cc: stable@dpdk.org

Signed-off-by: Rasesh Mody <rmody@marvell.com>
Signed-off-by: Igor Russkikh <irusskikh@marvell.com>
---
 drivers/net/qede/qede_ethdev.c | 33 ++++++++++++++++-----------------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
index 0b1fca9ac..d3d916e81 100644
--- a/drivers/net/qede/qede_ethdev.c
+++ b/drivers/net/qede/qede_ethdev.c
@@ -320,13 +320,19 @@ qede_interrupt_handler(void *param)
 }
 
 static void
-qede_assign_rxtx_handlers(struct rte_eth_dev *dev)
+qede_assign_rxtx_handlers(struct rte_eth_dev *dev, bool is_dummy)
 {
 	uint64_t tx_offloads = dev->data->dev_conf.txmode.offloads;
 	struct qede_dev *qdev = dev->data->dev_private;
 	struct ecore_dev *edev = &qdev->edev;
 	bool use_tx_offload = false;
 
+	if (is_dummy) {
+		dev->rx_pkt_burst = qede_rxtx_pkts_dummy;
+		dev->tx_pkt_burst = qede_rxtx_pkts_dummy;
+		return;
+	}
+
 	if (ECORE_IS_CMT(edev)) {
 		dev->rx_pkt_burst = qede_recv_pkts_cmt;
 		dev->tx_pkt_burst = qede_xmit_pkts_cmt;
@@ -1153,7 +1159,9 @@ static int qede_dev_start(struct rte_eth_dev *eth_dev)
 	/* Start/resume traffic */
 	qede_fastpath_start(edev);
 
-	qede_assign_rxtx_handlers(eth_dev);
+	/* Assign I/O handlers */
+	qede_assign_rxtx_handlers(eth_dev, false);
+
 	DP_INFO(edev, "Device started\n");
 
 	return 0;
@@ -1175,6 +1183,11 @@ static void qede_dev_stop(struct rte_eth_dev *eth_dev)
 	/* Update link status */
 	qede_link_update(eth_dev, 0);
 
+	/* Replace I/O functions with dummy ones. It cannot
+	 * be set to NULL because rte_eth_rx_burst() doesn't check for NULL.
+	 */
+	qede_assign_rxtx_handlers(eth_dev, true);
+
 	/* Disable vport */
 	if (qede_activate_vport(eth_dev, false))
 		return;
@@ -2323,11 +2336,6 @@ static int qede_set_mtu(struct rte_eth_dev *dev, uint16_t mtu)
 			dev->data->min_rx_buf_size);
 		return -EINVAL;
 	}
-	/* Temporarily replace I/O functions with dummy ones. It cannot
-	 * be set to NULL because rte_eth_rx_burst() doesn't check for NULL.
-	 */
-	dev->rx_pkt_burst = qede_rxtx_pkts_dummy;
-	dev->tx_pkt_burst = qede_rxtx_pkts_dummy;
 	if (dev->data->dev_started) {
 		dev->data->dev_started = 0;
 		qede_dev_stop(dev);
@@ -2366,15 +2374,6 @@ static int qede_set_mtu(struct rte_eth_dev *dev, uint16_t mtu)
 	/* update max frame size */
 	dev->data->dev_conf.rxmode.max_rx_pkt_len = max_rx_pkt_len;
 
-	/* Reassign back */
-	qede_assign_rxtx_handlers(dev);
-	if (ECORE_IS_CMT(edev)) {
-		dev->rx_pkt_burst = qede_recv_pkts_cmt;
-		dev->tx_pkt_burst = qede_xmit_pkts_cmt;
-	} else {
-		dev->rx_pkt_burst = qede_recv_pkts;
-		dev->tx_pkt_burst = qede_xmit_pkts;
-	}
 	return 0;
 }
 
@@ -2577,7 +2576,7 @@ static int qede_common_dev_init(struct rte_eth_dev *eth_dev, bool is_vf)
 	strncpy((char *)params.name, QEDE_PMD_VER_PREFIX,
 		QEDE_PMD_DRV_VER_STR_SIZE);
 
-	qede_assign_rxtx_handlers(eth_dev);
+	qede_assign_rxtx_handlers(eth_dev, true);
 	eth_dev->tx_pkt_prepare = qede_xmit_prep_pkts;
 
 	/* For CMT mode device do periodic polling for slowpath events.
-- 
2.18.0


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

* [dpdk-dev] [PATCH v2 2/2] examples/kni: fix MTU change to setup Tx queue
  2020-05-05  3:09 [dpdk-dev] [PATCH 1/2] net/qede: fix assignment of Rx/Tx handlers Rasesh Mody
                   ` (2 preceding siblings ...)
  2020-05-14  4:09 ` [dpdk-dev] [PATCH v2 " Rasesh Mody
@ 2020-05-14  4:09 ` Rasesh Mody
  2020-05-14 15:33   ` Ferruh Yigit
  2020-05-15 11:29   ` Ferruh Yigit
  2020-05-15  6:34 ` [dpdk-dev] [PATCH v3 1/2] net/qede: fix assignment of Rx/Tx handlers Rasesh Mody
  2020-05-15  6:34 ` [dpdk-dev] [PATCH v3 2/2] net/qede: fix port reconfiguration Rasesh Mody
  5 siblings, 2 replies; 22+ messages in thread
From: Rasesh Mody @ 2020-05-14  4:09 UTC (permalink / raw)
  To: dev, jerinj, ferruh.yigit; +Cc: Rasesh Mody, GR-Everest-DPDK-Dev, stable

This patch adds a fix to setup Tx queue when changing KNI interface MTU.
It ensures device can safely start txq post MTU change operation.

Fixes: fc9ee41b7016 ("examples/kni: convert to new ethdev offloads API")
Cc: stable@dpdk.org

Signed-off-by: Rasesh Mody <rmody@marvell.com>
---
 examples/kni/main.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/examples/kni/main.c b/examples/kni/main.c
index 6b4ab3b5b..7a927a50c 100644
--- a/examples/kni/main.c
+++ b/examples/kni/main.c
@@ -774,9 +774,11 @@ kni_change_mtu(uint16_t port_id, unsigned int new_mtu)
 {
 	int ret;
 	uint16_t nb_rxd = NB_RXD;
+	uint16_t nb_txd = NB_TXD;
 	struct rte_eth_conf conf;
 	struct rte_eth_dev_info dev_info;
 	struct rte_eth_rxconf rxq_conf;
+	struct rte_eth_txconf txq_conf;
 
 	if (!rte_eth_dev_is_valid_port(port_id)) {
 		RTE_LOG(ERR, APP, "Invalid port id %d\n", port_id);
@@ -804,7 +806,7 @@ kni_change_mtu(uint16_t port_id, unsigned int new_mtu)
 		return ret;
 	}
 
-	ret = rte_eth_dev_adjust_nb_rx_tx_desc(port_id, &nb_rxd, NULL);
+	ret = rte_eth_dev_adjust_nb_rx_tx_desc(port_id, &nb_rxd, &nb_txd);
 	if (ret < 0)
 		rte_exit(EXIT_FAILURE, "Could not adjust number of descriptors "
 				"for port%u (%d)\n", (unsigned int)port_id,
@@ -829,6 +831,16 @@ kni_change_mtu(uint16_t port_id, unsigned int new_mtu)
 		return ret;
 	}
 
+	txq_conf = dev_info.default_txconf;
+	txq_conf.offloads = conf.txmode.offloads;
+	ret = rte_eth_tx_queue_setup(port_id, 0, nb_txd,
+		rte_eth_dev_socket_id(port_id), &txq_conf);
+	if (ret < 0) {
+		RTE_LOG(ERR, APP, "Fail to setup Tx queue of port %d\n",
+				port_id);
+		return ret;
+	}
+
 	/* Restart specific port */
 	ret = rte_eth_dev_start(port_id);
 	if (ret < 0) {
-- 
2.18.0


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

* Re: [dpdk-dev] [EXT] Re: [PATCH 2/2] net/qede: restore Tx queue setup
  2020-05-10  7:04           ` Jerin Jacob
@ 2020-05-14  4:10             ` Rasesh Mody
  2020-05-14  7:56               ` Jerin Jacob
  0 siblings, 1 reply; 22+ messages in thread
From: Rasesh Mody @ 2020-05-14  4:10 UTC (permalink / raw)
  To: Jerin Jacob
  Cc: Thomas Monjalon, Ferruh Yigit, dpdk-dev,
	Jerin Jacob Kollanukkaran, GR-Everest-DPDK-Dev, Igor Russkikh,
	Andrew Rybchenko

Hi Jerin,

>From: Jerin Jacob <jerinjacobk@gmail.com>
>Sent: Sunday, May 10, 2020 12:04 AM
>
>On Wed, May 6, 2020 at 8:13 AM Rasesh Mody <rmody@marvell.com> wrote:
>>
>> Hi,
>>
>> >From: Thomas Monjalon <thomas@monjalon.net>
>> >Sent: Tuesday, May 05, 2020 2:15 AM
>> >
>> >05/05/2020 10:59, Ferruh Yigit:
>> >> On 5/5/2020 7:44 AM, Jerin Jacob wrote:
>> >> > On Tue, May 5, 2020 at 8:39 AM Rasesh Mody <rmody@marvell.com>
>> >wrote:
>> >> >>
>> >> >> Some applications do not explicitly restore Tx queues setup
>> >> >> during port re-configuration. This patch adds changes to check
>> >> >> for released Tx queues and restore the setup if application
>> >> >> doesn't explicitly does that.
>> >> >
>> >> > +ethdev maintainers.
>> >> >
>> >> > I think, Ideally, the fix should be in common code if we need to
>> >> > support such applications.
>> >>
>> >> Is this a case application re-configures to increase the number of
>> >> queues but doesn't setup new queues?
>> >> If so this looks like application error and application should be
>> >> fixed instead of recover this in the ethdev.
>> >
>> >+1
>> >
>>
>> This is a case of KNI application performing device re-configuration to
>change MTU. The application explicitly calls Rx queue setup, however doesn't
>call Tx queue setup. When MTU for KNI interface is changed it runs into a
>segfault when trying to start Tx queues.
>
>Can we fix the KNI application then?

Yes, I have submitted a v2 series with changes to KNI application. Please apply for -rc3.

Thanks!
-Rasesh
>
>> Some other applications make use of rte_eth_dev_set_mtu() ethdev op,
>which looks to be cleaner approach.
>>
>> Thanks!
>> -Rasesh
>>


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

* Re: [dpdk-dev] [EXT] Re: [PATCH 2/2] net/qede: restore Tx queue setup
  2020-05-14  4:10             ` Rasesh Mody
@ 2020-05-14  7:56               ` Jerin Jacob
  0 siblings, 0 replies; 22+ messages in thread
From: Jerin Jacob @ 2020-05-14  7:56 UTC (permalink / raw)
  To: Rasesh Mody
  Cc: Thomas Monjalon, Ferruh Yigit, dpdk-dev,
	Jerin Jacob Kollanukkaran, GR-Everest-DPDK-Dev, Igor Russkikh,
	Andrew Rybchenko

On Thu, May 14, 2020 at 9:40 AM Rasesh Mody <rmody@marvell.com> wrote:
>

> >> >> > +ethdev maintainers.
> >> >> >
> >> >> > I think, Ideally, the fix should be in common code if we need to
> >> >> > support such applications.
> >> >>
> >> >> Is this a case application re-configures to increase the number of
> >> >> queues but doesn't setup new queues?
> >> >> If so this looks like application error and application should be
> >> >> fixed instead of recover this in the ethdev.
> >> >
> >> >+1
> >> >
> >>
> >> This is a case of KNI application performing device re-configuration to
> >change MTU. The application explicitly calls Rx queue setup, however doesn't
> >call Tx queue setup. When MTU for KNI interface is changed it runs into a
> >segfault when trying to start Tx queues.
> >
> >Can we fix the KNI application then?
>
> Yes, I have submitted a v2 series with changes to KNI application. Please apply for -rc3.

Since it has the KNI app change, I have delegated the series to
@Ferruh Yigit to take it over next-net instead of next-net-mrvl.

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

* Re: [dpdk-dev] [EXT] Re: [PATCH 2/2] net/qede: restore Tx queue setup
  2020-05-06  2:43         ` [dpdk-dev] [EXT] " Rasesh Mody
  2020-05-10  7:04           ` Jerin Jacob
@ 2020-05-14 14:32           ` Ferruh Yigit
  2020-05-14 22:43             ` Rasesh Mody
  1 sibling, 1 reply; 22+ messages in thread
From: Ferruh Yigit @ 2020-05-14 14:32 UTC (permalink / raw)
  To: Rasesh Mody, Thomas Monjalon, Jerin Jacob
  Cc: dpdk-dev, Jerin Jacob Kollanukkaran, GR-Everest-DPDK-Dev,
	Igor Russkikh, Andrew Rybchenko

On 5/6/2020 3:43 AM, Rasesh Mody wrote:
> Hi,
> 
>> From: Thomas Monjalon <thomas@monjalon.net>
>> Sent: Tuesday, May 05, 2020 2:15 AM
>>
>> 05/05/2020 10:59, Ferruh Yigit:
>>> On 5/5/2020 7:44 AM, Jerin Jacob wrote:
>>>> On Tue, May 5, 2020 at 8:39 AM Rasesh Mody <rmody@marvell.com>
>> wrote:
>>>>>
>>>>> Some applications do not explicitly restore Tx queues setup during
>>>>> port re-configuration. This patch adds changes to check for
>>>>> released Tx queues and restore the setup if application doesn't
>>>>> explicitly does that.
>>>>
>>>> +ethdev maintainers.
>>>>
>>>> I think, Ideally, the fix should be in common code if we need to
>>>> support such applications.
>>>
>>> Is this a case application re-configures to increase the number of
>>> queues but doesn't setup new queues?
>>> If so this looks like application error and application should be
>>> fixed instead of recover this in the ethdev.
>>
>> +1
>>
> 
> This is a case of KNI application performing device re-configuration to change MTU. The application explicitly calls Rx queue setup, however doesn't call Tx queue setup. When MTU for KNI interface is changed it runs into a segfault when trying to start Tx queues.

Why it crash? Device re-configuration should not be changing the number of the
queues, it is always 1. What is missing/wrong without the Tx queue setup?

> Some other applications make use of rte_eth_dev_set_mtu() ethdev op, which looks to be cleaner approach.
> 

+1. I don't know history if there is a specific reason this way selected, but
after release I think we can try this.

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

* Re: [dpdk-dev] [PATCH v2 2/2] examples/kni: fix MTU change to setup Tx queue
  2020-05-14  4:09 ` [dpdk-dev] [PATCH v2 2/2] examples/kni: fix MTU change to setup Tx queue Rasesh Mody
@ 2020-05-14 15:33   ` Ferruh Yigit
  2020-05-15 11:29   ` Ferruh Yigit
  1 sibling, 0 replies; 22+ messages in thread
From: Ferruh Yigit @ 2020-05-14 15:33 UTC (permalink / raw)
  To: Rasesh Mody, dev, jerinj; +Cc: GR-Everest-DPDK-Dev, stable

On 5/14/2020 5:09 AM, Rasesh Mody wrote:
> This patch adds a fix to setup Tx queue when changing KNI interface MTU.
> It ensures device can safely start txq post MTU change operation.
> 
> Fixes: fc9ee41b7016 ("examples/kni: convert to new ethdev offloads API")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Rasesh Mody <rmody@marvell.com>
> ---
>  examples/kni/main.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/examples/kni/main.c b/examples/kni/main.c
> index 6b4ab3b5b..7a927a50c 100644
> --- a/examples/kni/main.c
> +++ b/examples/kni/main.c
> @@ -774,9 +774,11 @@ kni_change_mtu(uint16_t port_id, unsigned int new_mtu)
>  {
>  	int ret;
>  	uint16_t nb_rxd = NB_RXD;
> +	uint16_t nb_txd = NB_TXD;
>  	struct rte_eth_conf conf;
>  	struct rte_eth_dev_info dev_info;
>  	struct rte_eth_rxconf rxq_conf;
> +	struct rte_eth_txconf txq_conf;
>  
>  	if (!rte_eth_dev_is_valid_port(port_id)) {
>  		RTE_LOG(ERR, APP, "Invalid port id %d\n", port_id);
> @@ -804,7 +806,7 @@ kni_change_mtu(uint16_t port_id, unsigned int new_mtu)
>  		return ret;
>  	}
>  
> -	ret = rte_eth_dev_adjust_nb_rx_tx_desc(port_id, &nb_rxd, NULL);
> +	ret = rte_eth_dev_adjust_nb_rx_tx_desc(port_id, &nb_rxd, &nb_txd);
>  	if (ret < 0)
>  		rte_exit(EXIT_FAILURE, "Could not adjust number of descriptors "
>  				"for port%u (%d)\n", (unsigned int)port_id,
> @@ -829,6 +831,16 @@ kni_change_mtu(uint16_t port_id, unsigned int new_mtu)
>  		return ret;
>  	}
>  
> +	txq_conf = dev_info.default_txconf;
> +	txq_conf.offloads = conf.txmode.offloads;
> +	ret = rte_eth_tx_queue_setup(port_id, 0, nb_txd,
> +		rte_eth_dev_socket_id(port_id), &txq_conf);
> +	if (ret < 0) {
> +		RTE_LOG(ERR, APP, "Fail to setup Tx queue of port %d\n",
> +				port_id);
> +		return ret;
> +	}
> +
>  	/* Restart specific port */
>  	ret = rte_eth_dev_start(port_id);
>  	if (ret < 0) {
> 

Hi Rasesh,

The change looks OK but I am trying to understand the need for it, already asked
in other tread, will follow there.

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

* Re: [dpdk-dev] [EXT] Re: [PATCH 2/2] net/qede: restore Tx queue setup
  2020-05-14 14:32           ` Ferruh Yigit
@ 2020-05-14 22:43             ` Rasesh Mody
  2020-05-15 10:39               ` Ferruh Yigit
  0 siblings, 1 reply; 22+ messages in thread
From: Rasesh Mody @ 2020-05-14 22:43 UTC (permalink / raw)
  To: Ferruh Yigit, Thomas Monjalon, Jerin Jacob
  Cc: dpdk-dev, Jerin Jacob Kollanukkaran, GR-Everest-DPDK-Dev,
	Igor Russkikh, Andrew Rybchenko

>From: Ferruh Yigit <ferruh.yigit@intel.com>
>Sent: Thursday, May 14, 2020 7:33 AM
>
>On 5/6/2020 3:43 AM, Rasesh Mody wrote:
>> Hi,
>>
>>> From: Thomas Monjalon <thomas@monjalon.net>
>>> Sent: Tuesday, May 05, 2020 2:15 AM
>>>
>>> 05/05/2020 10:59, Ferruh Yigit:
>>>> On 5/5/2020 7:44 AM, Jerin Jacob wrote:
>>>>> On Tue, May 5, 2020 at 8:39 AM Rasesh Mody <rmody@marvell.com>
>>> wrote:
>>>>>>
>>>>>> Some applications do not explicitly restore Tx queues setup during
>>>>>> port re-configuration. This patch adds changes to check for
>>>>>> released Tx queues and restore the setup if application doesn't
>>>>>> explicitly does that.
>>>>>
>>>>> +ethdev maintainers.
>>>>>
>>>>> I think, Ideally, the fix should be in common code if we need to
>>>>> support such applications.
>>>>
>>>> Is this a case application re-configures to increase the number of
>>>> queues but doesn't setup new queues?
>>>> If so this looks like application error and application should be
>>>> fixed instead of recover this in the ethdev.
>>>
>>> +1
>>>
>>
>> This is a case of KNI application performing device re-configuration to
>change MTU. The application explicitly calls Rx queue setup, however doesn't
>call Tx queue setup. When MTU for KNI interface is changed it runs into a
>segfault when trying to start Tx queues.
>
>Why it crash? Device re-configuration should not be changing the number of
>the queues, it is always 1. What is missing/wrong without the Tx queue setup?
>

QEDE PMD deallocates all fastpath resources unconditionally, including Tx queue, when re-configuring the device. When reallocating resources PMD relies on application to explicitly setup the Rx/Tx queue. 
Deallocation of all the resources is only required if the Rx/Tx queue configuration changes. So when KNI MTU change performs device re-configuration it exposes a bug, where there are no Tx queue resources setup. Then while starting the Tx queue in HW from PMD, application crashes.
I'll submit a PMD fix for device re-configuration.

>> Some other applications make use of rte_eth_dev_set_mtu() ethdev op,
>which looks to be cleaner approach.
>>
>
>+1. I don't know history if there is a specific reason this way
>+selected, but
>after release I think we can try this.

Sure.
Thanks!
-Rasesh


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

* [dpdk-dev] [PATCH v3 1/2] net/qede: fix assignment of Rx/Tx handlers
  2020-05-05  3:09 [dpdk-dev] [PATCH 1/2] net/qede: fix assignment of Rx/Tx handlers Rasesh Mody
                   ` (3 preceding siblings ...)
  2020-05-14  4:09 ` [dpdk-dev] [PATCH v2 2/2] examples/kni: fix MTU change to setup Tx queue Rasesh Mody
@ 2020-05-15  6:34 ` Rasesh Mody
  2020-05-15 12:26   ` Jerin Jacob
  2020-05-15  6:34 ` [dpdk-dev] [PATCH v3 2/2] net/qede: fix port reconfiguration Rasesh Mody
  5 siblings, 1 reply; 22+ messages in thread
From: Rasesh Mody @ 2020-05-15  6:34 UTC (permalink / raw)
  To: dev, jerinj, ferruh.yigit
  Cc: Rasesh Mody, GR-Everest-DPDK-Dev, stable, Igor Russkikh

Fix to assign dummy Rx/Tx handlers in dev_stop.
For MTU set, assignment of the appropriate Rx/Tx handlers will be
handled by dev_start/dev_stop.

Fixes: 81f8804992c9 ("net/qede: enhance Rx CPU utilization")
Fixes: 8de0c4201926 ("net/qede: fix odd number of queues usage in 100G mode")
Cc: stable@dpdk.org

Signed-off-by: Rasesh Mody <rmody@marvell.com>
Signed-off-by: Igor Russkikh <irusskikh@marvell.com>
---
 drivers/net/qede/qede_ethdev.c | 33 ++++++++++++++++-----------------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
index 0b1fca9ac..d3d916e81 100644
--- a/drivers/net/qede/qede_ethdev.c
+++ b/drivers/net/qede/qede_ethdev.c
@@ -320,13 +320,19 @@ qede_interrupt_handler(void *param)
 }
 
 static void
-qede_assign_rxtx_handlers(struct rte_eth_dev *dev)
+qede_assign_rxtx_handlers(struct rte_eth_dev *dev, bool is_dummy)
 {
 	uint64_t tx_offloads = dev->data->dev_conf.txmode.offloads;
 	struct qede_dev *qdev = dev->data->dev_private;
 	struct ecore_dev *edev = &qdev->edev;
 	bool use_tx_offload = false;
 
+	if (is_dummy) {
+		dev->rx_pkt_burst = qede_rxtx_pkts_dummy;
+		dev->tx_pkt_burst = qede_rxtx_pkts_dummy;
+		return;
+	}
+
 	if (ECORE_IS_CMT(edev)) {
 		dev->rx_pkt_burst = qede_recv_pkts_cmt;
 		dev->tx_pkt_burst = qede_xmit_pkts_cmt;
@@ -1153,7 +1159,9 @@ static int qede_dev_start(struct rte_eth_dev *eth_dev)
 	/* Start/resume traffic */
 	qede_fastpath_start(edev);
 
-	qede_assign_rxtx_handlers(eth_dev);
+	/* Assign I/O handlers */
+	qede_assign_rxtx_handlers(eth_dev, false);
+
 	DP_INFO(edev, "Device started\n");
 
 	return 0;
@@ -1175,6 +1183,11 @@ static void qede_dev_stop(struct rte_eth_dev *eth_dev)
 	/* Update link status */
 	qede_link_update(eth_dev, 0);
 
+	/* Replace I/O functions with dummy ones. It cannot
+	 * be set to NULL because rte_eth_rx_burst() doesn't check for NULL.
+	 */
+	qede_assign_rxtx_handlers(eth_dev, true);
+
 	/* Disable vport */
 	if (qede_activate_vport(eth_dev, false))
 		return;
@@ -2323,11 +2336,6 @@ static int qede_set_mtu(struct rte_eth_dev *dev, uint16_t mtu)
 			dev->data->min_rx_buf_size);
 		return -EINVAL;
 	}
-	/* Temporarily replace I/O functions with dummy ones. It cannot
-	 * be set to NULL because rte_eth_rx_burst() doesn't check for NULL.
-	 */
-	dev->rx_pkt_burst = qede_rxtx_pkts_dummy;
-	dev->tx_pkt_burst = qede_rxtx_pkts_dummy;
 	if (dev->data->dev_started) {
 		dev->data->dev_started = 0;
 		qede_dev_stop(dev);
@@ -2366,15 +2374,6 @@ static int qede_set_mtu(struct rte_eth_dev *dev, uint16_t mtu)
 	/* update max frame size */
 	dev->data->dev_conf.rxmode.max_rx_pkt_len = max_rx_pkt_len;
 
-	/* Reassign back */
-	qede_assign_rxtx_handlers(dev);
-	if (ECORE_IS_CMT(edev)) {
-		dev->rx_pkt_burst = qede_recv_pkts_cmt;
-		dev->tx_pkt_burst = qede_xmit_pkts_cmt;
-	} else {
-		dev->rx_pkt_burst = qede_recv_pkts;
-		dev->tx_pkt_burst = qede_xmit_pkts;
-	}
 	return 0;
 }
 
@@ -2577,7 +2576,7 @@ static int qede_common_dev_init(struct rte_eth_dev *eth_dev, bool is_vf)
 	strncpy((char *)params.name, QEDE_PMD_VER_PREFIX,
 		QEDE_PMD_DRV_VER_STR_SIZE);
 
-	qede_assign_rxtx_handlers(eth_dev);
+	qede_assign_rxtx_handlers(eth_dev, true);
 	eth_dev->tx_pkt_prepare = qede_xmit_prep_pkts;
 
 	/* For CMT mode device do periodic polling for slowpath events.
-- 
2.18.0


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

* [dpdk-dev] [PATCH v3 2/2] net/qede: fix port reconfiguration
  2020-05-05  3:09 [dpdk-dev] [PATCH 1/2] net/qede: fix assignment of Rx/Tx handlers Rasesh Mody
                   ` (4 preceding siblings ...)
  2020-05-15  6:34 ` [dpdk-dev] [PATCH v3 1/2] net/qede: fix assignment of Rx/Tx handlers Rasesh Mody
@ 2020-05-15  6:34 ` Rasesh Mody
  5 siblings, 0 replies; 22+ messages in thread
From: Rasesh Mody @ 2020-05-15  6:34 UTC (permalink / raw)
  To: dev, jerinj, ferruh.yigit
  Cc: Rasesh Mody, GR-Everest-DPDK-Dev, stable, Igor Russkikh

This patch fixes deallocation of all fastpath resources unconditionally,
when re-configuring the device. When re-allocating resources PMD depends
on application to explicitly setup the Rx/Tx queue.

Deallocation of all the resources is only required if the Rx/Tx queue
configuration changes. For other scenarios like KNI MTU change we'd keep
existing configuration.

Fixes: 8de0c4201926 ("net/qede: fix odd number of queues usage in 100G
mode")
Fixes: dd28bc8c6ef4 ("net/qede: fix VF port creation sequence")
Cc: stable@dpdk.org

Signed-off-by: Rasesh Mody <rmody@marvell.com>
Signed-off-by: Igor Russkikh <irusskikh@marvell.com>
---
 drivers/net/qede/qede_ethdev.c | 19 +++++++++++++------
 drivers/net/qede/qede_rxtx.c   |  4 +++-
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
index d3d916e81..c4f8f1258 100644
--- a/drivers/net/qede/qede_ethdev.c
+++ b/drivers/net/qede/qede_ethdev.c
@@ -1273,6 +1273,8 @@ static int qede_dev_configure(struct rte_eth_dev *eth_dev)
 	struct qede_dev *qdev = QEDE_INIT_QDEV(eth_dev);
 	struct ecore_dev *edev = QEDE_INIT_EDEV(qdev);
 	struct rte_eth_rxmode *rxmode = &eth_dev->data->dev_conf.rxmode;
+	uint8_t num_rxqs;
+	uint8_t num_txqs;
 	int ret;
 
 	PMD_INIT_FUNC_TRACE(edev);
@@ -1305,12 +1307,17 @@ static int qede_dev_configure(struct rte_eth_dev *eth_dev)
 	if (qede_check_fdir_support(eth_dev))
 		return -ENOTSUP;
 
-	qede_dealloc_fp_resc(eth_dev);
-	qdev->num_tx_queues = eth_dev->data->nb_tx_queues * edev->num_hwfns;
-	qdev->num_rx_queues = eth_dev->data->nb_rx_queues * edev->num_hwfns;
-
-	if (qede_alloc_fp_resc(qdev))
-		return -ENOMEM;
+	/* Allocate/reallocate fastpath resources only for new queue config */
+	num_txqs = eth_dev->data->nb_tx_queues * edev->num_hwfns;
+	num_rxqs = eth_dev->data->nb_rx_queues * edev->num_hwfns;
+	if (qdev->num_tx_queues != num_txqs ||
+	    qdev->num_rx_queues != num_rxqs) {
+		qede_dealloc_fp_resc(eth_dev);
+		qdev->num_tx_queues = num_txqs;
+		qdev->num_rx_queues = num_rxqs;
+		if (qede_alloc_fp_resc(qdev))
+			return -ENOMEM;
+	}
 
 	/* If jumbo enabled adjust MTU */
 	if (rxmode->offloads & DEV_RX_OFFLOAD_JUMBO_FRAME)
diff --git a/drivers/net/qede/qede_rxtx.c b/drivers/net/qede/qede_rxtx.c
index b81788ca4..9878ba50e 100644
--- a/drivers/net/qede/qede_rxtx.c
+++ b/drivers/net/qede/qede_rxtx.c
@@ -593,12 +593,14 @@ qede_alloc_mem_sb(struct qede_dev *qdev, struct ecore_sb_info *sb_info,
 
 int qede_alloc_fp_resc(struct qede_dev *qdev)
 {
-	struct ecore_dev *edev = &qdev->edev;
+	struct ecore_dev *edev = QEDE_INIT_EDEV(qdev);
 	struct qede_fastpath *fp;
 	uint32_t num_sbs;
 	uint16_t sb_idx;
 	int i;
 
+	PMD_INIT_FUNC_TRACE(edev);
+
 	if (IS_VF(edev))
 		ecore_vf_get_num_sbs(ECORE_LEADING_HWFN(edev), &num_sbs);
 	else
-- 
2.18.0


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

* Re: [dpdk-dev] [EXT] Re: [PATCH 2/2] net/qede: restore Tx queue setup
  2020-05-14 22:43             ` Rasesh Mody
@ 2020-05-15 10:39               ` Ferruh Yigit
  0 siblings, 0 replies; 22+ messages in thread
From: Ferruh Yigit @ 2020-05-15 10:39 UTC (permalink / raw)
  To: Rasesh Mody, Thomas Monjalon, Jerin Jacob
  Cc: dpdk-dev, Jerin Jacob Kollanukkaran, GR-Everest-DPDK-Dev,
	Igor Russkikh, Andrew Rybchenko

On 5/14/2020 11:43 PM, Rasesh Mody wrote:
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Thursday, May 14, 2020 7:33 AM
>>
>> On 5/6/2020 3:43 AM, Rasesh Mody wrote:
>>> Hi,
>>>
>>>> From: Thomas Monjalon <thomas@monjalon.net>
>>>> Sent: Tuesday, May 05, 2020 2:15 AM
>>>>
>>>> 05/05/2020 10:59, Ferruh Yigit:
>>>>> On 5/5/2020 7:44 AM, Jerin Jacob wrote:
>>>>>> On Tue, May 5, 2020 at 8:39 AM Rasesh Mody <rmody@marvell.com>
>>>> wrote:
>>>>>>>
>>>>>>> Some applications do not explicitly restore Tx queues setup during
>>>>>>> port re-configuration. This patch adds changes to check for
>>>>>>> released Tx queues and restore the setup if application doesn't
>>>>>>> explicitly does that.
>>>>>>
>>>>>> +ethdev maintainers.
>>>>>>
>>>>>> I think, Ideally, the fix should be in common code if we need to
>>>>>> support such applications.
>>>>>
>>>>> Is this a case application re-configures to increase the number of
>>>>> queues but doesn't setup new queues?
>>>>> If so this looks like application error and application should be
>>>>> fixed instead of recover this in the ethdev.
>>>>
>>>> +1
>>>>
>>>
>>> This is a case of KNI application performing device re-configuration to
>> change MTU. The application explicitly calls Rx queue setup, however doesn't
>> call Tx queue setup. When MTU for KNI interface is changed it runs into a
>> segfault when trying to start Tx queues.
>>
>> Why it crash? Device re-configuration should not be changing the number of
>> the queues, it is always 1. What is missing/wrong without the Tx queue setup?
>>
> 
> QEDE PMD deallocates all fastpath resources unconditionally, including Tx queue, when re-configuring the device. When reallocating resources PMD relies on application to explicitly setup the Rx/Tx queue. 
> Deallocation of all the resources is only required if the Rx/Tx queue configuration changes. So when KNI MTU change performs device re-configuration it exposes a bug, where there are no Tx queue resources setup. Then while starting the Tx queue in HW from PMD, application crashes.
> I'll submit a PMD fix for device re-configuration.

Got it.

But if there is no change in queue number in the re-configure, application
doesn't have to call the Rx/Tx setup. In this case even you fix the KNI sample
app, any other user application can hit same issue.

> 
>>> Some other applications make use of rte_eth_dev_set_mtu() ethdev op,
>> which looks to be cleaner approach.
>>>
>>
>> +1. I don't know history if there is a specific reason this way
>> +selected, but
>> after release I think we can try this.
> 
> Sure.
> Thanks!
> -Rasesh
> 


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

* Re: [dpdk-dev] [PATCH v2 2/2] examples/kni: fix MTU change to setup Tx queue
  2020-05-14  4:09 ` [dpdk-dev] [PATCH v2 2/2] examples/kni: fix MTU change to setup Tx queue Rasesh Mody
  2020-05-14 15:33   ` Ferruh Yigit
@ 2020-05-15 11:29   ` Ferruh Yigit
  2020-05-19 16:31     ` Thomas Monjalon
  1 sibling, 1 reply; 22+ messages in thread
From: Ferruh Yigit @ 2020-05-15 11:29 UTC (permalink / raw)
  To: Rasesh Mody, dev, jerinj; +Cc: GR-Everest-DPDK-Dev, stable

On 5/14/2020 5:09 AM, Rasesh Mody wrote:
> This patch adds a fix to setup Tx queue when changing KNI interface MTU.
> It ensures device can safely start txq post MTU change operation.
> 
> Fixes: fc9ee41b7016 ("examples/kni: convert to new ethdev offloads API")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Rasesh Mody <rmody@marvell.com>

I see you are going with a fix in the PMD, but the fix in the KNI sample also
looks reasonable, so I guess we can keep this.

Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>


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

* Re: [dpdk-dev] [PATCH v3 1/2] net/qede: fix assignment of Rx/Tx handlers
  2020-05-15  6:34 ` [dpdk-dev] [PATCH v3 1/2] net/qede: fix assignment of Rx/Tx handlers Rasesh Mody
@ 2020-05-15 12:26   ` Jerin Jacob
  0 siblings, 0 replies; 22+ messages in thread
From: Jerin Jacob @ 2020-05-15 12:26 UTC (permalink / raw)
  To: Rasesh Mody
  Cc: dpdk-dev, Jerin Jacob, Ferruh Yigit, GR-Everest-DPDK-Dev,
	dpdk stable, Igor Russkikh

On Fri, May 15, 2020 at 12:05 PM Rasesh Mody <rmody@marvell.com> wrote:
>
> Fix to assign dummy Rx/Tx handlers in dev_stop.
> For MTU set, assignment of the appropriate Rx/Tx handlers will be
> handled by dev_start/dev_stop.
>
> Fixes: 81f8804992c9 ("net/qede: enhance Rx CPU utilization")
> Fixes: 8de0c4201926 ("net/qede: fix odd number of queues usage in 100G mode")
> Cc: stable@dpdk.org
>
> Signed-off-by: Rasesh Mody <rmody@marvell.com>
> Signed-off-by: Igor Russkikh <irusskikh@marvell.com>

Series applied to dpdk-next-net-mrvl/master after fixing the below issue.

Wrong tag:
        Fixes: 8de0c4201926 ("net/qede: fix odd number of queues usage in 100G
Wrong 'Fixes' reference:
        Fixes: 8de0c4201926 ("net/qede: fix odd number of queues usage in 100G


> ---
>  drivers/net/qede/qede_ethdev.c | 33 ++++++++++++++++-----------------
>  1 file changed, 16 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
> index 0b1fca9ac..d3d916e81 100644
> --- a/drivers/net/qede/qede_ethdev.c
> +++ b/drivers/net/qede/qede_ethdev.c
> @@ -320,13 +320,19 @@ qede_interrupt_handler(void *param)
>  }
>
>  static void
> -qede_assign_rxtx_handlers(struct rte_eth_dev *dev)
> +qede_assign_rxtx_handlers(struct rte_eth_dev *dev, bool is_dummy)
>  {
>         uint64_t tx_offloads = dev->data->dev_conf.txmode.offloads;
>         struct qede_dev *qdev = dev->data->dev_private;
>         struct ecore_dev *edev = &qdev->edev;
>         bool use_tx_offload = false;
>
> +       if (is_dummy) {
> +               dev->rx_pkt_burst = qede_rxtx_pkts_dummy;
> +               dev->tx_pkt_burst = qede_rxtx_pkts_dummy;
> +               return;
> +       }
> +
>         if (ECORE_IS_CMT(edev)) {
>                 dev->rx_pkt_burst = qede_recv_pkts_cmt;
>                 dev->tx_pkt_burst = qede_xmit_pkts_cmt;
> @@ -1153,7 +1159,9 @@ static int qede_dev_start(struct rte_eth_dev *eth_dev)
>         /* Start/resume traffic */
>         qede_fastpath_start(edev);
>
> -       qede_assign_rxtx_handlers(eth_dev);
> +       /* Assign I/O handlers */
> +       qede_assign_rxtx_handlers(eth_dev, false);
> +
>         DP_INFO(edev, "Device started\n");
>
>         return 0;
> @@ -1175,6 +1183,11 @@ static void qede_dev_stop(struct rte_eth_dev *eth_dev)
>         /* Update link status */
>         qede_link_update(eth_dev, 0);
>
> +       /* Replace I/O functions with dummy ones. It cannot
> +        * be set to NULL because rte_eth_rx_burst() doesn't check for NULL.
> +        */
> +       qede_assign_rxtx_handlers(eth_dev, true);
> +
>         /* Disable vport */
>         if (qede_activate_vport(eth_dev, false))
>                 return;
> @@ -2323,11 +2336,6 @@ static int qede_set_mtu(struct rte_eth_dev *dev, uint16_t mtu)
>                         dev->data->min_rx_buf_size);
>                 return -EINVAL;
>         }
> -       /* Temporarily replace I/O functions with dummy ones. It cannot
> -        * be set to NULL because rte_eth_rx_burst() doesn't check for NULL.
> -        */
> -       dev->rx_pkt_burst = qede_rxtx_pkts_dummy;
> -       dev->tx_pkt_burst = qede_rxtx_pkts_dummy;
>         if (dev->data->dev_started) {
>                 dev->data->dev_started = 0;
>                 qede_dev_stop(dev);
> @@ -2366,15 +2374,6 @@ static int qede_set_mtu(struct rte_eth_dev *dev, uint16_t mtu)
>         /* update max frame size */
>         dev->data->dev_conf.rxmode.max_rx_pkt_len = max_rx_pkt_len;
>
> -       /* Reassign back */
> -       qede_assign_rxtx_handlers(dev);
> -       if (ECORE_IS_CMT(edev)) {
> -               dev->rx_pkt_burst = qede_recv_pkts_cmt;
> -               dev->tx_pkt_burst = qede_xmit_pkts_cmt;
> -       } else {
> -               dev->rx_pkt_burst = qede_recv_pkts;
> -               dev->tx_pkt_burst = qede_xmit_pkts;
> -       }
>         return 0;
>  }
>
> @@ -2577,7 +2576,7 @@ static int qede_common_dev_init(struct rte_eth_dev *eth_dev, bool is_vf)
>         strncpy((char *)params.name, QEDE_PMD_VER_PREFIX,
>                 QEDE_PMD_DRV_VER_STR_SIZE);
>
> -       qede_assign_rxtx_handlers(eth_dev);
> +       qede_assign_rxtx_handlers(eth_dev, true);
>         eth_dev->tx_pkt_prepare = qede_xmit_prep_pkts;
>
>         /* For CMT mode device do periodic polling for slowpath events.
> --
> 2.18.0
>

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

* Re: [dpdk-dev] [PATCH v2 2/2] examples/kni: fix MTU change to setup Tx queue
  2020-05-15 11:29   ` Ferruh Yigit
@ 2020-05-19 16:31     ` Thomas Monjalon
  0 siblings, 0 replies; 22+ messages in thread
From: Thomas Monjalon @ 2020-05-19 16:31 UTC (permalink / raw)
  To: Rasesh Mody; +Cc: dev, jerinj, GR-Everest-DPDK-Dev, stable, Ferruh Yigit

15/05/2020 13:29, Ferruh Yigit:
> On 5/14/2020 5:09 AM, Rasesh Mody wrote:
> > This patch adds a fix to setup Tx queue when changing KNI interface MTU.
> > It ensures device can safely start txq post MTU change operation.
> > 
> > Fixes: fc9ee41b7016 ("examples/kni: convert to new ethdev offloads API")
> > Cc: stable@dpdk.org
> > 
> > Signed-off-by: Rasesh Mody <rmody@marvell.com>
> 
> I see you are going with a fix in the PMD, but the fix in the KNI sample also
> looks reasonable, so I guess we can keep this.
> 
> Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

Applied, thanks




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

end of thread, other threads:[~2020-05-19 16:31 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-05  3:09 [dpdk-dev] [PATCH 1/2] net/qede: fix assignment of Rx/Tx handlers Rasesh Mody
2020-05-05  3:09 ` [dpdk-dev] [PATCH 2/2] net/qede: restore Tx queue setup Rasesh Mody
2020-05-05  6:44   ` Jerin Jacob
2020-05-05  8:59     ` Ferruh Yigit
2020-05-05  9:15       ` Thomas Monjalon
2020-05-06  2:43         ` [dpdk-dev] [EXT] " Rasesh Mody
2020-05-10  7:04           ` Jerin Jacob
2020-05-14  4:10             ` Rasesh Mody
2020-05-14  7:56               ` Jerin Jacob
2020-05-14 14:32           ` Ferruh Yigit
2020-05-14 22:43             ` Rasesh Mody
2020-05-15 10:39               ` Ferruh Yigit
2020-05-05  9:01 ` [dpdk-dev] [PATCH 1/2] net/qede: fix assignment of Rx/Tx handlers Ferruh Yigit
2020-05-06  2:34   ` [dpdk-dev] [EXT] " Rasesh Mody
2020-05-14  4:09 ` [dpdk-dev] [PATCH v2 " Rasesh Mody
2020-05-14  4:09 ` [dpdk-dev] [PATCH v2 2/2] examples/kni: fix MTU change to setup Tx queue Rasesh Mody
2020-05-14 15:33   ` Ferruh Yigit
2020-05-15 11:29   ` Ferruh Yigit
2020-05-19 16:31     ` Thomas Monjalon
2020-05-15  6:34 ` [dpdk-dev] [PATCH v3 1/2] net/qede: fix assignment of Rx/Tx handlers Rasesh Mody
2020-05-15 12:26   ` Jerin Jacob
2020-05-15  6:34 ` [dpdk-dev] [PATCH v3 2/2] net/qede: fix port reconfiguration Rasesh Mody

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).