[dpdk-dev] [PATCH v8 5/9] nfp: adding link functionality

[Thomas Monjalon]

2015-11-25 08:29, Stephen Hemminger:
> On Wed, 25 Nov 2015 16:19:51 +
> "Alejandro.Lucero"  wrote:
> 
> > +/*
> > + * 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 = >data->dev_link;
> > +
> > +   if (rte_atomic64_cmpset((uint64_t *)dst, *(uint64_t *)dst,
> > +   *(uint64_t *)src) == 0)
> > +   return -1;
> > +
> > +   return 0;
> > +}
> > +
> > +/
> 
> Sigh, this code has been copied and pasted to every driver.
> Why is it not part of standard rte_ethdev code.

Because nobody made the patch.

Stephen, how is your mood today?

[dpdk-dev] [PATCH v8 5/9] nfp: adding link functionality

[Alejandro Lucero]

I tried to do that but there is some issue with the inlining. I think this
is due to inline keyword being processed (also) as static by the compiler.

On Wed, Nov 25, 2015 at 4:29 PM, Stephen Hemminger <
stephen at networkplumber.org> wrote:

> On Wed, 25 Nov 2015 16:19:51 +
> "Alejandro.Lucero"  wrote:
>
> > +/*
> > + * 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 = >data->dev_link;
> > +
> > + if (rte_atomic64_cmpset((uint64_t *)dst, *(uint64_t *)dst,
> > + *(uint64_t *)src) == 0)
> > + return -1;
> > +
> > + return 0;
> > +}
> > +
> > +/
>
> Sigh, this code has been copied and pasted to every driver.
> Why is it not part of standard rte_ethdev code.
>

[dpdk-dev] [PATCH v8 5/9] nfp: adding link functionality

[Alejandro.Lucero]

From: "Alejandro.Lucero" 

Signed-off-by: Alejandro.Lucero 
Signed-off-by: Rolf.Neugebauer 
---
 drivers/net/nfp/nfp_net.c |   96 +
 1 file changed, 96 insertions(+)

diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index 49c71bf..a7db4bc 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -74,6 +74,7 @@
 static void nfp_net_close(struct rte_eth_dev *dev);
 static int nfp_net_configure(struct rte_eth_dev *dev);
 static int nfp_net_init(struct rte_eth_dev *eth_dev);
+static int nfp_net_link_update(struct rte_eth_dev *dev, int wait_to_complete);
 static int nfp_net_rx_fill_freelist(struct nfp_net_rxq *rxq);
 static uint32_t nfp_net_rx_queue_count(struct rte_eth_dev *dev,
   uint16_t queue_idx);
@@ -226,6 +227,57 @@ ring_dma_zone_reserve(struct rte_eth_dev *dev, const char 
*ring_name,
   NFP_MEMZONE_ALIGN);
 }

+/*
+ * 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 = >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 = >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)
 {
@@ -682,6 +734,49 @@ nfp_net_close(struct rte_eth_dev *dev)
 */
 }

+/*
+ * return 0 means link status changed, -1 means not changed
+ *
+ * Wait to complete is needed as it can take up to 9 seconds to get the Link
+ * status.
+ */
+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;
+   uint32_t nn_link_status;
+
+   PMD_DRV_LOG(DEBUG, "Link update\n");
+
+   hw = NFP_NET_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+   memset(, 0, sizeof(old));
+   nfp_net_dev_atomic_read_link_status(dev, );
+
+   nn_link_status = nn_cfg_readl(hw, NFP_NET_CFG_STS);
+
+   memset(, 0, sizeof(struct rte_eth_link));
+
+   if (nn_link_status & NFP_NET_CFG_STS_LINK)
+   link.link_status = 1;
+
+   link.link_duplex = ETH_LINK_FULL_DUPLEX;
+   /* Other cards can limit the tx and rx rate per VF */
+   link.link_speed = ETH_LINK_SPEED_40G;
+
+   if (old.link_status != link.link_status) {
+   nfp_net_dev_atomic_write_link_status(dev, );
+   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;
+}
+
 static void
 nfp_net_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 {
@@ -1895,6 +1990,7 @@ static struct eth_dev_ops nfp_net_eth_dev_ops = {
.dev_start  = nfp_net_start,
.dev_stop   = nfp_net_stop,
.dev_close  = nfp_net_close,
+   .link_update= nfp_net_link_update,
.stats_get  = nfp_net_stats_get,
.stats_reset= nfp_net_stats_reset,
.reta_update= nfp_net_reta_update,
-- 
1.7.9.5

[dpdk-dev] [PATCH v8 5/9] nfp: adding link functionality

[Stephen Hemminger]

On Wed, 25 Nov 2015 16:19:51 +
"Alejandro.Lucero"  wrote:

> +/*
> + * 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 = >data->dev_link;
> +
> + if (rte_atomic64_cmpset((uint64_t *)dst, *(uint64_t *)dst,
> + *(uint64_t *)src) == 0)
> + return -1;
> +
> + return 0;
> +}
> +
> +/

Sigh, this code has been copied and pasted to every driver.
Why is it not part of standard rte_ethdev code.