[PATCH 1/6] net/vmxnet3: retain counters on restart

[PATCH 1/6] net/vmxnet3: retain counters on restart

[Charles (Chas) Williams]

From: Nachiketa Prachanda <nprachan@brocade.com>

Most nics like virtio, igb/ixgbe etc. don't reset counters on
dev_start and arguably this helps in monitoring the counters
across a longer time span with multiple device start/stops.
vmxnet3 behavior is opposite to that and counters are reset by
the host side implementation each time the device is restarted.

Change the driver to save the counters in its private context
before it is reset by writing CMD_ACTIVATE to REG_CMD.

Signed-off-by: Nachiketa Prachanda <nprachan@brocade.com>
---
 drivers/net/vmxnet3/vmxnet3_ethdev.c | 102 ++++++++++++++++++++++++++++-------
 drivers/net/vmxnet3/vmxnet3_ethdev.h |   2 +
 2 files changed, 86 insertions(+), 18 deletions(-)

diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c
index 17b471f..4d2c024 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
@@ -85,6 +85,7 @@ static void vmxnet3_dev_allmulticast_enable(struct rte_eth_dev *dev);
 static void vmxnet3_dev_allmulticast_disable(struct rte_eth_dev *dev);
 static int vmxnet3_dev_link_update(struct rte_eth_dev *dev,
 				   int wait_to_complete);
+static void vmxnet3_hw_stats_save(struct vmxnet3_hw *hw);
 static void vmxnet3_dev_stats_get(struct rte_eth_dev *dev,
 				  struct rte_eth_stats *stats);
 static void vmxnet3_dev_info_get(struct rte_eth_dev *dev,
@@ -351,6 +352,10 @@ eth_vmxnet3_dev_init(struct rte_eth_dev *eth_dev)
 	RTE_ASSERT((hw->rxdata_desc_size & ~VMXNET3_RXDATA_DESC_SIZE_MASK) ==
 		   hw->rxdata_desc_size);
 
+	/* clear shadow stats */
+	memset(hw->saved_tx_stats, 0, sizeof(hw->saved_tx_stats));
+	memset(hw->saved_rx_stats, 0, sizeof(hw->saved_rx_stats));
+
 	return 0;
 }
 
@@ -707,6 +712,9 @@ vmxnet3_dev_start(struct rte_eth_dev *dev)
 
 	PMD_INIT_FUNC_TRACE();
 
+	/* Save stats before it is reset by CMD_ACTIVATE */
+	vmxnet3_hw_stats_save(hw);
+
 	ret = vmxnet3_setup_driver_shared(dev);
 	if (ret != VMXNET3_SUCCESS)
 		return ret;
@@ -820,47 +828,105 @@ vmxnet3_dev_close(struct rte_eth_dev *dev)
 }
 
 static void
+vmxnet3_hw_tx_stats_get(struct vmxnet3_hw *hw, unsigned int q,
+			struct UPT1_TxStats *res)
+{
+#define VMXNET3_UPDATE_TX_STAT(h, i, f, r)		\
+		((r)->f = (h)->tqd_start[(i)].stats.f +	\
+			(h)->saved_tx_stats[(i)].f)
+
+	VMXNET3_UPDATE_TX_STAT(hw, q, ucastPktsTxOK, res);
+	VMXNET3_UPDATE_TX_STAT(hw, q, mcastPktsTxOK, res);
+	VMXNET3_UPDATE_TX_STAT(hw, q, bcastPktsTxOK, res);
+	VMXNET3_UPDATE_TX_STAT(hw, q, ucastBytesTxOK, res);
+	VMXNET3_UPDATE_TX_STAT(hw, q, mcastBytesTxOK, res);
+	VMXNET3_UPDATE_TX_STAT(hw, q, bcastBytesTxOK, res);
+	VMXNET3_UPDATE_TX_STAT(hw, q, pktsTxError, res);
+	VMXNET3_UPDATE_TX_STAT(hw, q, pktsTxDiscard, res);
+
+#undef VMXNET3_UPDATE_TX_STAT
+}
+
+static void
+vmxnet3_hw_rx_stats_get(struct vmxnet3_hw *hw, unsigned int q,
+			struct UPT1_RxStats *res)
+{
+#define VMXNET3_UPDATE_RX_STAT(h, i, f, r)		\
+		((r)->f = (h)->rqd_start[(i)].stats.f +	\
+			(h)->saved_rx_stats[(i)].f)
+
+	VMXNET3_UPDATE_RX_STAT(hw, q, ucastPktsRxOK, res);
+	VMXNET3_UPDATE_RX_STAT(hw, q, mcastPktsRxOK, res);
+	VMXNET3_UPDATE_RX_STAT(hw, q, bcastPktsRxOK, res);
+	VMXNET3_UPDATE_RX_STAT(hw, q, ucastBytesRxOK, res);
+	VMXNET3_UPDATE_RX_STAT(hw, q, mcastBytesRxOK, res);
+	VMXNET3_UPDATE_RX_STAT(hw, q, bcastBytesRxOK, res);
+	VMXNET3_UPDATE_RX_STAT(hw, q, pktsRxError, res);
+	VMXNET3_UPDATE_RX_STAT(hw, q, pktsRxOutOfBuf, res);
+
+#undef VMXNET3_UPDATE_RX_STATS
+}
+
+static void
+vmxnet3_hw_stats_save(struct vmxnet3_hw *hw)
+{
+	unsigned int i;
+
+	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD, VMXNET3_CMD_GET_STATS);
+
+	RTE_BUILD_BUG_ON(RTE_ETHDEV_QUEUE_STAT_CNTRS < VMXNET3_MAX_TX_QUEUES);
+
+	for (i = 0; i < hw->num_tx_queues; i++)
+		vmxnet3_hw_tx_stats_get(hw, i, &hw->saved_tx_stats[i]);
+	for (i = 0; i < hw->num_rx_queues; i++)
+		vmxnet3_hw_rx_stats_get(hw, i, &hw->saved_rx_stats[i]);
+}
+
+static void
 vmxnet3_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 {
 	unsigned int i;
 	struct vmxnet3_hw *hw = dev->data->dev_private;
+	struct UPT1_TxStats txStats;
+	struct UPT1_RxStats rxStats;
 
 	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD, VMXNET3_CMD_GET_STATS);
 
 	RTE_BUILD_BUG_ON(RTE_ETHDEV_QUEUE_STAT_CNTRS < VMXNET3_MAX_TX_QUEUES);
 	for (i = 0; i < hw->num_tx_queues; i++) {
-		struct UPT1_TxStats *txStats = &hw->tqd_start[i].stats;
+		vmxnet3_hw_tx_stats_get(hw, i, &txStats);
+
+		stats->q_opackets[i] = txStats.ucastPktsTxOK +
+			txStats.mcastPktsTxOK +
+			txStats.bcastPktsTxOK;
 
-		stats->q_opackets[i] = txStats->ucastPktsTxOK +
-					txStats->mcastPktsTxOK +
-					txStats->bcastPktsTxOK;
-		stats->q_obytes[i] = txStats->ucastBytesTxOK +
-					txStats->mcastBytesTxOK +
-					txStats->bcastBytesTxOK;
+		stats->q_obytes[i] = txStats.ucastBytesTxOK +
+			txStats.mcastBytesTxOK +
+			txStats.bcastBytesTxOK;
 
 		stats->opackets += stats->q_opackets[i];
 		stats->obytes += stats->q_obytes[i];
-		stats->oerrors += txStats->pktsTxError + txStats->pktsTxDiscard;
+		stats->oerrors += txStats.pktsTxError + txStats.pktsTxDiscard;
 	}
 
 	RTE_BUILD_BUG_ON(RTE_ETHDEV_QUEUE_STAT_CNTRS < VMXNET3_MAX_RX_QUEUES);
 	for (i = 0; i < hw->num_rx_queues; i++) {
-		struct UPT1_RxStats *rxStats = &hw->rqd_start[i].stats;
+		vmxnet3_hw_rx_stats_get(hw, i, &rxStats);
 
-		stats->q_ipackets[i] = rxStats->ucastPktsRxOK +
-					rxStats->mcastPktsRxOK +
-					rxStats->bcastPktsRxOK;
+		stats->q_ipackets[i] = rxStats.ucastPktsRxOK +
+			rxStats.mcastPktsRxOK +
+			rxStats.bcastPktsRxOK;
 
-		stats->q_ibytes[i] = rxStats->ucastBytesRxOK +
-					rxStats->mcastBytesRxOK +
-					rxStats->bcastBytesRxOK;
+		stats->q_ibytes[i] = rxStats.ucastBytesRxOK +
+			rxStats.mcastBytesRxOK +
+			rxStats.bcastBytesRxOK;
 
 		stats->ipackets += stats->q_ipackets[i];
 		stats->ibytes += stats->q_ibytes[i];
 
-		stats->q_errors[i] = rxStats->pktsRxError;
-		stats->ierrors += rxStats->pktsRxError;
-		stats->rx_nombuf += rxStats->pktsRxOutOfBuf;
+		stats->q_errors[i] = rxStats.pktsRxError;
+		stats->ierrors += rxStats.pktsRxError;
+		stats->rx_nombuf += rxStats.pktsRxOutOfBuf;
 	}
 }
 
diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.h b/drivers/net/vmxnet3/vmxnet3_ethdev.h
index 7a03262..e93e4a7 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethdev.h
+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.h
@@ -122,6 +122,8 @@ struct vmxnet3_hw {
 	Vmxnet3_MemRegs	      *memRegs;
 	uint64_t	      memRegsPA;
 #define VMXNET3_VFT_TABLE_SIZE     (VMXNET3_VFT_SIZE * sizeof(uint32_t))
+	UPT1_TxStats	      saved_tx_stats[VMXNET3_MAX_TX_QUEUES];
+	UPT1_RxStats	      saved_rx_stats[VMXNET3_MAX_RX_QUEUES];
 };
 
 #define VMXNET3_REV_3		2		/* Vmxnet3 Rev. 3 */
-- 
2.1.4

[PATCH 2/6] net/vmxnet3: Implement retrieval of extended stats

[Charles (Chas) Williams]

From: Robert Shearman <rshearma@brocade.com>

Implement xstats_get() to allow a number of driver-specific tx and rx
stats to be retrieved.

Signed-off-by: Robert Shearman <rshearma@brocade.com>
---
 drivers/net/vmxnet3/vmxnet3_ethdev.c | 111 +++++++++++++++++++++++++++++++++++
 1 file changed, 111 insertions(+)

diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c
index 4d2c024..a3378ad 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
@@ -88,6 +88,11 @@ static int vmxnet3_dev_link_update(struct rte_eth_dev *dev,
 static void vmxnet3_hw_stats_save(struct vmxnet3_hw *hw);
 static void vmxnet3_dev_stats_get(struct rte_eth_dev *dev,
 				  struct rte_eth_stats *stats);
+static int vmxnet3_dev_xstats_get_names(struct rte_eth_dev *dev,
+					struct rte_eth_xstat_name *xstats,
+					unsigned int n);
+static int vmxnet3_dev_xstats_get(struct rte_eth_dev *dev,
+				  struct rte_eth_xstat *xstats, unsigned int n);
 static void vmxnet3_dev_info_get(struct rte_eth_dev *dev,
 				 struct rte_eth_dev_info *dev_info);
 static const uint32_t *
@@ -122,6 +127,8 @@ static const struct eth_dev_ops vmxnet3_eth_dev_ops = {
 	.allmulticast_disable = vmxnet3_dev_allmulticast_disable,
 	.link_update          = vmxnet3_dev_link_update,
 	.stats_get            = vmxnet3_dev_stats_get,
+	.xstats_get_names     = vmxnet3_dev_xstats_get_names,
+	.xstats_get           = vmxnet3_dev_xstats_get,
 	.mac_addr_set         = vmxnet3_mac_addr_set,
 	.dev_infos_get        = vmxnet3_dev_info_get,
 	.dev_supported_ptypes_get = vmxnet3_dev_supported_ptypes_get,
@@ -133,6 +140,27 @@ static const struct eth_dev_ops vmxnet3_eth_dev_ops = {
 	.tx_queue_release     = vmxnet3_dev_tx_queue_release,
 };
 
+struct vmxnet3_xstats_name_off {
+	char name[RTE_ETH_XSTATS_NAME_SIZE];
+	unsigned int offset;
+};
+
+/* tx_qX_ is prepended to the name string here */
+static const struct vmxnet3_xstats_name_off vmxnet3_txq_stat_strings[] = {
+	{"drop_total",         offsetof(struct vmxnet3_txq_stats, drop_total)},
+	{"drop_too_many_segs", offsetof(struct vmxnet3_txq_stats, drop_too_many_segs)},
+	{"drop_tso",           offsetof(struct vmxnet3_txq_stats, drop_tso)},
+	{"tx_ring_full",       offsetof(struct vmxnet3_txq_stats, tx_ring_full)},
+};
+
+/* rx_qX_ is prepended to the name string here */
+static const struct vmxnet3_xstats_name_off vmxnet3_rxq_stat_strings[] = {
+	{"drop_total",           offsetof(struct vmxnet3_rxq_stats, drop_total)},
+	{"drop_err",             offsetof(struct vmxnet3_rxq_stats, drop_err)},
+	{"drop_fcs",             offsetof(struct vmxnet3_rxq_stats, drop_fcs)},
+	{"rx_buf_alloc_failure", offsetof(struct vmxnet3_rxq_stats, rx_buf_alloc_failure)},
+};
+
 static const struct rte_memzone *
 gpa_zone_reserve(struct rte_eth_dev *dev, uint32_t size,
 		 const char *post_string, int socket_id,
@@ -882,6 +910,89 @@ vmxnet3_hw_stats_save(struct vmxnet3_hw *hw)
 		vmxnet3_hw_rx_stats_get(hw, i, &hw->saved_rx_stats[i]);
 }
 
+static int
+vmxnet3_dev_xstats_get_names(struct rte_eth_dev *dev,
+			     struct rte_eth_xstat_name *xstats_names,
+			     unsigned int n)
+{
+	unsigned int i, t, count = 0;
+	unsigned int nstats =
+		dev->data->nb_tx_queues * RTE_DIM(vmxnet3_txq_stat_strings) +
+		dev->data->nb_rx_queues * RTE_DIM(vmxnet3_rxq_stat_strings);
+
+	if (!xstats_names || n < nstats)
+		return nstats;
+
+	for (i = 0; i < dev->data->nb_rx_queues; i++) {
+		if (!dev->data->rx_queues[i])
+			continue;
+
+		for (t = 0; t < RTE_DIM(vmxnet3_rxq_stat_strings); t++) {
+			snprintf(xstats_names[count].name,
+				 sizeof(xstats_names[count].name),
+				 "rx_q%u_%s", i,
+				 vmxnet3_rxq_stat_strings[t].name);
+			count++;
+		}
+	}
+
+	for (i = 0; i < dev->data->nb_tx_queues; i++) {
+		if (!dev->data->tx_queues[i])
+			continue;
+
+		for (t = 0; t < RTE_DIM(vmxnet3_txq_stat_strings); t++) {
+			snprintf(xstats_names[count].name,
+				 sizeof(xstats_names[count].name),
+				 "tx_q%u_%s", i,
+				 vmxnet3_txq_stat_strings[t].name);
+			count++;
+		}
+	}
+
+	return count;
+}
+
+static int
+vmxnet3_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
+		       unsigned int n)
+{
+	unsigned int i, t, count = 0;
+	unsigned int nstats =
+		dev->data->nb_tx_queues * RTE_DIM(vmxnet3_txq_stat_strings) +
+		dev->data->nb_rx_queues * RTE_DIM(vmxnet3_rxq_stat_strings);
+
+	if (n < nstats)
+		return nstats;
+
+	for (i = 0; i < dev->data->nb_rx_queues; i++) {
+		struct vmxnet3_rx_queue *rxq = dev->data->rx_queues[i];
+
+		if (rxq == NULL)
+			continue;
+
+		for (t = 0; t < RTE_DIM(vmxnet3_rxq_stat_strings); t++) {
+			xstats[count].value = *(uint64_t *)(((char *)&rxq->stats) +
+				vmxnet3_rxq_stat_strings[t].offset);
+			count++;
+		}
+	}
+
+	for (i = 0; i < dev->data->nb_tx_queues; i++) {
+		struct vmxnet3_tx_queue *txq = dev->data->tx_queues[i];
+
+		if (txq == NULL)
+			continue;
+
+		for (t = 0; t < RTE_DIM(vmxnet3_txq_stat_strings); t++) {
+			xstats[count].value = *(uint64_t *)(((char *)&txq->stats) +
+				vmxnet3_txq_stat_strings[t].offset);
+			count++;
+		}
+	}
+
+	return count;
+}
+
 static void
 vmxnet3_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 {
-- 
2.1.4

Re: [PATCH 2/6] net/vmxnet3: Implement retrieval of extended stats

[Shrikrishna Khare]

On Fri, 19 May 2017, Charles (Chas) Williams wrote:

> From: Robert Shearman <rshearma@brocade.com>
> 
> Implement xstats_get() to allow a number of driver-specific tx and rx
> stats to be retrieved.
> 
> Signed-off-by: Robert Shearman <rshearma@brocade.com>

Acked-by: Shrikrishna Khare <skhare@vmware.com>

[PATCH 3/6] net/vmxnet3: Generate link-state change notifications

[Charles (Chas) Williams]

From: Robert Shearman <rshearma@brocade.com>

Generate link-state change notifications by listening to interrupts
generated by the device. Make use of the existing
vmxnet3_process_events function that was compiled out, but change it
to call vmxnet3_dev_link_update on a VMXNET3_ECR_LINK event and to not
be so noisy in its log messages.

Enable interrupts on starting the device, using a new helper function,
vmxnet3_enable_intr, based on vmxnet3_disable_intr and validated
against the FreeBSD driver.

Keep track of the number of interrupts registered for to avoid
hardcoding these in vmxnet3_enable/disable_intr and to provision for
any future rxq intr support.

Factor out the guts of vmxnet3_dev_link_update minus the started check
to allow the new function to be called from vmxnet3_dev_start in the
lsc-enabled case to ensure that the link state is correctly set from
the actual state at that point.

Signed-off-by: Robert Shearman <rshearma@brocade.com>
---
 drivers/net/vmxnet3/vmxnet3_ethdev.c | 116 +++++++++++++++++++++++++++--------
 drivers/net/vmxnet3/vmxnet3_ethdev.h |   2 +
 2 files changed, 91 insertions(+), 27 deletions(-)

diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c
index a3378ad..8f6e0fc 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
@@ -83,6 +83,8 @@ static void vmxnet3_dev_promiscuous_enable(struct rte_eth_dev *dev);
 static void vmxnet3_dev_promiscuous_disable(struct rte_eth_dev *dev);
 static void vmxnet3_dev_allmulticast_enable(struct rte_eth_dev *dev);
 static void vmxnet3_dev_allmulticast_disable(struct rte_eth_dev *dev);
+static int __vmxnet3_dev_link_update(struct rte_eth_dev *dev,
+				     int wait_to_complete);
 static int vmxnet3_dev_link_update(struct rte_eth_dev *dev,
 				   int wait_to_complete);
 static void vmxnet3_hw_stats_save(struct vmxnet3_hw *hw);
@@ -102,10 +104,8 @@ static int vmxnet3_dev_vlan_filter_set(struct rte_eth_dev *dev,
 static void vmxnet3_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask);
 static void vmxnet3_mac_addr_set(struct rte_eth_dev *dev,
 				 struct ether_addr *mac_addr);
+static void vmxnet3_interrupt_handler(void *param);
 
-#if PROCESS_SYS_EVENTS == 1
-static void vmxnet3_process_events(struct vmxnet3_hw *);
-#endif
 /*
  * The set of PCI devices this driver supports
  */
@@ -250,10 +250,22 @@ vmxnet3_disable_intr(struct vmxnet3_hw *hw)
 	PMD_INIT_FUNC_TRACE();
 
 	hw->shared->devRead.intrConf.intrCtrl |= VMXNET3_IC_DISABLE_ALL;
-	for (i = 0; i < VMXNET3_MAX_INTRS; i++)
+	for (i = 0; i < hw->num_intrs; i++)
 		VMXNET3_WRITE_BAR0_REG(hw, VMXNET3_REG_IMR + i * 8, 1);
 }
 
+static void
+vmxnet3_enable_intr(struct vmxnet3_hw *hw)
+{
+	int i;
+
+	PMD_INIT_FUNC_TRACE();
+
+	hw->shared->devRead.intrConf.intrCtrl &= ~VMXNET3_IC_DISABLE_ALL;
+	for (i = 0; i < hw->num_intrs; i++)
+		VMXNET3_WRITE_BAR0_REG(hw, VMXNET3_REG_IMR + i * 8, 0);
+}
+
 /*
  * Gets tx data ring descriptor size.
  */
@@ -425,7 +437,7 @@ static int eth_vmxnet3_pci_remove(struct rte_pci_device *pci_dev)
 
 static struct rte_pci_driver rte_vmxnet3_pmd = {
 	.id_table = pci_id_vmxnet3_map,
-	.drv_flags = RTE_PCI_DRV_NEED_MAPPING,
+	.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
 	.probe = eth_vmxnet3_pci_probe,
 	.remove = eth_vmxnet3_pci_remove,
 };
@@ -648,11 +660,11 @@ vmxnet3_setup_driver_shared(struct rte_eth_dev *dev)
 
 	/*
 	 * Set number of interrupts to 1
-	 * PMD disables all the interrupts but this is MUST to activate device
-	 * It needs at least one interrupt for link events to handle
-	 * So we'll disable it later after device activation if needed
+	 * PMD by default disables all the interrupts but this is MUST
+	 * to activate device. It needs at least one interrupt for
+	 * link events to handle
 	 */
-	devRead->intrConf.numIntrs = 1;
+	hw->num_intrs = devRead->intrConf.numIntrs = 1;
 	devRead->intrConf.intrCtrl |= VMXNET3_IC_DISABLE_ALL;
 
 	for (i = 0; i < hw->num_tx_queues; i++) {
@@ -747,6 +759,20 @@ vmxnet3_dev_start(struct rte_eth_dev *dev)
 	if (ret != VMXNET3_SUCCESS)
 		return ret;
 
+	/* check if lsc interrupt feature is enabled */
+	if (dev->data->dev_conf.intr_conf.lsc) {
+		struct rte_pci_device *pci_dev = RTE_DEV_TO_PCI(dev->device);
+
+		/* Setup interrupt callback  */
+		rte_intr_callback_register(&pci_dev->intr_handle,
+					   vmxnet3_interrupt_handler, dev);
+
+		if (rte_intr_enable(&pci_dev->intr_handle) < 0) {
+			PMD_INIT_LOG(ERR, "interrupt enable failed");
+			return -EIO;
+		}
+	}
+
 	/* Exchange shared data with device */
 	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_DSAL,
 			       VMXNET3_GET_ADDR_LO(hw->sharedPA));
@@ -794,14 +820,19 @@ vmxnet3_dev_start(struct rte_eth_dev *dev)
 	/* Setting proper Rx Mode and issue Rx Mode Update command */
 	vmxnet3_dev_set_rxmode(hw, VMXNET3_RXM_UCAST | VMXNET3_RXM_BCAST, 1);
 
-	/*
-	 * Don't need to handle events for now
-	 */
-#if PROCESS_SYS_EVENTS == 1
-	events = VMXNET3_READ_BAR1_REG(hw, VMXNET3_REG_ECR);
-	PMD_INIT_LOG(DEBUG, "Reading events: 0x%X", events);
-	vmxnet3_process_events(hw);
-#endif
+	if (dev->data->dev_conf.intr_conf.lsc) {
+		vmxnet3_enable_intr(hw);
+
+		/*
+		 * Update link state from device since this won't be
+		 * done upon starting with lsc in use. This is done
+		 * only after enabling interrupts to avoid any race
+		 * where the link state could change without an
+		 * interrupt being fired.
+		 */
+		__vmxnet3_dev_link_update(dev, 0);
+	}
+
 	return VMXNET3_SUCCESS;
 }
 
