diff mbox

[dpdk-dev] i40e: fix no effect wait_to_complete on link_get

Message ID 1427855614-8654-1-git-send-email-cunming.liang@intel.com (mailing list archive)
State Accepted, archived
Headers

Commit Message

Cunming Liang April 1, 2015, 2:33 a.m. UTC 
  API *rte_eth_link_get* expect to call a wait to complete link_update.
That's the difference between *rte_eth_link_get_nowait*.
The patch fixes the issue that i40e link_update ignores the wait_to_complete flag.
The issue impacts those applications calling rte_eth_link_get to get wrong intermediate link status.

Signed-off-by: Cunming Liang <cunming.liang@intel.com>
---
 lib/librte_pmd_i40e/i40e_ethdev.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

Comments

Zhang, Helin April 1, 2015, 2:50 a.m. UTC | #1 
> -----Original Message-----
> From: Liang, Cunming
> Sent: Wednesday, April 1, 2015 10:34 AM
> To: dev@dpdk.org
> Cc: Zhang, Helin; Zhang, XiaonanX; Dumitrescu, Cristian; Liang, Cunming
> Subject: [PATCH] i40e: fix no effect wait_to_complete on link_get
> 
> API *rte_eth_link_get* expect to call a wait to complete link_update.
> That's the difference between *rte_eth_link_get_nowait*.
> The patch fixes the issue that i40e link_update ignores the wait_to_complete
> flag.
> The issue impacts those applications calling rte_eth_link_get to get wrong
> intermediate link status.
> 
> Signed-off-by: Cunming Liang <cunming.liang@intel.com>
Acked-by: Helin Zhang <helin.zhang@intel.com>

> ---
>  lib/librte_pmd_i40e/i40e_ethdev.c | 29 +++++++++++++++++++----------
>  1 file changed, 19 insertions(+), 10 deletions(-)
Zhang, XiaonanX April 1, 2015, 6:10 a.m. UTC | #2 
Tested-by: Xiaonan zhang<xiaonanx.zhang@intel.com>

- OS: Fedora21 3.19.1-201.fc21.x86_64
- GCC: gcc version 4.9.1 20140930 (Red Hat 4.9.1-11) (GCC)
- CPU: Intel(R) Xeon(R) CPU E5-2680 v2 @ 2.80GHz
- NIC: Ethernet controller [0200]: Intel Corporation Ethernet Controller X710 for 10GbE SFP+ [8086:1572] (rev 01)
- Default x86_64-native-linuxapp-gcc configuration
- Total 1 cases, 1 passed, 0 failed

- Test case: Used Qos example to verified 
-------------------------------------
 
Traffic shaping for subport. Check that the subport rate is enforced.
 
Set the subport output rate to x% of line rate (x = 10 .. 100). Set the subport TC limits high (100% line rate each), so they do not constitute limitations. Input traffic is 100% line rate.
 
Different tb period and tb credits, therefore different output rate, are tried: 25%, 50%, 75%, 90% and 100% the lineal rate. (The output for subport is Tb credits per period / Tb period.)
The traffic is injected change subport value random.
 
Other parameters are same before tests and they don't change here.

Cmdline:   ./examples/qos_sched/build/qos_sched  -c 0xe -n 4 -- --pfc "0,1,2,3,3" --cfg "/root/profile_sched_pipe_1.cfg"
 
The result is this table:
 
 
+-----------------------+----------------------+
|  Subport output rate  | Subport output rate  |
|     (% line rate)     |     (Mpps)           |
+-----------+-----------+----------+-----------+
|  Expected | Actual    | Expected | Actual    |
+-----------+-----------+----------+-----------+
 

Signed-off-by: Xiaonan Zhang <xiaonanx.zhang@intel.com>


-----Original Message-----
From: Zhang, Helin 
Sent: Wednesday, April 01, 2015 10:50 AM
To: Liang, Cunming; dev@dpdk.org
Cc: Zhang, XiaonanX; Dumitrescu, Cristian
Subject: RE: [PATCH] i40e: fix no effect wait_to_complete on link_get



