net/e1000: fix link status update

Message ID 20191113173243.90826-1-lunyuanx.cui@intel.com (mailing list archive)
State Superseded, archived
Delegated to: xiaolong ye
Headers
Series net/e1000: fix link status update |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-compilation success Compile Testing PASS
ci/Intel-compilation success Compilation OK
ci/travis-robot success Travis build: passed

Commit Message

Cui, LunyuanX Nov. 13, 2019, 5:32 p.m. UTC
  Unassigned variable should not be used as judgment, and there
is no need to update link status according to old link status.
This patch fix the issue.

Fixes: 80ba61115e77 ("net/e1000: use link status helper functions")
Cc: stable@dpdk.org

Signed-off-by: Lunyuan Cui <lunyuanx.cui@intel.com>
---
 drivers/net/e1000/em_ethdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Xiaolong Ye Nov. 14, 2019, 6:41 a.m. UTC | #1
Hi,

On 11/13, Cui LunyuanX wrote:
>Unassigned variable should not be used as judgment, and there

The issue here is link structure variable has been memset first, which makes it
meaningless to compare the value of link.link_status in the conditions.

>is no need to update link status according to old link status.
>This patch fix the issue.
>
>Fixes: 80ba61115e77 ("net/e1000: use link status helper functions")
>Cc: stable@dpdk.org
>
>Signed-off-by: Lunyuan Cui <lunyuanx.cui@intel.com>
>---
> drivers/net/e1000/em_ethdev.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
>index 9a88b50b2..a3d39a935 100644
>--- a/drivers/net/e1000/em_ethdev.c
>+++ b/drivers/net/e1000/em_ethdev.c
>@@ -1157,7 +1157,7 @@ eth_em_link_update(struct rte_eth_dev *dev, int wait_to_complete)
> 	memset(&link, 0, sizeof(link));
> 
> 	/* Now we check if a transition has happened */
>-	if (link_check && (link.link_status == ETH_LINK_DOWN)) {
>+	if (link_check) {
> 		uint16_t duplex, speed;
> 		hw->mac.ops.get_link_up_info(hw, &speed, &duplex);
> 		link.link_duplex = (duplex == FULL_DUPLEX) ?

Seems you also need to fix the `else if` judgment.

Thanks,
Xiaolong

>-- 
>2.17.1
>
  
Wenzhuo Lu Nov. 26, 2019, 2:04 a.m. UTC | #2
Hi,

> -----Original Message-----
> From: Cui, LunyuanX
> Sent: Thursday, November 14, 2019 1:33 AM
> To: dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Yang, Qiming
> <qiming.yang@intel.com>; Cui, LunyuanX <lunyuanx.cui@intel.com>;
> stable@dpdk.org
> Subject: [PATCH] net/e1000: fix link status update
> 
> Unassigned variable should not be used as judgment, and there is no need
> to update link status according to old link status.
> This patch fix the issue.
> 
> Fixes: 80ba61115e77 ("net/e1000: use link status helper functions")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Lunyuan Cui <lunyuanx.cui@intel.com>
Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
  

Patch

diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
index 9a88b50b2..a3d39a935 100644
--- a/drivers/net/e1000/em_ethdev.c
+++ b/drivers/net/e1000/em_ethdev.c
@@ -1157,7 +1157,7 @@  eth_em_link_update(struct rte_eth_dev *dev, int wait_to_complete)
 	memset(&link, 0, sizeof(link));
 
 	/* Now we check if a transition has happened */
-	if (link_check && (link.link_status == ETH_LINK_DOWN)) {
+	if (link_check) {
 		uint16_t duplex, speed;
 		hw->mac.ops.get_link_up_info(hw, &speed, &duplex);
 		link.link_duplex = (duplex == FULL_DUPLEX) ?