@@ -824,6 +855,15 @@ vmxnet3_dev_stop(struct rte_eth_dev *dev)
 	/* disable interrupts */
 	vmxnet3_disable_intr(hw);
 
+	if (dev->data->dev_conf.intr_conf.lsc) {
+		struct rte_pci_device *pci_dev = RTE_DEV_TO_PCI(dev->device);
+
+		rte_intr_disable(&pci_dev->intr_handle);
+
+		rte_intr_callback_unregister(&pci_dev->intr_handle,
+					     vmxnet3_interrupt_handler, dev);
+	}
+
 	/* quiesce the device first */
 	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD, VMXNET3_CMD_QUIESCE_DEV);
 	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_DSAL, 0);
@@ -1108,17 +1148,13 @@ vmxnet3_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr)
 
 /* return 0 means link status changed, -1 means not changed */
 static int
-vmxnet3_dev_link_update(struct rte_eth_dev *dev,
-			__rte_unused int wait_to_complete)
+__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;
 	uint32_t ret;
 
-	/* Link status doesn't change for stopped dev */
-	if (dev->data->dev_started == 0)
-		return -1;
-
 	memset(&link, 0, sizeof(link));
 	vmxnet3_dev_atomic_read_link_status(dev, &old);
 
@@ -1137,6 +1173,16 @@ vmxnet3_dev_link_update(struct rte_eth_dev *dev,
 	return (old.link_status == link.link_status) ? -1 : 0;
 }
 
+static int
+vmxnet3_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete)
+{
+	/* Link status doesn't change for stopped dev */
+	if (dev->data->dev_started == 0)
+		return -1;
+
+	return __vmxnet3_dev_link_update(dev, wait_to_complete);
+}
+
 /* Updating rxmode through Vmxnet3_DriverShared structure in adapter */
 static void
 vmxnet3_dev_set_rxmode(struct vmxnet3_hw *hw, uint32_t feature, int set)
@@ -1253,10 +1299,10 @@ vmxnet3_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask)
 	}
 }
 
-#if PROCESS_SYS_EVENTS == 1
 static void
-vmxnet3_process_events(struct vmxnet3_hw *hw)
+vmxnet3_process_events(struct rte_eth_dev *dev)
 {
+	struct vmxnet3_hw *hw = dev->data->dev_private;
 	uint32_t events = hw->shared->ecr;
 
 	if (!events) {
@@ -1271,10 +1317,15 @@ vmxnet3_process_events(struct vmxnet3_hw *hw)
 	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_ECR, events);
 
 	/* Check if link state has changed */
-	if (events & VMXNET3_ECR_LINK)
+	if (events & VMXNET3_ECR_LINK) {
 		PMD_INIT_LOG(ERR,
 			     "Process events in %s(): VMXNET3_ECR_LINK event",
 			     __func__);
+		if (vmxnet3_dev_link_update(dev, 0) == 0)
+			_rte_eth_dev_callback_process(dev,
+						      RTE_ETH_EVENT_INTR_LSC,
+						      NULL);
+	}
 
 	/* Check if there is an error on xmit/recv queues */
 	if (events & (VMXNET3_ECR_TQERR | VMXNET3_ECR_RQERR)) {
@@ -1299,7 +1350,18 @@ vmxnet3_process_events(struct vmxnet3_hw *hw)
 	if (events & VMXNET3_ECR_DEBUG)
 		PMD_INIT_LOG(ERR, "Debug event generated by device.");
 }
-#endif
+
+static void
+vmxnet3_interrupt_handler(void *param)
+{
+	struct rte_eth_dev *dev = param;
+	struct rte_pci_device *pci_dev = RTE_DEV_TO_PCI(dev->device);
+
+	vmxnet3_process_events(dev);
+
+	if (rte_intr_enable(&pci_dev->intr_handle) < 0)
+		PMD_DRV_LOG(ERR, "interrupt enable failed");
+}
 
 RTE_PMD_REGISTER_PCI(net_vmxnet3, rte_vmxnet3_pmd);
 RTE_PMD_REGISTER_PCI_TABLE(net_vmxnet3, pci_id_vmxnet3_map);
diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.h b/drivers/net/vmxnet3/vmxnet3_ethdev.h
index e93e4a7..b48058a 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethdev.h
+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.h
@@ -106,6 +106,8 @@ struct vmxnet3_hw {
 	uint16_t txdata_desc_size; /* tx data ring buffer size */
 	uint16_t rxdata_desc_size; /* rx data ring buffer size */
 
+	uint8_t num_intrs;
+
 	Vmxnet3_TxQueueDesc   *tqd_start;	/* start address of all tx queue desc */
 	Vmxnet3_RxQueueDesc   *rqd_start;	/* start address of all rx queue desc */
 
-- 
2.1.4

[PATCH 4/6] net/vmxnet3: Make vmxnet3_process_events less noisy

[Charles (Chas) Williams]

From: Robert Shearman <rshearma@brocade.com>

Make vmxnet3_process_events less noisy by removing logging when there
are no events to process and by making link, device-change and debug
events DEBUG level rather than ERR.

Change these to use PMD_DRV_LOG instead of PMD_INIT_LOG since they
don't happen at device init.

Signed-off-by: Robert Shearman <rshearma@brocade.com>
---
 drivers/net/vmxnet3/vmxnet3_ethdev.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c
index 8f6e0fc..d241499 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
@@ -1305,10 +1305,8 @@ vmxnet3_process_events(struct rte_eth_dev *dev)
 	struct vmxnet3_hw *hw = dev->data->dev_private;
 	uint32_t events = hw->shared->ecr;
 
-	if (!events) {
-		PMD_INIT_LOG(ERR, "No events to process");
+	if (!events)
 		return;
-	}
 
 	/*
 	 * ECR bits when written with 1b are cleared. Hence write
@@ -1318,9 +1316,7 @@ vmxnet3_process_events(struct rte_eth_dev *dev)
 
 	/* Check if link state has changed */
 	if (events & VMXNET3_ECR_LINK) {
-		PMD_INIT_LOG(ERR,
-			     "Process events in %s(): VMXNET3_ECR_LINK event",
-			     __func__);
+		PMD_DRV_LOG(DEBUG, "Process events: VMXNET3_ECR_LINK event");
 		if (vmxnet3_dev_link_update(dev, 0) == 0)
 			_rte_eth_dev_callback_process(dev,
 						      RTE_ETH_EVENT_INTR_LSC,
@@ -1333,11 +1329,11 @@ vmxnet3_process_events(struct rte_eth_dev *dev)
 				       VMXNET3_CMD_GET_QUEUE_STATUS);
 
 		if (hw->tqd_start->status.stopped)
-			PMD_INIT_LOG(ERR, "tq error 0x%x",
-				     hw->tqd_start->status.error);
+			PMD_DRV_LOG(ERR, "tq error 0x%x",
+				    hw->tqd_start->status.error);
 
 		if (hw->rqd_start->status.stopped)
-			PMD_INIT_LOG(ERR, "rq error 0x%x",
+			PMD_DRV_LOG(ERR, "rq error 0x%x",
 				     hw->rqd_start->status.error);
 
 		/* Reset the device */
@@ -1345,10 +1341,10 @@ vmxnet3_process_events(struct rte_eth_dev *dev)
 	}
 
 	if (events & VMXNET3_ECR_DIC)
-		PMD_INIT_LOG(ERR, "Device implementation change event.");
+		PMD_DRV_LOG(DEBUG, "Device implementation change event.");
 
 	if (events & VMXNET3_ECR_DEBUG)
-		PMD_INIT_LOG(ERR, "Debug event generated by device.");
+		PMD_DRV_LOG(DEBUG, "Debug event generated by device.");
 }
 
 static void
-- 
2.1.4

Re: [PATCH 4/6] net/vmxnet3: Make vmxnet3_process_events less noisy

[Shrikrishna Khare]

On Fri, 19 May 2017, Charles (Chas) Williams wrote:

> From: Robert Shearman <rshearma@brocade.com>
> 
> Make vmxnet3_process_events less noisy by removing logging when there
> are no events to process and by making link, device-change and debug
> events DEBUG level rather than ERR.
> 
> Change these to use PMD_DRV_LOG instead of PMD_INIT_LOG since they
> don't happen at device init.
> 
> Signed-off-by: Robert Shearman <rshearma@brocade.com>

Acked-by: Shrikrishna Khare <skhare@vmware.com>

[PATCH 5/6] net/vmxnet3: receive queue lockup and memleak

[Charles (Chas) Williams]

From: Mandeep Rohilla <mrohilla@brocade.com>

The receive queue can lockup if all the rx descriptors have lost
their mbufs and temporarily there are no mbufs available. This
can happen if there is an mbuf leak or if the application holds
on to the mbuf for a while.

This also addresses an mbuf leak in an error condition during
packet receive.

Signed-off-by: Mandeep Rohilla <mrohilla@brocade.com>
---
 drivers/net/vmxnet3/vmxnet3_rxtx.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/net/vmxnet3/vmxnet3_rxtx.c b/drivers/net/vmxnet3/vmxnet3_rxtx.c
index d8713a1..d21679d 100644
--- a/drivers/net/vmxnet3/vmxnet3_rxtx.c
+++ b/drivers/net/vmxnet3/vmxnet3_rxtx.c
@@ -731,6 +731,7 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 	uint16_t nb_rx;
 	uint32_t nb_rxd, idx;
 	uint8_t ring_idx;
+	uint8_t i;
 	vmxnet3_rx_queue_t *rxq;
 	Vmxnet3_RxCompDesc *rcd;
 	vmxnet3_buf_info_t *rbi;
@@ -800,6 +801,12 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 				   (int)(rcd - (struct Vmxnet3_RxCompDesc *)
 					 rxq->comp_ring.base), rcd->rxdIdx);
 			rte_pktmbuf_free_seg(rxm);
+			if (rxq->start_seg) {
+				struct rte_mbuf *start = rxq->start_seg;
+
+				rxq->start_seg = NULL;
+				rte_pktmbuf_free(start);
+			}
 			goto rcd_done;
 		}
 
@@ -893,6 +900,18 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 		}
 	}
 
+	/*
+	 * Try to replenish the rx descriptors with the new mbufs
+	 */
+	for (i = 0; i < VMXNET3_RX_CMDRING_SIZE; i++) {
+		vmxnet3_post_rx_bufs(rxq, i);
+		if (unlikely(rxq->shared->ctrl.updateRxProd)) {
+			VMXNET3_WRITE_BAR0_REG(hw,
+				rxprod_reg[i] +
+					(rxq->queue_id * VMXNET3_REG_ALIGN),
+				rxq->cmd_ring[i].next2fill);
+		}
+	}
 	return nb_rx;
 }
 
-- 
2.1.4

Re: [PATCH 5/6] net/vmxnet3: receive queue lockup and memleak

[Charles (Chas) Williams]

While looking at another issue, I think one of the issues fixed in this
commit has already been fixed in the last DPDK release by:

	commit 8fce14b789aecdb4345a62f6980e7b6e7f4ba245
	Author: Stefan Puiu <stefan.puiu@gmail.com>
	Date:   Mon Dec 19 11:40:53 2016 +0200

	    net/vmxnet3: fix Rx deadlock

	    Our use case is that we have an app that needs to keep mbufs around
	    for a while. We've seen cases when calling vmxnet3_post_rx_bufs() from
	    vmxet3_recv_pkts(), it might not succeed to add any mbufs to any RX
	    descriptors (where it returns -err). Since there are no mbufs that the
	    virtual hardware can use, no packets will be received after this; the
	    driver won't refill the mbuf after this so it gets stuck in this
	    state. I call this a deadlock for lack of a better term - the virtual
	    HW waits for free mbufs, while the app waits for the hardware to
	    notify it for data (by flipping the generation bit on the used Rx
	    descriptors). Note that after this, the app can't recover.
	...

