[dpdk-dev] [PATCH v7 00/15] common linkstatus functions

[dpdk-dev] [PATCH v7 00/15] common linkstatus functions

[Stephen Hemminger]

While reviewing drivers, noticed a lot of unnecessary
duplication of code in drivers for handling the eth_dev link status
information. While consolidating this, it also became obvious that
some drivers behave differently for no good reason.

It also was a good chance to introduce atomic exchange primitives
in EAL because there are other places using cmpset where not
necessary (such as bonding).

Mostly only compile tested only, don't have all of the hardware
available (except ixgbe and virtio) to test.


v7
  - add exchange functions for PPC64
  - move linkstatus helpers to rte_ethdev_driver
  - rebase to 18.02



Stephen Hemminger (15):
  eal: introduce atomic exchange operation
  ethdev: add linkstatus get/set helper functions
  net/virtio: use eth_linkstatus_set
  net/vmxnet3: use rte_eth_linkstatus_set
  net/dpaa2: use rte_eth_linkstatus_set
  net/nfp: use rte_eth_linkstatus functions
  net/e1000: use rte_eth_linkstatus helpers
  net/ixgbe: use rte_eth_linkstatus functions
  net/sfc: use new rte_eth_linkstatus functions
  net/i40e: use rte_eth_linkstatus functions
  net/liquidio: use rte_eth_linkstatus_set
  net/thunderx: use rte_eth_linkstatus_set
  net/szedata: use _rte_eth_linkstatus_set
  net/octeontx: use rte_eth_linkstatus_set
  net/enic: use rte_eth_linkstatus_set

 drivers/net/dpaa2/dpaa2_ethdev.c                   | 75 ++---------------
 drivers/net/e1000/em_ethdev.c                      | 69 ++--------------
 drivers/net/e1000/igb_ethdev.c                     | 70 ++--------------
 drivers/net/enic/enic_ethdev.c                     |  5 +-
 drivers/net/enic/enic_main.c                       | 17 ++--
 drivers/net/i40e/i40e_ethdev.c                     | 43 ++--------
 drivers/net/i40e/i40e_ethdev_vf.c                  | 18 +---
 drivers/net/ixgbe/ixgbe_ethdev.c                   | 96 ++++------------------
 drivers/net/liquidio/lio_ethdev.c                  | 53 ++----------
 drivers/net/nfp/nfp_net.c                          | 77 ++---------------
 drivers/net/octeontx/octeontx_ethdev.c             | 17 +---
 drivers/net/sfc/sfc_ethdev.c                       | 21 +----
 drivers/net/sfc/sfc_ev.c                           | 20 +----
 drivers/net/szedata2/rte_eth_szedata2.c            | 19 ++---
 drivers/net/thunderx/nicvf_ethdev.c                | 48 +++++------
 drivers/net/virtio/virtio_ethdev.c                 | 65 +++------------
 drivers/net/vmxnet3/vmxnet3_ethdev.c               | 86 ++++---------------
 .../common/include/arch/ppc_64/rte_atomic.h        | 21 ++++-
 .../common/include/arch/x86/rte_atomic.h           | 24 ++++++
 .../common/include/arch/x86/rte_atomic_32.h        | 12 +++
 .../common/include/arch/x86/rte_atomic_64.h        | 12 +++
 lib/librte_eal/common/include/generic/rte_atomic.h | 78 ++++++++++++++++++
 lib/librte_ether/rte_ethdev.c                      | 22 +----
 lib/librte_ether/rte_ethdev_driver.h               | 61 ++++++++++++++
 24 files changed, 338 insertions(+), 691 deletions(-)

-- 
2.15.1

[dpdk-dev] [PATCH v7 01/15] eal: introduce atomic exchange operation

[Stephen Hemminger]

To handle atomic update of link status (64 bit), every driver
was doing its own version using cmpset.
Atomic exchange is a useful primitive in its own right;
therefore make it a EAL routine.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 .../common/include/arch/ppc_64/rte_atomic.h        | 21 +++++-
 .../common/include/arch/x86/rte_atomic.h           | 24 +++++++
 .../common/include/arch/x86/rte_atomic_32.h        | 12 ++++
 .../common/include/arch/x86/rte_atomic_64.h        | 12 ++++
 lib/librte_eal/common/include/generic/rte_atomic.h | 78 ++++++++++++++++++++++
 5 files changed, 146 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eal/common/include/arch/ppc_64/rte_atomic.h b/lib/librte_eal/common/include/arch/ppc_64/rte_atomic.h
index 150810cdb365..11fa1117a255 100644
--- a/lib/librte_eal/common/include/arch/ppc_64/rte_atomic.h
+++ b/lib/librte_eal/common/include/arch/ppc_64/rte_atomic.h
@@ -132,6 +132,12 @@ static inline int rte_atomic16_dec_and_test(rte_atomic16_t *v)
 	return __atomic_sub_fetch(&v->cnt, 1, __ATOMIC_ACQUIRE) == 0;
 }
 
+static inline uint16_t
+rte_atomic16_exchange(volatile uint16_t *dst, uint16_t val)
+{
+	return __atomic_exchange_2(dst, val, __ATOMIC_SEQ_CST);
+}
+
 /*------------------------- 32 bit atomic operations -------------------------*/
 
 static inline int
@@ -233,6 +239,13 @@ static inline int rte_atomic32_dec_and_test(rte_atomic32_t *v)
 
 	return ret == 0;
 }
+
+static inline uint32_t
+rte_atomic32_exchange(volatile uint32_t *dst, uint32_t val)
+{
+	return __atomic_exchange_4(dst, val, __ATOMIC_SEQ_CST);
+}
+
 /*------------------------- 64 bit atomic operations -------------------------*/
 
 static inline int
@@ -438,6 +450,13 @@ static inline void rte_atomic64_clear(rte_atomic64_t *v)
 {
 	v->cnt = 0;
 }
+
+static inline uint64_t
+rte_atomic64_exchange(volatile uint64_t *dst, uint64_t val)
+{
+	return __atomic_exchange_4(dst, val, __ATOMIC_SEQ_CST);
+}
+
 #endif
 
 #ifdef __cplusplus
diff --git a/lib/librte_eal/common/include/arch/x86/rte_atomic.h b/lib/librte_eal/common/include/arch/x86/rte_atomic.h
index 36cfabc38f09..55bfc3903cdc 100644
--- a/lib/librte_eal/common/include/arch/x86/rte_atomic.h
+++ b/lib/librte_eal/common/include/arch/x86/rte_atomic.h
@@ -60,6 +60,18 @@ rte_atomic16_cmpset(volatile uint16_t *dst, uint16_t exp, uint16_t src)
 	return res;
 }
 
+static inline uint16_t
+rte_atomic16_exchange(volatile uint16_t *dst, uint16_t val)
+{
+	asm volatile(
+			MPLOCKED
+			"xchgw %0, %1;"
+			: "=r" (val), "=m" (*dst)
+			: "0" (val),  "m" (*dst)
+			: "memory");         /* no-clobber list */
+	return val;
+}
+
 static inline int rte_atomic16_test_and_set(rte_atomic16_t *v)
 {
 	return rte_atomic16_cmpset((volatile uint16_t *)&v->cnt, 0, 1);
@@ -134,6 +146,18 @@ rte_atomic32_cmpset(volatile uint32_t *dst, uint32_t exp, uint32_t src)
 	return res;
 }
 
+static inline uint32_t
+rte_atomic32_exchange(volatile uint32_t *dst, uint32_t val)
+{
+	asm volatile(
+			MPLOCKED
+			"xchgl %0, %1;"
+			: "=r" (val), "=m" (*dst)
+			: "0" (val),  "m" (*dst)
+			: "memory");         /* no-clobber list */
+	return val;
+}
+
 static inline int rte_atomic32_test_and_set(rte_atomic32_t *v)
 {
 	return rte_atomic32_cmpset((volatile uint32_t *)&v->cnt, 0, 1);
diff --git a/lib/librte_eal/common/include/arch/x86/rte_atomic_32.h b/lib/librte_eal/common/include/arch/x86/rte_atomic_32.h
index fb3abf187998..43fa59355ac5 100644
--- a/lib/librte_eal/common/include/arch/x86/rte_atomic_32.h
+++ b/lib/librte_eal/common/include/arch/x86/rte_atomic_32.h
@@ -98,6 +98,18 @@ rte_atomic64_cmpset(volatile uint64_t *dst, uint64_t exp, uint64_t src)
 	return res;
 }
 
+static inline uint64_t
+rte_atomic64_exchange(volatile uint64_t *dest, uint64_t val)
+{
+	uint64_t old;
+
+	do {
+		old = *dest;
+	} while (rte_atomic64_t_cmpset(dest, old, val));
+
+	return old;
+}
+
 static inline void
 rte_atomic64_init(rte_atomic64_t *v)
 {
diff --git a/lib/librte_eal/common/include/arch/x86/rte_atomic_64.h b/lib/librte_eal/common/include/arch/x86/rte_atomic_64.h
index 1a53a766bd72..fd2ec9c53796 100644
--- a/lib/librte_eal/common/include/arch/x86/rte_atomic_64.h
+++ b/lib/librte_eal/common/include/arch/x86/rte_atomic_64.h
@@ -71,6 +71,18 @@ rte_atomic64_cmpset(volatile uint64_t *dst, uint64_t exp, uint64_t src)
 	return res;
 }
 
+static inline uint64_t
+rte_atomic64_exchange(volatile uint64_t *dst, uint64_t val)
+{
+	asm volatile(
+			MPLOCKED
+			"xchgq %0, %1;"
+			: "=r" (val), "=m" (*dst)
+			: "0" (val),  "m" (*dst)
+			: "memory");         /* no-clobber list */
+	return val;
+}
+
 static inline void
 rte_atomic64_init(rte_atomic64_t *v)
 {
diff --git a/lib/librte_eal/common/include/generic/rte_atomic.h b/lib/librte_eal/common/include/generic/rte_atomic.h
index 3ba7245a3e17..bf12162d02f4 100644
--- a/lib/librte_eal/common/include/generic/rte_atomic.h
+++ b/lib/librte_eal/common/include/generic/rte_atomic.h
@@ -139,6 +139,32 @@ rte_atomic16_cmpset(volatile uint16_t *dst, uint16_t exp, uint16_t src)
 }
 #endif
 
