[dpdk-dev,2/7] cxgbe: update link state when link speed changes

Message ID e61f3eb95823b8695e0ae9cda739051f7170c390.1517685185.git.rahul.lakkireddy@chelsio.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Rahul Lakkireddy Feb. 4, 2018, 6:06 a.m. UTC
  From: Kumar Sanghvi <kumaras@chelsio.com>

Original work by Surendra Mobiya <surendra@chelsio.com>

Fixes: cdac6e2eeafc ("cxgbe: add link related functions")
Signed-off-by: Kumar Sanghvi <kumaras@chelsio.com>
Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
---
 drivers/net/cxgbe/cxgbe_ethdev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
  

Comments

Ferruh Yigit Feb. 5, 2018, 5:05 p.m. UTC | #1
On 2/4/2018 6:06 AM, Rahul Lakkireddy wrote:
> From: Kumar Sanghvi <kumaras@chelsio.com>
> 
> Original work by Surendra Mobiya <surendra@chelsio.com>
> 
> Fixes: cdac6e2eeafc ("cxgbe: add link related functions")
> Signed-off-by: Kumar Sanghvi <kumaras@chelsio.com>
> Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
> ---
>  drivers/net/cxgbe/cxgbe_ethdev.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c b/drivers/net/cxgbe/cxgbe_ethdev.c
> index 6d56f3c1b..5a25125fe 100644
> --- a/drivers/net/cxgbe/cxgbe_ethdev.c
> +++ b/drivers/net/cxgbe/cxgbe_ethdev.c
> @@ -227,7 +227,8 @@ static int cxgbe_dev_link_update(struct rte_eth_dev *eth_dev,
>  	unsigned int work_done, budget = 4;
>  
>  	cxgbe_poll(&s->fw_evtq, NULL, budget, &work_done);
> -	if (old_link->link_status == pi->link_cfg.link_ok)
> +	if (old_link->link_status == pi->link_cfg.link_ok &&
> +	    old_link->link_speed == pi->link_cfg.speed)

As Stephen's patch tried to clean this up, link_update dev_ops return value is
not very clear now and this return value not used at all in ethdev layer. And as
far as I can see you are also not using this locally in driver, so there is no
effect of updating this code, good or bad, I suggest postponing this update
until return value cleared more.

>  		return -1;  /* link not changed */
>  
>  	eth_dev->data->dev_link.link_status = pi->link_cfg.link_ok;
>
  

Patch

diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c b/drivers/net/cxgbe/cxgbe_ethdev.c
index 6d56f3c1b..5a25125fe 100644
--- a/drivers/net/cxgbe/cxgbe_ethdev.c
+++ b/drivers/net/cxgbe/cxgbe_ethdev.c
@@ -227,7 +227,8 @@  static int cxgbe_dev_link_update(struct rte_eth_dev *eth_dev,
 	unsigned int work_done, budget = 4;
 
 	cxgbe_poll(&s->fw_evtq, NULL, budget, &work_done);
-	if (old_link->link_status == pi->link_cfg.link_ok)
+	if (old_link->link_status == pi->link_cfg.link_ok &&
+	    old_link->link_speed == pi->link_cfg.speed)
 		return -1;  /* link not changed */
 
 	eth_dev->data->dev_link.link_status = pi->link_cfg.link_ok;