The mbuf leak due to an error during receive still exists.  That can be
refactored into a new commit.

On 05/19/2017 01:55 PM, Charles (Chas) Williams wrote:
> From: Mandeep Rohilla <mrohilla@brocade.com>
>
> The receive queue can lockup if all the rx descriptors have lost
> their mbufs and temporarily there are no mbufs available. This
> can happen if there is an mbuf leak or if the application holds
> on to the mbuf for a while.
>
> This also addresses an mbuf leak in an error condition during
> packet receive.
>
> Signed-off-by: Mandeep Rohilla <mrohilla@brocade.com>
> ---
>  drivers/net/vmxnet3/vmxnet3_rxtx.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
>
> diff --git a/drivers/net/vmxnet3/vmxnet3_rxtx.c b/drivers/net/vmxnet3/vmxnet3_rxtx.c
> index d8713a1..d21679d 100644
> --- a/drivers/net/vmxnet3/vmxnet3_rxtx.c
> +++ b/drivers/net/vmxnet3/vmxnet3_rxtx.c
> @@ -731,6 +731,7 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
>  	uint16_t nb_rx;
>  	uint32_t nb_rxd, idx;
>  	uint8_t ring_idx;
> +	uint8_t i;
>  	vmxnet3_rx_queue_t *rxq;
>  	Vmxnet3_RxCompDesc *rcd;
>  	vmxnet3_buf_info_t *rbi;
> @@ -800,6 +801,12 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
>  				   (int)(rcd - (struct Vmxnet3_RxCompDesc *)
>  					 rxq->comp_ring.base), rcd->rxdIdx);
>  			rte_pktmbuf_free_seg(rxm);
> +			if (rxq->start_seg) {
> +				struct rte_mbuf *start = rxq->start_seg;
> +
> +				rxq->start_seg = NULL;
> +				rte_pktmbuf_free(start);
> +			}
>  			goto rcd_done;
>  		}
>
> @@ -893,6 +900,18 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
>  		}
>  	}
>
> +	/*
> +	 * Try to replenish the rx descriptors with the new mbufs
> +	 */
> +	for (i = 0; i < VMXNET3_RX_CMDRING_SIZE; i++) {
> +		vmxnet3_post_rx_bufs(rxq, i);
> +		if (unlikely(rxq->shared->ctrl.updateRxProd)) {
> +			VMXNET3_WRITE_BAR0_REG(hw,
> +				rxprod_reg[i] +
> +					(rxq->queue_id * VMXNET3_REG_ALIGN),
> +				rxq->cmd_ring[i].next2fill);
> +		}
> +	}
>  	return nb_rx;
>  }
>
>

Re: [PATCH 1/6] net/vmxnet3: retain counters on restart

[Shrikrishna Khare]

On Fri, 19 May 2017, Charles (Chas) Williams wrote:

> From: Nachiketa Prachanda <nprachan@brocade.com>
> 
> Most nics like virtio, igb/ixgbe etc. don't reset counters on
> dev_start and arguably this helps in monitoring the counters
> across a longer time span with multiple device start/stops.
> vmxnet3 behavior is opposite to that and counters are reset by
> the host side implementation each time the device is restarted.
> 
> Change the driver to save the counters in its private context
> before it is reset by writing CMD_ACTIVATE to REG_CMD.
> 
> Signed-off-by: Nachiketa Prachanda <nprachan@brocade.com>

This won't be able to deal with vMotion or suspend/resume?

Re: [PATCH 1/6] net/vmxnet3: retain counters on restart

[Nachi Prachanda]

> From: Shrikrishna Khare [mailto:skhare@shri-linux.eng.vmware.com]
> Sent: Wednesday, May 24, 2017 2:10 PM
> 
> On Fri, 19 May 2017, Charles (Chas) Williams wrote:
> 
> > From: Nachiketa Prachanda <nprachan@brocade.com>
> >
> > Most nics like virtio, igb/ixgbe etc. don't reset counters on
> > dev_start and arguably this helps in monitoring the counters across a
> > longer time span with multiple device start/stops.
> > vmxnet3 behavior is opposite to that and counters are reset by the
> > host side implementation each time the device is restarted.
> >
> > Change the driver to save the counters in its private context before
> > it is reset by writing CMD_ACTIVATE to REG_CMD.
> >
> > Signed-off-by: Nachiketa Prachanda <nprachan@brocade.com>
> 
> This won't be able to deal with vMotion or suspend/resume?

Correct - this can't deal with the VM suspend/resume unless hypervisor maintains the counter. But this patch doesn't make that behavior any worse than what it was before.

Re: [PATCH 1/6] net/vmxnet3: retain counters on restart

[Shrikrishna Khare]

On Thu, 25 May 2017, Nachi Prachanda wrote:

> > From: Shrikrishna Khare [mailto:skhare@shri-linux.eng.vmware.com]
> > Sent: Wednesday, May 24, 2017 2:10 PM
> > 
> > On Fri, 19 May 2017, Charles (Chas) Williams wrote:
> > 
> > > From: Nachiketa Prachanda <nprachan@brocade.com>
> > >
> > > Most nics like virtio, igb/ixgbe etc. don't reset counters on
> > > dev_start and arguably this helps in monitoring the counters across a
> > > longer time span with multiple device start/stops.
> > > vmxnet3 behavior is opposite to that and counters are reset by the
> > > host side implementation each time the device is restarted.
> > >
> > > Change the driver to save the counters in its private context before
> > > it is reset by writing CMD_ACTIVATE to REG_CMD.
> > >
> > > Signed-off-by: Nachiketa Prachanda <nprachan@brocade.com>
> > 
> > This won't be able to deal with vMotion or suspend/resume?
> 
> Correct - this can't deal with the VM suspend/resume unless hypervisor maintains the counter. But this patch doesn't make that behavior any worse than what it was before.

The current code always resets stats, but am concerned that this patch 
will make the behavior inconsistent for cases like suspend/resume.

Wondering if this will be better handled by the device emulation instead 
of the driver (for igb/ixgbe, is this handled by the hardware?).

If we were to handle this in the device emulation, what would be the 
goals/requirements:
 - device start/stop should not reset stats?
 - any other operations where we would like to maintain/reset stats?
 - what might be the expectation around how accurate the stats need to be?
 - any other requirement on the device?

Also, note that if we proceed with this patch, and later extend device 
support to not reset stats, driver with this patch running on the extended 
device will report incorrect stats.

Thanks,
Shri

Re: [PATCH 1/6] net/vmxnet3: retain counters on restart

[Nachi Prachanda]

> From: Shrikrishna Khare [mailto:skhare@shri-linux.eng.vmware.com]
> Sent: Thursday, May 25, 2017 1:27 PM
> 
> On Thu, 25 May 2017, Nachi Prachanda wrote:
> 
> > > From: Shrikrishna Khare [mailto:skhare@shri-linux.eng.vmware.com]
> > > Sent: Wednesday, May 24, 2017 2:10 PM
> > >
> > > On Fri, 19 May 2017, Charles (Chas) Williams wrote:
> > >
> > > > From: Nachiketa Prachanda <nprachan@brocade.com>
> > > >
> > > > Most nics like virtio, igb/ixgbe etc. don't reset counters on
> > > > dev_start and arguably this helps in monitoring the counters
> > > > across a longer time span with multiple device start/stops.
> > > > vmxnet3 behavior is opposite to that and counters are reset by the
> > > > host side implementation each time the device is restarted.
> > > >
> > > > Change the driver to save the counters in its private context
> > > > before it is reset by writing CMD_ACTIVATE to REG_CMD.
> > > >
> > > > Signed-off-by: Nachiketa Prachanda <nprachan@brocade.com>
> > >
> > > This won't be able to deal with vMotion or suspend/resume?
> >
> > Correct - this can't deal with the VM suspend/resume unless hypervisor
> maintains the counter. But this patch doesn't make that behavior any worse
> than what it was before.
> 
> The current code always resets stats, but am concerned that this patch will
> make the behavior inconsistent for cases like suspend/resume.
> 
> Wondering if this will be better handled by the device emulation instead of the
> driver (for igb/ixgbe, is this handled by the hardware?).

A little more nuanced.. - see below.

> If we were to handle this in the device emulation, what would be the
> goals/requirements:
>  - device start/stop should not reset stats?
>  - any other operations where we would like to maintain/reset stats?
>  - what might be the expectation around how accurate the stats need to be?
>  - any other requirement on the device?

I haven't thought about dealing it at the emulation layer - but the expectation
would be not clear counters not cleared for the lifetime of the device - and have
a way to clear them from the driver when needed.

don't know if there is a standard behavior about resetting counters. But
for igb/ixgb the counters are read/clear registers and they are maintained
at driver. May not be always accurate if the hardware is reset without updating
the driver's counter - but at least ensures that it is monotonically increasing since
on each read driver only gets the delta.

virtio emulation doesn't provide the counters - mostly the receive/send functions
updates the counters. 

> 
> Also, note that if we proceed with this patch, and later extend device support
> to not reset stats, driver with this patch running on the extended device will
> report incorrect stats.

Agree - it will need some work if the emulation changes.

Regards,
Nachi

> Thanks,
> Shri

Re: [PATCH 1/6] net/vmxnet3: retain counters on restart

[Shrikrishna Khare]

On Thu, 25 May 2017, Nachi Prachanda wrote:

> > From: Shrikrishna Khare [mailto:skhare@shri-linux.eng.vmware.com]
> > Sent: Thursday, May 25, 2017 1:27 PM
> > 
> > On Thu, 25 May 2017, Nachi Prachanda wrote:
> > 
> > > > From: Shrikrishna Khare [mailto:skhare@shri-linux.eng.vmware.com]
> > > > Sent: Wednesday, May 24, 2017 2:10 PM
> > > >
> > > > On Fri, 19 May 2017, Charles (Chas) Williams wrote:
> > > >
> > > > > From: Nachiketa Prachanda <nprachan@brocade.com>
> > > > >
> > > > > Most nics like virtio, igb/ixgbe etc. don't reset counters on
> > > > > dev_start and arguably this helps in monitoring the counters
> > > > > across a longer time span with multiple device start/stops.
> > > > > vmxnet3 behavior is opposite to that and counters are reset by the
> > > > > host side implementation each time the device is restarted.
> > > > >
> > > > > Change the driver to save the counters in its private context
> > > > > before it is reset by writing CMD_ACTIVATE to REG_CMD.
> > > > >
> > > > > Signed-off-by: Nachiketa Prachanda <nprachan@brocade.com>
> > > >
> > > > This won't be able to deal with vMotion or suspend/resume?
> > >
> > > Correct - this can't deal with the VM suspend/resume unless hypervisor
> > maintains the counter. But this patch doesn't make that behavior any worse
> > than what it was before.
> > 
> > The current code always resets stats, but am concerned that this patch will
> > make the behavior inconsistent for cases like suspend/resume.
> > 
> > Wondering if this will be better handled by the device emulation instead of the
> > driver (for igb/ixgbe, is this handled by the hardware?).
> 
> A little more nuanced.. - see below.
> 
> > If we were to handle this in the device emulation, what would be the
> > goals/requirements:
> >  - device start/stop should not reset stats?
> >  - any other operations where we would like to maintain/reset stats?
> >  - what might be the expectation around how accurate the stats need to be?
> >  - any other requirement on the device?
> 
> I haven't thought about dealing it at the emulation layer - but the expectation
> would be not clear counters not cleared for the lifetime of the device - and have
> a way to clear them from the driver when needed.
> 
> don't know if there is a standard behavior about resetting counters. But
> for igb/ixgb the counters are read/clear registers and they are maintained
> at driver. May not be always accurate if the hardware is reset without updating
> the driver's counter - but at least ensures that it is monotonically increasing since
> on each read driver only gets the delta.
> 
> virtio emulation doesn't provide the counters - mostly the receive/send functions
> updates the counters. 
> 
> > 
> > Also, note that if we proceed with this patch, and later extend device support
> > to not reset stats, driver with this patch running on the extended device will
> > report incorrect stats.
> 
> Agree - it will need some work if the emulation changes.