+/**
+ * Atomic exchange.
+ *
+ * (atomic) equivalent to:
+ *   ret = *dst
+ *   *dst = val;
+ *   return ret;
+ *
+ * @param dst
+ *   The destination location into which the value will be written.
+ * @param val
+ *   The new value.
+ * @return
+ *   The original value at that location
+ */
+static inline uint16_t
+rte_atomic16_exchange(volatile uint16_t *dst, uint16_t val);
+
+#ifdef RTE_FORCE_INTRINSICS
+static inline uint16_t
+rte_atomic16_exchange(volatile uint16_t *dst, uint16_t val)
+{
+	return __atomic_exchange_2(dst, val, __ATOMIC_SEQ_CST);
+}
+#endif
+
 /**
  * The atomic counter structure.
  */
@@ -392,6 +418,32 @@ rte_atomic32_cmpset(volatile uint32_t *dst, uint32_t exp, uint32_t src)
 }
 #endif
 
+/**
+ * Atomic exchange.
+ *
+ * (atomic) equivalent to:
+ *   ret = *dst
+ *   *dst = val;
+ *   return ret;
+ *
+ * @param dst
+ *   The destination location into which the value will be written.
+ * @param val
+ *   The new value.
+ * @return
+ *   The original value at that location
+ */
+static inline uint32_t
+rte_atomic32_exchange(volatile uint32_t *dst, uint32_t val);
+
+#ifdef RTE_FORCE_INTRINSICS
+static inline uint32_t
+rte_atomic32_exchange(volatile uint32_t *dst, uint32_t val)
+{
+	return __atomic_exchange_4(dst, val, __ATOMIC_SEQ_CST);
+}
+#endif
+
 /**
  * The atomic counter structure.
  */
@@ -644,6 +696,32 @@ rte_atomic64_cmpset(volatile uint64_t *dst, uint64_t exp, uint64_t src)
 }
 #endif
 
+/**
+ * Atomic exchange.
+ *
+ * (atomic) equivalent to:
+ *   ret = *dst
+ *   *dst = val;
+ *   return ret;
+ *
+ * @param dst
+ *   The destination location into which the value will be written.
+ * @param val
+ *   The new value.
+ * @return
+ *   The original value at that location
+ */
+static inline uint64_t
+rte_atomic64_exchange(volatile uint64_t *dst, uint64_t val);
+
+#ifdef RTE_FORCE_INTRINSICS
+static inline uint64_t
+rte_atomic64_exchange(volatile uint64_t *dst, uint64_t val)
+{
+	return __atomic_exchange_8(dst, val, __ATOMIC_SEQ_CST);
+}
+#endif
+
 /**
  * The atomic counter structure.
  */
-- 
2.15.1

[dpdk-dev] [PATCH v7 02/15] ethdev: add linkstatus get/set helper functions

[Stephen Hemminger]

Many drivers are all doing copy/paste of the same code to atomically
update the link status. Reduce duplication, and allow for future
changes by having common function for this.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Shreyansh Jain <shreyansh.jain@nxp.com>

---
 lib/librte_ether/rte_ethdev.c        | 22 +++----------
 lib/librte_ether/rte_ethdev_driver.h | 61 ++++++++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+), 18 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index f285ba278967..344397101abd 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -1578,20 +1578,6 @@ rte_eth_allmulticast_get(uint16_t port_id)
 	return dev->data->all_multicast;
 }
 
