Re: [dpdk-dev] [v1] net/i40e: fix RX/TX setup when restart port

Re: [dpdk-dev] [v1] net/i40e: fix RX/TX setup when restart port

[Ilya Maximets]

On 28.11.2018 19:51, Zhirun Yan wrote:
> Before this patch, there are two functions that will clear RX/TX queues
> number: rte_eth_dev_close() and i40e_dev_free_queues(). But if also
> i40e_dev_free_queues() clear it, RX/TX queues will not set up correctly
> when restart port.

According to DPDK API device could not be restarted after rte_eth_dev_close:

  "Close a stopped Ethernet device. The device cannot be restarted!"
  http://doc.dpdk.org/api/rte__ethdev_8h.html#a93eeb672a2f9cd18e338aad10c77687c

You should not close the device if you're willing to use it later.
If you really want to close it, you'll need to detach it and attach
back when needed.

> 
> Fixes: 6b4537128394 ("i40e: free queue memory when closing")
> 
> Signed-off-by: Zhirun Yan <zhirun.yan@intel.com>
> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> ---
>  drivers/net/i40e/i40e_rxtx.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
> index e1152ff0e..cc953ad58 100644
> --- a/drivers/net/i40e/i40e_rxtx.c
> +++ b/drivers/net/i40e/i40e_rxtx.c
> @@ -2753,7 +2753,6 @@ i40e_dev_free_queues(struct rte_eth_dev *dev)
>  		i40e_dev_rx_queue_release(dev->data->rx_queues[i]);
>  		dev->data->rx_queues[i] = NULL;
>  	}
> -	dev->data->nb_rx_queues = 0;
>  
>  	for (i = 0; i < dev->data->nb_tx_queues; i++) {
>  		if (!dev->data->tx_queues[i])
> @@ -2761,7 +2760,6 @@ i40e_dev_free_queues(struct rte_eth_dev *dev)
>  		i40e_dev_tx_queue_release(dev->data->tx_queues[i]);
>  		dev->data->tx_queues[i] = NULL;
>  	}
> -	dev->data->nb_tx_queues = 0;
>  }
>  
>  #define I40E_FDIR_NUM_TX_DESC  I40E_MIN_RING_DESC
> 

[dpdk-dev] [PATCH v1] net/i40e: fix RX/TX setup when restart port

[Zhirun Yan]

Before this patch, there are two functions that will clear RX/TX queues
number: rte_eth_dev_close() and i40e_dev_free_queues(). But if also
i40e_dev_free_queues() clear it, RX/TX queues will not set up correctly
when restart port.

Fixes: 6b4537128394 ("i40e: free queue memory when closing")

Signed-off-by: Zhirun Yan <zhirun.yan@intel.com>
Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
---
 drivers/net/i40e/i40e_rxtx.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index e1152ff0e..cc953ad58 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -2753,7 +2753,6 @@ i40e_dev_free_queues(struct rte_eth_dev *dev)
 		i40e_dev_rx_queue_release(dev->data->rx_queues[i]);
 		dev->data->rx_queues[i] = NULL;
 	}
-	dev->data->nb_rx_queues = 0;
 
 	for (i = 0; i < dev->data->nb_tx_queues; i++) {
 		if (!dev->data->tx_queues[i])
@@ -2761,7 +2760,6 @@ i40e_dev_free_queues(struct rte_eth_dev *dev)
 		i40e_dev_tx_queue_release(dev->data->tx_queues[i]);
 		dev->data->tx_queues[i] = NULL;
 	}
-	dev->data->nb_tx_queues = 0;
 }
 
 #define I40E_FDIR_NUM_TX_DESC  I40E_MIN_RING_DESC
-- 
2.17.1

Re: [dpdk-dev] [v1] net/i40e: fix RX/TX setup when restart port

[Yan, Zhirun]

Hi Ilya,

> -----Original Message-----
> From: Ilya Maximets [mailto:i.maximets@samsung.com]
> Sent: Wednesday, November 28, 2018 5:32 PM
> To: Yan, Zhirun <zhirun.yan@intel.com>; dev@dpdk.org; Zhang, Qi Z
> <qi.z.zhang@intel.com>
> Cc: Wang, Haiyue <haiyue.wang@intel.com>
> Subject: Re: [v1] net/i40e: fix RX/TX setup when restart port
> 
> On 28.11.2018 19:51, Zhirun Yan wrote:
> > Before this patch, there are two functions that will clear RX/TX
> > queues
> > number: rte_eth_dev_close() and i40e_dev_free_queues(). But if also
> > i40e_dev_free_queues() clear it, RX/TX queues will not set up
> > correctly when restart port.
> 
> According to DPDK API device could not be restarted after rte_eth_dev_close:
> 
>   "Close a stopped Ethernet device. The device cannot be restarted!"
> 
> http://doc.dpdk.org/api/rte__ethdev_8h.html#a93eeb672a2f9cd18e338aad10
> c77687c
> 
> You should not close the device if you're willing to use it later.
> If you really want to close it, you'll need to detach it and attach back when
> needed.
> 

Thanks for your advice. I am agree with you.

By the way, I think the close will do some clean job in i40e. For example, the reset
function will call it in uninit  to release resource.

Rte_eth_dev_rx\tx_queue_config() malloc the dev->data->rx_queues, so
 dev->data->nb_rx_queues = 0 should be in rte layer when rte_eth_dev_close try to
release the queues.

I think I have write a bad commit, I will modify it in V2.


> >
> > Fixes: 6b4537128394 ("i40e: free queue memory when closing")
> >
> > Signed-off-by: Zhirun Yan <zhirun.yan@intel.com>
> > Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> > ---
> >  drivers/net/i40e/i40e_rxtx.c | 2 --
> >  1 file changed, 2 deletions(-)
> >
> > diff --git a/drivers/net/i40e/i40e_rxtx.c
> > b/drivers/net/i40e/i40e_rxtx.c index e1152ff0e..cc953ad58 100644
> > --- a/drivers/net/i40e/i40e_rxtx.c
> > +++ b/drivers/net/i40e/i40e_rxtx.c
> > @@ -2753,7 +2753,6 @@ i40e_dev_free_queues(struct rte_eth_dev *dev)
> >  		i40e_dev_rx_queue_release(dev->data->rx_queues[i]);
> >  		dev->data->rx_queues[i] = NULL;
> >  	}
> > -	dev->data->nb_rx_queues = 0;
> >
> >  	for (i = 0; i < dev->data->nb_tx_queues; i++) {
> >  		if (!dev->data->tx_queues[i])
> > @@ -2761,7 +2760,6 @@ i40e_dev_free_queues(struct rte_eth_dev *dev)
> >  		i40e_dev_tx_queue_release(dev->data->tx_queues[i]);
> >  		dev->data->tx_queues[i] = NULL;
> >  	}
> > -	dev->data->nb_tx_queues = 0;
> >  }
> >
> >  #define I40E_FDIR_NUM_TX_DESC  I40E_MIN_RING_DESC
> >