I spent some time looking at the emulation code. It turns out that these 
stats are already retained across vMotion, suspend/resume. And we have no 
immediate plan to change the device behavior to retain these stats across 
device start/stop. So am fine with this patch. Thank you for being patient 
with this.

Thanks,
Shri


> 
> Regards,
> Nachi
> 
> > Thanks,
> > Shri
> 

Re: [PATCH 1/6] net/vmxnet3: retain counters on restart

[Nachi Prachanda]

> -----Original Message-----
> From: Shrikrishna Khare [mailto:skhare@shri-linux.eng.vmware.com]
> Sent: Friday, May 26, 2017 10:29 AM
> To: Nachi Prachanda
> Cc: skhare@vmware.com; Chas Williams III; dev@dpdk.org
> Subject: RE: [PATCH 1/6] net/vmxnet3: retain counters on restart
> 
> 
> 
> On Thu, 25 May 2017, Nachi Prachanda wrote:
> 
> > > From: Shrikrishna Khare [mailto:skhare@shri-linux.eng.vmware.com]
> > > Sent: Thursday, May 25, 2017 1:27 PM
> > >
> > > On Thu, 25 May 2017, Nachi Prachanda wrote:
> > >
> > > > > From: Shrikrishna Khare
> > > > > [mailto:skhare@shri-linux.eng.vmware.com]
> > > > > Sent: Wednesday, May 24, 2017 2:10 PM
> > > > >
> > > > > On Fri, 19 May 2017, Charles (Chas) Williams wrote:
> > > > >
> > > > > > From: Nachiketa Prachanda <nprachan@brocade.com>
> > > > > >
> > > > > > Most nics like virtio, igb/ixgbe etc. don't reset counters on
> > > > > > dev_start and arguably this helps in monitoring the counters
> > > > > > across a longer time span with multiple device start/stops.
> > > > > > vmxnet3 behavior is opposite to that and counters are reset by
> > > > > > the host side implementation each time the device is restarted.
> > > > > >
> > > > > > Change the driver to save the counters in its private context
> > > > > > before it is reset by writing CMD_ACTIVATE to REG_CMD.
> > > > > >
> > > > > > Signed-off-by: Nachiketa Prachanda <nprachan@brocade.com>
> > > > >
> > > > > This won't be able to deal with vMotion or suspend/resume?
> > > >
> > > > Correct - this can't deal with the VM suspend/resume unless
> > > > hypervisor
> > > maintains the counter. But this patch doesn't make that behavior any
> > > worse than what it was before.
> > >
> > > The current code always resets stats, but am concerned that this
> > > patch will make the behavior inconsistent for cases like suspend/resume.
> > >
> > > Wondering if this will be better handled by the device emulation
> > > instead of the driver (for igb/ixgbe, is this handled by the hardware?).
> >
> > A little more nuanced.. - see below.
> >
> > > If we were to handle this in the device emulation, what would be the
> > > goals/requirements:
> > >  - device start/stop should not reset stats?
> > >  - any other operations where we would like to maintain/reset stats?
> > >  - what might be the expectation around how accurate the stats need to
> be?
> > >  - any other requirement on the device?
> >
> > I haven't thought about dealing it at the emulation layer - but the
> > expectation would be not clear counters not cleared for the lifetime
> > of the device - and have a way to clear them from the driver when needed.
> >
> > don't know if there is a standard behavior about resetting counters.
> > But for igb/ixgb the counters are read/clear registers and they are
> > maintained at driver. May not be always accurate if the hardware is
> > reset without updating the driver's counter - but at least ensures
> > that it is monotonically increasing since on each read driver only gets the
> delta.
> >
> > virtio emulation doesn't provide the counters - mostly the
> > receive/send functions updates the counters.
> >
> > >
> > > Also, note that if we proceed with this patch, and later extend
> > > device support to not reset stats, driver with this patch running on
> > > the extended device will report incorrect stats.
> >
> > Agree - it will need some work if the emulation changes.
> 
> I spent some time looking at the emulation code. It turns out that these stats
> are already retained across vMotion, suspend/resume. And we have no
> immediate plan to change the device behavior to retain these stats across
> device start/stop. So am fine with this patch. Thank you for being patient with
> this.

Thanks for checking this out.
Nachi

> 
> Thanks,
> Shri
> 
> 
> >
> > Regards,
> > Nachi
> >
> > > Thanks,
> > > Shri
> >

Re: [PATCH 1/6] net/vmxnet3: retain counters on restart

[Shrikrishna Khare]

On Fri, 19 May 2017, Charles (Chas) Williams wrote:

> From: Nachiketa Prachanda <nprachan@brocade.com>
> 
> Most nics like virtio, igb/ixgbe etc. don't reset counters on
> dev_start and arguably this helps in monitoring the counters
> across a longer time span with multiple device start/stops.
> vmxnet3 behavior is opposite to that and counters are reset by
> the host side implementation each time the device is restarted.
> 
> Change the driver to save the counters in its private context
> before it is reset by writing CMD_ACTIVATE to REG_CMD.
> 
> Signed-off-by: Nachiketa Prachanda <nprachan@brocade.com>

Acked-by: Shrikrishna Khare <skhare@vmware.com>

[PATCH V2 0/6] some local vmxnet3 patches

[Charles (Chas) Williams]

This series addresses some local issues with the vmxnet3 driver that
others might find of interest.

Changes in v2:

- net/vmxnet3: Implement retrieval of extended stats

    .id field wasn't being filled in xstats.

- net/vmxnet3: receive queue memory leak

    The leak and buffer reallcation were split.  The buffer reallocation
    patch may be superceded by another commit so isn't included in this
    series while under discussion.

[PATCH v2 1/6] net/vmxnet3: retain counters on restart

[Charles (Chas) Williams]

From: Nachiketa Prachanda <nprachan@brocade.com>

Most nics like virtio, igb/ixgbe etc. don't reset counters on
dev_start and arguably this helps in monitoring the counters
across a longer time span with multiple device start/stops.
vmxnet3 behavior is opposite to that and counters are reset by
the host side implementation each time the device is restarted.

Change the driver to save the counters in its private context
before it is reset by writing CMD_ACTIVATE to REG_CMD.

Signed-off-by: Nachiketa Prachanda <nprachan@brocade.com>
Acked-by: Shrikrishna Khare <skhare@vmware.com>
---
 drivers/net/vmxnet3/vmxnet3_ethdev.c | 102 ++++++++++++++++++++++++++++-------
 drivers/net/vmxnet3/vmxnet3_ethdev.h |   2 +
 2 files changed, 86 insertions(+), 18 deletions(-)

diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c
index 292a671..1301f50 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
@@ -85,6 +85,7 @@ static void vmxnet3_dev_allmulticast_enable(struct rte_eth_dev *dev);
 static void vmxnet3_dev_allmulticast_disable(struct rte_eth_dev *dev);
 static int vmxnet3_dev_link_update(struct rte_eth_dev *dev,
 				   int wait_to_complete);
+static void vmxnet3_hw_stats_save(struct vmxnet3_hw *hw);
 static void vmxnet3_dev_stats_get(struct rte_eth_dev *dev,
 				  struct rte_eth_stats *stats);
 static void vmxnet3_dev_info_get(struct rte_eth_dev *dev,
@@ -351,6 +352,10 @@ eth_vmxnet3_dev_init(struct rte_eth_dev *eth_dev)
 	RTE_ASSERT((hw->rxdata_desc_size & ~VMXNET3_RXDATA_DESC_SIZE_MASK) ==
 		   hw->rxdata_desc_size);
 
+	/* clear shadow stats */
+	memset(hw->saved_tx_stats, 0, sizeof(hw->saved_tx_stats));
+	memset(hw->saved_rx_stats, 0, sizeof(hw->saved_rx_stats));
+
 	return 0;
 }
 
@@ -707,6 +712,9 @@ vmxnet3_dev_start(struct rte_eth_dev *dev)
 
 	PMD_INIT_FUNC_TRACE();
 
+	/* Save stats before it is reset by CMD_ACTIVATE */
+	vmxnet3_hw_stats_save(hw);
+
 	ret = vmxnet3_setup_driver_shared(dev);
 	if (ret != VMXNET3_SUCCESS)
 		return ret;
@@ -820,47 +828,105 @@ vmxnet3_dev_close(struct rte_eth_dev *dev)
 }
 
 static void
+vmxnet3_hw_tx_stats_get(struct vmxnet3_hw *hw, unsigned int q,
+			struct UPT1_TxStats *res)
+{
+#define VMXNET3_UPDATE_TX_STAT(h, i, f, r)		\
+		((r)->f = (h)->tqd_start[(i)].stats.f +	\
+			(h)->saved_tx_stats[(i)].f)
+
+	VMXNET3_UPDATE_TX_STAT(hw, q, ucastPktsTxOK, res);
+	VMXNET3_UPDATE_TX_STAT(hw, q, mcastPktsTxOK, res);
+	VMXNET3_UPDATE_TX_STAT(hw, q, bcastPktsTxOK, res);
+	VMXNET3_UPDATE_TX_STAT(hw, q, ucastBytesTxOK, res);
+	VMXNET3_UPDATE_TX_STAT(hw, q, mcastBytesTxOK, res);
+	VMXNET3_UPDATE_TX_STAT(hw, q, bcastBytesTxOK, res);
+	VMXNET3_UPDATE_TX_STAT(hw, q, pktsTxError, res);
+	VMXNET3_UPDATE_TX_STAT(hw, q, pktsTxDiscard, res);
+
+#undef VMXNET3_UPDATE_TX_STAT
+}
+
+static void
+vmxnet3_hw_rx_stats_get(struct vmxnet3_hw *hw, unsigned int q,
+			struct UPT1_RxStats *res)
+{
+#define VMXNET3_UPDATE_RX_STAT(h, i, f, r)		\
+		((r)->f = (h)->rqd_start[(i)].stats.f +	\
+			(h)->saved_rx_stats[(i)].f)
+
+	VMXNET3_UPDATE_RX_STAT(hw, q, ucastPktsRxOK, res);
+	VMXNET3_UPDATE_RX_STAT(hw, q, mcastPktsRxOK, res);
+	VMXNET3_UPDATE_RX_STAT(hw, q, bcastPktsRxOK, res);
+	VMXNET3_UPDATE_RX_STAT(hw, q, ucastBytesRxOK, res);
+	VMXNET3_UPDATE_RX_STAT(hw, q, mcastBytesRxOK, res);
+	VMXNET3_UPDATE_RX_STAT(hw, q, bcastBytesRxOK, res);
+	VMXNET3_UPDATE_RX_STAT(hw, q, pktsRxError, res);
+	VMXNET3_UPDATE_RX_STAT(hw, q, pktsRxOutOfBuf, res);
+
+#undef VMXNET3_UPDATE_RX_STATS
+}
+
+static void
+vmxnet3_hw_stats_save(struct vmxnet3_hw *hw)
+{
+	unsigned int i;
+
+	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD, VMXNET3_CMD_GET_STATS);
+
+	RTE_BUILD_BUG_ON(RTE_ETHDEV_QUEUE_STAT_CNTRS < VMXNET3_MAX_TX_QUEUES);
+
+	for (i = 0; i < hw->num_tx_queues; i++)
+		vmxnet3_hw_tx_stats_get(hw, i, &hw->saved_tx_stats[i]);
+	for (i = 0; i < hw->num_rx_queues; i++)
+		vmxnet3_hw_rx_stats_get(hw, i, &hw->saved_rx_stats[i]);
+}
+
+static void
 vmxnet3_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 {
 	unsigned int i;
 	struct vmxnet3_hw *hw = dev->data->dev_private;
+	struct UPT1_TxStats txStats;
+	struct UPT1_RxStats rxStats;
 
 	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD, VMXNET3_CMD_GET_STATS);
 
 	RTE_BUILD_BUG_ON(RTE_ETHDEV_QUEUE_STAT_CNTRS < VMXNET3_MAX_TX_QUEUES);
 	for (i = 0; i < hw->num_tx_queues; i++) {
-		struct UPT1_TxStats *txStats = &hw->tqd_start[i].stats;
+		vmxnet3_hw_tx_stats_get(hw, i, &txStats);
+
+		stats->q_opackets[i] = txStats.ucastPktsTxOK +
+			txStats.mcastPktsTxOK +
+			txStats.bcastPktsTxOK;
 
-		stats->q_opackets[i] = txStats->ucastPktsTxOK +
-					txStats->mcastPktsTxOK +
-					txStats->bcastPktsTxOK;
-		stats->q_obytes[i] = txStats->ucastBytesTxOK +
-					txStats->mcastBytesTxOK +
-					txStats->bcastBytesTxOK;
+		stats->q_obytes[i] = txStats.ucastBytesTxOK +
+			txStats.mcastBytesTxOK +
+			txStats.bcastBytesTxOK;
 
 		stats->opackets += stats->q_opackets[i];
 		stats->obytes += stats->q_obytes[i];
-		stats->oerrors += txStats->pktsTxError + txStats->pktsTxDiscard;
+		stats->oerrors += txStats.pktsTxError + txStats.pktsTxDiscard;
 	}
 
 	RTE_BUILD_BUG_ON(RTE_ETHDEV_QUEUE_STAT_CNTRS < VMXNET3_MAX_RX_QUEUES);
 	for (i = 0; i < hw->num_rx_queues; i++) {
-		struct UPT1_RxStats *rxStats = &hw->rqd_start[i].stats;
+		vmxnet3_hw_rx_stats_get(hw, i, &rxStats);
 
-		stats->q_ipackets[i] = rxStats->ucastPktsRxOK +
-					rxStats->mcastPktsRxOK +
-					rxStats->bcastPktsRxOK;
+		stats->q_ipackets[i] = rxStats.ucastPktsRxOK +
+			rxStats.mcastPktsRxOK +
+			rxStats.bcastPktsRxOK;
 
-		stats->q_ibytes[i] = rxStats->ucastBytesRxOK +
-					rxStats->mcastBytesRxOK +
-					rxStats->bcastBytesRxOK;
+		stats->q_ibytes[i] = rxStats.ucastBytesRxOK +
+			rxStats.mcastBytesRxOK +
+			rxStats.bcastBytesRxOK;
 
 		stats->ipackets += stats->q_ipackets[i];
 		stats->ibytes += stats->q_ibytes[i];
 
-		stats->q_errors[i] = rxStats->pktsRxError;
-		stats->ierrors += rxStats->pktsRxError;
-		stats->rx_nombuf += rxStats->pktsRxOutOfBuf;
+		stats->q_errors[i] = rxStats.pktsRxError;
+		stats->ierrors += rxStats.pktsRxError;
+		stats->rx_nombuf += rxStats.pktsRxOutOfBuf;
 	}
 }
 
diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.h b/drivers/net/vmxnet3/vmxnet3_ethdev.h
index 7a03262..e93e4a7 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethdev.h
+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.h
@@ -122,6 +122,8 @@ struct vmxnet3_hw {
 	Vmxnet3_MemRegs	      *memRegs;
 	uint64_t	      memRegsPA;
 #define VMXNET3_VFT_TABLE_SIZE     (VMXNET3_VFT_SIZE * sizeof(uint32_t))
+	UPT1_TxStats	      saved_tx_stats[VMXNET3_MAX_TX_QUEUES];
+	UPT1_RxStats	      saved_rx_stats[VMXNET3_MAX_RX_QUEUES];
 };
 
 #define VMXNET3_REV_3		2		/* Vmxnet3 Rev. 3 */
-- 
2.1.4

[PATCH v2 2/6] net/vmxnet3: Implement retrieval of extended stats

[Charles (Chas) Williams]

From: Robert Shearman <rshearma@brocade.com>

Implement xstats_get() to allow a number of driver-specific tx and rx
stats to be retrieved.

Signed-off-by: Robert Shearman <rshearma@brocade.com>
---
 drivers/net/vmxnet3/vmxnet3_ethdev.c | 113 +++++++++++++++++++++++++++++++++++
 1 file changed, 113 insertions(+)

diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c
index 1301f50..f5a9832 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
@@ -88,6 +88,11 @@ static int vmxnet3_dev_link_update(struct rte_eth_dev *dev,
 static void vmxnet3_hw_stats_save(struct vmxnet3_hw *hw);
 static void vmxnet3_dev_stats_get(struct rte_eth_dev *dev,
 				  struct rte_eth_stats *stats);
+static int vmxnet3_dev_xstats_get_names(struct rte_eth_dev *dev,
+					struct rte_eth_xstat_name *xstats,
+					unsigned int n);
+static int vmxnet3_dev_xstats_get(struct rte_eth_dev *dev,
+				  struct rte_eth_xstat *xstats, unsigned int n);
 static void vmxnet3_dev_info_get(struct rte_eth_dev *dev,
 				 struct rte_eth_dev_info *dev_info);
 static const uint32_t *
@@ -122,6 +127,8 @@ static const struct eth_dev_ops vmxnet3_eth_dev_ops = {
 	.allmulticast_disable = vmxnet3_dev_allmulticast_disable,
 	.link_update          = vmxnet3_dev_link_update,
 	.stats_get            = vmxnet3_dev_stats_get,
+	.xstats_get_names     = vmxnet3_dev_xstats_get_names,
+	.xstats_get           = vmxnet3_dev_xstats_get,
 	.mac_addr_set         = vmxnet3_mac_addr_set,
 	.dev_infos_get        = vmxnet3_dev_info_get,
 	.dev_supported_ptypes_get = vmxnet3_dev_supported_ptypes_get,
@@ -133,6 +140,27 @@ static const struct eth_dev_ops vmxnet3_eth_dev_ops = {
 	.tx_queue_release     = vmxnet3_dev_tx_queue_release,
 };
 
+struct vmxnet3_xstats_name_off {
+	char name[RTE_ETH_XSTATS_NAME_SIZE];
+	unsigned int offset;
+};
+
+/* tx_qX_ is prepended to the name string here */
+static const struct vmxnet3_xstats_name_off vmxnet3_txq_stat_strings[] = {
+	{"drop_total",         offsetof(struct vmxnet3_txq_stats, drop_total)},
+	{"drop_too_many_segs", offsetof(struct vmxnet3_txq_stats, drop_too_many_segs)},
+	{"drop_tso",           offsetof(struct vmxnet3_txq_stats, drop_tso)},
+	{"tx_ring_full",       offsetof(struct vmxnet3_txq_stats, tx_ring_full)},
+};
+
+/* rx_qX_ is prepended to the name string here */
+static const struct vmxnet3_xstats_name_off vmxnet3_rxq_stat_strings[] = {
+	{"drop_total",           offsetof(struct vmxnet3_rxq_stats, drop_total)},
+	{"drop_err",             offsetof(struct vmxnet3_rxq_stats, drop_err)},
+	{"drop_fcs",             offsetof(struct vmxnet3_rxq_stats, drop_fcs)},
+	{"rx_buf_alloc_failure", offsetof(struct vmxnet3_rxq_stats, rx_buf_alloc_failure)},
+};
+
 static const struct rte_memzone *
 gpa_zone_reserve(struct rte_eth_dev *dev, uint32_t size,
 		 const char *post_string, int socket_id,
@@ -882,6 +910,91 @@ vmxnet3_hw_stats_save(struct vmxnet3_hw *hw)
 		vmxnet3_hw_rx_stats_get(hw, i, &hw->saved_rx_stats[i]);
 }
 
+static int
+vmxnet3_dev_xstats_get_names(struct rte_eth_dev *dev,
+			     struct rte_eth_xstat_name *xstats_names,
+			     unsigned int n)
+{
+	unsigned int i, t, count = 0;
+	unsigned int nstats =
+		dev->data->nb_tx_queues * RTE_DIM(vmxnet3_txq_stat_strings) +
+		dev->data->nb_rx_queues * RTE_DIM(vmxnet3_rxq_stat_strings);
+
+	if (!xstats_names || n < nstats)
+		return nstats;
+
+	for (i = 0; i < dev->data->nb_rx_queues; i++) {
+		if (!dev->data->rx_queues[i])
+			continue;
+
+		for (t = 0; t < RTE_DIM(vmxnet3_rxq_stat_strings); t++) {
+			snprintf(xstats_names[count].name,
+				 sizeof(xstats_names[count].name),
+				 "rx_q%u_%s", i,
+				 vmxnet3_rxq_stat_strings[t].name);
+			count++;
+		}
+	}
+
+	for (i = 0; i < dev->data->nb_tx_queues; i++) {
+		if (!dev->data->tx_queues[i])
+			continue;
+
+		for (t = 0; t < RTE_DIM(vmxnet3_txq_stat_strings); t++) {
+			snprintf(xstats_names[count].name,
+				 sizeof(xstats_names[count].name),
+				 "tx_q%u_%s", i,
+				 vmxnet3_txq_stat_strings[t].name);
+			count++;
+		}
+	}
+
+	return count;
+}
+
+static int
+vmxnet3_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
+		       unsigned int n)
+{
+	unsigned int i, t, count = 0;
+	unsigned int nstats =
+		dev->data->nb_tx_queues * RTE_DIM(vmxnet3_txq_stat_strings) +
+		dev->data->nb_rx_queues * RTE_DIM(vmxnet3_rxq_stat_strings);
+
+	if (n < nstats)
+		return nstats;
+
+	for (i = 0; i < dev->data->nb_rx_queues; i++) {
+		struct vmxnet3_rx_queue *rxq = dev->data->rx_queues[i];
+
+		if (rxq == NULL)
+			continue;
+
+		for (t = 0; t < RTE_DIM(vmxnet3_rxq_stat_strings); t++) {
+			xstats[count].value = *(uint64_t *)(((char *)&rxq->stats) +
+				vmxnet3_rxq_stat_strings[t].offset);
+			xstats[count].id = count;
+			count++;
+		}
+	}
+
+	for (i = 0; i < dev->data->nb_tx_queues; i++) {
+		struct vmxnet3_tx_queue *txq = dev->data->tx_queues[i];
+
+		if (txq == NULL)
+			continue;
+
+		for (t = 0; t < RTE_DIM(vmxnet3_txq_stat_strings); t++) {
+			xstats[count].value = *(uint64_t *)(((char *)&txq->stats) +
+				vmxnet3_txq_stat_strings[t].offset);
+			xstats[count].id = count;
+			count++;
+		}
+	}
+
+	return count;
+}
+
 static void
 vmxnet3_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 {
-- 
2.1.4

Re: [PATCH v2 2/6] net/vmxnet3: Implement retrieval of extended stats

[Shrikrishna Khare]

On Thu, 15 Jun 2017, Charles (Chas) Williams wrote:

> From: Robert Shearman <rshearma@brocade.com>
> 
> Implement xstats_get() to allow a number of driver-specific tx and rx
> stats to be retrieved.
> 
> Signed-off-by: Robert Shearman <rshearma@brocade.com>

Acked-by: Shrikrishna Khare <skhare@vmware.com>

[PATCH v2 3/6] net/vmxnet3: Generate link-state change notifications

[Charles (Chas) Williams]

From: Robert Shearman <rshearma@brocade.com>

Generate link-state change notifications by listening to interrupts
generated by the device. Make use of the existing
vmxnet3_process_events function that was compiled out, but change it
to call vmxnet3_dev_link_update on a VMXNET3_ECR_LINK event and to not
be so noisy in its log messages.

Enable interrupts on starting the device, using a new helper function,
vmxnet3_enable_intr, based on vmxnet3_disable_intr and validated
against the FreeBSD driver.

Keep track of the number of interrupts registered for to avoid
hardcoding these in vmxnet3_enable/disable_intr and to provision for
any future rxq intr support.

Factor out the guts of vmxnet3_dev_link_update minus the started check
to allow the new function to be called from vmxnet3_dev_start in the
lsc-enabled case to ensure that the link state is correctly set from
the actual state at that point.

Signed-off-by: Robert Shearman <rshearma@brocade.com>
---
 drivers/net/vmxnet3/vmxnet3_ethdev.c | 116 +++++++++++++++++++++++++++--------
 drivers/net/vmxnet3/vmxnet3_ethdev.h |   2 +
 2 files changed, 91 insertions(+), 27 deletions(-)

diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c
index f5a9832..73277fb 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
@@ -83,6 +83,8 @@ static void vmxnet3_dev_promiscuous_enable(struct rte_eth_dev *dev);
 static void vmxnet3_dev_promiscuous_disable(struct rte_eth_dev *dev);
 static void vmxnet3_dev_allmulticast_enable(struct rte_eth_dev *dev);
 static void vmxnet3_dev_allmulticast_disable(struct rte_eth_dev *dev);
+static int __vmxnet3_dev_link_update(struct rte_eth_dev *dev,
+				     int wait_to_complete);
 static int vmxnet3_dev_link_update(struct rte_eth_dev *dev,
 				   int wait_to_complete);
 static void vmxnet3_hw_stats_save(struct vmxnet3_hw *hw);
@@ -102,10 +104,8 @@ static int vmxnet3_dev_vlan_filter_set(struct rte_eth_dev *dev,
 static void vmxnet3_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask);
 static void vmxnet3_mac_addr_set(struct rte_eth_dev *dev,
 				 struct ether_addr *mac_addr);
+static void vmxnet3_interrupt_handler(void *param);
 
-#if PROCESS_SYS_EVENTS == 1
-static void vmxnet3_process_events(struct vmxnet3_hw *);
-#endif
 /*
  * The set of PCI devices this driver supports
  */
@@ -250,10 +250,22 @@ vmxnet3_disable_intr(struct vmxnet3_hw *hw)
 	PMD_INIT_FUNC_TRACE();
 
 	hw->shared->devRead.intrConf.intrCtrl |= VMXNET3_IC_DISABLE_ALL;
-	for (i = 0; i < VMXNET3_MAX_INTRS; i++)
+	for (i = 0; i < hw->num_intrs; i++)
 		VMXNET3_WRITE_BAR0_REG(hw, VMXNET3_REG_IMR + i * 8, 1);
 }
 
+static void
+vmxnet3_enable_intr(struct vmxnet3_hw *hw)
+{
+	int i;
+
+	PMD_INIT_FUNC_TRACE();
+
+	hw->shared->devRead.intrConf.intrCtrl &= ~VMXNET3_IC_DISABLE_ALL;
+	for (i = 0; i < hw->num_intrs; i++)
+		VMXNET3_WRITE_BAR0_REG(hw, VMXNET3_REG_IMR + i * 8, 0);
+}
+
 /*
  * Gets tx data ring descriptor size.
  */
@@ -425,7 +437,7 @@ static int eth_vmxnet3_pci_remove(struct rte_pci_device *pci_dev)
 
 static struct rte_pci_driver rte_vmxnet3_pmd = {
 	.id_table = pci_id_vmxnet3_map,
-	.drv_flags = RTE_PCI_DRV_NEED_MAPPING,
+	.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
 	.probe = eth_vmxnet3_pci_probe,
 	.remove = eth_vmxnet3_pci_remove,
 };
@@ -648,11 +660,11 @@ vmxnet3_setup_driver_shared(struct rte_eth_dev *dev)
 
 	/*
 	 * Set number of interrupts to 1
-	 * PMD disables all the interrupts but this is MUST to activate device
-	 * It needs at least one interrupt for link events to handle
-	 * So we'll disable it later after device activation if needed
+	 * PMD by default disables all the interrupts but this is MUST
+	 * to activate device. It needs at least one interrupt for
+	 * link events to handle
 	 */
-	devRead->intrConf.numIntrs = 1;
+	hw->num_intrs = devRead->intrConf.numIntrs = 1;
 	devRead->intrConf.intrCtrl |= VMXNET3_IC_DISABLE_ALL;
 
 	for (i = 0; i < hw->num_tx_queues; i++) {
@@ -747,6 +759,20 @@ vmxnet3_dev_start(struct rte_eth_dev *dev)
 	if (ret != VMXNET3_SUCCESS)
 		return ret;
 
+	/* check if lsc interrupt feature is enabled */
+	if (dev->data->dev_conf.intr_conf.lsc) {
+		struct rte_pci_device *pci_dev = RTE_DEV_TO_PCI(dev->device);
+
+		/* Setup interrupt callback  */
+		rte_intr_callback_register(&pci_dev->intr_handle,
+					   vmxnet3_interrupt_handler, dev);
+
+		if (rte_intr_enable(&pci_dev->intr_handle) < 0) {
+			PMD_INIT_LOG(ERR, "interrupt enable failed");
+			return -EIO;
+		}
+	}
+
 	/* Exchange shared data with device */
 	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_DSAL,
 			       VMXNET3_GET_ADDR_LO(hw->sharedPA));
@@ -794,14 +820,19 @@ vmxnet3_dev_start(struct rte_eth_dev *dev)
 	/* Setting proper Rx Mode and issue Rx Mode Update command */
 	vmxnet3_dev_set_rxmode(hw, VMXNET3_RXM_UCAST | VMXNET3_RXM_BCAST, 1);
 
-	/*
-	 * Don't need to handle events for now
-	 */
-#if PROCESS_SYS_EVENTS == 1
-	events = VMXNET3_READ_BAR1_REG(hw, VMXNET3_REG_ECR);
-	PMD_INIT_LOG(DEBUG, "Reading events: 0x%X", events);
-	vmxnet3_process_events(hw);
-#endif
+	if (dev->data->dev_conf.intr_conf.lsc) {
+		vmxnet3_enable_intr(hw);
+
+		/*
+		 * Update link state from device since this won't be
+		 * done upon starting with lsc in use. This is done
+		 * only after enabling interrupts to avoid any race
+		 * where the link state could change without an
+		 * interrupt being fired.
+		 */
+		__vmxnet3_dev_link_update(dev, 0);
+	}
+
 	return VMXNET3_SUCCESS;
 }
 