-static inline int
-rte_eth_dev_atomic_read_link_status(struct rte_eth_dev *dev,
-				struct rte_eth_link *link)
-{
-	struct rte_eth_link *dst = link;
-	struct rte_eth_link *src = &(dev->data->dev_link);
-
-	if (rte_atomic64_cmpset((uint64_t *)dst, *(uint64_t *)dst,
-					*(uint64_t *)src) == 0)
-		return -1;
-
-	return 0;
-}
-
 void
 rte_eth_link_get(uint16_t port_id, struct rte_eth_link *eth_link)
 {
@@ -1600,8 +1586,8 @@ rte_eth_link_get(uint16_t port_id, struct rte_eth_link *eth_link)
 	RTE_ETH_VALID_PORTID_OR_RET(port_id);
 	dev = &rte_eth_devices[port_id];
 
-	if (dev->data->dev_conf.intr_conf.lsc != 0)
-		rte_eth_dev_atomic_read_link_status(dev, eth_link);
+	if (dev->data->dev_conf.intr_conf.lsc)
+		rte_eth_linkstatus_get(dev, eth_link);
 	else {
 		RTE_FUNC_PTR_OR_RET(*dev->dev_ops->link_update);
 		(*dev->dev_ops->link_update)(dev, 1);
@@ -1617,8 +1603,8 @@ rte_eth_link_get_nowait(uint16_t port_id, struct rte_eth_link *eth_link)
 	RTE_ETH_VALID_PORTID_OR_RET(port_id);
 	dev = &rte_eth_devices[port_id];
 
-	if (dev->data->dev_conf.intr_conf.lsc != 0)
-		rte_eth_dev_atomic_read_link_status(dev, eth_link);
+	if (dev->data->dev_conf.intr_conf.lsc)
+		rte_eth_linkstatus_get(dev, eth_link);
 	else {
 		RTE_FUNC_PTR_OR_RET(*dev->dev_ops->link_update);
 		(*dev->dev_ops->link_update)(dev, 0);
diff --git a/lib/librte_ether/rte_ethdev_driver.h b/lib/librte_ether/rte_ethdev_driver.h
index 45f08c65ec1e..66da1fb68bbf 100644
--- a/lib/librte_ether/rte_ethdev_driver.h
+++ b/lib/librte_ether/rte_ethdev_driver.h
@@ -68,6 +68,67 @@ struct rte_eth_dev *rte_eth_dev_attach_secondary(const char *name);
  */
 int rte_eth_dev_release_port(struct rte_eth_dev *eth_dev);
 
+/**
+ * @internal
+ * Atomically set the link status for the specific device.
+ * It is for use by DPDK device driver use only.
+ * User applications should not call it
+ *
+ * @param dev
+ *  Pointer to struct rte_eth_dev.
+ * @param link
+ *  New link status value.
+ * @return
+ *  1 if link status has changed;
+ *  0 if link status is unchanged.
+ */
+static inline int
+rte_eth_linkstatus_set(struct rte_eth_dev *dev,
+		       const struct rte_eth_link *new_link)
+{
+	volatile uint64_t *dev_link
+		 = (volatile uint64_t *)&(dev->data->dev_link);
+	union {
+		uint64_t val64;
+		struct rte_eth_link link;
+	} orig;
+
+	RTE_BUILD_BUG_ON(sizeof(*new_link) != sizeof(uint64_t));
+
+	orig.val64 = rte_atomic64_exchange(dev_link,
+					   *(const uint64_t *)new_link);
+
+	return orig.link.link_status != new_link->link_status;
+}
+
+/**
+ * @internal
+ * Atomically get the link speed and status.
+ *
+ * @param dev
+ *  Pointer to struct rte_eth_dev.
+ * @param link
+ *  link status value.
+ */
+static inline void
+rte_eth_linkstatus_get(const struct rte_eth_dev *dev,
+		       struct rte_eth_link *link)
+{
+	volatile uint64_t *src = (uint64_t *)&(dev->data->dev_link);
+	uint64_t *dst = (uint64_t *)link;
+
+	RTE_BUILD_BUG_ON(sizeof(*link) != sizeof(uint64_t));
+
+	/* can't use rte_atomic64_read because it returns signed int */
+#ifdef __LP64__
+	*dst = *src;
+#else
+	do {
+		*dst = *src;
+	} while (!rte_atomic64_cmpset(src, *dst, *dst));
+#endif
+}
+
 /**
  * @internal
  * Release device queues and clear its configuration to force the user
-- 
2.15.1

[dpdk-dev] [PACH v7 02a] typo in i686 version of rte_atomic64_exchange

[Stephen Hemminger]

Fix typo in 32 bit version of rte_atomic64_exchange.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

---
 lib/librte_eal/common/include/arch/x86/rte_atomic_32.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_eal/common/include/arch/x86/rte_atomic_32.h b/lib/librte_eal/common/include/arch/x86/rte_atomic_32.h
index 43fa59355ac5..8d711b6f685d 100644
--- a/lib/librte_eal/common/include/arch/x86/rte_atomic_32.h
+++ b/lib/librte_eal/common/include/arch/x86/rte_atomic_32.h
@@ -105,7 +105,7 @@ rte_atomic64_exchange(volatile uint64_t *dest, uint64_t val)
 
 	do {
 		old = *dest;
-	} while (rte_atomic64_t_cmpset(dest, old, val));
+	} while (rte_atomic64_cmpset(dest, old, val));
 
 	return old;
 }
-- 
2.15.1

[dpdk-dev] [PATCH v7 03/15] net/virtio: use eth_linkstatus_set

[Stephen Hemminger]

Use the new comon code in ethdev to handle link status update.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>

---
 drivers/net/virtio/virtio_ethdev.c | 65 +++++++-------------------------------
 1 file changed, 12 insertions(+), 53 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 65cb71fa65cc..ff87ee7e9e51 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -14,7 +14,6 @@
 #include <rte_string_fns.h>
 #include <rte_memzone.h>
 #include <rte_malloc.h>
-#include <rte_atomic.h>
 #include <rte_branch_prediction.h>
 #include <rte_pci.h>
 #include <rte_bus_pci.h>
@@ -785,46 +784,6 @@ static const struct eth_dev_ops virtio_eth_dev_ops = {
 	.mac_addr_set            = virtio_mac_addr_set,
 };
 
-static inline int
-virtio_dev_atomic_read_link_status(struct rte_eth_dev *dev,
-				struct rte_eth_link *link)
-{
-	struct rte_eth_link *dst = link;
-	struct rte_eth_link *src = &(dev->data->dev_link);
-
-	if (rte_atomic64_cmpset((uint64_t *)dst, *(uint64_t *)dst,
-			*(uint64_t *)src) == 0)
-		return -1;
-
-	return 0;
-}
-
-/**
- * Atomically writes the link status information into global
- * structure rte_eth_dev.
- *
- * @param dev
- *   - Pointer to the structure rte_eth_dev to read from.
- *   - Pointer to the buffer to be saved with the link status.
- *
- * @return
- *   - On success, zero.
- *   - On failure, negative value.
- */
-static inline int
-virtio_dev_atomic_write_link_status(struct rte_eth_dev *dev,
-		struct rte_eth_link *link)
-{
-	struct rte_eth_link *dst = &(dev->data->dev_link);
-	struct rte_eth_link *src = link;
-
-	if (rte_atomic64_cmpset((uint64_t *)dst, *(uint64_t *)dst,
-					*(uint64_t *)src) == 0)
-		return -1;
-
-	return 0;
-}
-
 static void
 virtio_update_stats(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 {
@@ -2023,8 +1982,10 @@ static void
 virtio_dev_stop(struct rte_eth_dev *dev)
 {
 	struct virtio_hw *hw = dev->data->dev_private;
-	struct rte_eth_link link;
 	struct rte_intr_conf *intr_conf = &dev->data->dev_conf.intr_conf;
+	struct rte_eth_link link = {
+		.link_status = ETH_LINK_DOWN,
+	};
 
 	PMD_INIT_LOG(DEBUG, "stop");
 
@@ -2033,22 +1994,20 @@ virtio_dev_stop(struct rte_eth_dev *dev)
 		virtio_intr_disable(dev);
 
 	hw->started = 0;
-	memset(&link, 0, sizeof(link));
-	virtio_dev_atomic_write_link_status(dev, &link);
+	rte_eth_linkstatus_set(dev, &link);
 	rte_spinlock_unlock(&hw->state_lock);
 }
 
 static int
 virtio_dev_link_update(struct rte_eth_dev *dev, __rte_unused int wait_to_complete)
 {
-	struct rte_eth_link link, old;
-	uint16_t status;
 	struct virtio_hw *hw = dev->data->dev_private;
-	memset(&link, 0, sizeof(link));
-	virtio_dev_atomic_read_link_status(dev, &link);
-	old = link;
-	link.link_duplex = ETH_LINK_FULL_DUPLEX;
-	link.link_speed  = ETH_SPEED_NUM_10G;
+	uint16_t status;
+	struct rte_eth_link link = {
+		.link_speed = ETH_SPEED_NUM_10G,
+		.link_duplex = ETH_LINK_FULL_DUPLEX,
+		.link_autoneg = ETH_LINK_SPEED_FIXED,
+	};
 
 	if (hw->started == 0) {
 		link.link_status = ETH_LINK_DOWN;
@@ -2069,9 +2028,9 @@ virtio_dev_link_update(struct rte_eth_dev *dev, __rte_unused int wait_to_complet
 	} else {
 		link.link_status = ETH_LINK_UP;
 	}
-	virtio_dev_atomic_write_link_status(dev, &link);
+	rte_eth_linkstatus_set(dev, &link);
 
-	return (old.link_status == link.link_status) ? -1 : 0;
+	return 0;
 }
 
 static int
-- 
2.15.1

[dpdk-dev] [PATCH v7 04/15] net/vmxnet3: use rte_eth_linkstatus_set

[Stephen Hemminger]

Use new rte_eth_link_update helper.
Also remove no longer necessary include of rte_atomic.h

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 drivers/net/vmxnet3/vmxnet3_ethdev.c | 86 +++++++-----------------------------
 1 file changed, 16 insertions(+), 70 deletions(-)

diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c
index a2fb93cc984c..3a4601a491b1 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
@@ -20,7 +20,6 @@
 #include <rte_debug.h>
 #include <rte_pci.h>
 #include <rte_bus_pci.h>
-#include <rte_atomic.h>
 #include <rte_branch_prediction.h>
 #include <rte_memory.h>
 #include <rte_memzone.h>
@@ -160,59 +159,6 @@ gpa_zone_reserve(struct rte_eth_dev *dev, uint32_t size,
 	return rte_memzone_reserve_aligned(z_name, size, socket_id, 0, align);
 }
 
-/**
- * Atomically reads the link status information from global
- * structure rte_eth_dev.
- *
- * @param dev
- *   - Pointer to the structure rte_eth_dev to read from.
- *   - Pointer to the buffer to be saved with the link status.
- *
- * @return
- *   - On success, zero.
- *   - On failure, negative value.
- */
-
-static int
-vmxnet3_dev_atomic_read_link_status(struct rte_eth_dev *dev,
-				    struct rte_eth_link *link)
-{
-	struct rte_eth_link *dst = link;
-	struct rte_eth_link *src = &(dev->data->dev_link);
-
-	if (rte_atomic64_cmpset((uint64_t *)dst, *(uint64_t *)dst,
-				*(uint64_t *)src) == 0)
-		return -1;
-
-	return 0;
-}
-
-/**
- * Atomically writes the link status information into global
- * structure rte_eth_dev.
- *
- * @param dev
- *   - Pointer to the structure rte_eth_dev to write to.
- *   - Pointer to the buffer to be saved with the link status.
- *
- * @return
- *   - On success, zero.
- *   - On failure, negative value.
- */
-static int
-vmxnet3_dev_atomic_write_link_status(struct rte_eth_dev *dev,
-				     struct rte_eth_link *link)
-{
-	struct rte_eth_link *dst = &(dev->data->dev_link);
-	struct rte_eth_link *src = link;
-
-	if (rte_atomic64_cmpset((uint64_t *)dst, *(uint64_t *)dst,
-				*(uint64_t *)src) == 0)
-		return -1;
-
-	return 0;
-}
-
 /*
  * This function is based on vmxnet3_disable_intr()
  */
@@ -817,9 +763,13 @@ vmxnet3_dev_start(struct rte_eth_dev *dev)
 static void
 vmxnet3_dev_stop(struct rte_eth_dev *dev)
 {
-	struct rte_eth_link link;
 	struct vmxnet3_hw *hw = dev->data->dev_private;
-
+	struct rte_eth_link link = {
+		.link_speed = ETH_SPEED_NUM_10G,
+		.link_duplex = ETH_LINK_FULL_DUPLEX,
+		.link_autoneg = ETH_LINK_SPEED_FIXED,
+		.link_status = ETH_LINK_DOWN,
+	};
 	PMD_INIT_FUNC_TRACE();
 
 	if (hw->adapter_stopped == 1) {
@@ -852,8 +802,7 @@ vmxnet3_dev_stop(struct rte_eth_dev *dev)
 	vmxnet3_dev_clear_queues(dev);
 
 	/* Clear recorded link status */
-	memset(&link, 0, sizeof(link));
-	vmxnet3_dev_atomic_write_link_status(dev, &link);
+	rte_eth_linkstatus_set(dev, &link);
 }
 
 /*
@@ -1132,25 +1081,22 @@ __vmxnet3_dev_link_update(struct rte_eth_dev *dev,
 			  __rte_unused int wait_to_complete)
 {
 	struct vmxnet3_hw *hw = dev->data->dev_private;
-	struct rte_eth_link old = { 0 }, link;
+	struct rte_eth_link link = {
+		.link_speed = ETH_SPEED_NUM_10G,
+		.link_duplex = ETH_LINK_FULL_DUPLEX,
+		.link_autoneg = ETH_LINK_SPEED_FIXED,
+		.link_status = ETH_LINK_DOWN,
+	};
 	uint32_t ret;
 
-	memset(&link, 0, sizeof(link));
-	vmxnet3_dev_atomic_read_link_status(dev, &old);
-
 	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD, VMXNET3_CMD_GET_LINK);
 	ret = VMXNET3_READ_BAR1_REG(hw, VMXNET3_REG_CMD);
 
-	if (ret & 0x1) {
+	if (ret & 0x1)
 		link.link_status = ETH_LINK_UP;
-		link.link_duplex = ETH_LINK_FULL_DUPLEX;
-		link.link_speed = ETH_SPEED_NUM_10G;
-		link.link_autoneg = ETH_LINK_AUTONEG;
-	}
-
-	vmxnet3_dev_atomic_write_link_status(dev, &link);
 
-	return (old.link_status == link.link_status) ? -1 : 0;
+	rte_eth_linkstatus_set(dev, &link);
+	return 0;
 }
 
 static int
-- 
2.15.1

[dpdk-dev] [PATCH v7 05/15] net/dpaa2: use rte_eth_linkstatus_set

[Stephen Hemminger]

Use new helper function to update the link status.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Shreyansh Jain <shreyansh.jain@nxp.com>
---
 drivers/net/dpaa2/dpaa2_ethdev.c | 75 ++++------------------------------------
 1 file changed, 7 insertions(+), 68 deletions(-)

diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c b/drivers/net/dpaa2/dpaa2_ethdev.c
index 09a11d65af93..66d1b1eaad4a 100644
--- a/drivers/net/dpaa2/dpaa2_ethdev.c
+++ b/drivers/net/dpaa2/dpaa2_ethdev.c
@@ -57,58 +57,6 @@ static int dpaa2_dev_set_link_up(struct rte_eth_dev *dev);
 static int dpaa2_dev_set_link_down(struct rte_eth_dev *dev);
 static int dpaa2_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu);
 
-/**
- * Atomically reads the link status information from global
- * structure rte_eth_dev.
- *
- * @param dev
- *   - Pointer to the structure rte_eth_dev to read from.
- *   - Pointer to the buffer to be saved with the link status.
- *
- * @return
- *   - On success, zero.
- *   - On failure, negative value.
- */
-static inline int
-dpaa2_dev_atomic_read_link_status(struct rte_eth_dev *dev,
-				  struct rte_eth_link *link)
-{
-	struct rte_eth_link *dst = link;
-	struct rte_eth_link *src = &dev->data->dev_link;
-
-	if (rte_atomic64_cmpset((uint64_t *)dst, *(uint64_t *)dst,
-				*(uint64_t *)src) == 0)
-		return -1;
-
-	return 0;
-}
-
-/**
- * Atomically writes the link status information into global
- * structure rte_eth_dev.
- *
- * @param dev
- *   - Pointer to the structure rte_eth_dev to read from.
- *   - Pointer to the buffer to be saved with the link status.
- *
- * @return
- *   - On success, zero.
- *   - On failure, negative value.
- */
-static inline int
-dpaa2_dev_atomic_write_link_status(struct rte_eth_dev *dev,
-				   struct rte_eth_link *link)
-{
-	struct rte_eth_link *dst = &dev->data->dev_link;
-	struct rte_eth_link *src = link;
-
-	if (rte_atomic64_cmpset((uint64_t *)dst, *(uint64_t *)dst,
-				*(uint64_t *)src) == 0)
-		return -1;
-
-	return 0;
-}
-
 static int
 dpaa2_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on)
 {
@@ -852,7 +800,7 @@ dpaa2_dev_stop(struct rte_eth_dev *dev)
 
 	/* clear the recorded link status */
 	memset(&link, 0, sizeof(link));
-	dpaa2_dev_atomic_write_link_status(dev, &link);
+	_rte_eth_link_write(dev, &link);
 }
 
 static void
@@ -884,7 +832,7 @@ dpaa2_dev_close(struct rte_eth_dev *dev)
 	}
 
 	memset(&link, 0, sizeof(link));
-	dpaa2_dev_atomic_write_link_status(dev, &link);
+	_rte_eth_link_write(dev, &link);
 }
 
 static void
@@ -1335,15 +1283,13 @@ dpaa2_dev_link_update(struct rte_eth_dev *dev,
 	int ret;
 	struct dpaa2_dev_priv *priv = dev->data->dev_private;
 	struct fsl_mc_io *dpni = (struct fsl_mc_io *)priv->hw;
-	struct rte_eth_link link, old;
+	struct rte_eth_link link;
 	struct dpni_link_state state = {0};
 
 	if (dpni == NULL) {
 		RTE_LOG(ERR, PMD, "dpni is NULL\n");
 		return 0;
 	}
-	memset(&old, 0, sizeof(old));
-	dpaa2_dev_atomic_read_link_status(dev, &old);
 
 	ret = dpni_get_link_state(dpni, CMD_PRI_LOW, priv->token, &state);
 	if (ret < 0) {
@@ -1351,11 +1297,6 @@ dpaa2_dev_link_update(struct rte_eth_dev *dev,
 		return -1;
 	}
 
-	if ((old.link_status == state.up) && (old.link_speed == state.rate)) {
-		RTE_LOG(DEBUG, PMD, "No change in status\n");
-		return -1;
-	}
-
 	memset(&link, 0, sizeof(struct rte_eth_link));
 	link.link_status = state.up;
 	link.link_speed = state.rate;
@@ -1365,12 +1306,10 @@ dpaa2_dev_link_update(struct rte_eth_dev *dev,
 	else
 		link.link_duplex = ETH_LINK_FULL_DUPLEX;
 
-	dpaa2_dev_atomic_write_link_status(dev, &link);
-
-	if (link.link_status)
-		PMD_DRV_LOG(INFO, "Port %d Link is Up\n", dev->data->port_id);
-	else
-		PMD_DRV_LOG(INFO, "Port %d Link is Down", dev->data->port_id);
+	if (rte_eth_linkstatus_set(dev, &link)) {
+		PMD_DRV_LOG(INFO, "Port %d Link is %s\n", dev->data->port_id,
+			    link.link_status ? "Up" : "Down");
+	}
 	return 0;
 }
 
-- 
2.15.1

[dpdk-dev] [PATCH v7 06/15] net/nfp: use rte_eth_linkstatus functions

[Stephen Hemminger]

Use new rte_eth_linkstatus_get/set helper function.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>

---
 drivers/net/nfp/nfp_net.c | 77 +++++------------------------------------------
 1 file changed, 7 insertions(+), 70 deletions(-)

diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index 0044699237ac..7c9832374027 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -213,57 +213,6 @@ nn_cfg_writeq(struct nfp_net_hw *hw, int off, uint64_t val)
 	nn_writeq(rte_cpu_to_le_64(val), hw->ctrl_bar + off);
 }
 
-/*
- * Atomically reads link status information from global structure rte_eth_dev.
- *
- * @param dev
- *   - Pointer to the structure rte_eth_dev to read from.
- *   - Pointer to the buffer to be saved with the link status.
- *
- * @return
- *   - On success, zero.
- *   - On failure, negative value.
- */
-static inline int
-nfp_net_dev_atomic_read_link_status(struct rte_eth_dev *dev,
-				    struct rte_eth_link *link)
-{
-	struct rte_eth_link *dst = link;
-	struct rte_eth_link *src = &dev->data->dev_link;
-
-	if (rte_atomic64_cmpset((uint64_t *)dst, *(uint64_t *)dst,
-				*(uint64_t *)src) == 0)
-		return -1;
-
-	return 0;
-}
-
-/*
- * Atomically writes the link status information into global
- * structure rte_eth_dev.
- *
- * @param dev
- *   - Pointer to the structure rte_eth_dev to read from.
- *   - Pointer to the buffer to be saved with the link status.
- *
- * @return
- *   - On success, zero.
- *   - On failure, negative value.
- */
-static inline int
-nfp_net_dev_atomic_write_link_status(struct rte_eth_dev *dev,
-				     struct rte_eth_link *link)
-{
-	struct rte_eth_link *dst = &dev->data->dev_link;
-	struct rte_eth_link *src = link;
-
-	if (rte_atomic64_cmpset((uint64_t *)dst, *(uint64_t *)dst,
-				*(uint64_t *)src) == 0)
-		return -1;
-
-	return 0;
-}
-
 static void
 nfp_net_rx_queue_release_mbufs(struct nfp_net_rxq *rxq)
 {
@@ -999,7 +948,7 @@ static int
 nfp_net_link_update(struct rte_eth_dev *dev, __rte_unused int wait_to_complete)
 {
 	struct nfp_net_hw *hw;
-	struct rte_eth_link link, old;
+	struct rte_eth_link link;
 	uint32_t nn_link_status;
 
 	static const uint32_t ls_to_ethtool[] = {
@@ -1017,9 +966,6 @@ nfp_net_link_update(struct rte_eth_dev *dev, __rte_unused int wait_to_complete)
 
 	hw = NFP_NET_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
-	memset(&old, 0, sizeof(old));
-	nfp_net_dev_atomic_read_link_status(dev, &old);
-
 	nn_link_status = nn_cfg_readl(hw, NFP_NET_CFG_STS);
 
 	memset(&link, 0, sizeof(struct rte_eth_link));
@@ -1037,16 +983,10 @@ nfp_net_link_update(struct rte_eth_dev *dev, __rte_unused int wait_to_complete)
 	else
 		link.link_speed = ls_to_ethtool[nn_link_status];
 
-	if (old.link_status != link.link_status) {
-		nfp_net_dev_atomic_write_link_status(dev, &link);
-		if (link.link_status)
-			PMD_DRV_LOG(INFO, "NIC Link is Up\n");
-		else
-			PMD_DRV_LOG(INFO, "NIC Link is Down\n");
-		return 0;
-	}
-
-	return -1;
+	if (rte_eth_linkstatus_set(dev, &link))
+		PMD_DRV_LOG(INFO, "NIC Link is %s\n",
+			    link.link_status ? "Up" : "Down");
+	return 0;
 }
 
 static int
@@ -1376,8 +1316,7 @@ nfp_net_dev_link_status_print(struct rte_eth_dev *dev)
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
 	struct rte_eth_link link;
 
-	memset(&link, 0, sizeof(link));
-	nfp_net_dev_atomic_read_link_status(dev, &link);
+	rte_eth_linkstatus_get(dev, &link);
 	if (link.link_status)
 		RTE_LOG(INFO, PMD, "Port %d: Link Up - speed %u Mbps - %s\n",
 			dev->data->port_id, link.link_speed,
@@ -1430,9 +1369,7 @@ nfp_net_dev_interrupt_handler(void *param)
 
 	PMD_DRV_LOG(DEBUG, "We got a LSC interrupt!!!\n");
 
-	/* get the link status */
-	memset(&link, 0, sizeof(link));
-	nfp_net_dev_atomic_read_link_status(dev, &link);
+	rte_eth_linkstatus_get(dev, &link);
 
 	nfp_net_link_update(dev, 0);
 
-- 
2.15.1

[dpdk-dev] [PATCH v7 07/15] net/e1000: use rte_eth_linkstatus helpers

[Stephen Hemminger]

Use new rte_eth_linkstatus_get/set API.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 drivers/net/e1000/em_ethdev.c  | 69 +++--------------------------------------
 drivers/net/e1000/igb_ethdev.c | 70 +++---------------------------------------
 2 files changed, 10 insertions(+), 129 deletions(-)

diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
index 07ee25f84f09..3e68ef9e0581 100644
--- a/drivers/net/e1000/em_ethdev.c
+++ b/drivers/net/e1000/em_ethdev.c
@@ -20,7 +20,6 @@
 #include <rte_ethdev_pci.h>
 #include <rte_memory.h>
 #include <rte_eal.h>
-#include <rte_atomic.h>
 #include <rte_malloc.h>
 #include <rte_dev.h>
 
@@ -197,57 +196,6 @@ static const struct eth_dev_ops eth_em_ops = {
 	.txq_info_get         = em_txq_info_get,
 };
 
-/**
- * Atomically reads the link status information from global
- * structure rte_eth_dev.
- *
- * @param dev
- *   - Pointer to the structure rte_eth_dev to read from.
- *   - Pointer to the buffer to be saved with the link status.
- *
- * @return
- *   - On success, zero.
- *   - On failure, negative value.
- */
-static inline int
-rte_em_dev_atomic_read_link_status(struct rte_eth_dev *dev,
-				struct rte_eth_link *link)
-{
-	struct rte_eth_link *dst = link;
-	struct rte_eth_link *src = &(dev->data->dev_link);
-
-	if (rte_atomic64_cmpset((uint64_t *)dst, *(uint64_t *)dst,
-					*(uint64_t *)src) == 0)
-		return -1;
-
-	return 0;
-}
-
-/**
- * Atomically writes the link status information into global
- * structure rte_eth_dev.
- *
- * @param dev
- *   - Pointer to the structure rte_eth_dev to read from.
- *   - Pointer to the buffer to be saved with the link status.
- *
- * @return
- *   - On success, zero.
- *   - On failure, negative value.
- */
-static inline int
-rte_em_dev_atomic_write_link_status(struct rte_eth_dev *dev,
-				struct rte_eth_link *link)
-{
-	struct rte_eth_link *dst = &(dev->data->dev_link);
-	struct rte_eth_link *src = link;
-
-	if (rte_atomic64_cmpset((uint64_t *)dst, *(uint64_t *)dst,
-					*(uint64_t *)src) == 0)
-		return -1;
-
-	return 0;
-}
 
 /**
  *  eth_em_dev_is_ich8 - Check for ICH8 device
@@ -802,7 +750,7 @@ eth_em_stop(struct rte_eth_dev *dev)
 
 	/* clear the recorded link status */
 	memset(&link, 0, sizeof(link));
-	rte_em_dev_atomic_write_link_status(dev, &link);
+	rte_eth_linkstatus_set(dev, &link);
 
 	if (!rte_intr_allow_others(intr_handle))
 		/* resume to the default handler */
@@ -1160,7 +1108,7 @@ eth_em_link_update(struct rte_eth_dev *dev, int wait_to_complete)
 {
 	struct e1000_hw *hw =
 		E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-	struct rte_eth_link link, old;
+	struct rte_eth_link link;
 	int link_check, count;
 
 	link_check = 0;
@@ -1195,8 +1143,6 @@ eth_em_link_update(struct rte_eth_dev *dev, int wait_to_complete)
 		rte_delay_ms(EM_LINK_UPDATE_CHECK_INTERVAL);
 	}
 	memset(&link, 0, sizeof(link));
-	rte_em_dev_atomic_read_link_status(dev, &link);
-	old = link;
 
 	/* Now we check if a transition has happened */
 	if (link_check && (link.link_status == ETH_LINK_DOWN)) {
@@ -1215,13 +1161,8 @@ eth_em_link_update(struct rte_eth_dev *dev, int wait_to_complete)
 		link.link_status = ETH_LINK_DOWN;
 		link.link_autoneg = ETH_LINK_FIXED;
 	}
-	rte_em_dev_atomic_write_link_status(dev, &link);
+	rte_eth_linkstatus_set(dev, &link);
 
-	/* not changed */
-	if (old.link_status == link.link_status)
-		return -1;
-
-	/* changed */
 	return 0;
 }
 
@@ -1631,8 +1572,8 @@ eth_em_interrupt_action(struct rte_eth_dev *dev,
 	if (ret < 0)
 		return 0;
 
-	memset(&link, 0, sizeof(link));
-	rte_em_dev_atomic_read_link_status(dev, &link);
+	rte_eth_linkstatus_get(dev, &link);
+
 	if (link.link_status) {
 		PMD_INIT_LOG(INFO, " Port %d: Link Up - speed %u Mbps - %s",
 			     dev->data->port_id, link.link_speed,
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 53b8ffa45fda..c23683e60779 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -20,7 +20,6 @@
 #include <rte_ethdev_pci.h>
 #include <rte_memory.h>
 #include <rte_eal.h>
-#include <rte_atomic.h>
 #include <rte_malloc.h>
 #include <rte_dev.h>
 
@@ -522,57 +521,6 @@ static const struct rte_igb_xstats_name_off rte_igbvf_stats_strings[] = {
 #define IGBVF_NB_XSTATS (sizeof(rte_igbvf_stats_strings) / \
 		sizeof(rte_igbvf_stats_strings[0]))
 
-/**
- * Atomically reads the link status information from global
- * structure rte_eth_dev.
- *
- * @param dev
- *   - Pointer to the structure rte_eth_dev to read from.
- *   - Pointer to the buffer to be saved with the link status.
- *
- * @return
- *   - On success, zero.
- *   - On failure, negative value.
- */
-static inline int
-rte_igb_dev_atomic_read_link_status(struct rte_eth_dev *dev,
-				struct rte_eth_link *link)
-{
-	struct rte_eth_link *dst = link;
-	struct rte_eth_link *src = &(dev->data->dev_link);
-
-	if (rte_atomic64_cmpset((uint64_t *)dst, *(uint64_t *)dst,
-					*(uint64_t *)src) == 0)
-		return -1;
-
-	return 0;
-}
-
-/**
- * Atomically writes the link status information into global
- * structure rte_eth_dev.
- *
- * @param dev
- *   - Pointer to the structure rte_eth_dev to read from.
- *   - Pointer to the buffer to be saved with the link status.
- *
- * @return
- *   - On success, zero.
- *   - On failure, negative value.
- */
-static inline int
-rte_igb_dev_atomic_write_link_status(struct rte_eth_dev *dev,
-				struct rte_eth_link *link)
-{
-	struct rte_eth_link *dst = &(dev->data->dev_link);
-	struct rte_eth_link *src = link;
-
-	if (rte_atomic64_cmpset((uint64_t *)dst, *(uint64_t *)dst,
-					*(uint64_t *)src) == 0)
-		return -1;
-
-	return 0;
-}
 
 static inline void
 igb_intr_enable(struct rte_eth_dev *dev)
@@ -1559,7 +1507,7 @@ eth_igb_stop(struct rte_eth_dev *dev)
 
 	/* clear the recorded link status */
 	memset(&link, 0, sizeof(link));
-	rte_igb_dev_atomic_write_link_status(dev, &link);
+	rte_eth_linkstatus_set(dev, &link);
 
 	if (!rte_intr_allow_others(intr_handle))
 		/* resume to the default handler */
@@ -1635,7 +1583,7 @@ eth_igb_close(struct rte_eth_dev *dev)
 	}
 
 	memset(&link, 0, sizeof(link));
-	rte_igb_dev_atomic_write_link_status(dev, &link);
+	rte_eth_linkstatus_set(dev, &link);
 }
 
 static int
@@ -2382,7 +2330,7 @@ eth_igb_link_update(struct rte_eth_dev *dev, int wait_to_complete)
 {
 	struct e1000_hw *hw =
 		E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-	struct rte_eth_link link, old;
+	struct rte_eth_link link;
 	int link_check, count;
 
 	link_check = 0;
@@ -2423,8 +2371,6 @@ eth_igb_link_update(struct rte_eth_dev *dev, int wait_to_complete)
 		rte_delay_ms(IGB_LINK_UPDATE_CHECK_INTERVAL);
 	}
 	memset(&link, 0, sizeof(link));
-	rte_igb_dev_atomic_read_link_status(dev, &link);
-	old = link;
 
 	/* Now we check if a transition has happened */
 	if (link_check) {
@@ -2443,13 +2389,8 @@ eth_igb_link_update(struct rte_eth_dev *dev, int wait_to_complete)
 		link.link_status = ETH_LINK_DOWN;
 		link.link_autoneg = ETH_LINK_FIXED;
 	}
-	rte_igb_dev_atomic_write_link_status(dev, &link);
-
-	/* not changed */
-	if (old.link_status == link.link_status)
-		return -1;
+	rte_eth_linkstatus_set(dev, &link);
 
-	/* changed */
 	return 0;
 }
 
@@ -2887,8 +2828,7 @@ eth_igb_interrupt_action(struct rte_eth_dev *dev,
 		if (ret < 0)
 			return 0;
 
-		memset(&link, 0, sizeof(link));
-		rte_igb_dev_atomic_read_link_status(dev, &link);
+		rte_eth_linkstatus_get(dev, &link);
 		if (link.link_status) {
 			PMD_INIT_LOG(INFO,
 				     " Port %d: Link Up - speed %u Mbps - %s",
-- 
2.15.1

[dpdk-dev] [PATCH v7 08/15] net/ixgbe: use rte_eth_linkstatus functions

[Stephen Hemminger]

Use the new helper functions from eth_dev for
handling atomic link_info update.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>

---
 drivers/net/ixgbe/ixgbe_ethdev.c | 96 +++++++---------------------------------
 1 file changed, 17 insertions(+), 79 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 58217680c29c..7b74cbe92c0d 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -20,7 +20,6 @@
 #include <rte_debug.h>
 #include <rte_pci.h>
 #include <rte_bus_pci.h>
-#include <rte_atomic.h>
 #include <rte_branch_prediction.h>
 #include <rte_memory.h>
 #include <rte_eal.h>
@@ -787,58 +786,6 @@ static const struct rte_ixgbe_xstats_name_off rte_ixgbevf_stats_strings[] = {
 #define IXGBEVF_NB_XSTATS (sizeof(rte_ixgbevf_stats_strings) /	\
 		sizeof(rte_ixgbevf_stats_strings[0]))
 
-/**
- * Atomically reads the link status information from global
- * structure rte_eth_dev.
- *
- * @param dev
- *   - Pointer to the structure rte_eth_dev to read from.
- *   - Pointer to the buffer to be saved with the link status.
- *
- * @return
- *   - On success, zero.
- *   - On failure, negative value.
- */
-static inline int
-rte_ixgbe_dev_atomic_read_link_status(struct rte_eth_dev *dev,
-				struct rte_eth_link *link)
-{
-	struct rte_eth_link *dst = link;
-	struct rte_eth_link *src = &(dev->data->dev_link);
-
-	if (rte_atomic64_cmpset((uint64_t *)dst, *(uint64_t *)dst,
-					*(uint64_t *)src) == 0)
-		return -1;
-
-	return 0;
-}
-
-/**
- * Atomically writes the link status information into global
- * structure rte_eth_dev.
- *
- * @param dev
- *   - Pointer to the structure rte_eth_dev to read from.
- *   - Pointer to the buffer to be saved with the link status.
- *
- * @return
- *   - On success, zero.
- *   - On failure, negative value.
- */
-static inline int
-rte_ixgbe_dev_atomic_write_link_status(struct rte_eth_dev *dev,
-				struct rte_eth_link *link)
-{
-	struct rte_eth_link *dst = &(dev->data->dev_link);
-	struct rte_eth_link *src = link;
-
-	if (rte_atomic64_cmpset((uint64_t *)dst, *(uint64_t *)dst,
-					*(uint64_t *)src) == 0)
-		return -1;
-
-	return 0;
-}
-
 /*
  * This function is the same as ixgbe_is_sfp() in base/ixgbe.h.
  */
@@ -2758,7 +2705,7 @@ ixgbe_dev_stop(struct rte_eth_dev *dev)
 
 	/* Clear recorded link status */
 	memset(&link, 0, sizeof(link));
-	rte_ixgbe_dev_atomic_write_link_status(dev, &link);
+	rte_eth_linkstatus_set(dev, &link);
 
 	if (!rte_intr_allow_others(intr_handle))
 		/* resume to the default handler */
@@ -3943,7 +3890,6 @@ ixgbe_dev_link_update_share(struct rte_eth_dev *dev,
 			    int wait_to_complete, int vf)
 {
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-	struct rte_eth_link link, old;
 	ixgbe_link_speed link_speed = IXGBE_LINK_SPEED_UNKNOWN;
 	struct ixgbe_interrupt *intr =
 		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
@@ -3952,13 +3898,11 @@ ixgbe_dev_link_update_share(struct rte_eth_dev *dev,
 	u32 speed = 0;
 	int wait = 1;
 	bool autoneg = false;
-
-	link.link_status = ETH_LINK_DOWN;
-	link.link_speed = 0;
-	link.link_duplex = ETH_LINK_HALF_DUPLEX;
-	link.link_autoneg = ETH_LINK_AUTONEG;
-	memset(&old, 0, sizeof(old));
-	rte_ixgbe_dev_atomic_read_link_status(dev, &old);
+	struct rte_eth_link link = {
+		.link_status = ETH_LINK_DOWN,
+		.link_duplex = ETH_LINK_HALF_DUPLEX,
+		.link_autoneg = ETH_LINK_AUTONEG,
+	};
 
 	hw->mac.get_link_status = true;
 
@@ -3982,19 +3926,17 @@ ixgbe_dev_link_update_share(struct rte_eth_dev *dev,
 	if (diag != 0) {
 		link.link_speed = ETH_SPEED_NUM_100M;
 		link.link_duplex = ETH_LINK_FULL_DUPLEX;
-		rte_ixgbe_dev_atomic_write_link_status(dev, &link);
-		if (link.link_status == old.link_status)
-			return -1;
+		rte_eth_linkstatus_set(dev, &link);
+
 		return 0;
 	}
 
 	if (link_up == 0) {
-		rte_ixgbe_dev_atomic_write_link_status(dev, &link);
 		intr->flags |= IXGBE_FLAG_NEED_LINK_CONFIG;
-		if (link.link_status == old.link_status)
-			return -1;
+		rte_eth_linkstatus_set(dev, &link);
 		return 0;
 	}
+
 	intr->flags &= ~IXGBE_FLAG_NEED_LINK_CONFIG;
 	link.link_status = ETH_LINK_UP;
 	link.link_duplex = ETH_LINK_FULL_DUPLEX;
@@ -4026,11 +3968,8 @@ ixgbe_dev_link_update_share(struct rte_eth_dev *dev,
 		link.link_speed = ETH_SPEED_NUM_10G;
 		break;
 	}
-	rte_ixgbe_dev_atomic_write_link_status(dev, &link);
-
-	if (link.link_status == old.link_status)
-		return -1;
 
+	rte_eth_linkstatus_set(dev, &link);
 	return 0;
 }
 
@@ -4230,8 +4169,8 @@ ixgbe_dev_link_status_print(struct rte_eth_dev *dev)
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
 	struct rte_eth_link link;
 
-	memset(&link, 0, sizeof(link));
-	rte_ixgbe_dev_atomic_read_link_status(dev, &link);
+	rte_eth_linkstatus_get(dev, &link);
+
 	if (link.link_status) {
 		PMD_INIT_LOG(INFO, "Port %d: Link Up - speed %u Mbps - %s",
 					(int)(dev->data->port_id),
@@ -4266,7 +4205,6 @@ ixgbe_dev_interrupt_action(struct rte_eth_dev *dev,
 	struct ixgbe_interrupt *intr =
 		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
 	int64_t timeout;
-	struct rte_eth_link link;
 	struct ixgbe_hw *hw =
 		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
@@ -4283,9 +4221,10 @@ ixgbe_dev_interrupt_action(struct rte_eth_dev *dev,
 	}
 
 	if (intr->flags & IXGBE_FLAG_NEED_LINK_UPDATE) {
+		struct rte_eth_link link;
+
 		/* get the link status before link update, for predicting later */
-		memset(&link, 0, sizeof(link));
-		rte_ixgbe_dev_atomic_read_link_status(dev, &link);
+		rte_eth_linkstatus_get(dev, &link);
 
 		ixgbe_dev_link_update(dev, 0);
 
@@ -6804,9 +6743,8 @@ ixgbe_start_timecounters(struct rte_eth_dev *dev)
 	uint32_t shift = 0;
 
 	/* Get current link speed. */
-	memset(&link, 0, sizeof(link));
 	ixgbe_dev_link_update(dev, 1);
-	rte_ixgbe_dev_atomic_read_link_status(dev, &link);
+	rte_eth_linkstatus_get(dev, &link);
 
 	switch (link.link_speed) {
 	case ETH_SPEED_NUM_100M:
-- 
2.15.1

Re: [dpdk-dev] [PATCH v7 08/15] net/ixgbe: use rte_eth_linkstatus functions

[Neil Horman]

On Mon, Jan 22, 2018 at 03:54:51PM -0800, Stephen Hemminger wrote:
> Use the new helper functions from eth_dev for
> handling atomic link_info update.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 
> ---
>  drivers/net/ixgbe/ixgbe_ethdev.c | 96 +++++++---------------------------------
>  1 file changed, 17 insertions(+), 79 deletions(-)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
> index 58217680c29c..7b74cbe92c0d 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -20,7 +20,6 @@
>  #include <rte_debug.h>
>  #include <rte_pci.h>
>  #include <rte_bus_pci.h>
> -#include <rte_atomic.h>
>  #include <rte_branch_prediction.h>
>  #include <rte_memory.h>
>  #include <rte_eal.h>
> @@ -787,58 +786,6 @@ static const struct rte_ixgbe_xstats_name_off rte_ixgbevf_stats_strings[] = {
>  #define IXGBEVF_NB_XSTATS (sizeof(rte_ixgbevf_stats_strings) /	\
>  		sizeof(rte_ixgbevf_stats_strings[0]))
>  
Getting some build errors here:

CC igb_ethdev.o
/home/nhorman/git/dpdk/drivers/net/e1000/igb_ethdev.c: In function ‘eth_igb_stop’:
/home/nhorman/git/dpdk/drivers/net/e1000/igb_ethdev.c:1510:2: error: implicit declaration of function ‘rte_eth_linkstatus_set’; did you mean ‘rte_eth_xstats_get’? [-Werror=implicit-function-declaration]
  rte_eth_linkstatus_set(dev, &link);
  ^~~~~~~~~~~~~~~~~~~~~~
  rte_eth_xstats_get
/home/nhorman/git/dpdk/drivers/net/e1000/igb_ethdev.c:1510:2: error: nested extern declaration of ‘rte_eth_linkstatus_set’ [-Werror=nested-externs]
/home/nhorman/git/dpdk/drivers/net/e1000/igb_ethdev.c: In function ‘eth_igb_interrupt_action’:
/home/nhorman/git/dpdk/drivers/net/e1000/igb_ethdev.c:2831:3: error: implicit declaration of function ‘rte_eth_linkstatus_get’; did you mean ‘rte_eth_xstats_get’? [-Werror=implicit-function-declaration]
   rte_eth_linkstatus_get(dev, &link);

Re: [dpdk-dev] [PATCH v7 08/15] net/ixgbe: use rte_eth_linkstatus functions

[Ferruh Yigit]

On 1/23/2018 12:27 PM, Neil Horman wrote:
> On Mon, Jan 22, 2018 at 03:54:51PM -0800, Stephen Hemminger wrote:
>> Use the new helper functions from eth_dev for
>> handling atomic link_info update.
>>
>> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>
>> ---
>>  drivers/net/ixgbe/ixgbe_ethdev.c | 96 +++++++---------------------------------
>>  1 file changed, 17 insertions(+), 79 deletions(-)
>>
>> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
>> index 58217680c29c..7b74cbe92c0d 100644
>> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
>> @@ -20,7 +20,6 @@
>>  #include <rte_debug.h>
>>  #include <rte_pci.h>
>>  #include <rte_bus_pci.h>
>> -#include <rte_atomic.h>
>>  #include <rte_branch_prediction.h>
>>  #include <rte_memory.h>
>>  #include <rte_eal.h>
>> @@ -787,58 +786,6 @@ static const struct rte_ixgbe_xstats_name_off rte_ixgbevf_stats_strings[] = {
>>  #define IXGBEVF_NB_XSTATS (sizeof(rte_ixgbevf_stats_strings) /	\
>>  		sizeof(rte_ixgbevf_stats_strings[0]))
>>  
> Getting some build errors here:
> 
> CC igb_ethdev.o
> /home/nhorman/git/dpdk/drivers/net/e1000/igb_ethdev.c: In function ‘eth_igb_stop’:
> /home/nhorman/git/dpdk/drivers/net/e1000/igb_ethdev.c:1510:2: error: implicit declaration of function ‘rte_eth_linkstatus_set’; did you mean ‘rte_eth_xstats_get’? [-Werror=implicit-function-declaration]
>   rte_eth_linkstatus_set(dev, &link);
>   ^~~~~~~~~~~~~~~~~~~~~~
>   rte_eth_xstats_get
> /home/nhorman/git/dpdk/drivers/net/e1000/igb_ethdev.c:1510:2: error: nested extern declaration of ‘rte_eth_linkstatus_set’ [-Werror=nested-externs]
> /home/nhorman/git/dpdk/drivers/net/e1000/igb_ethdev.c: In function ‘eth_igb_interrupt_action’:
> /home/nhorman/git/dpdk/drivers/net/e1000/igb_ethdev.c:2831:3: error: implicit declaration of function ‘rte_eth_linkstatus_get’; did you mean ‘rte_eth_xstats_get’? [-Werror=implicit-function-declaration]
>    rte_eth_linkstatus_get(dev, &link);
> 

Compiles fine for me, any change that you are using old version of dpdk that
driver doesn't use rte_ethdev_driver.h yet which these functions are implemented in?

[dpdk-dev] [PATCH v7 09/15] net/sfc: use new rte_eth_linkstatus functions

[Stephen Hemminger]

Use the new API (_rte_eth_linkstatus_set) to handle link status update.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/sfc/sfc_ethdev.c | 21 +++------------------
 drivers/net/sfc/sfc_ev.c     | 20 ++------------------
 2 files changed, 5 insertions(+), 36 deletions(-)

diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index 89a452907621..01a2c608915e 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -236,22 +236,12 @@ static int
 sfc_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete)
 {
 	struct sfc_adapter *sa = dev->data->dev_private;
-	struct rte_eth_link *dev_link = &dev->data->dev_link;
-	struct rte_eth_link old_link;
 	struct rte_eth_link current_link;
 
 	sfc_log_init(sa, "entry");
 
-retry:
-	EFX_STATIC_ASSERT(sizeof(*dev_link) == sizeof(rte_atomic64_t));
-	*(int64_t *)&old_link = rte_atomic64_read((rte_atomic64_t *)dev_link);
-
 	if (sa->state != SFC_ADAPTER_STARTED) {
 		sfc_port_link_mode_to_info(EFX_LINK_UNKNOWN, &current_link);
-		if (!rte_atomic64_cmpset((volatile uint64_t *)dev_link,
-					 *(uint64_t *)&old_link,
-					 *(uint64_t *)&current_link))
-			goto retry;
 	} else if (wait_to_complete) {
 		efx_link_mode_t link_mode;
 
@@ -259,21 +249,16 @@ sfc_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete)
 			link_mode = EFX_LINK_UNKNOWN;
 		sfc_port_link_mode_to_info(link_mode, &current_link);
 
-		if (!rte_atomic64_cmpset((volatile uint64_t *)dev_link,
-					 *(uint64_t *)&old_link,
-					 *(uint64_t *)&current_link))
-			goto retry;
 	} else {
 		sfc_ev_mgmt_qpoll(sa);
-		*(int64_t *)&current_link =
-			rte_atomic64_read((rte_atomic64_t *)dev_link);
+		rte_eth_linkstatus_get(dev, &current_link);
 	}
 
-	if (old_link.link_status != current_link.link_status)
+	if (rte_eth_linkstatus_set(dev, &current_link))
 		sfc_info(sa, "Link status is %s",
 			 current_link.link_status ? "UP" : "DOWN");
 
-	return old_link.link_status == current_link.link_status ? 0 : -1;
+	return 0;
 }
 
 static void
diff --git a/drivers/net/sfc/sfc_ev.c b/drivers/net/sfc/sfc_ev.c
index 7abe61ae5ff1..273a92ccb222 100644
--- a/drivers/net/sfc/sfc_ev.c
+++ b/drivers/net/sfc/sfc_ev.c
@@ -382,27 +382,11 @@ sfc_ev_link_change(void *arg, efx_link_mode_t link_mode)
 {
 	struct sfc_evq *evq = arg;
 	struct sfc_adapter *sa = evq->sa;
-	struct rte_eth_link *dev_link = &sa->eth_dev->data->dev_link;
 	struct rte_eth_link new_link;
-	uint64_t new_link_u64;
-	uint64_t old_link_u64;
-
-	EFX_STATIC_ASSERT(sizeof(*dev_link) == sizeof(rte_atomic64_t));
 
 	sfc_port_link_mode_to_info(link_mode, &new_link);
-
-	new_link_u64 = *(uint64_t *)&new_link;
-	do {
-		old_link_u64 = rte_atomic64_read((rte_atomic64_t *)dev_link);
-		if (old_link_u64 == new_link_u64)
-			break;
-
-		if (rte_atomic64_cmpset((volatile uint64_t *)dev_link,
-					old_link_u64, new_link_u64)) {
-			evq->sa->port.lsc_seq++;
-			break;
-		}
-	} while (B_TRUE);
+	if (rte_eth_linkstatus_set(sa->eth_dev, &new_link))
+		evq->sa->port.lsc_seq++;
 
 	return B_FALSE;
 }
-- 
2.15.1

[dpdk-dev] [PATCH v7 10/15] net/i40e: use rte_eth_linkstatus functions

[Stephen Hemminger]

Use new rte_linkstatus update API

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>

---
 drivers/net/i40e/i40e_ethdev.c    | 43 +++++----------------------------------
 drivers/net/i40e/i40e_ethdev_vf.c | 18 ++--------------
 2 files changed, 7 insertions(+), 54 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index c4df65df0cf6..15f18776e198 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -628,34 +628,6 @@ static struct rte_pci_driver rte_i40e_pmd = {
 	.remove = eth_i40e_pci_remove,
 };
 
-static inline int
-rte_i40e_dev_atomic_read_link_status(struct rte_eth_dev *dev,
-				     struct rte_eth_link *link)
-{
-	struct rte_eth_link *dst = link;
-	struct rte_eth_link *src = &(dev->data->dev_link);
-
-	if (rte_atomic64_cmpset((uint64_t *)dst, *(uint64_t *)dst,
-					*(uint64_t *)src) == 0)
-		return -1;
-
-	return 0;
-}
-
-static inline int
-rte_i40e_dev_atomic_write_link_status(struct rte_eth_dev *dev,
-				      struct rte_eth_link *link)
-{
-	struct rte_eth_link *dst = &(dev->data->dev_link);
-	struct rte_eth_link *src = link;
-
-	if (rte_atomic64_cmpset((uint64_t *)dst, *(uint64_t *)dst,
-					*(uint64_t *)src) == 0)
-		return -1;
-
-	return 0;
-}
-
 RTE_PMD_REGISTER_PCI(net_i40e, rte_i40e_pmd);
 RTE_PMD_REGISTER_PCI_TABLE(net_i40e, pci_id_i40e_map);
 RTE_PMD_REGISTER_KMOD_DEP(net_i40e, "* igb_uio | uio_pci_generic | vfio-pci");
@@ -2347,17 +2319,16 @@ i40e_dev_link_update(struct rte_eth_dev *dev,
 #define MAX_REPEAT_TIME 10  /* 1s (10 * 100ms) in total */
 	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct i40e_link_status link_status;
-	struct rte_eth_link link, old;
+	struct rte_eth_link link;
 	int status;
 	unsigned rep_cnt = MAX_REPEAT_TIME;
 	bool enable_lse = dev->data->dev_conf.intr_conf.lsc ? true : false;
 
 	memset(&link, 0, sizeof(link));
-	memset(&old, 0, sizeof(old));
-	memset(&link_status, 0, sizeof(link_status));
-	rte_i40e_dev_atomic_read_link_status(dev, &old);
 
 	do {
+		memset(&link_status, 0, sizeof(link_status));
+
 		/* Get link status information from hardware */
 		status = i40e_aq_get_link_info(hw, enable_lse,
 						&link_status, NULL);
@@ -2410,10 +2381,7 @@ i40e_dev_link_update(struct rte_eth_dev *dev,
 			ETH_LINK_SPEED_FIXED);
 
 out:
-	rte_i40e_dev_atomic_write_link_status(dev, &link);
-	if (link.link_status == old.link_status)
-		return -1;
-
+	rte_eth_linkstatus_set(dev, &link);
 	i40e_notify_all_vfs_link_status(dev);
 
 	return 0;
@@ -10034,9 +10002,8 @@ i40e_start_timecounters(struct rte_eth_dev *dev)
 	uint32_t tsync_inc_h;
 
 	/* Get current link speed. */
-	memset(&link, 0, sizeof(link));
 	i40e_dev_link_update(dev, 1);
-	rte_i40e_dev_atomic_read_link_status(dev, &link);
+	rte_eth_linkstatus_get(dev, &link);
 
 	switch (link.link_speed) {
 	case ETH_SPEED_NUM_40G:
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 6ac3f8c04a50..e34a2bf93787 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -1036,20 +1036,6 @@ static const struct rte_pci_id pci_id_i40evf_map[] = {
 	{ .vendor_id = 0, /* sentinel */ },
 };
 
-static inline int
-i40evf_dev_atomic_write_link_status(struct rte_eth_dev *dev,
-				    struct rte_eth_link *link)
-{
-	struct rte_eth_link *dst = &(dev->data->dev_link);
-	struct rte_eth_link *src = link;
-
-	if (rte_atomic64_cmpset((uint64_t *)dst, *(uint64_t *)dst,
-					*(uint64_t *)src) == 0)
-		return -1;
-
-	return 0;
-}
-
 /* Disable IRQ0 */
 static inline void
 i40evf_disable_irq0(struct i40e_hw *hw)
@@ -2077,6 +2063,7 @@ i40evf_dev_link_update(struct rte_eth_dev *dev,
 	 * while Linux driver does not
 	 */
 
+	memset(&new_link, 0, sizeof(new_link));
 	/* Linux driver PF host */
 	switch (vf->link_speed) {
 	case I40E_LINK_SPEED_100MB:
@@ -2108,8 +2095,7 @@ i40evf_dev_link_update(struct rte_eth_dev *dev,
 	new_link.link_autoneg =
 		dev->data->dev_conf.link_speeds & ETH_LINK_SPEED_FIXED;
 
-	i40evf_dev_atomic_write_link_status(dev, &new_link);
-
+	rte_eth_linkstatus_set(dev, &new_link);
 	return 0;
 }
 
-- 
2.15.1

[dpdk-dev] [PATCH v7 11/15] net/liquidio: use rte_eth_linkstatus_set

[Stephen Hemminger]

Use the new link update API, and cleanup the logic in the the
link update routine.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Tested-by: Shijith Thotton <shijith.thotton@caviumnetworks.com>
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 drivers/net/liquidio/lio_ethdev.c | 53 ++++++---------------------------------
 1 file changed, 8 insertions(+), 45 deletions(-)

diff --git a/drivers/net/liquidio/lio_ethdev.c b/drivers/net/liquidio/lio_ethdev.c
index 843d023383f7..3aabefcbabe2 100644
--- a/drivers/net/liquidio/lio_ethdev.c
+++ b/drivers/net/liquidio/lio_ethdev.c
@@ -904,32 +904,6 @@ lio_dev_vlan_filter_set(struct rte_eth_dev *eth_dev, uint16_t vlan_id, int on)
 	return 0;
 }
 
-/**
- * Atomically writes the link status information into global
- * structure rte_eth_dev.
- *
- * @param eth_dev
- *   - Pointer to the structure rte_eth_dev to read from.
- *   - Pointer to the buffer to be saved with the link status.
- *
- * @return
- *   - On success, zero.
- *   - On failure, negative value.
- */
-static inline int
-lio_dev_atomic_write_link_status(struct rte_eth_dev *eth_dev,
-				 struct rte_eth_link *link)
-{
-	struct rte_eth_link *dst = &eth_dev->data->dev_link;
-	struct rte_eth_link *src = link;
-
-	if (rte_atomic64_cmpset((uint64_t *)dst, *(uint64_t *)dst,
-				*(uint64_t *)src) == 0)
-		return -1;
-
-	return 0;
-}
-
 static uint64_t
 lio_hweight64(uint64_t w)
 {
@@ -949,23 +923,17 @@ lio_dev_link_update(struct rte_eth_dev *eth_dev,
 		    int wait_to_complete __rte_unused)
 {
 	struct lio_device *lio_dev = LIO_DEV(eth_dev);
-	struct rte_eth_link link, old;
-
-	/* Initialize */
-	link.link_status = ETH_LINK_DOWN;
-	link.link_speed = ETH_SPEED_NUM_NONE;
-	link.link_duplex = ETH_LINK_HALF_DUPLEX;
-	link.link_autoneg = ETH_LINK_AUTONEG;
-	memset(&old, 0, sizeof(old));
+	struct rte_eth_link link = {
+		.link_status = ETH_LINK_DOWN,
+		.link_speed = ETH_SPEED_NUM_NONE,
+		.link_duplex = ETH_LINK_HALF_DUPLEX,
+		.link_autoneg = ETH_LINK_AUTONEG,
+	};
 
 	/* Return what we found */
 	if (lio_dev->linfo.link.s.link_up == 0) {
 		/* Interface is down */
-		if (lio_dev_atomic_write_link_status(eth_dev, &link))
-			return -1;
-		if (link.link_status == old.link_status)
-			return -1;
-		return 0;
+		return rte_eth_linkstatus_set(eth_dev, &link);
 	}
 
 	link.link_status = ETH_LINK_UP; /* Interface is up */
@@ -982,12 +950,7 @@ lio_dev_link_update(struct rte_eth_dev *eth_dev,
 		link.link_duplex = ETH_LINK_HALF_DUPLEX;
 	}
 
-	if (lio_dev_atomic_write_link_status(eth_dev, &link))
-		return -1;
-
-	if (link.link_status == old.link_status)
-		return -1;
-
+	rte_eth_linkstatus_set(eth_dev, &link);
 	return 0;
 }
 
-- 
2.15.1

[dpdk-dev] [PATCH v7 12/15] net/thunderx: use rte_eth_linkstatus_set

[Stephen Hemminger]

Use new helper function.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/thunderx/nicvf_ethdev.c | 48 ++++++++++++++++---------------------
 1 file changed, 20 insertions(+), 28 deletions(-)

diff --git a/drivers/net/thunderx/nicvf_ethdev.c b/drivers/net/thunderx/nicvf_ethdev.c
index d34938c64b1d..f23170c5a4a9 100644
--- a/drivers/net/thunderx/nicvf_ethdev.c
+++ b/drivers/net/thunderx/nicvf_ethdev.c
@@ -15,7 +15,6 @@
 #include <sys/queue.h>
 
 #include <rte_alarm.h>
-#include <rte_atomic.h>
 #include <rte_branch_prediction.h>
 #include <rte_byteorder.h>
 #include <rte_common.h>
@@ -69,31 +68,20 @@ nicvf_init_log(void)
 		rte_log_set_level(nicvf_logtype_driver, RTE_LOG_NOTICE);
 }
 
-static inline int
-nicvf_atomic_write_link_status(struct rte_eth_dev *dev,
-			       struct rte_eth_link *link)
+static void
+nicvf_link_status_update(struct nicvf *nic,
+			 struct rte_eth_link *link)
 {
-	struct rte_eth_link *dst = &dev->data->dev_link;
-	struct rte_eth_link *src = link;
+	memset(link, 0, sizeof(*link));
 
-	if (rte_atomic64_cmpset((uint64_t *)dst, *(uint64_t *)dst,
-		*(uint64_t *)src) == 0)
-		return -1;
-
-	return 0;
-}
+	link->link_autoneg = ETH_LINK_AUTONEG;
+	link->link_status = nic->link_up ? ETH_LINK_UP : ETH_LINK_DOWN;
 
-static inline void
-nicvf_set_eth_link_status(struct nicvf *nic, struct rte_eth_link *link)
-{
-	link->link_status = nic->link_up;
-	link->link_duplex = ETH_LINK_AUTONEG;
 	if (nic->duplex == NICVF_HALF_DUPLEX)
 		link->link_duplex = ETH_LINK_HALF_DUPLEX;
 	else if (nic->duplex == NICVF_FULL_DUPLEX)
 		link->link_duplex = ETH_LINK_FULL_DUPLEX;
 	link->link_speed = nic->speed;
-	link->link_autoneg = ETH_LINK_AUTONEG;
 }
 
 static void
@@ -101,12 +89,16 @@ nicvf_interrupt(void *arg)
 {
 	struct rte_eth_dev *dev = arg;
 	struct nicvf *nic = nicvf_pmd_priv(dev);
+	struct rte_eth_link link;
 
 	if (nicvf_reg_poll_interrupts(nic) == NIC_MBOX_MSG_BGX_LINK_CHANGE) {
-		if (dev->data->dev_conf.intr_conf.lsc)
-			nicvf_set_eth_link_status(nic, &dev->data->dev_link);
-		_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC,
-					      NULL);
+		if (dev->data->dev_conf.intr_conf.lsc) {
+			nicvf_link_status_update(nic, &link);
+			rte_eth_linkstatus_set(dev, &link);
+
+			_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC,
+						      NULL);
+		}
 	}
 
 	rte_eal_alarm_set(NICVF_INTR_POLL_INTERVAL_MS * 1000,
@@ -153,17 +145,17 @@ nicvf_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete)
 	if (wait_to_complete) {
 		/* rte_eth_link_get() might need to wait up to 9 seconds */
 		for (i = 0; i < MAX_CHECK_TIME; i++) {
-			memset(&link, 0, sizeof(link));
-			nicvf_set_eth_link_status(nic, &link);
-			if (link.link_status)
+			nicvf_link_status_update(nic, &link);
+			if (link.link_status == ETH_LINK_UP)
 				break;
 			rte_delay_ms(CHECK_INTERVAL);
 		}
 	} else {
-		memset(&link, 0, sizeof(link));
-		nicvf_set_eth_link_status(nic, &link);
+		nicvf_link_status_update(nic, &link);
 	}
-	return nicvf_atomic_write_link_status(dev, &link);
+
+	rte_eth_linkstatus_set(dev, &link);
+	return 0;
 }
 
 static int
-- 
2.15.1

[dpdk-dev] [PATCH v7 13/15] net/szedata: use _rte_eth_linkstatus_set

[Stephen Hemminger]

Yet another driver which was not returing correct value on
link change.

Since this driver can't be built on x86 could not even
do a compile test.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/szedata2/rte_eth_szedata2.c | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/drivers/net/szedata2/rte_eth_szedata2.c b/drivers/net/szedata2/rte_eth_szedata2.c
index e53c738db546..a331ed3c5cdc 100644
--- a/drivers/net/szedata2/rte_eth_szedata2.c
+++ b/drivers/net/szedata2/rte_eth_szedata2.c
@@ -50,7 +50,6 @@
 #include <rte_memcpy.h>
 #include <rte_kvargs.h>
 #include <rte_dev.h>
-#include <rte_atomic.h>
 
 #include "rte_eth_szedata2.h"
 #include "szedata2_iobuf.h"
@@ -1173,14 +1172,15 @@ static int
 eth_link_update(struct rte_eth_dev *dev,
 		int wait_to_complete __rte_unused)
 {
-	struct rte_eth_link link;
-	struct rte_eth_link *link_ptr = &link;
-	struct rte_eth_link *dev_link = &dev->data->dev_link;
 	struct pmd_internals *internals = (struct pmd_internals *)
 		dev->data->dev_private;
 	const volatile struct szedata2_ibuf *ibuf;
 	uint32_t i;
 	bool link_is_up = false;
+	struct rte_eth_link link = {
+		.link_duplex = ETH_LINK_FULL_DUPLEX,
+		.link_autoneg = ETH_LINK_SPEED_FIXED,
+	};
 
 	switch (get_link_speed(internals)) {
 	case SZEDATA2_LINK_SPEED_10G:
@@ -1197,9 +1197,6 @@ eth_link_update(struct rte_eth_dev *dev,
 		break;
 	}
 
-	/* szedata2 uses only full duplex */
-	link.link_duplex = ETH_LINK_FULL_DUPLEX;
-
 	for (i = 0; i < szedata2_ibuf_count; i++) {
 		ibuf = ibuf_ptr_by_index(internals->pci_rsc, i);
 		/*
@@ -1212,13 +1209,9 @@ eth_link_update(struct rte_eth_dev *dev,
 		}
 	}
 
-	link.link_status = (link_is_up) ? ETH_LINK_UP : ETH_LINK_DOWN;
-
-	link.link_autoneg = ETH_LINK_FIXED;
-
-	rte_atomic64_cmpset((uint64_t *)dev_link, *(uint64_t *)dev_link,
-			*(uint64_t *)link_ptr);
+	link.link_status = link_is_up ? ETH_LINK_UP : ETH_LINK_DOWN;
 
+	rte_eth_linkstatus_set(dev, &link);
 	return 0;
 }
 
-- 
2.15.1

[dpdk-dev] [PATCH v7 14/15] net/octeontx: use rte_eth_linkstatus_set

[Stephen Hemminger]

Common function matches this drivers usage.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/octeontx/octeontx_ethdev.c | 17 ++---------------
 1 file changed, 2 insertions(+), 15 deletions(-)

diff --git a/drivers/net/octeontx/octeontx_ethdev.c b/drivers/net/octeontx/octeontx_ethdev.c
index adca3435e4af..a54ea1d2219a 100644
--- a/drivers/net/octeontx/octeontx_ethdev.c
+++ b/drivers/net/octeontx/octeontx_ethdev.c
@@ -487,20 +487,6 @@ octeontx_dev_promisc_disable(struct rte_eth_dev *dev)
 	octeontx_port_promisc_set(nic, 0);
 }
 
-static inline int
-octeontx_atomic_write_link_status(struct rte_eth_dev *dev,
-				  struct rte_eth_link *link)
-{
-	struct rte_eth_link *dst = &dev->data->dev_link;
-	struct rte_eth_link *src = link;
-
-	if (rte_atomic64_cmpset((uint64_t *)dst, *(uint64_t *)dst,
-		*(uint64_t *)src) == 0)
-		return -1;
-
-	return 0;
-}
-
 static int
 octeontx_port_link_status(struct octeontx_nic *nic)
 {
@@ -572,7 +558,8 @@ octeontx_dev_link_update(struct rte_eth_dev *dev,
 	link.link_duplex = ETH_LINK_FULL_DUPLEX;
 	link.link_autoneg = ETH_LINK_AUTONEG;
 
-	return octeontx_atomic_write_link_status(dev, &link);
+	rte_eth_linkstatus_set(dev, &link);
+	return 0;
 }
 
 static int
-- 
2.15.1

[dpdk-dev] [PATCH v7 15/15] net/enic: use rte_eth_linkstatus_set

[Stephen Hemminger]

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>
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

---
 drivers/net/enic/enic_ethdev.c |  5 ++---
 drivers/net/enic/enic_main.c   | 17 ++++++++---------
 2 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c
index e1ff9dcbd713..7d10b70a341a 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 9274defd998e..7e64706d5623 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -379,16 +379,15 @@ 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;
+	rte_eth_linkstatus_set(eth_dev, &link);
+	return 0;
 }
 
 static void
-- 
2.15.1

Re: [dpdk-dev] [PATCH v7 00/15] common linkstatus functions

[Ferruh Yigit]

On 1/22/2018 11:54 PM, Stephen Hemminger wrote:
> While reviewing drivers, noticed a lot of unnecessary
> duplication of code in drivers for handling the eth_dev link status
> information. While consolidating this, it also became obvious that
> some drivers behave differently for no good reason.
> 
> It also was a good chance to introduce atomic exchange primitives
> in EAL because there are other places using cmpset where not
> necessary (such as bonding).
> 
> Mostly only compile tested only, don't have all of the hardware
> available (except ixgbe and virtio) to test.
> 
> 
> v7
>   - add exchange functions for PPC64
>   - move linkstatus helpers to rte_ethdev_driver
>   - rebase to 18.02
> 
> 
> 
> Stephen Hemminger (15):
>   eal: introduce atomic exchange operation
>   ethdev: add linkstatus get/set helper functions
>   net/virtio: use eth_linkstatus_set
>   net/vmxnet3: use rte_eth_linkstatus_set
>   net/dpaa2: use rte_eth_linkstatus_set
>   net/nfp: use rte_eth_linkstatus functions
>   net/e1000: use rte_eth_linkstatus helpers
>   net/ixgbe: use rte_eth_linkstatus functions
>   net/sfc: use new rte_eth_linkstatus functions
>   net/i40e: use rte_eth_linkstatus functions
>   net/liquidio: use rte_eth_linkstatus_set
>   net/thunderx: use rte_eth_linkstatus_set
>   net/szedata: use _rte_eth_linkstatus_set
>   net/octeontx: use rte_eth_linkstatus_set
>   net/enic: use rte_eth_linkstatus_set

This patchset is not able to make rc1, I am for postponing it to next release
instead of getting in later rc.

It is good to have eal atomic exchange APIs and it is good that some duplicated
code removed from APIs but I think it would be better to discuss and have a
consensus on what dev_link_update() function should return, and get some acks
from PMD maintainers first.

What do you think?