[dpdk-dev,v4,15/15] net/enic: use _rte_eth_linkstatus_set

Message ID 20180111170658.2809-15-ferruh.yigit@intel.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 fail apply patch file failure

Commit Message

Ferruh Yigit Jan. 11, 2018, 5:06 p.m. UTC
  From: Stephen Hemminger <stephen@networkplumber.org>

This driver was not doing atomic update of link status information.
And the return value was different than others.
The hardware also does not do autonegotiation (at least on Linux).

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/enic/enic_ethdev.c |  5 ++---
 drivers/net/enic/enic_main.c   | 16 +++++++---------
 2 files changed, 9 insertions(+), 12 deletions(-)
  

Comments

Hyong Youb Kim (hyonkim) Jan. 12, 2018, 2:05 p.m. UTC | #1
On Thu, Jan 11, 2018 at 05:06:58PM +0000, Ferruh Yigit wrote:
> From: Stephen Hemminger <stephen@networkplumber.org>
> 
> This driver was not doing atomic update of link status information.
> And the return value was different than others.
> The hardware also does not do autonegotiation (at least on Linux).
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---

Here is an ack, in case you need it. Please feel free to add it to v4 if you
make it (i.e. dropping the leading _).

Acked-by: Hyong Youb Kim <hyonkim@cisco.com>

FWIW, the NIC hardware (Cisco VIC) does autonegotiate. But, the link
settings (autoneg/fixed/10/25/40) are controlled by a management
entity (e.g. UCS manager). The drivers including the netdev enic
driver cannot change them and currently do not know if the current
speed has been autonegotiated or fixed by admin. So, they simply
report the current link speed as fixed. I know, it is a little
unconventional.

-Hyong
  

Patch

diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c
index ad7a4e306..08b990dc2 100644
--- a/drivers/net/enic/enic_ethdev.c
+++ b/drivers/net/enic/enic_ethdev.c
@@ -426,10 +426,9 @@  static void enicpmd_dev_stop(struct rte_eth_dev *eth_dev)
 
 	ENICPMD_FUNC_TRACE();
 	enic_disable(enic);
+
 	memset(&link, 0, sizeof(link));
-	rte_atomic64_cmpset((uint64_t *)&eth_dev->data->dev_link,
-		*(uint64_t *)&eth_dev->data->dev_link,
-		*(uint64_t *)&link);
+	_rte_eth_linkstatus_set(eth_dev, &link);
 }
 
 /*
diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index 9bbe054b3..e2a75de66 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -379,16 +379,14 @@  enic_free_consistent(void *priv,
 int enic_link_update(struct enic *enic)
 {
 	struct rte_eth_dev *eth_dev = enic->rte_dev;
-	int ret;
-	int link_status = 0;
+	struct rte_eth_link link = {
+		.link_status = enic_get_link_status(enic),
+		.link_duplex = ETH_LINK_FULL_DUPLEX,
+		.link_speed = vnic_dev_port_speed(enic->vdev),
+		.link_autoneg = ETH_LINK_FIXED,
+	};
 
-	link_status = enic_get_link_status(enic);
-	ret = (link_status == enic->link_status);
-	enic->link_status = link_status;
-	eth_dev->data->dev_link.link_status = link_status;
-	eth_dev->data->dev_link.link_duplex = ETH_LINK_FULL_DUPLEX;
-	eth_dev->data->dev_link.link_speed = vnic_dev_port_speed(enic->vdev);
-	return ret;
+	return _rte_eth_linkstatus_set(eth_dev, &link);
 }
 
 static void