[dpdk-dev,v5,02/15] ethdev: add linkstatus get/set helper functions
Checks
Commit Message
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>
---
lib/librte_ether/rte_ethdev.c | 22 +++------------
lib/librte_ether/rte_ethdev.h | 62 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 66 insertions(+), 18 deletions(-)
Comments
On 01/16/2018 09:37 PM, Stephen Hemminger wrote:
> 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>
> ---
> lib/librte_ether/rte_ethdev.c | 22 +++------------
> lib/librte_ether/rte_ethdev.h | 62 +++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 66 insertions(+), 18 deletions(-)
>
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index a524af740f4a..d6433af35a19 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -1447,20 +1447,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)
> {
> @@ -1469,8 +1455,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);
> @@ -1486,8 +1472,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.h b/lib/librte_ether/rte_ethdev.h
> index b10e2a92da71..ec8255656967 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -2218,6 +2218,68 @@ int rte_eth_dev_set_link_down(uint16_t port_id);
> */
> void rte_eth_dev_close(uint16_t port_id);
>
> +
> +/**
> + * @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.
Here "link status" is used as up/down status of the link.
> + */
> +static inline int
> +rte_eth_linkstatus_set(struct rte_eth_dev *dev,
> + const struct rte_eth_link *new_link)
Here (and function and the second parameter description) "link status"
is used as all information about link.
rte_ethdev API calls it just "link" (link_update, link_get_wait/nowait).
It is not a big problem, but it is better to avoid it.
Thanks.
> +{
> + 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
> +}
> +
> /**
> * Reset a Ethernet device and keep its port id.
> *
@@ -1447,20 +1447,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)
{
@@ -1469,8 +1455,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);
@@ -1486,8 +1472,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);
@@ -2218,6 +2218,68 @@ int rte_eth_dev_set_link_down(uint16_t port_id);
*/
void rte_eth_dev_close(uint16_t port_id);
+
+/**
+ * @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
+}
+
/**
* Reset a Ethernet device and keep its port id.
*