Re: [PATCH v11 2/8] ethdev: use constants for link duplex
From: Zhang, Helin <hidden>
Date: 2016-03-23 02:44:42
quoted hunk ↗ jump to hunk
-----Original Message----- From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] Sent: Friday, March 18, 2016 2:09 AM To: marcdevel@gmail.com; Richardson, Bruce; Doherty, Declan; Ananyev, Konstantin; Lu, Wenzhuo; Zhang, Helin; Chen, Jing D; harish.patil@qlogic.com; rahul.lakkireddy@chelsio.com; johndale@cisco.com; vido@cesnet.cz; adrien.mazarguil@6wind.com; alejandro.lucero@netronome.com Cc: dev@dpdk.org Subject: [PATCH v11 2/8] ethdev: use constants for link duplex From: Marc Sune <redacted> Some duplex values are replaced from 0 to half-duplex when link is down. Some drivers are still using their own constants for duplex modes. Signed-off-by: Marc Sune <redacted> --- drivers/net/e1000/em_ethdev.c | 2 +- drivers/net/e1000/igb_ethdev.c | 2 +- drivers/net/ixgbe/ixgbe_ethdev.c | 2 +- drivers/net/virtio/virtio_ethdev.c | 2 +- drivers/net/virtio/virtio_ethdev.h | 2 -- lib/librte_ether/rte_ethdev.h | 2 +- 6 files changed, 5 insertions(+), 7 deletions(-)diff --git a/drivers/net/e1000/em_ethdev.cb/drivers/net/e1000/em_ethdev.c index 58093c6..943a270 100644--- a/drivers/net/e1000/em_ethdev.c +++ b/drivers/net/e1000/em_ethdev.c@@ -1108,7 +1108,7 @@ eth_em_link_update(struct rte_eth_dev *dev, intwait_to_complete) link.link_status = ETH_LINK_UP; } else if (!link_check && (link.link_status == ETH_LINK_UP)) { link.link_speed = 0; - link.link_duplex = 0; + link.link_duplex = ETH_LINK_HALF_DUPLEX; link.link_status = ETH_LINK_DOWN; } rte_em_dev_atomic_write_link_status(dev, &link); diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c index 311f866..ea156ce 100644--- a/drivers/net/e1000/igb_ethdev.c +++ b/drivers/net/e1000/igb_ethdev.c@@ -2033,7 +2033,7 @@ eth_igb_link_update(struct rte_eth_dev *dev, intwait_to_complete) link.link_status = ETH_LINK_UP; } else if (!link_check) { link.link_speed = 0; - link.link_duplex = 0; + link.link_duplex = ETH_LINK_HALF_DUPLEX; link.link_status = ETH_LINK_DOWN; } rte_igb_dev_atomic_write_link_status(dev, &link); diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c index be28f7e..35dac49 100644--- a/drivers/net/ixgbe/ixgbe_ethdev.c +++ b/drivers/net/ixgbe/ixgbe_ethdev.c@@ -2997,7 +2997,7 @@ ixgbe_dev_link_update(struct rte_eth_dev *dev,int wait_to_complete) link.link_status = ETH_LINK_DOWN; link.link_speed = 0; - link.link_duplex = 0; + link.link_duplex = ETH_LINK_HALF_DUPLEX; memset(&old, 0, sizeof(old)); rte_ixgbe_dev_atomic_read_link_status(dev, &old);diff --git a/drivers/net/virtio/virtio_ethdev.cb/drivers/net/virtio/virtio_ethdev.c index 3ebc221..63a368a 100644--- a/drivers/net/virtio/virtio_ethdev.c +++ b/drivers/net/virtio/virtio_ethdev.c@@ -1401,7 +1401,7 @@ virtio_dev_link_update(struct rte_eth_dev *dev,__rte_unused int wait_to_complet memset(&link, 0, sizeof(link)); virtio_dev_atomic_read_link_status(dev, &link); old = link; - link.link_duplex = FULL_DUPLEX; + link.link_duplex = ETH_LINK_FULL_DUPLEX; link.link_speed = SPEED_10G; if (vtpci_with_feature(hw, VIRTIO_NET_F_STATUS)) { diff --git a/drivers/net/virtio/virtio_ethdev.h b/drivers/net/virtio/virtio_ethdev.h index fed9571..66423a0 100644--- a/drivers/net/virtio/virtio_ethdev.h +++ b/drivers/net/virtio/virtio_ethdev.h@@ -42,8 +42,6 @@ #define SPEED_100 100 #define SPEED_1000 1000 #define SPEED_10G 10000 -#define HALF_DUPLEX 1 -#define FULL_DUPLEX 2 #ifndef PAGE_SIZE #define PAGE_SIZE 4096diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h index ec8d6b1..5379bee 100644 --- a/lib/librte_ether/rte_ethdev.h +++ b/lib/librte_ether/rte_ethdev.h@@ -246,7 +246,7 @@ struct rte_eth_stats { */ struct rte_eth_link { uint16_t link_speed; /**< ETH_LINK_SPEED_[10, 100, 1000, 10000]*/ - uint16_t link_duplex; /**< ETH_LINK_[HALF_DUPLEX, FULL_DUPLEX] */ + uint16_t link_duplex; /**< ETH_LINK_[HALF/FULL]_DUPLEX */ uint8_t link_status : 1; /**< ETH_LINK_[DOWN/UP] */ }__attribute__((aligned(8))); /**< aligned for atomic64 read/write */
For link speed and link duplex, I'd suggest to add one more status of 'UNKNOWN'. Because, sometimes it cannot get all the information from hardware. For link stauts, assume it in DOWN state is acceptable, while for other two, I don't think so. Currently it can be seen that a default link speed and duplex will be set if it cannot get the accurate info from hardware. That's not good, and I think UNKNOWN could be better. What do you think? Thanks, Helin
-- 2.7.0