diff mbox

[dpdk-dev] net/qede: fix advertising link speed capability

Message ID	1477636922-18546-1-git-send-email-rasesh.mody@qlogic.com (mailing list archive)
State	Superseded, archived
Delegated to:	Bruce Richardson
Headers	From: Rasesh Mody <rasesh.mody@qlogic.com> To: <thomas.monjalon@6wind.com> Date: Thu, 27 Oct 2016 23:42:02 -0700 Message-ID: <1477636922-18546-1-git-send-email-rasesh.mody@qlogic.com> In-Reply-To: <1772382.AymLqBrjhc@xps13> References: <1772382.AymLqBrjhc@xps13> MIME-Version: 1.0 Content-Type: text/plain disclaimer: bypass Cc: dev@dpdk.org, Dept-EngDPDKDev@qlogic.com Subject: [dpdk-dev] [PATCH] net/qede: fix advertising link speed capability Precedence: list Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org>

Commit Message

Rasesh Mody Oct. 28, 2016, 6:42 a.m. UTC

  From: Harish Patil <harish.patil@qlogic.com>

Fix to advertise device's link speed capability based on current
link speed rather than returning driver supported speeds.

Fixes: 95e67b479506 ("net/qede: add 100G link speed capability")

Signed-off-by: Harish Patil <harish.patil@qlogic.com>
---
 drivers/net/qede/qede_ethdev.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Thomas Monjalon Oct. 28, 2016, 7:26 a.m. UTC | #1

2016-10-27 23:42, Rasesh Mody:
> From: Harish Patil <harish.patil@qlogic.com>
> 
> Fix to advertise device's link speed capability based on current
> link speed rather than returning driver supported speeds.
[...]
> -	dev_info->speed_capa = ETH_LINK_SPEED_25G | ETH_LINK_SPEED_40G |
> -			       ETH_LINK_SPEED_100G;
> +	memset(&link, 0, sizeof(struct qed_link_output));
> +	qdev->ops->common->get_link(edev, &link);
> +	dev_info->speed_capa = rte_eth_speed_bitflag(link.speed, 0);
>  }

No, that's wrong.
You must advertise a capability!
So what the device supports?
Are every qede devices support ETH_LINK_SPEED_100G?

Harish Patil Oct. 29, 2016, 1:11 a.m. UTC | #2

>2016-10-27 23:42, Rasesh Mody:

>> From: Harish Patil <harish.patil@qlogic.com>

>> 

>> Fix to advertise device's link speed capability based on current

>> link speed rather than returning driver supported speeds.

>[...]

>> -	dev_info->speed_capa = ETH_LINK_SPEED_25G | ETH_LINK_SPEED_40G |

>> -			       ETH_LINK_SPEED_100G;

>> +	memset(&link, 0, sizeof(struct qed_link_output));

>> +	qdev->ops->common->get_link(edev, &link);

>> +	dev_info->speed_capa = rte_eth_speed_bitflag(link.speed, 0);

>>  }

>

>No, that's wrong.

>You must advertise a capability!

Got it. It was not very clear to me. Let me send out v2 patch that would
advertise native link speed based on the NIC personality configured (and
not based on current/negotiated link speed).

>So what the device supports?

10, 25, 40, 50, 100G speeds
>Are every qede devices support ETH_LINK_SPEED_100G?

No

diff mbox

Patch

diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
index b91b478..4c4c669 100644
--- a/drivers/net/qede/qede_ethdev.c
+++ b/drivers/net/qede/qede_ethdev.c
@@ -646,6 +646,7 @@  qede_dev_info_get(struct rte_eth_dev *eth_dev,
 {
 	struct qede_dev *qdev = eth_dev->data->dev_private;
 	struct ecore_dev *edev = &qdev->edev;
+	struct qed_link_output link;
 
 	PMD_INIT_FUNC_TRACE(edev);
 
@@ -678,8 +679,9 @@  qede_dev_info_get(struct rte_eth_dev *eth_dev,
 				     DEV_TX_OFFLOAD_UDP_CKSUM |
 				     DEV_TX_OFFLOAD_TCP_CKSUM);
 
-	dev_info->speed_capa = ETH_LINK_SPEED_25G | ETH_LINK_SPEED_40G |
-			       ETH_LINK_SPEED_100G;
+	memset(&link, 0, sizeof(struct qed_link_output));
+	qdev->ops->common->get_link(edev, &link);
+	dev_info->speed_capa = rte_eth_speed_bitflag(link.speed, 0);
 }
 
 /* return 0 means link status changed, -1 means not changed */