> -----Original Message-----
> From: Liang, Cunming
> Sent: Wednesday, April 1, 2015 10:34 AM
> To: dev@dpdk.org
> Cc: Zhang, Helin; Zhang, XiaonanX; Dumitrescu, Cristian; Liang, Cunming
> Subject: [PATCH] i40e: fix no effect wait_to_complete on link_get
> 
> API *rte_eth_link_get* expect to call a wait to complete link_update.
> That's the difference between *rte_eth_link_get_nowait*.
> The patch fixes the issue that i40e link_update ignores the wait_to_complete
> flag.
> The issue impacts those applications calling rte_eth_link_get to get wrong
> intermediate link status.
> 
> Signed-off-by: Cunming Liang <cunming.liang@intel.com>
Acked-by: Helin Zhang <helin.zhang@intel.com>

> ---
>  lib/librte_pmd_i40e/i40e_ethdev.c | 29 +++++++++++++++++++----------
>  1 file changed, 19 insertions(+), 10 deletions(-)
Thomas Monjalon April 1, 2015, 7:51 p.m. UTC | #3 
Hi,

2015-04-01 06:10, Zhang, XiaonanX:
> 
> Tested-by: Xiaonan zhang<xiaonanx.zhang@intel.com>
> 
> - OS: Fedora21 3.19.1-201.fc21.x86_64
> - GCC: gcc version 4.9.1 20140930 (Red Hat 4.9.1-11) (GCC)
> - CPU: Intel(R) Xeon(R) CPU E5-2680 v2 @ 2.80GHz
> - NIC: Ethernet controller [0200]: Intel Corporation Ethernet Controller X710 for 10GbE SFP+ [8086:1572] (rev 01)
> - Default x86_64-native-linuxapp-gcc configuration
> - Total 1 cases, 1 passed, 0 failed
> 
> - Test case: Used Qos example to verified 
> -------------------------------------

What is the relation between link status timeout and qos_sched?

> Traffic shaping for subport. Check that the subport rate is enforced.
>  
> Set the subport output rate to x% of line rate (x = 10 .. 100). Set the subport TC limits high (100% line rate each), so they do not constitute limitations. Input traffic is 100% line rate.
>  
> Different tb period and tb credits, therefore different output rate, are tried: 25%, 50%, 75%, 90% and 100% the lineal rate. (The output for subport is Tb credits per period / Tb period.)
> The traffic is injected change subport value random.
>  
> Other parameters are same before tests and they don't change here.
> 
> Cmdline:   ./examples/qos_sched/build/qos_sched  -c 0xe -n 4 -- --pfc "0,1,2,3,3" --cfg "/root/profile_sched_pipe_1.cfg"
>  
> The result is this table:
>  
>  
> +-----------------------+----------------------+
> |  Subport output rate  | Subport output rate  |
> |     (% line rate)     |     (Mpps)           |
> +-----------+-----------+----------+-----------+
> |  Expected | Actual    | Expected | Actual    |
> +-----------+-----------+----------+-----------+

This table is empty.

> 
> Signed-off-by: Xiaonan Zhang <xiaonanx.zhang@intel.com>

It seems that this test report is not relevant.
It will be ignored in the commit message. Sorry
Thomas Monjalon April 1, 2015, 7:53 p.m. UTC | #4 
> > API *rte_eth_link_get* expect to call a wait to complete link_update.
> > That's the difference between *rte_eth_link_get_nowait*.
> > The patch fixes the issue that i40e link_update ignores the wait_to_complete
> > flag.
> > The issue impacts those applications calling rte_eth_link_get to get wrong
> > intermediate link status.
> > 
> > Signed-off-by: Cunming Liang <cunming.liang@intel.com>
> Acked-by: Helin Zhang <helin.zhang@intel.com>

Applied, thanks
Cunming Liang April 2, 2015, 1:44 a.m. UTC | #5 
Hi Thomas,

> What is the relation between link status timeout and qos_sched?
[LCM] Validation team found qos_sched test failure on i40e. The sample depends on link speed to calc the percentage.
The root cause comes from that i40e link_get hasn't support wait_to_complete well. 
I agree with you it should add more description in test report why 'Used QoS example to verified'.

