diff mbox

[dpdk-dev,v7,2/7] ixgbe:clean scattered_rx configure in dev_stop

Message ID 1415773476-31004-3-git-send-email-cunming.liang@intel.com (mailing list archive)
State Accepted, archived
Headers

Commit Message

Cunming Liang Nov. 12, 2014, 6:24 a.m. UTC 
  Signed-off-by: Cunming Liang <cunming.liang@intel.com>
---
 lib/librte_pmd_ixgbe/ixgbe_ethdev.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

Comments

Thomas Monjalon Nov. 12, 2014, 7:53 a.m. UTC | #1 
Hi Cunming,

Please, could you provide an explanation for the commit log?
It should answer to the question "what was the issue?"
If it's a fix, the title should start with "fix".

Maybe that the same kind of fix is needed for em, igb and i40e?

Thanks
Cunming Liang Nov. 12, 2014, 8:21 a.m. UTC | #2 
The scattered_rx is update in dev_start.
In this unit test, we will re-configure and change the scatter mode.
When we stop, re-configure and then re-start, it expect using the new configure.
But during re-configure, the stored data may still old.
The patch clean the configure anyway in dev_stop.
For em, igb and i40e, we haven't provide so much rx/tx pair for switching.

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, November 12, 2014 3:53 PM
> To: Liang, Cunming
> Cc: dev@dpdk.org; nhorman@tuxdriver.com; Ananyev, Konstantin; Richardson,
> Bruce; De Lara Guarch, Pablo
> Subject: Re: [PATCH v7 2/7] ixgbe:clean scattered_rx configure in dev_stop
> 
> Hi Cunming,
> 
> Please, could you provide an explanation for the commit log?
> It should answer to the question "what was the issue?"
> If it's a fix, the title should start with "fix".
> 
> Maybe that the same kind of fix is needed for em, igb and i40e?
> 
> Thanks
> --
> Thomas
Thomas Monjalon Nov. 12, 2014, 9:24 a.m. UTC | #3 
2014-11-12 08:21, Liang, Cunming:
> For em, igb and i40e, we haven't provide so much rx/tx pair for switching.

Sorry, I don't understand. Which pair are you telling about?
em, igb and i40e have scattered Rx functions.
Cunming Liang Nov. 12, 2014, 10:29 a.m. UTC | #4 
Maybe pair is not accurate, I means the different rx/tx register function, like:
Ixgbe_recv_bulk_alloc/ixgbe_recv_(scattered_)pkts_vec/ixgbe_recv_scattered_pkts
ixgbe_xmit_pkts_simple/Ixgbe_xmit_pkts_vec/ixgbe_xmit_pkts

-Liang Cunming

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, November 12, 2014 5:25 PM
> To: Liang, Cunming
> Cc: dev@dpdk.org; nhorman@tuxdriver.com; Ananyev, Konstantin; Richardson,
> Bruce; De Lara Guarch, Pablo
> Subject: Re: [PATCH v7 2/7] ixgbe:clean scattered_rx configure in dev_stop
> 
> 2014-11-12 08:21, Liang, Cunming:
> > For em, igb and i40e, we haven't provide so much rx/tx pair for switching.
> 
> Sorry, I don't understand. Which pair are you telling about?
> em, igb and i40e have scattered Rx functions.
> 
> --
> Thomas
Thomas Monjalon Nov. 12, 2014, 10:32 a.m. UTC | #5 
2014-11-12 10:29, Liang, Cunming:
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > 2014-11-12 08:21, Liang, Cunming:
> > > For em, igb and i40e, we haven't provide so much rx/tx pair for switching.
> > 
> > Sorry, I don't understand. Which pair are you telling about?
> > em, igb and i40e have scattered Rx functions.
> 
> Maybe pair is not accurate, I means the different rx/tx register function, like:
> Ixgbe_recv_bulk_alloc/ixgbe_recv_(scattered_)pkts_vec/ixgbe_recv_scattered_pkts
> ixgbe_xmit_pkts_simple/Ixgbe_xmit_pkts_vec/ixgbe_xmit_pkts

OK that's what I understood.
However, you should check the scatter/non-scatter Rx functions of other drivers.
I think they need the same fix.

Thanks
Cunming Liang Nov. 12, 2014, 10:42 a.m. UTC | #6 
scatter/non-scatter always be checked during dev_start.
For others, it's only have the two. Won't do additional check during rx/tx_queue_setup(before dev_start).
So they won't have problem, I think.
But for ixgbe, it will check it meets vector condition or not, then choose the best performance function.
As the happens before dev_start, so the old value will impact the condition check.

-Liang Cunming

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, November 12, 2014 6:33 PM
> To: Liang, Cunming
> Cc: dev@dpdk.org; nhorman@tuxdriver.com; Ananyev, Konstantin; Richardson,
> Bruce; De Lara Guarch, Pablo
> Subject: Re: [PATCH v7 2/7] ixgbe:clean scattered_rx configure in dev_stop
> 
> 2014-11-12 10:29, Liang, Cunming:
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > 2014-11-12 08:21, Liang, Cunming:
> > > > For em, igb and i40e, we haven't provide so much rx/tx pair for switching.
> > >
> > > Sorry, I don't understand. Which pair are you telling about?
> > > em, igb and i40e have scattered Rx functions.
> >
> > Maybe pair is not accurate, I means the different rx/tx register function, like:
> >
> Ixgbe_recv_bulk_alloc/ixgbe_recv_(scattered_)pkts_vec/ixgbe_recv_scattered_p
> kts
> > ixgbe_xmit_pkts_simple/Ixgbe_xmit_pkts_vec/ixgbe_xmit_pkts
> 
> OK that's what I understood.
> However, you should check the scatter/non-scatter Rx functions of other drivers.
> I think they need the same fix.
> 
> Thanks
> --
> Thomas
diff mbox

Patch

diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
index 9c73a30..05490dc 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
@@ -1600,6 +1600,9 @@  ixgbe_dev_stop(struct rte_eth_dev *dev)
 
 	ixgbe_dev_clear_queues(dev);
 
+	/* Clear stored conf */
+	dev->data->scattered_rx = 0;
+
 	/* Clear recorded link status */
 	memset(&link, 0, sizeof(link));
 	rte_ixgbe_dev_atomic_write_link_status(dev, &link);
@@ -2889,6 +2892,9 @@  ixgbevf_dev_stop(struct rte_eth_dev *dev)
 	  */
 	ixgbevf_set_vfta_all(dev,0);
 
+	/* Clear stored conf */
+	dev->data->scattered_rx = 0;
+
 	ixgbe_dev_clear_queues(dev);
 }