diff mbox series

[v1] net/i40e: fix RX/TX setup when restart port

Message ID 20181128165152.78676-1-zhirun.yan@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Qi Zhang
Headers
Series [v1] net/i40e: fix RX/TX setup when restart port |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/intel-Performance-Testing success Performance Testing PASS

Commit Message

Yan, Zhirun Nov. 28, 2018, 4:51 p.m. UTC 
  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(-)

Comments

Ilya Maximets Nov. 28, 2018, 9:32 a.m. UTC | #1 
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
>
Yan, Zhirun Nov. 30, 2018, 6:52 a.m. UTC | #2 
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
> >
diff mbox series

Patch

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