@@ -824,6 +855,15 @@ vmxnet3_dev_stop(struct rte_eth_dev *dev)
 	/* disable interrupts */
 	vmxnet3_disable_intr(hw);
 
+	if (dev->data->dev_conf.intr_conf.lsc) {
+		struct rte_pci_device *pci_dev = RTE_DEV_TO_PCI(dev->device);
+
+		rte_intr_disable(&pci_dev->intr_handle);
+
+		rte_intr_callback_unregister(&pci_dev->intr_handle,
+					     vmxnet3_interrupt_handler, dev);
+	}
+
 	/* quiesce the device first */
 	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD, VMXNET3_CMD_QUIESCE_DEV);
 	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_DSAL, 0);
@@ -1110,17 +1150,13 @@ vmxnet3_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr)
 
 /* return 0 means link status changed, -1 means not changed */
 static int
-vmxnet3_dev_link_update(struct rte_eth_dev *dev,
-			__rte_unused int wait_to_complete)
+__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;
 	uint32_t ret;
 
-	/* Link status doesn't change for stopped dev */
-	if (dev->data->dev_started == 0)
-		return -1;
-
 	memset(&link, 0, sizeof(link));
 	vmxnet3_dev_atomic_read_link_status(dev, &old);
 
@@ -1139,6 +1175,16 @@ vmxnet3_dev_link_update(struct rte_eth_dev *dev,
 	return (old.link_status == link.link_status) ? -1 : 0;
 }
 
+static int
+vmxnet3_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete)
+{
+	/* Link status doesn't change for stopped dev */
+	if (dev->data->dev_started == 0)
+		return -1;
+
+	return __vmxnet3_dev_link_update(dev, wait_to_complete);
+}
+
 /* Updating rxmode through Vmxnet3_DriverShared structure in adapter */
 static void
 vmxnet3_dev_set_rxmode(struct vmxnet3_hw *hw, uint32_t feature, int set)
@@ -1255,10 +1301,10 @@ vmxnet3_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask)
 	}
 }
 
-#if PROCESS_SYS_EVENTS == 1
 static void