> > +-----------------------+----------------------+
> > |  Subport output rate  | Subport output rate  |
> > |     (% line rate)     |     (Mpps)           |
> > +-----------+-----------+----------+-----------+
> > |  Expected | Actual    | Expected | Actual    |
> > +-----------+-----------+----------+-----------+
> 
> This table is empty.
[LCM] It's useless, should be omitted I think.

Cunming
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Thursday, April 02, 2015 3:52 AM
> To: Zhang, XiaonanX; Cao, Waterman
> Cc: dev@dpdk.org; Zhang, Helin; Liang, Cunming
> Subject: Re: [dpdk-dev] [PATCH] i40e: fix no effect wait_to_complete on link_get
> 
> Hi,
> 
> 2015-04-01 06:10, Zhang, XiaonanX:
> >
> > Tested-by: Xiaonan zhang<xiaonanx.zhang@intel.com>
> >
> > - OS: Fedora21 3.19.1-201.fc21.x86_64
> > - GCC: gcc version 4.9.1 20140930 (Red Hat 4.9.1-11) (GCC)
> > - CPU: Intel(R) Xeon(R) CPU E5-2680 v2 @ 2.80GHz
> > - NIC: Ethernet controller [0200]: Intel Corporation Ethernet Controller X710 for
> 10GbE SFP+ [8086:1572] (rev 01)
> > - Default x86_64-native-linuxapp-gcc configuration
> > - Total 1 cases, 1 passed, 0 failed
> >
> > - Test case: Used Qos example to verified
> > -------------------------------------
> 
> What is the relation between link status timeout and qos_sched?
> 
> > Traffic shaping for subport. Check that the subport rate is enforced.
> >
> > Set the subport output rate to x% of line rate (x = 10 .. 100). Set the subport TC
> limits high (100% line rate each), so they do not constitute limitations. Input traffic
> is 100% line rate.
> >
> > Different tb period and tb credits, therefore different output rate, are tried:
> 25%, 50%, 75%, 90% and 100% the lineal rate. (The output for subport is Tb credits
> per period / Tb period.)
> > The traffic is injected change subport value random.
> >
> > Other parameters are same before tests and they don't change here.
> >
> > Cmdline:   ./examples/qos_sched/build/qos_sched  -c 0xe -n 4 -- --pfc
> "0,1,2,3,3" --cfg "/root/profile_sched_pipe_1.cfg"
> >
> > The result is this table:
> >
> >
> > +-----------------------+----------------------+
> > |  Subport output rate  | Subport output rate  |
> > |     (% line rate)     |     (Mpps)           |
> > +-----------+-----------+----------+-----------+
> > |  Expected | Actual    | Expected | Actual    |
> > +-----------+-----------+----------+-----------+
> 
> This table is empty.
> 
> >
> > Signed-off-by: Xiaonan Zhang <xiaonanx.zhang@intel.com>
> 
> It seems that this test report is not relevant.
> It will be ignored in the commit message. Sorry
Zhang, XiaonanX April 2, 2015, 1:52 a.m. UTC | #6 
Hi Thomas,
  First and foremost, We found this problem with using Qos example on Fedora21 platform, there are our reproduce test steps as follows.
  We used this example found FVL 4*10G NIC link status caused to our Qos example could not boot normally in our dpdk2.0 Release Cycle;
  In addition, it is performance case about our FVL 4 * 10G NIC, as you know, our performance result currently should not be public;
  And also that's why our expectation performance data are empty.
  In a nutshell, We have verified this patch and confirmed the Qos example could run and get performance data after we applied this patch to our code.
  Thanks.


Best Regards,
Xiaonan
   
-----Original Message-----
From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] 
Sent: Thursday, April 02, 2015 3:52 AM
To: Zhang, XiaonanX; Cao, Waterman
Cc: dev@dpdk.org; Zhang, Helin; Liang, Cunming
Subject: Re: [dpdk-dev] [PATCH] i40e: fix no effect wait_to_complete on link_get

Hi,

