Re: [dpdk-dev] [PATCH 2/2] net/qede: restore Tx queue setup

[Ferruh Yigit <ferruh.yigit@intel.com>]

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