-vmxnet3_process_events(struct vmxnet3_hw *hw)
+vmxnet3_process_events(struct rte_eth_dev *dev)
 {
+	struct vmxnet3_hw *hw = dev->data->dev_private;
 	uint32_t events = hw->shared->ecr;
 
 	if (!events) {
@@ -1273,10 +1319,15 @@ vmxnet3_process_events(struct vmxnet3_hw *hw)
 	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_ECR, events);
 
 	/* Check if link state has changed */
-	if (events & VMXNET3_ECR_LINK)
+	if (events & VMXNET3_ECR_LINK) {
 		PMD_INIT_LOG(ERR,
 			     "Process events in %s(): VMXNET3_ECR_LINK event",
 			     __func__);
+		if (vmxnet3_dev_link_update(dev, 0) == 0)
+			_rte_eth_dev_callback_process(dev,
+						      RTE_ETH_EVENT_INTR_LSC,
+						      NULL);
+	}
 
 	/* Check if there is an error on xmit/recv queues */
 	if (events & (VMXNET3_ECR_TQERR | VMXNET3_ECR_RQERR)) {
@@ -1301,7 +1352,18 @@ vmxnet3_process_events(struct vmxnet3_hw *hw)
 	if (events & VMXNET3_ECR_DEBUG)
 		PMD_INIT_LOG(ERR, "Debug event generated by device.");
 }
-#endif
+
+static void
+vmxnet3_interrupt_handler(void *param)
+{
+	struct rte_eth_dev *dev = param;
+	struct rte_pci_device *pci_dev = RTE_DEV_TO_PCI(dev->device);
+
+	vmxnet3_process_events(dev);
+
+	if (rte_intr_enable(&pci_dev->intr_handle) < 0)
+		PMD_DRV_LOG(ERR, "interrupt enable failed");
+}
 
 RTE_PMD_REGISTER_PCI(net_vmxnet3, rte_vmxnet3_pmd);
 RTE_PMD_REGISTER_PCI_TABLE(net_vmxnet3, pci_id_vmxnet3_map);
diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.h b/drivers/net/vmxnet3/vmxnet3_ethdev.h
index e93e4a7..b48058a 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethdev.h
+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.h
@@ -106,6 +106,8 @@ struct vmxnet3_hw {
 	uint16_t txdata_desc_size; /* tx data ring buffer size */
 	uint16_t rxdata_desc_size; /* rx data ring buffer size */
 
+	uint8_t num_intrs;
+
 	Vmxnet3_TxQueueDesc   *tqd_start;	/* start address of all tx queue desc */
 	Vmxnet3_RxQueueDesc   *rqd_start;	/* start address of all rx queue desc */
 
-- 
2.1.4

Re: [PATCH v2 3/6] net/vmxnet3: Generate link-state change notifications

[Ferruh Yigit]

On 6/15/2017 1:16 PM, Charles (Chas) Williams wrote:
> From: Robert Shearman <rshearma@brocade.com>
> 
> Generate link-state change notifications by listening to interrupts
> generated by the device. Make use of the existing
> vmxnet3_process_events function that was compiled out, but change it
> to call vmxnet3_dev_link_update on a VMXNET3_ECR_LINK event and to not
> be so noisy in its log messages.
> 
> Enable interrupts on starting the device, using a new helper function,
> vmxnet3_enable_intr, based on vmxnet3_disable_intr and validated
> against the FreeBSD driver.
> 
> Keep track of the number of interrupts registered for to avoid
> hardcoding these in vmxnet3_enable/disable_intr and to provision for
> any future rxq intr support.
> 
> Factor out the guts of vmxnet3_dev_link_update minus the started check
> to allow the new function to be called from vmxnet3_dev_start in the
> lsc-enabled case to ensure that the link state is correctly set from
> the actual state at that point.
> 
> Signed-off-by: Robert Shearman <rshearma@brocade.com>

It is close to the integration deadline, and all patches in the set
acked except this one, I am for getting the set and if any issue found
related LSC, fix it after integration deadline.

If there is an objection to get the patchset, please comment soon.

Thanks,
ferruh

[PATCH v2 4/6] net/vmxnet3: Make vmxnet3_process_events less noisy

[Charles (Chas) Williams]

From: Robert Shearman <rshearma@brocade.com>

Make vmxnet3_process_events less noisy by removing logging when there
are no events to process and by making link, device-change and debug
events DEBUG level rather than ERR.

Change these to use PMD_DRV_LOG instead of PMD_INIT_LOG since they
don't happen at device init.

Signed-off-by: Robert Shearman <rshearma@brocade.com>
Acked-by: Shrikrishna Khare <skhare@vmware.com>
---
 drivers/net/vmxnet3/vmxnet3_ethdev.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c
index 73277fb..e8cbc85 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
@@ -1307,10 +1307,8 @@ vmxnet3_process_events(struct rte_eth_dev *dev)
 	struct vmxnet3_hw *hw = dev->data->dev_private;
 	uint32_t events = hw->shared->ecr;
 
-	if (!events) {
-		PMD_INIT_LOG(ERR, "No events to process");
+	if (!events)
 		return;
-	}
 
 	/*
 	 * ECR bits when written with 1b are cleared. Hence write
@@ -1320,9 +1318,7 @@ vmxnet3_process_events(struct rte_eth_dev *dev)
 
 	/* Check if link state has changed */
 	if (events & VMXNET3_ECR_LINK) {
-		PMD_INIT_LOG(ERR,
-			     "Process events in %s(): VMXNET3_ECR_LINK event",
-			     __func__);
+		PMD_DRV_LOG(DEBUG, "Process events: VMXNET3_ECR_LINK event");
 		if (vmxnet3_dev_link_update(dev, 0) == 0)
 			_rte_eth_dev_callback_process(dev,
 						      RTE_ETH_EVENT_INTR_LSC,
@@ -1335,11 +1331,11 @@ vmxnet3_process_events(struct rte_eth_dev *dev)
 				       VMXNET3_CMD_GET_QUEUE_STATUS);
 
 		if (hw->tqd_start->status.stopped)
-			PMD_INIT_LOG(ERR, "tq error 0x%x",
-				     hw->tqd_start->status.error);
+			PMD_DRV_LOG(ERR, "tq error 0x%x",
+				    hw->tqd_start->status.error);
 
 		if (hw->rqd_start->status.stopped)
-			PMD_INIT_LOG(ERR, "rq error 0x%x",
+			PMD_DRV_LOG(ERR, "rq error 0x%x",
 				     hw->rqd_start->status.error);
 
 		/* Reset the device */
@@ -1347,10 +1343,10 @@ vmxnet3_process_events(struct rte_eth_dev *dev)
 	}
 
 	if (events & VMXNET3_ECR_DIC)
-		PMD_INIT_LOG(ERR, "Device implementation change event.");
+		PMD_DRV_LOG(DEBUG, "Device implementation change event.");
 
 	if (events & VMXNET3_ECR_DEBUG)
-		PMD_INIT_LOG(ERR, "Debug event generated by device.");
+		PMD_DRV_LOG(DEBUG, "Debug event generated by device.");
 }
 
 static void
-- 
2.1.4

Re: [PATCH V2 0/6] some local vmxnet3 patches

[Ferruh Yigit]

On 6/15/2017 1:16 PM, Charles (Chas) Williams wrote:
> This series addresses some local issues with the vmxnet3 driver that
> others might find of interest.
> 
> Changes in v2:
> 
> - net/vmxnet3: Implement retrieval of extended stats
> 
>     .id field wasn't being filled in xstats.
> 
> - net/vmxnet3: receive queue memory leak
> 
>     The leak and buffer reallcation were split.  The buffer reallocation
>     patch may be superceded by another commit so isn't included in this
>     series while under discussion.
> 

Series applied to dpdk-next-net/master, thanks.

Re: [PATCH V2 0/6] some local vmxnet3 patches

[Ferruh Yigit]

On 6/28/2017 12:30 PM, Ferruh Yigit wrote:
> On 6/15/2017 1:16 PM, Charles (Chas) Williams wrote:
>> This series addresses some local issues with the vmxnet3 driver that
>> others might find of interest.
>>
>> Changes in v2:
>>
>> - net/vmxnet3: Implement retrieval of extended stats
>>
>>     .id field wasn't being filled in xstats.
>>
>> - net/vmxnet3: receive queue memory leak
>>
>>     The leak and buffer reallcation were split.  The buffer reallocation
>>     patch may be superceded by another commit so isn't included in this
>>     series while under discussion.
>>
> 
> Series applied to dpdk-next-net/master, thanks.

Hi Charles,

Patches require vmxnet3.ini update, "Extended stats" and "Link status
event" ones I guess. I missed them to before getting patchset.

Would you mind sending another patch to update features document?

Thanks,
ferruh

Re: [PATCH V2 0/6] some local vmxnet3 patches

[Charles (Chas) Williams]

On 06/28/2017 08:52 AM, Ferruh Yigit wrote:
> On 6/28/2017 12:30 PM, Ferruh Yigit wrote:
>> On 6/15/2017 1:16 PM, Charles (Chas) Williams wrote:
>>> This series addresses some local issues with the vmxnet3 driver that
>>> others might find of interest.
>>>
>>> Changes in v2:
>>>
>>> - net/vmxnet3: Implement retrieval of extended stats
>>>
>>>     .id field wasn't being filled in xstats.
>>>
>>> - net/vmxnet3: receive queue memory leak
>>>
>>>     The leak and buffer reallcation were split.  The buffer reallocation
>>>     patch may be superceded by another commit so isn't included in this
>>>     series while under discussion.
>>>
>>
>> Series applied to dpdk-next-net/master, thanks.
>
> Hi Charles,
>
> Patches require vmxnet3.ini update, "Extended stats" and "Link status
> event" ones I guess. I missed them to before getting patchset.
>
> Would you mind sending another patch to update features document?
>
> Thanks,
> ferruh
>

I will get to it today.

Re: [PATCH V2 0/6] some local vmxnet3 patches

[Charles (Chas) Williams]

On 06/28/2017 08:52 AM, Ferruh Yigit wrote:
> On 6/28/2017 12:30 PM, Ferruh Yigit wrote:
>> On 6/15/2017 1:16 PM, Charles (Chas) Williams wrote:
>>> This series addresses some local issues with the vmxnet3 driver that
>>> others might find of interest.
>>>
>>> Changes in v2:
>>>
>>> - net/vmxnet3: Implement retrieval of extended stats
>>>
>>>     .id field wasn't being filled in xstats.
>>>
>>> - net/vmxnet3: receive queue memory leak
>>>
>>>     The leak and buffer reallcation were split.  The buffer reallocation
>>>     patch may be superceded by another commit so isn't included in this
>>>     series while under discussion.
>>>
>>
>> Series applied to dpdk-next-net/master, thanks.
>
> Hi Charles,
>
> Patches require vmxnet3.ini update, "Extended stats" and "Link status
> event" ones I guess. I missed them to before getting patchset.
>
> Would you mind sending another patch to update features document?
>
> Thanks,
> ferruh
>

Curiously, the vmxnet3 already indicates that it is handling link
status event interrupts.


; Supported features of the 'vmxnet3' network poll mode driver.
;
; Refer to default.ini for the full list of available PMD features.
;
[Features]
Speed capabilities   = P
Link status          = Y
Link status event    = Y

Re: [PATCH V2 0/6] some local vmxnet3 patches

[Ferruh Yigit]

On 6/28/2017 6:15 PM, Charles (Chas) Williams wrote:
> 
> 
> On 06/28/2017 08:52 AM, Ferruh Yigit wrote:
>> On 6/28/2017 12:30 PM, Ferruh Yigit wrote:
>>> On 6/15/2017 1:16 PM, Charles (Chas) Williams wrote:
>>>> This series addresses some local issues with the vmxnet3 driver that
>>>> others might find of interest.
>>>>
>>>> Changes in v2:
>>>>
>>>> - net/vmxnet3: Implement retrieval of extended stats
>>>>
>>>>     .id field wasn't being filled in xstats.
>>>>
>>>> - net/vmxnet3: receive queue memory leak
>>>>
>>>>     The leak and buffer reallcation were split.  The buffer reallocation
>>>>     patch may be superceded by another commit so isn't included in this
>>>>     series while under discussion.
>>>>
>>>
>>> Series applied to dpdk-next-net/master, thanks.
>>
>> Hi Charles,
>>
>> Patches require vmxnet3.ini update, "Extended stats" and "Link status
>> event" ones I guess. I missed them to before getting patchset.
>>
>> Would you mind sending another patch to update features document?
>>
>> Thanks,
>> ferruh
>>
> 
> Curiously, the vmxnet3 already indicates that it is handling link
> status event interrupts.

You are right, this can be by mistake.

> 
> 
> ; Supported features of the 'vmxnet3' network poll mode driver.
> ;
> ; Refer to default.ini for the full list of available PMD features.
> ;
> [Features]
> Speed capabilities   = P
> Link status          = Y
> Link status event    = Y
> 

[PATCH v2 5/6] net/vmxnet3: receive queue memory leak

[Charles (Chas) Williams]

From: Mandeep Rohilla <mrohilla@brocade.com>

This addresses an mbuf leak in an error condition during packet
receive.

Fixes: dfaff37fc46d ("vmxnet3: import new vmxnet3 poll mode driver implementation")

Signed-off-by: Mandeep Rohilla <mrohilla@brocade.com>
---
 drivers/net/vmxnet3/vmxnet3_rxtx.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/vmxnet3/vmxnet3_rxtx.c b/drivers/net/vmxnet3/vmxnet3_rxtx.c
index d8713a1..13c73f6 100644
--- a/drivers/net/vmxnet3/vmxnet3_rxtx.c
+++ b/drivers/net/vmxnet3/vmxnet3_rxtx.c
@@ -800,6 +800,12 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 				   (int)(rcd - (struct Vmxnet3_RxCompDesc *)
 					 rxq->comp_ring.base), rcd->rxdIdx);
 			rte_pktmbuf_free_seg(rxm);
+			if (rxq->start_seg) {
+				struct rte_mbuf *start = rxq->start_seg;
+
+				rxq->start_seg = NULL;
+				rte_pktmbuf_free(start);
+			}
 			goto rcd_done;
 		}
 
-- 
2.1.4

Re: [PATCH v2 5/6] net/vmxnet3: receive queue memory leak

[Shrikrishna Khare]

On Thu, 15 Jun 2017, Charles (Chas) Williams wrote:

> From: Mandeep Rohilla <mrohilla@brocade.com>
> 
> This addresses an mbuf leak in an error condition during packet
> receive.
> 
> Fixes: dfaff37fc46d ("vmxnet3: import new vmxnet3 poll mode driver implementation")
> 
> Signed-off-by: Mandeep Rohilla <mrohilla@brocade.com>

Acked-by: Shrikrishna Khare <skhare@vmware.com>

[PATCH v2 6/6] net/vmxnet3: preserve configured MAC address

[Charles (Chas) Williams]

From: George Wilkie <gwilkie@brocade.com>

When starting a vmxnet3 device, it is always writing the permanent MAC
address, even if a different MAC address was configured.  Write from
the device data instead which holds the current one.

Signed-off-by: George Wilkie <gwilkie@brocade.com>
Acked-by: Shrikrishna Khare <skhare@vmware.com>
---
 drivers/net/vmxnet3/vmxnet3_ethdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c
index e8cbc85..265de35 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
@@ -734,7 +734,7 @@ vmxnet3_setup_driver_shared(struct rte_eth_dev *dev)
 	vmxnet3_dev_vlan_offload_set(dev,
 				     ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK);
 
-	vmxnet3_write_mac(hw, hw->perm_addr);
+	vmxnet3_write_mac(hw, dev->data->mac_addrs->addr_bytes);
 
 	return VMXNET3_SUCCESS;
 }
-- 
2.1.4