2015-04-01 06:10, Zhang, XiaonanX:
> 
> Tested-by: Xiaonan zhang<xiaonanx.zhang@intel.com>
> 
> - OS: Fedora21 3.19.1-201.fc21.x86_64
> - GCC: gcc version 4.9.1 20140930 (Red Hat 4.9.1-11) (GCC)
> - CPU: Intel(R) Xeon(R) CPU E5-2680 v2 @ 2.80GHz
> - NIC: Ethernet controller [0200]: Intel Corporation Ethernet Controller X710 for 10GbE SFP+ [8086:1572] (rev 01)
> - Default x86_64-native-linuxapp-gcc configuration
> - Total 1 cases, 1 passed, 0 failed
> 
> - Test case: Used Qos example to verified 
> -------------------------------------

What is the relation between link status timeout and qos_sched?

> Traffic shaping for subport. Check that the subport rate is enforced.
>  
> Set the subport output rate to x% of line rate (x = 10 .. 100). Set the subport TC limits high (100% line rate each), so they do not constitute limitations. Input traffic is 100% line rate.
>  
> Different tb period and tb credits, therefore different output rate, are tried: 25%, 50%, 75%, 90% and 100% the lineal rate. (The output for subport is Tb credits per period / Tb period.)
> The traffic is injected change subport value random.
>  
> Other parameters are same before tests and they don't change here.
> 
> Cmdline:   ./examples/qos_sched/build/qos_sched  -c 0xe -n 4 -- --pfc "0,1,2,3,3" --cfg "/root/profile_sched_pipe_1.cfg"
>  
> The result is this table:
>  
>  
> +-----------------------+----------------------+
> |  Subport output rate  | Subport output rate  |
> |     (% line rate)     |     (Mpps)           |
> +-----------+-----------+----------+-----------+
> |  Expected | Actual    | Expected | Actual    |
> +-----------+-----------+----------+-----------+

This table is empty.

> 
> Signed-off-by: Xiaonan Zhang <xiaonanx.zhang@intel.com>

It seems that this test report is not relevant.
It will be ignored in the commit message. Sorry
Stephen Hemminger April 2, 2015, 10:18 p.m. UTC | #7 
The qos_scheduler has a 32 bit speed value and therefore I doubt it will
work work at 40G

On Wed, Apr 1, 2015 at 6:44 PM, Liang, Cunming <cunming.liang@intel.com>
wrote:

> Hi Thomas,
>
> > What is the relation between link status timeout and qos_sched?
> [LCM] Validation team found qos_sched test failure on i40e. The sample
> depends on link speed to calc the percentage.
> The root cause comes from that i40e link_get hasn't support
> wait_to_complete well.
> I agree with you it should add more description in test report why 'Used
> QoS example to verified'.
>
> > > +-----------------------+----------------------+
> > > |  Subport output rate  | Subport output rate  |
> > > |     (% line rate)     |     (Mpps)           |
> > > +-----------+-----------+----------+-----------+
> > > |  Expected | Actual    | Expected | Actual    |
> > > +-----------+-----------+----------+-----------+
> >
> > This table is empty.
> [LCM] It's useless, should be omitted I think.
>
> Cunming
> > -----Original Message-----
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > Sent: Thursday, April 02, 2015 3:52 AM
> > To: Zhang, XiaonanX; Cao, Waterman
> > Cc: dev@dpdk.org; Zhang, Helin; Liang, Cunming
> > Subject: Re: [dpdk-dev] [PATCH] i40e: fix no effect wait_to_complete on
> link_get
> >
> > Hi,
> >
> > 2015-04-01 06:10, Zhang, XiaonanX:
> > >
> > > Tested-by: Xiaonan zhang<xiaonanx.zhang@intel.com>
> > >
> > > - OS: Fedora21 3.19.1-201.fc21.x86_64
> > > - GCC: gcc version 4.9.1 20140930 (Red Hat 4.9.1-11) (GCC)
> > > - CPU: Intel(R) Xeon(R) CPU E5-2680 v2 @ 2.80GHz
> > > - NIC: Ethernet controller [0200]: Intel Corporation Ethernet
> Controller X710 for
> > 10GbE SFP+ [8086:1572] (rev 01)
> > > - Default x86_64-native-linuxapp-gcc configuration
> > > - Total 1 cases, 1 passed, 0 failed
> > >
> > > - Test case: Used Qos example to verified
> > > -------------------------------------
> >
> > What is the relation between link status timeout and qos_sched?
> >
> > > Traffic shaping for subport. Check that the subport rate is enforced.
> > >
> > > Set the subport output rate to x% of line rate (x = 10 .. 100). Set
> the subport TC
> > limits high (100% line rate each), so they do not constitute
> limitations. Input traffic
> > is 100% line rate.
> > >
> > > Different tb period and tb credits, therefore different output rate,
> are tried:
> > 25%, 50%, 75%, 90% and 100% the lineal rate. (The output for subport is
> Tb credits
> > per period / Tb period.)
> > > The traffic is injected change subport value random.
> > >
> > > Other parameters are same before tests and they don't change here.
> > >
> > > Cmdline:   ./examples/qos_sched/build/qos_sched  -c 0xe -n 4 -- --pfc
> > "0,1,2,3,3" --cfg "/root/profile_sched_pipe_1.cfg"
> > >
> > > The result is this table:
> > >
> > >
> > > +-----------------------+----------------------+
> > > |  Subport output rate  | Subport output rate  |
> > > |     (% line rate)     |     (Mpps)           |
> > > +-----------+-----------+----------+-----------+
> > > |  Expected | Actual    | Expected | Actual    |
> > > +-----------+-----------+----------+-----------+
> >
> > This table is empty.
> >
> > >
> > > Signed-off-by: Xiaonan Zhang <xiaonanx.zhang@intel.com>
> >
> > It seems that this test report is not relevant.
> > It will be ignored in the commit message. Sorry
>
>
diff mbox

Patch

diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c b/lib/librte_pmd_i40e/i40e_ethdev.c
index 00d044f..6b8f96e 100644
--- a/lib/librte_pmd_i40e/i40e_ethdev.c
+++ b/lib/librte_pmd_i40e/i40e_ethdev.c
@@ -1081,28 +1081,37 @@  i40e_dev_set_link_down(__rte_unused struct rte_eth_dev *dev)
 
 int
 i40e_dev_link_update(struct rte_eth_dev *dev,
-		     __rte_unused int wait_to_complete)
+		     int wait_to_complete)
 {
+#define CHECK_INTERVAL 100  /* 100ms */
+#define MAX_REPEAT_TIME 10  /* 1s (10 * 100ms) in total */
 	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct i40e_link_status link_status;
 	struct rte_eth_link link, old;
 	int status;
+	unsigned rep_cnt = MAX_REPEAT_TIME;
 
 	memset(&link, 0, sizeof(link));
 	memset(&old, 0, sizeof(old));
 	memset(&link_status, 0, sizeof(link_status));
 	rte_i40e_dev_atomic_read_link_status(dev, &old);
 
-	/* Get link status information from hardware */
-	status = i40e_aq_get_link_info(hw, false, &link_status, NULL);
-	if (status != I40E_SUCCESS) {
-		link.link_speed = ETH_LINK_SPEED_100;
-		link.link_duplex = ETH_LINK_FULL_DUPLEX;
-		PMD_DRV_LOG(ERR, "Failed to get link info");
-		goto out;
-	}
+	do {
+		/* Get link status information from hardware */
+		status = i40e_aq_get_link_info(hw, false, &link_status, NULL);
+		if (status != I40E_SUCCESS) {
+			link.link_speed = ETH_LINK_SPEED_100;
+			link.link_duplex = ETH_LINK_FULL_DUPLEX;
+			PMD_DRV_LOG(ERR, "Failed to get link info");
+			goto out;
+		}
+
+		link.link_status = link_status.link_info & I40E_AQ_LINK_UP;
+		if (!wait_to_complete)
+			break;
 
-	link.link_status = link_status.link_info & I40E_AQ_LINK_UP;
+		rte_delay_ms(CHECK_INTERVAL);
+	} while (!link.link_status && rep_cnt--);
 
 	if (!link.link_status)
 		goto out;