[dpdk-dev] [PATCH v3 00/10] vmxnet3: multisegment and bugfixes

[dpdk-dev] [PATCH v3 00/10] vmxnet3: multisegment and bugfixes

[Stephen Hemminger]

From: Stephen Hemminger <shemming@brocade.com>

Revised version of earlier patches.
Incorporate the small packet optimization
Add more cleanups

Stephen Hemminger (10):
  vmxnet3: fix link state handling
  vmxnet3: enable VLAN filtering
  vmxnet3: remove mtu check
  vmxnet3: cleanup txq stats
  vmxnet3: add support for multi-segment transmit
  vmxnet3: support RSS and refactor offload
  vmxnet3: support jumbo frames
  vmxnet3: get rid of DEBUG ifdefs
  vmxnet3: add check for jumbo segment
  vmxnet3: remove excess inlining

 lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c | 162 +++++++++++---
 lib/librte_pmd_vmxnet3/vmxnet3_ethdev.h |  11 +-
 lib/librte_pmd_vmxnet3/vmxnet3_ring.h   |  20 +-
 lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c   | 360 ++++++++++++++++----------------
 4 files changed, 333 insertions(+), 220 deletions(-)

-- 
2.1.4

[dpdk-dev] [PATCH v3 01/10] vmxnet3: fix link state handling

[Stephen Hemminger]

From: Stephen Hemminger <shemming@brocade.com>

The Intel version of VMXNET3 driver does not handle link state properly.
The VMXNET3 API returns 1 if connected and 0 if disconnected.
Also need to return correct value to indicate state change.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Yong Wang <yongwang@vmware.com>
---
 lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c | 54 ++++++++++++++++++++++++---------
 1 file changed, 39 insertions(+), 15 deletions(-)

diff --git a/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c b/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c
index 6068c60..4c882ee 100644
--- a/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c
+++ b/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c
@@ -149,9 +149,36 @@ gpa_zone_reserve(struct rte_eth_dev *dev, uint32_t size,
  *   - On success, zero.
  *   - On failure, negative value.
  */
-static inline int
-rte_vmxnet3_dev_atomic_write_link_status(struct rte_eth_dev *dev,
-				struct rte_eth_link *link)
+
+static int
+vmxnet3_dev_atomic_read_link_status(struct rte_eth_dev *dev,
+				    struct rte_eth_link *link)
+{
+	struct rte_eth_link *dst = link;
+	struct rte_eth_link *src = &(dev->data->dev_link);
+
+	if (rte_atomic64_cmpset((uint64_t *)dst, *(uint64_t *)dst,
+				*(uint64_t *)src) == 0)
+		return -1;
+
+	return 0;
+}
+
+/**
+ * Atomically writes the link status information into global
+ * structure rte_eth_dev.
+ *
+ * @param dev
+ *   - Pointer to the structure rte_eth_dev to write to.
+ *   - Pointer to the buffer to be saved with the link status.
+ *
+ * @return
+ *   - On success, zero.
+ *   - On failure, negative value.
+ */
+static int
+vmxnet3_dev_atomic_write_link_status(struct rte_eth_dev *dev,
+				     struct rte_eth_link *link)
 {
 	struct rte_eth_link *dst = &(dev->data->dev_link);
 	struct rte_eth_link *src = link;
@@ -384,6 +411,7 @@ vmxnet3_setup_driver_shared(struct rte_eth_dev *dev)
 	devRead->misc.driverInfo.vmxnet3RevSpt = 1;
 	devRead->misc.driverInfo.uptVerSpt     = 1;
 
+	devRead->misc.mtu = rte_le_to_cpu_32(dev->data->mtu);
 	devRead->misc.queueDescPA  = hw->queueDescPA;
 	devRead->misc.queueDescLen = hw->queue_desc_len;
 	devRead->misc.mtu          = hw->cur_mtu;
@@ -570,7 +598,7 @@ vmxnet3_dev_stop(struct rte_eth_dev *dev)
 
 	/* Clear recorded link status */
 	memset(&link, 0, sizeof(link));
-	rte_vmxnet3_dev_atomic_write_link_status(dev, &link);
+	vmxnet3_dev_atomic_write_link_status(dev, &link);
 }
 
 /*
@@ -653,28 +681,24 @@ static int
 vmxnet3_dev_link_update(struct rte_eth_dev *dev, __attribute__((unused)) int wait_to_complete)
 {
 	struct vmxnet3_hw *hw = dev->data->dev_private;
-	struct rte_eth_link link;
+	struct rte_eth_link old, link;
 	uint32_t ret;
 
+	memset(&link, 0, sizeof(link));
+	vmxnet3_dev_atomic_read_link_status(dev, &old);
+
 	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD, VMXNET3_CMD_GET_LINK);
 	ret = VMXNET3_READ_BAR1_REG(hw, VMXNET3_REG_CMD);
 
-	if (!ret) {
-		PMD_INIT_LOG(ERR, "Link Status Negative : %s()", __func__);
-		return -1;
-	}
-
 	if (ret & 0x1) {
 		link.link_status = 1;
 		link.link_duplex = ETH_LINK_FULL_DUPLEX;
 		link.link_speed = ETH_LINK_SPEED_10000;
-
-		rte_vmxnet3_dev_atomic_write_link_status(dev, &link);
-
-		return 0;
 	}
 
-	return -1;
+	vmxnet3_dev_atomic_write_link_status(dev, &link);
+
+	return (old.link_status == link.link_status) ? -1 : 0;
 }
 
 /* Updating rxmode through Vmxnet3_DriverShared structure in adapter */
-- 
2.1.4

Re: [dpdk-dev] [PATCH v3 01/10] vmxnet3: fix link state handling

[Sanford, Robert]

Hi Stephen,

Have you considered supporting LSC interrupts in vmxnet3?

--
Thanks,
Robert


>From: Stephen Hemminger <shemming@brocade.com>
>
>The Intel version of VMXNET3 driver does not handle link state properly.
>The VMXNET3 API returns 1 if connected and 0 if disconnected.
>Also need to return correct value to indicate state change.
>
>Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>Acked-by: Yong Wang <yongwang@vmware.com>
>---
> lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c | 54
>++++++++++++++++++++++++---------
> 1 file changed, 39 insertions(+), 15 deletions(-)
>
>diff --git a/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c
>b/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c
>index 6068c60..4c882ee 100644
>--- a/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c
>+++ b/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c
>@@ -149,9 +149,36 @@ gpa_zone_reserve(struct rte_eth_dev *dev, uint32_t
>size,
>  *   - On success, zero.
>  *   - On failure, negative value.
>  */
>-static inline int
>-rte_vmxnet3_dev_atomic_write_link_status(struct rte_eth_dev *dev,
>-				struct rte_eth_link *link)
>+
>+static int
>+vmxnet3_dev_atomic_read_link_status(struct rte_eth_dev *dev,
>+				    struct rte_eth_link *link)
>+{
>+	struct rte_eth_link *dst = link;
>+	struct rte_eth_link *src = &(dev->data->dev_link);
>+
>+	if (rte_atomic64_cmpset((uint64_t *)dst, *(uint64_t *)dst,
>+				*(uint64_t *)src) == 0)
>+		return -1;
>+
>+	return 0;
>+}
>+
>+/**
>+ * Atomically writes the link status information into global
>+ * structure rte_eth_dev.
>+ *
>+ * @param dev
>+ *   - Pointer to the structure rte_eth_dev to write to.
>+ *   - Pointer to the buffer to be saved with the link status.
>+ *
>+ * @return
>+ *   - On success, zero.
>+ *   - On failure, negative value.
>+ */
>+static int
>+vmxnet3_dev_atomic_write_link_status(struct rte_eth_dev *dev,
>+				     struct rte_eth_link *link)
> {
> 	struct rte_eth_link *dst = &(dev->data->dev_link);
> 	struct rte_eth_link *src = link;
>@@ -384,6 +411,7 @@ vmxnet3_setup_driver_shared(struct rte_eth_dev *dev)
> 	devRead->misc.driverInfo.vmxnet3RevSpt = 1;
> 	devRead->misc.driverInfo.uptVerSpt     = 1;
> 
>+	devRead->misc.mtu = rte_le_to_cpu_32(dev->data->mtu);
> 	devRead->misc.queueDescPA  = hw->queueDescPA;
> 	devRead->misc.queueDescLen = hw->queue_desc_len;
> 	devRead->misc.mtu          = hw->cur_mtu;
>@@ -570,7 +598,7 @@ vmxnet3_dev_stop(struct rte_eth_dev *dev)
> 
> 	/* Clear recorded link status */
> 	memset(&link, 0, sizeof(link));
>-	rte_vmxnet3_dev_atomic_write_link_status(dev, &link);
>+	vmxnet3_dev_atomic_write_link_status(dev, &link);
> }
> 
> /*
>@@ -653,28 +681,24 @@ static int
> vmxnet3_dev_link_update(struct rte_eth_dev *dev, __attribute__((unused))
>int wait_to_complete)
> {
> 	struct vmxnet3_hw *hw = dev->data->dev_private;
>-	struct rte_eth_link link;
>+	struct rte_eth_link old, link;
> 	uint32_t ret;
> 
>+	memset(&link, 0, sizeof(link));
>+	vmxnet3_dev_atomic_read_link_status(dev, &old);
>+
> 	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD, VMXNET3_CMD_GET_LINK);
> 	ret = VMXNET3_READ_BAR1_REG(hw, VMXNET3_REG_CMD);
> 
>-	if (!ret) {
>-		PMD_INIT_LOG(ERR, "Link Status Negative : %s()", __func__);
>-		return -1;
>-	}
>-
> 	if (ret & 0x1) {
> 		link.link_status = 1;
> 		link.link_duplex = ETH_LINK_FULL_DUPLEX;
> 		link.link_speed = ETH_LINK_SPEED_10000;
>-
>-		rte_vmxnet3_dev_atomic_write_link_status(dev, &link);
>-
>-		return 0;
> 	}
> 
>-	return -1;
>+	vmxnet3_dev_atomic_write_link_status(dev, &link);
>+
>+	return (old.link_status == link.link_status) ? -1 : 0;
> }
> 
> /* Updating rxmode through Vmxnet3_DriverShared structure in adapter */
>-- 
>2.1.4
>

Re: [dpdk-dev] [PATCH v3 01/10] vmxnet3: fix link state handling

[Stephen Hemminger]

On Fri, 6 Mar 2015 17:21:36 +0000
"Sanford, Robert" <rsanford@akamai.com> wrote:

> Hi Stephen,
> 
> Have you considered supporting LSC interrupts in vmxnet3?
> 
> --
> Thanks,
> Robert

I tried implementing it but it is not possible since the VMXNET3 device
API does not have an interrupt mask. Therefore there is no way to get
Link State interrupts but not get an interrupt for every packet received.

[dpdk-dev] [PATCH v3 02/10] vmxnet3: enable VLAN filtering

[Stephen Hemminger]

From: Stephen Hemminger <shemming@brocade.com>

Support the VLAN filter functionality of the VMXNET3 interface.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c | 105 +++++++++++++++++++++++++++++---
 lib/librte_pmd_vmxnet3/vmxnet3_ethdev.h |   4 +-
 lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c   |  31 +---------
 3 files changed, 102 insertions(+), 38 deletions(-)

diff --git a/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c b/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c
index 4c882ee..f4c2f12 100644
--- a/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c
+++ b/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c
@@ -87,6 +87,12 @@ 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,
 				struct rte_eth_dev_info *dev_info);
+static int vmxnet3_dev_vlan_filter_set(struct rte_eth_dev *dev,
+				       uint16_t vid, int on);
+static void vmxnet3_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask);
+static void vmxnet3_dev_vlan_offload_set_clear(struct rte_eth_dev *dev,
+						int mask, int clear);
+
 #if PROCESS_SYS_EVENTS == 1
 static void vmxnet3_process_events(struct vmxnet3_hw *);
 #endif
@@ -113,6 +119,8 @@ static struct eth_dev_ops vmxnet3_eth_dev_ops = {
 	.link_update          = vmxnet3_dev_link_update,
 	.stats_get            = vmxnet3_dev_stats_get,
 	.dev_infos_get        = vmxnet3_dev_info_get,
+	.vlan_filter_set      = vmxnet3_dev_vlan_filter_set,
+	.vlan_offload_set     = vmxnet3_dev_vlan_offload_set,
 	.rx_queue_setup       = vmxnet3_dev_rx_queue_setup,
 	.rx_queue_release     = vmxnet3_dev_rx_queue_release,
 	.tx_queue_setup       = vmxnet3_dev_tx_queue_setup,
@@ -398,7 +406,7 @@ vmxnet3_setup_driver_shared(struct rte_eth_dev *dev)
 	Vmxnet3_DSDevRead *devRead = &shared->devRead;
 	uint32_t *mac_ptr;
 	uint32_t val, i;
-	int ret;
+	int ret, mask;
 
 	shared->magic = VMXNET3_REV1_MAGIC;
 	devRead->misc.driverInfo.version = VMXNET3_DRIVER_VERSION_NUM;
@@ -470,9 +478,6 @@ vmxnet3_setup_driver_shared(struct rte_eth_dev *dev)
 	if (dev->data->dev_conf.rxmode.hw_ip_checksum)
 		devRead->misc.uptFeatures |= VMXNET3_F_RXCSUM;
 
-	if (dev->data->dev_conf.rxmode.hw_vlan_strip)
-		devRead->misc.uptFeatures |= VMXNET3_F_RXVLAN;
-
 	if (port_conf.rxmode.mq_mode == ETH_MQ_RX_RSS) {
 		ret = vmxnet3_rss_configure(dev);
 		if (ret != VMXNET3_SUCCESS)
@@ -484,11 +489,14 @@ vmxnet3_setup_driver_shared(struct rte_eth_dev *dev)
 		devRead->rssConfDesc.confPA  = hw->rss_confPA;
 	}
 
-	if (dev->data->dev_conf.rxmode.hw_vlan_filter) {
-		ret = vmxnet3_vlan_configure(dev);
-		if (ret != VMXNET3_SUCCESS)
-			return ret;
-	}
+	mask = 0;
+	if (dev->data->dev_conf.rxmode.hw_vlan_strip)
+		mask |= ETH_VLAN_STRIP_MASK;
+
+	if (dev->data->dev_conf.rxmode.hw_vlan_filter)
+		mask |= ETH_VLAN_FILTER_MASK;
+
+	vmxnet3_dev_vlan_offload_set_clear(dev, mask, 1);
 
 	PMD_INIT_LOG(DEBUG,
 		     "Writing MAC Address : %02x:%02x:%02x:%02x:%02x:%02x",
@@ -720,8 +728,13 @@ static void
 vmxnet3_dev_promiscuous_enable(struct rte_eth_dev *dev)
 {
 	struct vmxnet3_hw *hw = dev->data->dev_private;
+	uint32_t *vf_table = hw->shared->devRead.rxFilterConf.vfTable;
 
+	memset(vf_table, 0, VMXNET3_VFT_TABLE_SIZE);
 	vmxnet3_dev_set_rxmode(hw, VMXNET3_RXM_PROMISC, 1);
+
+	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD,
+			       VMXNET3_CMD_UPDATE_VLAN_FILTERS);
 }
 
 /* Promiscuous supported only if Vmxnet3_DriverShared is initialized in adapter */
@@ -729,8 +742,12 @@ static void
 vmxnet3_dev_promiscuous_disable(struct rte_eth_dev *dev)
 {
 	struct vmxnet3_hw *hw = dev->data->dev_private;
+	uint32_t *vf_table = hw->shared->devRead.rxFilterConf.vfTable;
 
+	memcpy(vf_table, hw->shadow_vfta, VMXNET3_VFT_TABLE_SIZE);
 	vmxnet3_dev_set_rxmode(hw, VMXNET3_RXM_PROMISC, 0);
+	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD,
+			       VMXNET3_CMD_UPDATE_VLAN_FILTERS);
 }
 
 /* Allmulticast supported only if Vmxnet3_DriverShared is initialized in adapter */
@@ -751,6 +768,76 @@ vmxnet3_dev_allmulticast_disable(struct rte_eth_dev *dev)
 	vmxnet3_dev_set_rxmode(hw, VMXNET3_RXM_ALL_MULTI, 0);
 }
 
+/* Enable/disable filter on vlan */
+static int
+vmxnet3_dev_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vid, int on)
+{
+	struct vmxnet3_hw *hw = dev->data->dev_private;
+	struct Vmxnet3_RxFilterConf *rxConf = &hw->shared->devRead.rxFilterConf;
+	uint32_t *vf_table = rxConf->vfTable;
+
+	/* save state for restore */
+	if (on)
+		VMXNET3_SET_VFTABLE_ENTRY(hw->shadow_vfta, vid);
+	else
+		VMXNET3_CLEAR_VFTABLE_ENTRY(hw->shadow_vfta, vid);
+
+	/* don't change active filter if in promiscious mode */
+	if (rxConf->rxMode & VMXNET3_RXM_PROMISC)
+		return 0;
+
+	/* set in hardware */
+	if (on)
+		VMXNET3_SET_VFTABLE_ENTRY(vf_table, vid);
+	else
+		VMXNET3_CLEAR_VFTABLE_ENTRY(vf_table, vid);
+
+	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD,
+			       VMXNET3_CMD_UPDATE_VLAN_FILTERS);
+	return 0;
+}
+
+static void
+vmxnet3_dev_vlan_offload_set_clear(struct rte_eth_dev *dev,
+				   int mask, int clear)
+{
+	struct vmxnet3_hw *hw = dev->data->dev_private;
+	Vmxnet3_DSDevRead *devRead = &hw->shared->devRead;
+	uint32_t *vf_table = devRead->rxFilterConf.vfTable;
+
+	if (mask & ETH_VLAN_STRIP_MASK)
+		devRead->misc.uptFeatures |= UPT1_F_RXVLAN;
+	else
+		devRead->misc.uptFeatures &= ~UPT1_F_RXVLAN;
+
+	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD,
+			       VMXNET3_CMD_UPDATE_FEATURE);
+
+	if (mask & ETH_VLAN_FILTER_MASK) {
+		if (clear) {
+			memset(hw->shadow_vfta, 0,
+			       VMXNET3_VFT_TABLE_SIZE);
+			/* allow untagged pkts */
+			VMXNET3_SET_VFTABLE_ENTRY(hw->shadow_vfta, 0);
+		}
+		memcpy(vf_table, hw->shadow_vfta, VMXNET3_VFT_TABLE_SIZE);
+	} else {
+		/* allow any pkts -- no filtering */
+		if (clear)
+			memset(hw->shadow_vfta, 0xff, VMXNET3_VFT_TABLE_SIZE);
+		memset(vf_table, 0xff, VMXNET3_VFT_TABLE_SIZE);
+	}
+
+	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD,
+			       VMXNET3_CMD_UPDATE_VLAN_FILTERS);
+}
+
+static void
+vmxnet3_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask)
+{
+	vmxnet3_dev_vlan_offload_set_clear(dev, mask, 0);
+}
+
 #if PROCESS_SYS_EVENTS == 1
 static void
 vmxnet3_process_events(struct vmxnet3_hw *hw)
diff --git a/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.h b/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.h
index 09993cf..e9d67a7 100644
--- a/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.h
+++ b/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.h
@@ -121,8 +121,11 @@ struct vmxnet3_hw {
 	VMXNET3_RSSConf		 *rss_conf;
 	uint64_t			 rss_confPA;
 	vmxnet3_mf_table_t   *mf_table;
+	uint32_t	      shadow_vfta[VMXNET3_VFT_SIZE];
 };
 
+#define VMXNET3_VFT_TABLE_SIZE	   (VMXNET3_VFT_SIZE * sizeof(uint32_t))
+
 #define VMXNET3_GET_ADDR_LO(reg)   ((uint32_t)(reg))
 #define VMXNET3_GET_ADDR_HI(reg)   ((uint32_t)(((uint64_t)(reg)) >> 32))
 
@@ -173,7 +176,6 @@ int  vmxnet3_dev_tx_queue_setup(struct rte_eth_dev *dev, uint16_t tx_queue_id,
 int vmxnet3_dev_rxtx_init(struct rte_eth_dev *dev);
 
 int vmxnet3_rss_configure(struct rte_eth_dev *dev);
-int vmxnet3_vlan_configure(struct rte_eth_dev *dev);
 
 uint16_t vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 		uint16_t nb_pkts);
diff --git a/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c b/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
index 4d8a010..5fe3de5 100644
--- a/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
+++ b/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
@@ -738,9 +738,9 @@ vmxnet3_dev_tx_queue_setup(struct rte_eth_dev *dev,
 		return -EINVAL;
 	}
 
-	if ((tx_conf->txq_flags & ETH_TXQ_FLAGS_NOOFFLOADS) !=
-	    ETH_TXQ_FLAGS_NOOFFLOADS) {
-		PMD_INIT_LOG(ERR, "TX not support offload function yet");
+	if ((tx_conf->txq_flags & ETH_TXQ_FLAGS_NOXSUMS) !=
+	    ETH_TXQ_FLAGS_NOXSUMS) {
+		PMD_INIT_LOG(ERR, "TX no support for checksum offload yet");
 		return -EINVAL;
 	}
 
@@ -1045,28 +1045,3 @@ vmxnet3_rss_configure(struct rte_eth_dev *dev)
 
 	return VMXNET3_SUCCESS;
 }
-
-/*
- * Configure VLAN Filter feature
- */
-int
-vmxnet3_vlan_configure(struct rte_eth_dev *dev)
-{
-	uint8_t i;
-	struct vmxnet3_hw *hw = dev->data->dev_private;
-	uint32_t *vf_table = hw->shared->devRead.rxFilterConf.vfTable;
-
-	PMD_INIT_FUNC_TRACE();
-
-	/* Verify if this tag is already set */
-	for (i = 0; i < VMXNET3_VFT_SIZE; i++) {
-		/* Filter all vlan tags out by default */
-		vf_table[i] = 0;
-		/* To-Do: Provide another routine in dev_ops for user config */
-
-		PMD_INIT_LOG(DEBUG, "Registering VLAN portid: %"PRIu8" tag %u",
-					dev->data->port_id, vf_table[i]);
-	}
-
-	return VMXNET3_SUCCESS;
-}
-- 
2.1.4

Re: [dpdk-dev] [PATCH v3 02/10] vmxnet3: enable VLAN filtering

[Yong Wang]

On 3/5/15, 4:10 PM, "Stephen Hemminger" <stephen@networkplumber.org> wrote:

>From: Stephen Hemminger <shemming@brocade.com>
>
>Support the VLAN filter functionality of the VMXNET3 interface.
>
>Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

Acked-by: Yong Wang <yongwang@vmware.com>

>---
> lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c | 105
>+++++++++++++++++++++++++++++---
> lib/librte_pmd_vmxnet3/vmxnet3_ethdev.h |   4 +-
> lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c   |  31 +---------
> 3 files changed, 102 insertions(+), 38 deletions(-)
>
>diff --git a/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c
>b/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c
>index 4c882ee..f4c2f12 100644
>--- a/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c
>+++ b/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c
>@@ -87,6 +87,12 @@ 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,
> 				struct rte_eth_dev_info *dev_info);
>+static int vmxnet3_dev_vlan_filter_set(struct rte_eth_dev *dev,
>+				       uint16_t vid, int on);
>+static void vmxnet3_dev_vlan_offload_set(struct rte_eth_dev *dev, int
>mask);
>+static void vmxnet3_dev_vlan_offload_set_clear(struct rte_eth_dev *dev,
>+						int mask, int clear);
>+
> #if PROCESS_SYS_EVENTS == 1
> static void vmxnet3_process_events(struct vmxnet3_hw *);
> #endif
>@@ -113,6 +119,8 @@ static struct eth_dev_ops vmxnet3_eth_dev_ops = {
> 	.link_update          = vmxnet3_dev_link_update,
> 	.stats_get            = vmxnet3_dev_stats_get,
> 	.dev_infos_get        = vmxnet3_dev_info_get,
>+	.vlan_filter_set      = vmxnet3_dev_vlan_filter_set,
>+	.vlan_offload_set     = vmxnet3_dev_vlan_offload_set,
> 	.rx_queue_setup       = vmxnet3_dev_rx_queue_setup,
> 	.rx_queue_release     = vmxnet3_dev_rx_queue_release,
> 	.tx_queue_setup       = vmxnet3_dev_tx_queue_setup,
>@@ -398,7 +406,7 @@ vmxnet3_setup_driver_shared(struct rte_eth_dev *dev)
> 	Vmxnet3_DSDevRead *devRead = &shared->devRead;
> 	uint32_t *mac_ptr;
> 	uint32_t val, i;
>-	int ret;
>+	int ret, mask;
> 
> 	shared->magic = VMXNET3_REV1_MAGIC;
> 	devRead->misc.driverInfo.version = VMXNET3_DRIVER_VERSION_NUM;
>@@ -470,9 +478,6 @@ vmxnet3_setup_driver_shared(struct rte_eth_dev *dev)
> 	if (dev->data->dev_conf.rxmode.hw_ip_checksum)
> 		devRead->misc.uptFeatures |= VMXNET3_F_RXCSUM;
> 
>-	if (dev->data->dev_conf.rxmode.hw_vlan_strip)
>-		devRead->misc.uptFeatures |= VMXNET3_F_RXVLAN;
>-
> 	if (port_conf.rxmode.mq_mode == ETH_MQ_RX_RSS) {
> 		ret = vmxnet3_rss_configure(dev);
> 		if (ret != VMXNET3_SUCCESS)
>@@ -484,11 +489,14 @@ vmxnet3_setup_driver_shared(struct rte_eth_dev *dev)
> 		devRead->rssConfDesc.confPA  = hw->rss_confPA;
> 	}
> 
>-	if (dev->data->dev_conf.rxmode.hw_vlan_filter) {
>-		ret = vmxnet3_vlan_configure(dev);
>-		if (ret != VMXNET3_SUCCESS)
>-			return ret;
>-	}
>+	mask = 0;
>+	if (dev->data->dev_conf.rxmode.hw_vlan_strip)
>+		mask |= ETH_VLAN_STRIP_MASK;
>+
>+	if (dev->data->dev_conf.rxmode.hw_vlan_filter)
>+		mask |= ETH_VLAN_FILTER_MASK;
>+
>+	vmxnet3_dev_vlan_offload_set_clear(dev, mask, 1);
> 
> 	PMD_INIT_LOG(DEBUG,
> 		     "Writing MAC Address : %02x:%02x:%02x:%02x:%02x:%02x",
>@@ -720,8 +728,13 @@ static void
> vmxnet3_dev_promiscuous_enable(struct rte_eth_dev *dev)
> {
> 	struct vmxnet3_hw *hw = dev->data->dev_private;
>+	uint32_t *vf_table = hw->shared->devRead.rxFilterConf.vfTable;
> 
>+	memset(vf_table, 0, VMXNET3_VFT_TABLE_SIZE);
> 	vmxnet3_dev_set_rxmode(hw, VMXNET3_RXM_PROMISC, 1);
>+
>+	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD,
>+			       VMXNET3_CMD_UPDATE_VLAN_FILTERS);
> }
> 
> /* Promiscuous supported only if Vmxnet3_DriverShared is initialized in
>adapter */
>@@ -729,8 +742,12 @@ static void
> vmxnet3_dev_promiscuous_disable(struct rte_eth_dev *dev)
> {
> 	struct vmxnet3_hw *hw = dev->data->dev_private;
>+	uint32_t *vf_table = hw->shared->devRead.rxFilterConf.vfTable;
> 
>+	memcpy(vf_table, hw->shadow_vfta, VMXNET3_VFT_TABLE_SIZE);
> 	vmxnet3_dev_set_rxmode(hw, VMXNET3_RXM_PROMISC, 0);
>+	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD,
>+			       VMXNET3_CMD_UPDATE_VLAN_FILTERS);
> }
> 
> /* Allmulticast supported only if Vmxnet3_DriverShared is initialized in
>adapter */
>@@ -751,6 +768,76 @@ vmxnet3_dev_allmulticast_disable(struct rte_eth_dev
>*dev)
> 	vmxnet3_dev_set_rxmode(hw, VMXNET3_RXM_ALL_MULTI, 0);
> }
> 
>+/* Enable/disable filter on vlan */
>+static int
>+vmxnet3_dev_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vid, int
>on)
>+{
>+	struct vmxnet3_hw *hw = dev->data->dev_private;
>+	struct Vmxnet3_RxFilterConf *rxConf = &hw->shared->devRead.rxFilterConf;
>+	uint32_t *vf_table = rxConf->vfTable;
>+
>+	/* save state for restore */
>+	if (on)
>+		VMXNET3_SET_VFTABLE_ENTRY(hw->shadow_vfta, vid);
>+	else
>+		VMXNET3_CLEAR_VFTABLE_ENTRY(hw->shadow_vfta, vid);
>+
>+	/* don't change active filter if in promiscious mode */
>+	if (rxConf->rxMode & VMXNET3_RXM_PROMISC)
>+		return 0;
>+
>+	/* set in hardware */
>+	if (on)
>+		VMXNET3_SET_VFTABLE_ENTRY(vf_table, vid);
>+	else
>+		VMXNET3_CLEAR_VFTABLE_ENTRY(vf_table, vid);
>+
>+	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD,
>+			       VMXNET3_CMD_UPDATE_VLAN_FILTERS);
>+	return 0;
>+}
>+
>+static void
>+vmxnet3_dev_vlan_offload_set_clear(struct rte_eth_dev *dev,
>+				   int mask, int clear)
>+{
>+	struct vmxnet3_hw *hw = dev->data->dev_private;
>+	Vmxnet3_DSDevRead *devRead = &hw->shared->devRead;
>+	uint32_t *vf_table = devRead->rxFilterConf.vfTable;
>+
>+	if (mask & ETH_VLAN_STRIP_MASK)
>+		devRead->misc.uptFeatures |= UPT1_F_RXVLAN;
>+	else
>+		devRead->misc.uptFeatures &= ~UPT1_F_RXVLAN;
>+
>+	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD,
>+			       VMXNET3_CMD_UPDATE_FEATURE);
>+
>+	if (mask & ETH_VLAN_FILTER_MASK) {
>+		if (clear) {
>+			memset(hw->shadow_vfta, 0,
>+			       VMXNET3_VFT_TABLE_SIZE);
>+			/* allow untagged pkts */
>+			VMXNET3_SET_VFTABLE_ENTRY(hw->shadow_vfta, 0);
>+		}
>+		memcpy(vf_table, hw->shadow_vfta, VMXNET3_VFT_TABLE_SIZE);
>+	} else {
>+		/* allow any pkts -- no filtering */
>+		if (clear)
>+			memset(hw->shadow_vfta, 0xff, VMXNET3_VFT_TABLE_SIZE);
>+		memset(vf_table, 0xff, VMXNET3_VFT_TABLE_SIZE);
>+	}
>+
>+	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD,
>+			       VMXNET3_CMD_UPDATE_VLAN_FILTERS);
>+}
>+
>+static void
>+vmxnet3_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask)
>+{
>+	vmxnet3_dev_vlan_offload_set_clear(dev, mask, 0);
>+}
>+
> #if PROCESS_SYS_EVENTS == 1
> static void
> vmxnet3_process_events(struct vmxnet3_hw *hw)
>diff --git a/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.h
>b/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.h
>index 09993cf..e9d67a7 100644
>--- a/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.h
>+++ b/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.h
>@@ -121,8 +121,11 @@ struct vmxnet3_hw {
> 	VMXNET3_RSSConf		 *rss_conf;
> 	uint64_t			 rss_confPA;
> 	vmxnet3_mf_table_t   *mf_table;
>+	uint32_t	      shadow_vfta[VMXNET3_VFT_SIZE];
> };
> 
>+#define VMXNET3_VFT_TABLE_SIZE	   (VMXNET3_VFT_SIZE * sizeof(uint32_t))
>+
> #define VMXNET3_GET_ADDR_LO(reg)   ((uint32_t)(reg))
> #define VMXNET3_GET_ADDR_HI(reg)   ((uint32_t)(((uint64_t)(reg)) >> 32))
> 
>@@ -173,7 +176,6 @@ int  vmxnet3_dev_tx_queue_setup(struct rte_eth_dev
>*dev, uint16_t tx_queue_id,
> int vmxnet3_dev_rxtx_init(struct rte_eth_dev *dev);
> 
> int vmxnet3_rss_configure(struct rte_eth_dev *dev);
>-int vmxnet3_vlan_configure(struct rte_eth_dev *dev);
> 
> uint16_t vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
> 		uint16_t nb_pkts);
>diff --git a/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
>b/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
>index 4d8a010..5fe3de5 100644
>--- a/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
>+++ b/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
>@@ -738,9 +738,9 @@ vmxnet3_dev_tx_queue_setup(struct rte_eth_dev *dev,
> 		return -EINVAL;
> 	}
> 
>-	if ((tx_conf->txq_flags & ETH_TXQ_FLAGS_NOOFFLOADS) !=
>-	    ETH_TXQ_FLAGS_NOOFFLOADS) {
>-		PMD_INIT_LOG(ERR, "TX not support offload function yet");
>+	if ((tx_conf->txq_flags & ETH_TXQ_FLAGS_NOXSUMS) !=
>+	    ETH_TXQ_FLAGS_NOXSUMS) {
>+		PMD_INIT_LOG(ERR, "TX no support for checksum offload yet");
> 		return -EINVAL;
> 	}
> 
>@@ -1045,28 +1045,3 @@ vmxnet3_rss_configure(struct rte_eth_dev *dev)
> 
> 	return VMXNET3_SUCCESS;
> }
>-
>-/*
>- * Configure VLAN Filter feature
>- */
>-int
>-vmxnet3_vlan_configure(struct rte_eth_dev *dev)
>-{
>-	uint8_t i;
>-	struct vmxnet3_hw *hw = dev->data->dev_private;
>-	uint32_t *vf_table = hw->shared->devRead.rxFilterConf.vfTable;
>-
>-	PMD_INIT_FUNC_TRACE();
>-
>-	/* Verify if this tag is already set */
>-	for (i = 0; i < VMXNET3_VFT_SIZE; i++) {
>-		/* Filter all vlan tags out by default */
>-		vf_table[i] = 0;
>-		/* To-Do: Provide another routine in dev_ops for user config */
>-
>-		PMD_INIT_LOG(DEBUG, "Registering VLAN portid: %"PRIu8" tag %u",
>-					dev->data->port_id, vf_table[i]);
>-	}
>-
>-	return VMXNET3_SUCCESS;
>-}
>-- 
>2.1.4
>

[dpdk-dev] [PATCH v3 03/10] vmxnet3: remove mtu check

[Stephen Hemminger]

From: Stephen Hemminger <shemming@brocade.com>

Remove check for packets greater than MTU. No other driver does
this, it should be handled at higher layer

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Yong Wang <yongwang@vmware.com>
---
 lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c |  2 --
 lib/librte_pmd_vmxnet3/vmxnet3_ethdev.h |  1 -
 lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c   | 10 ----------
 3 files changed, 13 deletions(-)

diff --git a/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c b/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c
index f4c2f12..35bb561 100644
--- a/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c
+++ b/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c
@@ -246,7 +246,6 @@ eth_vmxnet3_dev_init(__attribute__((unused)) struct eth_driver *eth_drv,
 
 	hw->num_rx_queues = 1;
 	hw->num_tx_queues = 1;
-	hw->cur_mtu = ETHER_MTU;
 	hw->bufs_per_pkt = 1;
 
 	/* Check h/w version compatibility with driver. */
@@ -422,7 +421,6 @@ vmxnet3_setup_driver_shared(struct rte_eth_dev *dev)
 	devRead->misc.mtu = rte_le_to_cpu_32(dev->data->mtu);
 	devRead->misc.queueDescPA  = hw->queueDescPA;
 	devRead->misc.queueDescLen = hw->queue_desc_len;
-	devRead->misc.mtu          = hw->cur_mtu;
 	devRead->misc.numTxQueues  = hw->num_tx_queues;
 	devRead->misc.numRxQueues  = hw->num_rx_queues;
 
diff --git a/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.h b/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.h
index e9d67a7..83182e2 100644
--- a/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.h
+++ b/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.h
@@ -107,7 +107,6 @@ struct vmxnet3_hw {
 	uint8_t num_tx_queues;
 	uint8_t num_rx_queues;
 	uint8_t bufs_per_pkt;
-	uint16_t cur_mtu;
 
 	Vmxnet3_TxQueueDesc   *tqd_start;	/* start address of all tx queue desc */
 	Vmxnet3_RxQueueDesc   *rqd_start;	/* start address of all rx queue desc */
diff --git a/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c b/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
index 5fe3de5..38ac811 100644
--- a/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
+++ b/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
@@ -369,16 +369,6 @@ vmxnet3_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 				continue;
 			}
 
-			/* Needs to minus ether header len */
-			if (txm->data_len > (hw->cur_mtu + ETHER_HDR_LEN)) {
-				PMD_TX_LOG(DEBUG, "Packet data_len higher than MTU");
-				rte_pktmbuf_free(tx_pkts[nb_tx]);
-				txq->stats.drop_total++;
-
-				nb_tx++;
-				continue;
-			}
-
 			txd = (Vmxnet3_TxDesc *)(txq->cmd_ring.base + txq->cmd_ring.next2fill);
 			if (rte_pktmbuf_pkt_len(txm) <= VMXNET3_HDR_COPY_SIZE) {
 				struct Vmxnet3_TxDataDesc *tdd;
-- 
2.1.4

[dpdk-dev] [PATCH v3 04/10] vmxnet3: cleanup txq stats

[Stephen Hemminger]

From: Stephen Hemminger <shemming@brocade.com>

There are several stats here which are never set, and have no way
to be displayed.  Assume in future xstats could be used.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Yong Wang <yongwang@vmware.com>
---
 lib/librte_pmd_vmxnet3/vmxnet3_ring.h | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/lib/librte_pmd_vmxnet3/vmxnet3_ring.h b/lib/librte_pmd_vmxnet3/vmxnet3_ring.h
index c5abdb6..ebe6268 100644
--- a/lib/librte_pmd_vmxnet3/vmxnet3_ring.h
+++ b/lib/librte_pmd_vmxnet3/vmxnet3_ring.h
@@ -121,16 +121,12 @@ vmxnet3_comp_ring_adv_next2proc(struct vmxnet3_comp_ring *ring)
 }
 
 struct vmxnet3_txq_stats {
-	uint64_t               drop_total; /* # of pkts dropped by the driver, the
-									   * counters below track droppings due to
-									   * different reasons
-									   */
-	uint64_t               drop_oversized;
-	uint64_t               drop_hdr_inspect_err;
-	uint64_t               drop_tso;
-	uint64_t               deferred;
-	uint64_t               tx_ring_full;
-	uint64_t               linearized;  /* # of pkts linearized */
+	uint64_t	drop_total; /* # of pkts dropped by the driver,
+				     * the counters below track droppings due to
+				     * different reasons
+				     */
+	uint64_t	drop_tso;
+	uint64_t	tx_ring_full;
 };
 
 typedef struct vmxnet3_tx_ctx {
-- 
2.1.4

[dpdk-dev] [PATCH v3 05/10] vmxnet3: add support for multi-segment transmit

[Stephen Hemminger]

From: Stephen Hemminger <shemming@brocade.com>

Change sending loop to support multi-segment mbufs.
The VMXNET3 api has start-of-packet and end-packet flags, so it
is not hard to send multi-segment mbuf's.

Also, update descriptor in 32 bit value rather than toggling
bitfields which is slower and error prone.
Based on code in earlier driver, and the Linux kernel driver.

Add a compiler barrier to make sure that update of earlier descriptor
are completed prior to update of generation bit on start of packet.

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

---
v3 -- add back in small packet optimization

 lib/librte_pmd_vmxnet3/vmxnet3_ring.h |   1 +
 lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c | 155 +++++++++++++++++-----------------
 2 files changed, 79 insertions(+), 77 deletions(-)

diff --git a/lib/librte_pmd_vmxnet3/vmxnet3_ring.h b/lib/librte_pmd_vmxnet3/vmxnet3_ring.h
index ebe6268..612487e 100644
--- a/lib/librte_pmd_vmxnet3/vmxnet3_ring.h
+++ b/lib/librte_pmd_vmxnet3/vmxnet3_ring.h
@@ -125,6 +125,7 @@ struct vmxnet3_txq_stats {
 				     * the counters below track droppings due to
 				     * different reasons
 				     */
+	uint64_t	drop_too_many_segs;
 	uint64_t	drop_tso;
 	uint64_t	tx_ring_full;
 };
diff --git a/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c b/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
index 38ac811..5d0f227 100644
--- a/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
+++ b/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
@@ -306,26 +306,24 @@ vmxnet3_tq_tx_complete(vmxnet3_tx_queue_t *txq)
 		(comp_ring->base + comp_ring->next2proc);
 
 	while (tcd->gen == comp_ring->gen) {
-
 		/* Release cmd_ring descriptor and free mbuf */
 #ifdef RTE_LIBRTE_VMXNET3_DEBUG_DRIVER
 		VMXNET3_ASSERT(txq->cmd_ring.base[tcd->txdIdx].txd.eop == 1);
 #endif
-		mbuf = txq->cmd_ring.buf_info[tcd->txdIdx].m;
-		if (unlikely(mbuf == NULL))
-			rte_panic("EOP desc does not point to a valid mbuf");
-		else
-			rte_pktmbuf_free(mbuf);
 
+		while (txq->cmd_ring.next2comp != tcd->txdIdx) {
+			mbuf = txq->cmd_ring.buf_info[txq->cmd_ring.next2comp].m;
+			rte_pktmbuf_free_seg(mbuf);
+			txq->cmd_ring.buf_info[txq->cmd_ring.next2comp].m = NULL;
 
-		txq->cmd_ring.buf_info[tcd->txdIdx].m = NULL;
-		/* Mark the txd for which tcd was generated as completed */
-		vmxnet3_cmd_ring_adv_next2comp(&txq->cmd_ring);
+			/* Mark the txd for which tcd was generated as completed */
+			vmxnet3_cmd_ring_adv_next2comp(&txq->cmd_ring);
+			completed++;
+		}
 
 		vmxnet3_comp_ring_adv_next2proc(comp_ring);
 		tcd = (struct Vmxnet3_TxCompDesc *)(comp_ring->base +
 						    comp_ring->next2proc);
-		completed++;
 	}
 
 	PMD_TX_LOG(DEBUG, "Processed %d tx comps & command descs.", completed);
@@ -336,13 +334,8 @@ vmxnet3_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 		  uint16_t nb_pkts)
 {
 	uint16_t nb_tx;
-	Vmxnet3_TxDesc *txd = NULL;
-	vmxnet3_buf_info_t *tbi = NULL;
-	struct vmxnet3_hw *hw;
-	struct rte_mbuf *txm;
 	vmxnet3_tx_queue_t *txq = tx_queue;
-
-	hw = txq->hw;
+	struct vmxnet3_hw *hw = txq->hw;
 
 	if (unlikely(txq->stopped)) {
 		PMD_TX_LOG(DEBUG, "Tx queue is stopped.");
@@ -354,75 +347,89 @@ vmxnet3_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 
 	nb_tx = 0;
 	while (nb_tx < nb_pkts) {
+		Vmxnet3_GenericDesc *gdesc;
+		vmxnet3_buf_info_t *tbi;
+		uint32_t first2fill, avail, dw2;
+		struct rte_mbuf *txm = tx_pkts[nb_tx];
+
+		/* Is this packet execessively fragmented, then drop */
+		if (unlikely(txm->nb_segs > VMXNET3_MAX_TXD_PER_PKT)) {
+			++txq->stats.drop_too_many_segs;
+			++txq->stats.drop_total;
+			rte_pktmbuf_free(txm);
+			++nb_tx;
+			continue;
+		}
 
-		if (vmxnet3_cmd_ring_desc_avail(&txq->cmd_ring)) {
-			int copy_size = 0;
+		/* Is command ring full? */
+		avail = vmxnet3_cmd_ring_desc_avail(&txq->cmd_ring);
+		if (txm->nb_segs > avail) {
+			++txq->stats.tx_ring_full;
+			break;
+		}
 
-			txm = tx_pkts[nb_tx];
-			/* Don't support scatter packets yet, free them if met */
-			if (txm->nb_segs != 1) {
-				PMD_TX_LOG(DEBUG, "Don't support scatter packets yet, drop!");
-				rte_pktmbuf_free(tx_pkts[nb_tx]);
-				txq->stats.drop_total++;
+		/* use the previous gen bit for the SOP desc */
+		dw2 = (txq->cmd_ring.gen ^ 0x1) << VMXNET3_TXD_GEN_SHIFT;
+		first2fill = txq->cmd_ring.next2fill;
 
-				nb_tx++;
-				continue;
-			}
+		/* special case for small packets */
+		if (txm->nb_segs == 1 && txm->pkt_len <= VMXNET3_HDR_COPY_SIZE) {
+			struct Vmxnet3_TxDataDesc *tdd
+				= txq->data_ring.base + first2fill;
 
-			txd = (Vmxnet3_TxDesc *)(txq->cmd_ring.base + txq->cmd_ring.next2fill);
-			if (rte_pktmbuf_pkt_len(txm) <= VMXNET3_HDR_COPY_SIZE) {
-				struct Vmxnet3_TxDataDesc *tdd;
+			rte_memcpy(tdd->data, rte_pktmbuf_mtod(txm, char *), txm->pkt_len);
 
-				tdd = txq->data_ring.base + txq->cmd_ring.next2fill;
-				copy_size = rte_pktmbuf_pkt_len(txm);
-				rte_memcpy(tdd->data, rte_pktmbuf_mtod(txm, char *), copy_size);
-			}
+			tbi = txq->cmd_ring.buf_info + first2fill;
+			tbi->m = txm;
 
-			/* Fill the tx descriptor */
-			tbi = txq->cmd_ring.buf_info + txq->cmd_ring.next2fill;
-			tbi->bufPA = RTE_MBUF_DATA_DMA_ADDR(txm);
-			if (copy_size)
-				txd->addr = rte_cpu_to_le_64(txq->data_ring.basePA +
-							txq->cmd_ring.next2fill *
-							sizeof(struct Vmxnet3_TxDataDesc));
-			else
-				txd->addr = tbi->bufPA;
-			txd->len = txm->data_len;
+			gdesc = txq->cmd_ring.base + first2fill;
+			gdesc->txd.addr = rte_cpu_to_le_64(txq->data_ring.basePA
+				+ first2fill * sizeof(struct Vmxnet3_TxDataDesc));
+			gdesc->dword[2] = dw2 | txm->pkt_len;
+			gdesc->dword[3] = 0;
 
-			/* Mark the last descriptor as End of Packet. */
-			txd->cq = 1;
-			txd->eop = 1;
+			/* move to the next2fill descriptor */
+			vmxnet3_cmd_ring_adv_next2fill(&txq->cmd_ring);
+		} else {
+			struct rte_mbuf *m_seg = txm;
 
-			/* Add VLAN tag if requested */
-			if (txm->ol_flags & PKT_TX_VLAN_PKT) {
-				txd->ti = 1;
-				txd->tci = rte_cpu_to_le_16(txm->vlan_tci);
-			}
+			/* Multisegment and in/place transmit */
+			do {
+				/* Remember the transmit buffer for cleanup */
+				tbi = txq->cmd_ring.buf_info + txq->cmd_ring.next2fill;
+				tbi->m = m_seg;
 
-			/* Record current mbuf for freeing it later in tx complete */
-#ifdef RTE_LIBRTE_VMXNET3_DEBUG_DRIVER
-			VMXNET3_ASSERT(txm);
-#endif
-			tbi->m = txm;
+				gdesc = txq->cmd_ring.base + txq->cmd_ring.next2fill;
+				gdesc->txd.addr = RTE_MBUF_DATA_DMA_ADDR(m_seg);
+				gdesc->dword[2] = dw2 | m_seg->data_len;
+				gdesc->dword[3] = 0;
 
-			/* Set the offloading mode to default */
-			txd->hlen = 0;
-			txd->om = VMXNET3_OM_NONE;
-			txd->msscof = 0;
+				/* move to the next2fill descriptor */
+				vmxnet3_cmd_ring_adv_next2fill(&txq->cmd_ring);
 
-			/* finally flip the GEN bit of the SOP desc  */
-			txd->gen = txq->cmd_ring.gen;
-			txq->shared->ctrl.txNumDeferred++;
+				/* use the right gen for non-SOP desc */
+				dw2 = txq->cmd_ring.gen << VMXNET3_TXD_GEN_SHIFT;
+			} while ((m_seg = m_seg->next) != NULL);
+		}
 
-			/* move to the next2fill descriptor */
-			vmxnet3_cmd_ring_adv_next2fill(&txq->cmd_ring);
-			nb_tx++;
+		/* Update the EOP descriptor */
+		gdesc->dword[3] |= VMXNET3_TXD_EOP | VMXNET3_TXD_CQ;
 
-		} else {
-			PMD_TX_LOG(DEBUG, "No free tx cmd desc(s)");
-			txq->stats.drop_total += (nb_pkts - nb_tx);
-			break;
+		/* Add VLAN tag if present */
+		gdesc = txq->cmd_ring.base + first2fill;
+		if (txm->ol_flags & PKT_TX_VLAN_PKT) {
+			gdesc->txd.ti = 1;
+			gdesc->txd.tci = txm->vlan_tci;
 		}
+
+		/* TODO: Add transmit checksum offload here */
+
+		/* flip the GEN bit on the SOP */
+		rte_compiler_barrier();
+		gdesc->dword[2] ^= VMXNET3_TXD_GEN;
+
+		txq->shared->ctrl.txNumDeferred++;
+		nb_tx++;
 	}
 
 	PMD_TX_LOG(DEBUG, "vmxnet3 txThreshold: %u", txq->shared->ctrl.txThreshold);
@@ -722,12 +729,6 @@ vmxnet3_dev_tx_queue_setup(struct rte_eth_dev *dev,
 
 	PMD_INIT_FUNC_TRACE();
 
-	if ((tx_conf->txq_flags & ETH_TXQ_FLAGS_NOMULTSEGS) !=
-	    ETH_TXQ_FLAGS_NOMULTSEGS) {
-		PMD_INIT_LOG(ERR, "TX Multi segment not support yet");
-		return -EINVAL;
-	}
-
 	if ((tx_conf->txq_flags & ETH_TXQ_FLAGS_NOXSUMS) !=
 	    ETH_TXQ_FLAGS_NOXSUMS) {
 		PMD_INIT_LOG(ERR, "TX no support for checksum offload yet");
-- 
2.1.4

[dpdk-dev] [PATCH v3 06/10] vmxnet3: support RSS and refactor offload

[Stephen Hemminger]

From: Stephen Hemminger <shemming@brocade.com>

Refactor the logic to compute receive offload flags to a simpler
function. Andd add support for putting RSS flow hash into packet.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Bill Hong <bhong@brocade.com>
Acked-by: Yong Wang <yongwang@vmware.com>
---
 lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c | 69 ++++++++++++++++++++---------------
 1 file changed, 40 insertions(+), 29 deletions(-)

diff --git a/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c b/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
index 5d0f227..ba48a12 100644
--- a/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
+++ b/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
@@ -521,6 +521,43 @@ vmxnet3_post_rx_bufs(vmxnet3_rx_queue_t *rxq, uint8_t ring_id)
 		return i;
 }
 
+
+/* Receive side checksum and other offloads */
+static void
+vmxnet3_rx_offload(const Vmxnet3_RxCompDesc *rcd, struct rte_mbuf *rxm)
+{
+	/* Check for hardware stripped VLAN tag */
+	if (rcd->ts) {
+		rxm->ol_flags |= PKT_RX_VLAN_PKT;
+		rxm->vlan_tci = rte_le_to_cpu_16((uint16_t)rcd->tci);
+	}
+
+	/* Check for RSS */
+	if (rcd->rssType != VMXNET3_RCD_RSS_TYPE_NONE) {
+		rxm->ol_flags |= PKT_RX_RSS_HASH;
+		rxm->hash.rss = rcd->rssHash;
+	}
+
+	/* Check packet type, checksum errors, etc. Only support IPv4 for now. */
+	if (rcd->v4) {
+		struct ether_hdr *eth = rte_pktmbuf_mtod(rxm, struct ether_hdr *);
+		struct ipv4_hdr *ip = (struct ipv4_hdr *)(eth + 1);
+
+		if (((ip->version_ihl & 0xf) << 2) > (int)sizeof(struct ipv4_hdr))
+			rxm->ol_flags |= PKT_RX_IPV4_HDR_EXT;
+		else
+			rxm->ol_flags |= PKT_RX_IPV4_HDR;
+
+		if (!rcd->cnc) {
+			if (!rcd->ipc)
+				rxm->ol_flags |= PKT_RX_IP_CKSUM_BAD;
+
+			if ((rcd->tcp || rcd->udp) && !rcd->tuc)
+				rxm->ol_flags |= PKT_RX_L4_CKSUM_BAD;
+		}
+	}
+}
+
 /*
  * Process the Rx Completion Ring of given vmxnet3_rx_queue
  * for nb_pkts burst and return the number of packets received
@@ -621,17 +658,6 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 			goto rcd_done;
 		}
 
-		/* Check for hardware stripped VLAN tag */
-		if (rcd->ts) {
-			PMD_RX_LOG(DEBUG, "Received packet with vlan ID: %d.",
-				   rcd->tci);
-			rxm->ol_flags = PKT_RX_VLAN_PKT;
-			/* Copy vlan tag in packet buffer */
-			rxm->vlan_tci = rte_le_to_cpu_16((uint16_t)rcd->tci);
-		} else {
-			rxm->ol_flags = 0;
-			rxm->vlan_tci = 0;
-		}
 
 		/* Initialize newly received packet buffer */
 		rxm->port = rxq->port_id;
@@ -640,25 +666,10 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 		rxm->pkt_len = (uint16_t)rcd->len;
 		rxm->data_len = (uint16_t)rcd->len;
 		rxm->data_off = RTE_PKTMBUF_HEADROOM;
+		rxm->ol_flags = 0;
+		rxm->vlan_tci = 0;
 
-		/* Check packet type, checksum errors, etc. Only support IPv4 for now. */
-		if (rcd->v4) {
-			struct ether_hdr *eth = rte_pktmbuf_mtod(rxm, struct ether_hdr *);
-			struct ipv4_hdr *ip = (struct ipv4_hdr *)(eth + 1);
-
-			if (((ip->version_ihl & 0xf) << 2) > (int)sizeof(struct ipv4_hdr))
-				rxm->ol_flags |= PKT_RX_IPV4_HDR_EXT;
-			else
-				rxm->ol_flags |= PKT_RX_IPV4_HDR;
-
-			if (!rcd->cnc) {
-				if (!rcd->ipc)
-					rxm->ol_flags |= PKT_RX_IP_CKSUM_BAD;
-
-				if ((rcd->tcp || rcd->udp) && !rcd->tuc)
-					rxm->ol_flags |= PKT_RX_L4_CKSUM_BAD;
-			}
-		}
+		vmxnet3_rx_offload(rcd, rxm);
 
 		rx_pkts[nb_rx++] = rxm;
 rcd_done:
-- 
2.1.4

[dpdk-dev] [PATCH v3 07/10] vmxnet3: support jumbo frames

[Stephen Hemminger]

From: Stephen Hemminger <shemming@brocade.com>

Add support for linking multi-segment buffers together to
handle Jumbo packets.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c |  3 +-
 lib/librte_pmd_vmxnet3/vmxnet3_ring.h   |  2 +
 lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c   | 76 ++++++++++++++++++++-------------
 3 files changed, 50 insertions(+), 31 deletions(-)

diff --git a/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c b/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c
index 35bb561..4f1bc4f 100644
--- a/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c
+++ b/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c
@@ -401,6 +401,7 @@ vmxnet3_setup_driver_shared(struct rte_eth_dev *dev)
 {
 	struct rte_eth_conf port_conf = dev->data->dev_conf;
 	struct vmxnet3_hw *hw = dev->data->dev_private;
+	uint32_t mtu = dev->data->mtu;
 	Vmxnet3_DriverShared *shared = hw->shared;
 	Vmxnet3_DSDevRead *devRead = &shared->devRead;
 	uint32_t *mac_ptr;
@@ -418,7 +419,7 @@ vmxnet3_setup_driver_shared(struct rte_eth_dev *dev)
 	devRead->misc.driverInfo.vmxnet3RevSpt = 1;
 	devRead->misc.driverInfo.uptVerSpt     = 1;
 
-	devRead->misc.mtu = rte_le_to_cpu_32(dev->data->mtu);
+	devRead->misc.mtu = rte_le_to_cpu_32(mtu);
 	devRead->misc.queueDescPA  = hw->queueDescPA;
 	devRead->misc.queueDescLen = hw->queue_desc_len;
 	devRead->misc.numTxQueues  = hw->num_tx_queues;
diff --git a/lib/librte_pmd_vmxnet3/vmxnet3_ring.h b/lib/librte_pmd_vmxnet3/vmxnet3_ring.h
index 612487e..55ceadf 100644
--- a/lib/librte_pmd_vmxnet3/vmxnet3_ring.h
+++ b/lib/librte_pmd_vmxnet3/vmxnet3_ring.h
@@ -171,6 +171,8 @@ typedef struct vmxnet3_rx_queue {
 	uint32_t                    qid1;
 	uint32_t                    qid2;
 	Vmxnet3_RxQueueDesc         *shared;
+	struct rte_mbuf		    *start_seg;
+	struct rte_mbuf		    *last_seg;
 	struct vmxnet3_rxq_stats    stats;
 	bool                        stopped;
 	uint16_t                    queue_id;      /**< Device RX queue index. */
diff --git a/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c b/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
index ba48a12..5cf187a 100644
--- a/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
+++ b/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
@@ -571,7 +571,6 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 	vmxnet3_rx_queue_t *rxq;
 	Vmxnet3_RxCompDesc *rcd;
 	vmxnet3_buf_info_t *rbi;
-	Vmxnet3_RxDesc *rxd;
 	struct rte_mbuf *rxm = NULL;
 	struct vmxnet3_hw *hw;
 
@@ -596,42 +595,18 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 
 		idx = rcd->rxdIdx;
 		ring_idx = (uint8_t)((rcd->rqID == rxq->qid1) ? 0 : 1);
-		rxd = (Vmxnet3_RxDesc *)rxq->cmd_ring[ring_idx].base + idx;
 		rbi = rxq->cmd_ring[ring_idx].buf_info + idx;
 
-		if (unlikely(rcd->sop != 1 || rcd->eop != 1)) {
-			rte_pktmbuf_free_seg(rbi->m);
-			PMD_RX_LOG(DEBUG, "Packet spread across multiple buffers\n)");
-			goto rcd_done;
-		}
 
 		PMD_RX_LOG(DEBUG, "rxd idx: %d ring idx: %d.", idx, ring_idx);
 
 #ifdef RTE_LIBRTE_VMXNET3_DEBUG_DRIVER
+		Vmxnet3_RxDesc *rxd
+			= (Vmxnet3_RxDesc *)rxq->cmd_ring[ring_idx].base + idx;
 		VMXNET3_ASSERT(rcd->len <= rxd->len);
 		VMXNET3_ASSERT(rbi->m);
 #endif
-		if (unlikely(rcd->len == 0)) {
-			PMD_RX_LOG(DEBUG, "Rx buf was skipped. rxring[%d][%d]\n)",
-				   ring_idx, idx);
-#ifdef RTE_LIBRTE_VMXNET3_DEBUG_DRIVER
-			VMXNET3_ASSERT(rcd->sop && rcd->eop);
-#endif
-			rte_pktmbuf_free_seg(rbi->m);
-			goto rcd_done;
-		}
 
-		/* Assuming a packet is coming in a single packet buffer */
-		if (unlikely(rxd->btype != VMXNET3_RXD_BTYPE_HEAD)) {
-			PMD_RX_LOG(DEBUG,
-				   "Alert : Misbehaving device, incorrect "
-				   " buffer type used. iPacket dropped.");
-			rte_pktmbuf_free_seg(rbi->m);
-			goto rcd_done;
-		}
-#ifdef RTE_LIBRTE_VMXNET3_DEBUG_DRIVER
-		VMXNET3_ASSERT(rxd->btype == VMXNET3_RXD_BTYPE_HEAD);
-#endif
 		/* Get the packet buffer pointer from buf_info */
 		rxm = rbi->m;
 
@@ -643,7 +618,7 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 		rxq->cmd_ring[ring_idx].next2comp = idx;
 
 		/* For RCD with EOP set, check if there is frame error */
-		if (unlikely(rcd->err)) {
+		if (unlikely(rcd->eop && rcd->err)) {
 			rxq->stats.drop_total++;
 			rxq->stats.drop_err++;
 
@@ -669,9 +644,49 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 		rxm->ol_flags = 0;
 		rxm->vlan_tci = 0;
 
-		vmxnet3_rx_offload(rcd, rxm);
+		/*
+		 * If this is the first buffer of the received packet,
+		 * set the pointer to the first mbuf of the packet
+		 * Otherwise, update the total length and the number of segments
+		 * of the current scattered packet, and update the pointer to
+		 * the last mbuf of the current packet.
+		 */
+		if (rcd->sop) {
+#ifdef RTE_LIBRTE_VMXNET3_DEBUG_DRIVER
+			VMXNET3_ASSERT(!rxq->start_seg);
+			VMXNET3_ASSERT(rxd->btype == VMXNET3_RXD_BTYPE_HEAD);
+#endif
+
+			if (unlikely(rcd->len == 0)) {
+				PMD_RX_LOG(DEBUG,
+					   "Rx buf was skipped. rxring[%d][%d])",
+					   ring_idx, idx);
+				rte_pktmbuf_free_seg(rbi->m);
+				goto rcd_done;
+			}
+
+			rxq->start_seg = rxm;
+			vmxnet3_rx_offload(rcd, rxm);
+		} else {
+			struct rte_mbuf *start = rxq->start_seg;
+
+#ifdef RTE_LIBRTE_VMXNET3_DEBUG_DRIVER
+			VMXNET3_ASSERT(rxd->btype == VMXNET3_RXD_BTYPE_BODY);
+			VMXNET3_ASSERT(start != NULL);
+#endif
+
+			start->pkt_len += rxm->data_len;
+			start->nb_segs++;
+
+			rxq->last_seg->next = rxm;
+		}
+		rxq->last_seg = rxm;
+
+		if (rcd->eop) {
+			rx_pkts[nb_rx++] = rxq->start_seg;
+			rxq->start_seg = NULL;
+		}
 
-		rx_pkts[nb_rx++] = rxm;
 rcd_done:
 		rxq->cmd_ring[ring_idx].next2comp = idx;
 		VMXNET3_INC_RING_IDX_ONLY(rxq->cmd_ring[ring_idx].next2comp, rxq->cmd_ring[ring_idx].size);
@@ -975,6 +990,7 @@ vmxnet3_dev_rxtx_init(struct rte_eth_dev *dev)
 			}
 		}
 		rxq->stopped = FALSE;
+		rxq->start_seg = NULL;
 	}
 
 	for (i = 0; i < dev->data->nb_tx_queues; i++) {
-- 
2.1.4

Re: [dpdk-dev] [PATCH v3 07/10] vmxnet3: support jumbo frames

[Yong Wang]

On 3/5/15, 4:10 PM, "Stephen Hemminger" <stephen@networkplumber.org> wrote:

>From: Stephen Hemminger <shemming@brocade.com>
>
>Add support for linking multi-segment buffers together to
>handle Jumbo packets.
>
>Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>---
> lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c |  3 +-
> lib/librte_pmd_vmxnet3/vmxnet3_ring.h   |  2 +
> lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c   | 76
>++++++++++++++++++++-------------
> 3 files changed, 50 insertions(+), 31 deletions(-)
>
>diff --git a/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c
>b/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c
>index 35bb561..4f1bc4f 100644
>--- a/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c
>+++ b/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c
>@@ -401,6 +401,7 @@ vmxnet3_setup_driver_shared(struct rte_eth_dev *dev)
> {
> 	struct rte_eth_conf port_conf = dev->data->dev_conf;
> 	struct vmxnet3_hw *hw = dev->data->dev_private;
>+	uint32_t mtu = dev->data->mtu;
> 	Vmxnet3_DriverShared *shared = hw->shared;
> 	Vmxnet3_DSDevRead *devRead = &shared->devRead;
> 	uint32_t *mac_ptr;
>@@ -418,7 +419,7 @@ vmxnet3_setup_driver_shared(struct rte_eth_dev *dev)
> 	devRead->misc.driverInfo.vmxnet3RevSpt = 1;
> 	devRead->misc.driverInfo.uptVerSpt     = 1;
> 
>-	devRead->misc.mtu = rte_le_to_cpu_32(dev->data->mtu);
>+	devRead->misc.mtu = rte_le_to_cpu_32(mtu);

I didn’t see where mtu is used to calculate how many rx descriptors will
be needed for each packet.  Furthermore, as pointed out by the following
code comments, the device requires the first rx buffer of a packet be of
type VMXNET3_RXD_BTYPE_HEAD with the remaining buffers of type
VMXNET3_RXD_BTYPE_NODY.  This needs to be taken care of when populating rx
rings in vmxnet3_post_rx_bufs(). Currently we don’t do this because no
scatter-rx is supported and only one descriptor is needed for a packet
(thus all types should be HEAD). Otherwise, the device will complain with
error returned.  For the 2nd rx ring, type needs to be BODY for all
descriptors still.

Related to this, could you share what tests have been done to cover these
new features?

static int
vmxnet3_post_rx_bufs(vmxnet3_rx_queue_t *rxq, uint8_t ring_id)
{
        int err = 0;
        uint32_t i = 0, val = 0;
        struct vmxnet3_cmd_ring *ring = &rxq->cmd_ring[ring_id];

        if (ring_id == 0) {
	/* Usually: One HEAD type buf per packet
	* val = (ring->next2fill % rxq->hw->bufs_per_pkt) ?
	* VMXNET3_RXD_BTYPE_BODY : VMXNET3_RXD_BTYPE_HEAD;
	*/

	/* We use single packet buffer so all heads here */
	val = VMXNET3_RXD_BTYPE_HEAD;
        } else {
	/* All BODY type buffers for 2nd ring */
	val = VMXNET3_RXD_BTYPE_BODY;
        }



> 	devRead->misc.queueDescPA  = hw->queueDescPA;
> 	devRead->misc.queueDescLen = hw->queue_desc_len;
> 	devRead->misc.numTxQueues  = hw->num_tx_queues;
>diff --git a/lib/librte_pmd_vmxnet3/vmxnet3_ring.h
>b/lib/librte_pmd_vmxnet3/vmxnet3_ring.h
>index 612487e..55ceadf 100644
>--- a/lib/librte_pmd_vmxnet3/vmxnet3_ring.h
>+++ b/lib/librte_pmd_vmxnet3/vmxnet3_ring.h
>@@ -171,6 +171,8 @@ typedef struct vmxnet3_rx_queue {
> 	uint32_t                    qid1;
> 	uint32_t                    qid2;
> 	Vmxnet3_RxQueueDesc         *shared;
>+	struct rte_mbuf		    *start_seg;
>+	struct rte_mbuf		    *last_seg;
> 	struct vmxnet3_rxq_stats    stats;
> 	bool                        stopped;
> 	uint16_t                    queue_id;      /**< Device RX queue index.
>*/
>diff --git a/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
>b/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
>index ba48a12..5cf187a 100644
>--- a/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
>+++ b/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
>@@ -571,7 +571,6 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf
>**rx_pkts, uint16_t nb_pkts)
> 	vmxnet3_rx_queue_t *rxq;
> 	Vmxnet3_RxCompDesc *rcd;
> 	vmxnet3_buf_info_t *rbi;
>-	Vmxnet3_RxDesc *rxd;
> 	struct rte_mbuf *rxm = NULL;
> 	struct vmxnet3_hw *hw;
> 
>@@ -596,42 +595,18 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf
>**rx_pkts, uint16_t nb_pkts)
> 
> 		idx = rcd->rxdIdx;
> 		ring_idx = (uint8_t)((rcd->rqID == rxq->qid1) ? 0 : 1);
>-		rxd = (Vmxnet3_RxDesc *)rxq->cmd_ring[ring_idx].base + idx;
> 		rbi = rxq->cmd_ring[ring_idx].buf_info + idx;
> 
>-		if (unlikely(rcd->sop != 1 || rcd->eop != 1)) {
>-			rte_pktmbuf_free_seg(rbi->m);
>-			PMD_RX_LOG(DEBUG, "Packet spread across multiple buffers\n)");
>-			goto rcd_done;
>-		}
> 
> 		PMD_RX_LOG(DEBUG, "rxd idx: %d ring idx: %d.", idx, ring_idx);
> 
> #ifdef RTE_LIBRTE_VMXNET3_DEBUG_DRIVER
>+		Vmxnet3_RxDesc *rxd
>+			= (Vmxnet3_RxDesc *)rxq->cmd_ring[ring_idx].base + idx;
> 		VMXNET3_ASSERT(rcd->len <= rxd->len);
> 		VMXNET3_ASSERT(rbi->m);
> #endif
>-		if (unlikely(rcd->len == 0)) {
>-			PMD_RX_LOG(DEBUG, "Rx buf was skipped. rxring[%d][%d]\n)",
>-				   ring_idx, idx);
>-#ifdef RTE_LIBRTE_VMXNET3_DEBUG_DRIVER
>-			VMXNET3_ASSERT(rcd->sop && rcd->eop);
>-#endif
>-			rte_pktmbuf_free_seg(rbi->m);
>-			goto rcd_done;
>-		}
> 
>-		/* Assuming a packet is coming in a single packet buffer */
>-		if (unlikely(rxd->btype != VMXNET3_RXD_BTYPE_HEAD)) {
>-			PMD_RX_LOG(DEBUG,
>-				   "Alert : Misbehaving device, incorrect "
>-				   " buffer type used. iPacket dropped.");
>-			rte_pktmbuf_free_seg(rbi->m);
>-			goto rcd_done;
>-		}
>-#ifdef RTE_LIBRTE_VMXNET3_DEBUG_DRIVER
>-		VMXNET3_ASSERT(rxd->btype == VMXNET3_RXD_BTYPE_HEAD);
>-#endif
> 		/* Get the packet buffer pointer from buf_info */
> 		rxm = rbi->m;
> 
>@@ -643,7 +618,7 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf
>**rx_pkts, uint16_t nb_pkts)
> 		rxq->cmd_ring[ring_idx].next2comp = idx;
> 
> 		/* For RCD with EOP set, check if there is frame error */
>-		if (unlikely(rcd->err)) {
>+		if (unlikely(rcd->eop && rcd->err)) {
> 			rxq->stats.drop_total++;
> 			rxq->stats.drop_err++;
> 
>@@ -669,9 +644,49 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf
>**rx_pkts, uint16_t nb_pkts)
> 		rxm->ol_flags = 0;
> 		rxm->vlan_tci = 0;
> 
>-		vmxnet3_rx_offload(rcd, rxm);
>+		/*
>+		 * If this is the first buffer of the received packet,
>+		 * set the pointer to the first mbuf of the packet
>+		 * Otherwise, update the total length and the number of segments
>+		 * of the current scattered packet, and update the pointer to
>+		 * the last mbuf of the current packet.
>+		 */
>+		if (rcd->sop) {
>+#ifdef RTE_LIBRTE_VMXNET3_DEBUG_DRIVER
>+			VMXNET3_ASSERT(!rxq->start_seg);
>+			VMXNET3_ASSERT(rxd->btype == VMXNET3_RXD_BTYPE_HEAD);
>+#endif
>+
>+			if (unlikely(rcd->len == 0)) {
>+				PMD_RX_LOG(DEBUG,
>+					   "Rx buf was skipped. rxring[%d][%d])",
>+					   ring_idx, idx);
>+				rte_pktmbuf_free_seg(rbi->m);
>+				goto rcd_done;
>+			}
>+
>+			rxq->start_seg = rxm;
>+			vmxnet3_rx_offload(rcd, rxm);
>+		} else {
>+			struct rte_mbuf *start = rxq->start_seg;
>+
>+#ifdef RTE_LIBRTE_VMXNET3_DEBUG_DRIVER
>+			VMXNET3_ASSERT(rxd->btype == VMXNET3_RXD_BTYPE_BODY);
>+			VMXNET3_ASSERT(start != NULL);
>+#endif
>+
>+			start->pkt_len += rxm->data_len;
>+			start->nb_segs++;
>+
>+			rxq->last_seg->next = rxm;
>+		}
>+		rxq->last_seg = rxm;
>+
>+		if (rcd->eop) {
>+			rx_pkts[nb_rx++] = rxq->start_seg;
>+			rxq->start_seg = NULL;
>+		}
> 
>-		rx_pkts[nb_rx++] = rxm;
> rcd_done:
> 		rxq->cmd_ring[ring_idx].next2comp = idx;
> 		VMXNET3_INC_RING_IDX_ONLY(rxq->cmd_ring[ring_idx].next2comp,
>rxq->cmd_ring[ring_idx].size);
>@@ -975,6 +990,7 @@ vmxnet3_dev_rxtx_init(struct rte_eth_dev *dev)
> 			}
> 		}
> 		rxq->stopped = FALSE;
>+		rxq->start_seg = NULL;
> 	}
> 
> 	for (i = 0; i < dev->data->nb_tx_queues; i++) {
>-- 
>2.1.4
>

Re: [dpdk-dev] [PATCH v3 07/10] vmxnet3: support jumbo frames

[Yong Wang]

On 3/9/15, 4:28 PM, "Yong Wang" <yongwang@vmware.com> wrote:

>On 3/5/15, 4:10 PM, "Stephen Hemminger" <stephen@networkplumber.org>
>wrote:
>
>>From: Stephen Hemminger <shemming@brocade.com>
>>
>>Add support for linking multi-segment buffers together to
>>handle Jumbo packets.
>>
>>Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>>---
>> lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c |  3 +-
>> lib/librte_pmd_vmxnet3/vmxnet3_ring.h   |  2 +
>> lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c   | 76
>>++++++++++++++++++++-------------
>> 3 files changed, 50 insertions(+), 31 deletions(-)
>>
>>diff --git a/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c
>>b/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c
>>index 35bb561..4f1bc4f 100644
>>--- a/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c
>>+++ b/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c
>>@@ -401,6 +401,7 @@ vmxnet3_setup_driver_shared(struct rte_eth_dev *dev)
>> {
>> 	struct rte_eth_conf port_conf = dev->data->dev_conf;
>> 	struct vmxnet3_hw *hw = dev->data->dev_private;
>>+	uint32_t mtu = dev->data->mtu;
>> 	Vmxnet3_DriverShared *shared = hw->shared;
>> 	Vmxnet3_DSDevRead *devRead = &shared->devRead;
>> 	uint32_t *mac_ptr;
>>@@ -418,7 +419,7 @@ vmxnet3_setup_driver_shared(struct rte_eth_dev *dev)
>> 	devRead->misc.driverInfo.vmxnet3RevSpt = 1;
>> 	devRead->misc.driverInfo.uptVerSpt     = 1;
>> 
>>-	devRead->misc.mtu = rte_le_to_cpu_32(dev->data->mtu);
>>+	devRead->misc.mtu = rte_le_to_cpu_32(mtu);
>
>I didn’t see where mtu is used to calculate how many rx descriptors will
>be needed for each packet.  Furthermore, as pointed out by the following
>code comments, the device requires the first rx buffer of a packet be of
>type VMXNET3_RXD_BTYPE_HEAD with the remaining buffers of type
>VMXNET3_RXD_BTYPE_NODY.  This needs to be taken care of when populating rx
>rings in vmxnet3_post_rx_bufs(). Currently we don’t do this because no
>scatter-rx is supported and only one descriptor is needed for a packet
>(thus all types should be HEAD). Otherwise, the device will complain with

To clarify, in this case only the 1st ring will be used and thus all types
will be HEAD.

>error returned.  For the 2nd rx ring, type needs to be BODY for all
>descriptors still.
>
>Related to this, could you share what tests have been done to cover these
>new features?
>
>static int
>vmxnet3_post_rx_bufs(vmxnet3_rx_queue_t *rxq, uint8_t ring_id)
>{
>        int err = 0;
>        uint32_t i = 0, val = 0;
>        struct vmxnet3_cmd_ring *ring = &rxq->cmd_ring[ring_id];
>
>        if (ring_id == 0) {
>	/* Usually: One HEAD type buf per packet
>	* val = (ring->next2fill % rxq->hw->bufs_per_pkt) ?
>	* VMXNET3_RXD_BTYPE_BODY : VMXNET3_RXD_BTYPE_HEAD;
>	*/
>
>	/* We use single packet buffer so all heads here */
>	val = VMXNET3_RXD_BTYPE_HEAD;
>        } else {
>	/* All BODY type buffers for 2nd ring */
>	val = VMXNET3_RXD_BTYPE_BODY;
>        }
>
>
>
>> 	devRead->misc.queueDescPA  = hw->queueDescPA;
>> 	devRead->misc.queueDescLen = hw->queue_desc_len;
>> 	devRead->misc.numTxQueues  = hw->num_tx_queues;
>>diff --git a/lib/librte_pmd_vmxnet3/vmxnet3_ring.h
>>b/lib/librte_pmd_vmxnet3/vmxnet3_ring.h
>>index 612487e..55ceadf 100644
>>--- a/lib/librte_pmd_vmxnet3/vmxnet3_ring.h
>>+++ b/lib/librte_pmd_vmxnet3/vmxnet3_ring.h
>>@@ -171,6 +171,8 @@ typedef struct vmxnet3_rx_queue {
>> 	uint32_t                    qid1;
>> 	uint32_t                    qid2;
>> 	Vmxnet3_RxQueueDesc         *shared;
>>+	struct rte_mbuf		    *start_seg;
>>+	struct rte_mbuf		    *last_seg;
>> 	struct vmxnet3_rxq_stats    stats;
>> 	bool                        stopped;
>> 	uint16_t                    queue_id;      /**< Device RX queue index.
>>*/
>>diff --git a/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
>>b/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
>>index ba48a12..5cf187a 100644
>>--- a/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
>>+++ b/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
>>@@ -571,7 +571,6 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf
>>**rx_pkts, uint16_t nb_pkts)
>> 	vmxnet3_rx_queue_t *rxq;
>> 	Vmxnet3_RxCompDesc *rcd;
>> 	vmxnet3_buf_info_t *rbi;
>>-	Vmxnet3_RxDesc *rxd;
>> 	struct rte_mbuf *rxm = NULL;
>> 	struct vmxnet3_hw *hw;
>> 
>>@@ -596,42 +595,18 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf
>>**rx_pkts, uint16_t nb_pkts)
>> 
>> 		idx = rcd->rxdIdx;
>> 		ring_idx = (uint8_t)((rcd->rqID == rxq->qid1) ? 0 : 1);
>>-		rxd = (Vmxnet3_RxDesc *)rxq->cmd_ring[ring_idx].base + idx;
>> 		rbi = rxq->cmd_ring[ring_idx].buf_info + idx;
>> 
>>-		if (unlikely(rcd->sop != 1 || rcd->eop != 1)) {
>>-			rte_pktmbuf_free_seg(rbi->m);
>>-			PMD_RX_LOG(DEBUG, "Packet spread across multiple buffers\n)");
>>-			goto rcd_done;
>>-		}
>> 
>> 		PMD_RX_LOG(DEBUG, "rxd idx: %d ring idx: %d.", idx, ring_idx);
>> 
>> #ifdef RTE_LIBRTE_VMXNET3_DEBUG_DRIVER
>>+		Vmxnet3_RxDesc *rxd
>>+			= (Vmxnet3_RxDesc *)rxq->cmd_ring[ring_idx].base + idx;
>> 		VMXNET3_ASSERT(rcd->len <= rxd->len);
>> 		VMXNET3_ASSERT(rbi->m);
>> #endif
>>-		if (unlikely(rcd->len == 0)) {
>>-			PMD_RX_LOG(DEBUG, "Rx buf was skipped. rxring[%d][%d]\n)",
>>-				   ring_idx, idx);
>>-#ifdef RTE_LIBRTE_VMXNET3_DEBUG_DRIVER
>>-			VMXNET3_ASSERT(rcd->sop && rcd->eop);
>>-#endif
>>-			rte_pktmbuf_free_seg(rbi->m);
>>-			goto rcd_done;
>>-		}
>> 
>>-		/* Assuming a packet is coming in a single packet buffer */
>>-		if (unlikely(rxd->btype != VMXNET3_RXD_BTYPE_HEAD)) {
>>-			PMD_RX_LOG(DEBUG,
>>-				   "Alert : Misbehaving device, incorrect "
>>-				   " buffer type used. iPacket dropped.");
>>-			rte_pktmbuf_free_seg(rbi->m);
>>-			goto rcd_done;
>>-		}
>>-#ifdef RTE_LIBRTE_VMXNET3_DEBUG_DRIVER
>>-		VMXNET3_ASSERT(rxd->btype == VMXNET3_RXD_BTYPE_HEAD);
>>-#endif
>> 		/* Get the packet buffer pointer from buf_info */
>> 		rxm = rbi->m;
>> 
>>@@ -643,7 +618,7 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf
>>**rx_pkts, uint16_t nb_pkts)
>> 		rxq->cmd_ring[ring_idx].next2comp = idx;
>> 
>> 		/* For RCD with EOP set, check if there is frame error */
>>-		if (unlikely(rcd->err)) {
>>+		if (unlikely(rcd->eop && rcd->err)) {
>> 			rxq->stats.drop_total++;
>> 			rxq->stats.drop_err++;
>> 
>>@@ -669,9 +644,49 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf
>>**rx_pkts, uint16_t nb_pkts)
>> 		rxm->ol_flags = 0;
>> 		rxm->vlan_tci = 0;
>> 
>>-		vmxnet3_rx_offload(rcd, rxm);
>>+		/*
>>+		 * If this is the first buffer of the received packet,
>>+		 * set the pointer to the first mbuf of the packet
>>+		 * Otherwise, update the total length and the number of segments
>>+		 * of the current scattered packet, and update the pointer to
>>+		 * the last mbuf of the current packet.
>>+		 */
>>+		if (rcd->sop) {
>>+#ifdef RTE_LIBRTE_VMXNET3_DEBUG_DRIVER
>>+			VMXNET3_ASSERT(!rxq->start_seg);
>>+			VMXNET3_ASSERT(rxd->btype == VMXNET3_RXD_BTYPE_HEAD);
>>+#endif
>>+
>>+			if (unlikely(rcd->len == 0)) {
>>+				PMD_RX_LOG(DEBUG,
>>+					   "Rx buf was skipped. rxring[%d][%d])",
>>+					   ring_idx, idx);
>>+				rte_pktmbuf_free_seg(rbi->m);
>>+				goto rcd_done;
>>+			}
>>+
>>+			rxq->start_seg = rxm;
>>+			vmxnet3_rx_offload(rcd, rxm);
>>+		} else {
>>+			struct rte_mbuf *start = rxq->start_seg;
>>+
>>+#ifdef RTE_LIBRTE_VMXNET3_DEBUG_DRIVER
>>+			VMXNET3_ASSERT(rxd->btype == VMXNET3_RXD_BTYPE_BODY);
>>+			VMXNET3_ASSERT(start != NULL);
>>+#endif
>>+
>>+			start->pkt_len += rxm->data_len;
>>+			start->nb_segs++;
>>+
>>+			rxq->last_seg->next = rxm;
>>+		}
>>+		rxq->last_seg = rxm;
>>+
>>+		if (rcd->eop) {
>>+			rx_pkts[nb_rx++] = rxq->start_seg;
>>+			rxq->start_seg = NULL;
>>+		}
>> 
>>-		rx_pkts[nb_rx++] = rxm;
>> rcd_done:
>> 		rxq->cmd_ring[ring_idx].next2comp = idx;
>> 		VMXNET3_INC_RING_IDX_ONLY(rxq->cmd_ring[ring_idx].next2comp,
>>rxq->cmd_ring[ring_idx].size);
>>@@ -975,6 +990,7 @@ vmxnet3_dev_rxtx_init(struct rte_eth_dev *dev)
>> 			}
>> 		}
>> 		rxq->stopped = FALSE;
>>+		rxq->start_seg = NULL;
>> 	}
>> 
>> 	for (i = 0; i < dev->data->nb_tx_queues; i++) {
>>-- 
>>2.1.4
>>
>

Re: [dpdk-dev] [PATCH v3 07/10] vmxnet3: support jumbo frames

[Stephen Hemminger]

On Mon, 9 Mar 2015 23:32:48 +0000
Yong Wang <yongwang@vmware.com> wrote:

> >
> >I didn’t see where mtu is used to calculate how many rx descriptors will
> >be needed for each packet.  Furthermore, as pointed out by the following
> >code comments, the device requires the first rx buffer of a packet be of
> >type VMXNET3_RXD_BTYPE_HEAD with the remaining buffers of type
> >VMXNET3_RXD_BTYPE_NODY.  This needs to be taken care of when populating rx
> >rings in vmxnet3_post_rx_bufs(). Currently we don’t do this because no
> >scatter-rx is supported and only one descriptor is needed for a packet
> >(thus all types should be HEAD). Otherwise, the device will complain with  
> 
> To clarify, in this case only the 1st ring will be used and thus all types
> will be HEAD.
> 
> >error returned.  For the 2nd rx ring, type needs to be BODY for all
> >descriptors still.

Looking in more detail, the code as submitted looks fine:

* The # of rx descriptors needed for each packet is responsibility of
  the application. Before and after this patch the # of Rx descriptors
  for each ring (head/body) is determined by vmxnet3_rx_queue_setup.
  And the driver always allocates the same number of descriptors
  for both Rx rings.

  One could argue that was a bug in the original driver, since it needlessly
  allocated mbufs for the second ring and never used them. I suppose
  as a optimization in future, the second ring could be sized as 0
  and no buffers allocated unless doing rx-scatter (this is what the
  Linux driver does. But that change is independent of this.

* Buffers are correctly reallocated as needed when consumed.

* The code works for bulk transfer and performance tests in jumbo mode.

I see no requirement to change this patch.
  

Re: [dpdk-dev] [PATCH v3 07/10] vmxnet3: support jumbo frames

[Yong Wang]

On 3/10/15, 11:35 AM, "Stephen Hemminger" <stephen@networkplumber.org>
wrote:

>On Mon, 9 Mar 2015 23:32:48 +0000
>Yong Wang <yongwang@vmware.com> wrote:
>
>> >
>> >I didn’t see where mtu is used to calculate how many rx descriptors
>>will
>> >be needed for each packet.  Furthermore, as pointed out by the
>>following
>> >code comments, the device requires the first rx buffer of a packet be
>>of
>> >type VMXNET3_RXD_BTYPE_HEAD with the remaining buffers of type
>> >VMXNET3_RXD_BTYPE_NODY.  This needs to be taken care of when
>>populating rx
>> >rings in vmxnet3_post_rx_bufs(). Currently we don’t do this because no
>> >scatter-rx is supported and only one descriptor is needed for a packet
>> >(thus all types should be HEAD). Otherwise, the device will complain
>>with  
>> 
>> To clarify, in this case only the 1st ring will be used and thus all
>>types
>> will be HEAD.
>> 
>> >error returned.  For the 2nd rx ring, type needs to be BODY for all
>> >descriptors still.
>
>Looking in more detail, the code as submitted looks fine:
>
>* The # of rx descriptors needed for each packet is responsibility of
>  the application. Before and after this patch the # of Rx descriptors
>  for each ring (head/body) is determined by vmxnet3_rx_queue_setup.
>  And the driver always allocates the same number of descriptors
>  for both Rx rings.

Where is vmxnet3_rx_queue_setup? It’s not in the current code base nor in
your patch and I failed to see how rx descriptors are populated.

Could you explain what do you mean by responsibility of the application?
If mtu is set to 9000, the driver needs to make sure rx buf per packet is
set to the correct size with proper type set, right?

>
>  One could argue that was a bug in the original driver, since it
>needlessly
>  allocated mbufs for the second ring and never used them. I suppose
>  as a optimization in future, the second ring could be sized as 0
>  and no buffers allocated unless doing rx-scatter (this is what the
>  Linux driver does. But that change is independent of this.

We can just keep it as is as we will need it if someone decides to support
LRO for vmxnet3.

>
>* Buffers are correctly reallocated as needed when consumed.
>
>* The code works for bulk transfer and performance tests in jumbo mode.

You mean the standalone patch you posted without the
vmxnet3_rx_queue_setup you mentioned?

>
>I see no requirement to change this patch.
>  

Re: [dpdk-dev] [PATCH v3 07/10] vmxnet3: support jumbo frames

[Stephen Hemminger]

On Wed, 11 Mar 2015 01:03:37 +0000
Yong Wang <yongwang@vmware.com> wrote:

> >Looking in more detail, the code as submitted looks fine:
> >
> >* The # of rx descriptors needed for each packet is responsibility of
> >  the application. Before and after this patch the # of Rx descriptors
> >  for each ring (head/body) is determined by vmxnet3_rx_queue_setup.
> >  And the driver always allocates the same number of descriptors
> >  for both Rx rings.  
> 
> Where is vmxnet3_rx_queue_setup? It’s not in the current code base nor in
> your patch and I failed to see how rx descriptors are populated.



> Could you explain what do you mean by responsibility of the application?
> If mtu is set to 9000, the driver needs to make sure rx buf per packet is
> set to the correct size with proper type set, right?

vmxnet3_dev_rx_queue_setup is unchanged from original Intel code.

But you are right there is a bug there since it will error out
if max_rx_pkt_len > bufsize.  That error check should have been removed.
That was an oversight from the merge of our code with the DPDK/Intel
code. Just removing the check and surrounding code is all that is necessary.

After that driver will receive scattered mbufs.

None of the DPDK drivers do a good job of checking all the possible
misconfiguration of rxmode, jumbo and scattered flags. There is room
for improvement in all drivers around that.


> >  One could argue that was a bug in the original driver, since it
> >needlessly
> >  allocated mbufs for the second ring and never used them. I suppose
> >  as a optimization in future, the second ring could be sized as 0
> >  and no buffers allocated unless doing rx-scatter (this is what the
> >  Linux driver does. But that change is independent of this.  
> 
> We can just keep it as is as we will need it if someone decides to support
> LRO for vmxnet3.

But LRO must be optional since LRO is broken for the case of forwarding.
Linux driver handles the case of only allocating second ring if necessary.
Although the Linux vmxnet3 driver was broken about LRO setting for
several years, it is fixed now.


> >
> >* Buffers are correctly reallocated as needed when consumed.
> >
> >* The code works for bulk transfer and performance tests in jumbo mode.  
> 
> You mean the standalone patch you posted without the
> vmxnet3_rx_queue_setup you mentioned?

The code in recv that calls vmxnet3_post_rx_bufs works for both rings
already. It was just until the scattered logic was engaged the second
ring was never used.

Re: [dpdk-dev] [PATCH v3 07/10] vmxnet3: support jumbo frames

[Stephen Hemminger]

On Mon, 9 Mar 2015 23:28:39 +0000
Yong Wang <yongwang@vmware.com> wrote:

> I didn’t see where mtu is used to calculate how many rx descriptors will
> be needed for each packet.  Furthermore, as pointed out by the following
> code comments, the device requires the first rx buffer of a packet be of
> type VMXNET3_RXD_BTYPE_HEAD with the remaining buffers of type
> VMXNET3_RXD_BTYPE_NODY.  This needs to be taken care of when populating rx
> rings in vmxnet3_post_rx_bufs(). Currently we don’t do this because no
> scatter-rx is supported and only one descriptor is needed for a packet
> (thus all types should be HEAD). Otherwise, the device will complain with
> error returned.  For the 2nd rx ring, type needs to be BODY for all
> descriptors still.

Yeah the reload logic needs work.

> Related to this, could you share what tests have been done to cover these
> new features?

Not really. We only test with our product which is built on top of DPDK,
not standalone.

Most of these came from figuring what features where missing from the DPDK
driver that Intel supplied. I had independently developed a much better
virtio and vmxnet3 drivers which were well tested, but DPDK maintainers
would not accept wholesale replacement of the driver. Therefore we have
been trying to selective add the bits from my driver back into the
Intel driver.

[dpdk-dev] [PATCH v3 08/10] vmxnet3: get rid of DEBUG ifdefs

[Stephen Hemminger]

By defining macro as a stub it is possible to get rid of #ifdef's
in the actual code. Always evaluate the argument (even in the stub)
so that there are no extra unused variable errors.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_pmd_vmxnet3/vmxnet3_ethdev.h |  6 ++++--
 lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c   | 13 ++-----------
 2 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.h b/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.h
index 83182e2..1c0d95f 100644
--- a/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.h
+++ b/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.h
@@ -35,9 +35,11 @@
 #define _VMXNET3_ETHDEV_H_
 
 #ifdef RTE_LIBRTE_VMXNET3_DEBUG_DRIVER
-#define VMXNET3_ASSERT(x) do { \
-	if (!(x)) rte_panic("VMXNET3: x"); \
+#define VMXNET3_ASSERT(x) do {					\
+	if (unlikely(!(x))) rte_panic("VMXNET3: %s\n", #x);	\
 } while(0)
+#else
+#define VMXNET3_ASSERT(x) do { (void)(x); } while (0)
 #endif
 
 #define VMXNET3_MAX_MAC_ADDRS 1
diff --git a/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c b/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
index 5cf187a..3bd13ef 100644
--- a/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
+++ b/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
@@ -307,9 +307,7 @@ vmxnet3_tq_tx_complete(vmxnet3_tx_queue_t *txq)
 
 	while (tcd->gen == comp_ring->gen) {
 		/* Release cmd_ring descriptor and free mbuf */
-#ifdef RTE_LIBRTE_VMXNET3_DEBUG_DRIVER
 		VMXNET3_ASSERT(txq->cmd_ring.base[tcd->txdIdx].txd.eop == 1);
-#endif
 
 		while (txq->cmd_ring.next2comp != tcd->txdIdx) {
 			mbuf = txq->cmd_ring.buf_info[txq->cmd_ring.next2comp].m;
@@ -570,6 +568,7 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 	uint8_t ring_idx;
 	vmxnet3_rx_queue_t *rxq;
 	Vmxnet3_RxCompDesc *rcd;
+	Vmxnet3_RxDesc *rxd;
 	vmxnet3_buf_info_t *rbi;
 	struct rte_mbuf *rxm = NULL;
 	struct vmxnet3_hw *hw;
@@ -596,16 +595,12 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 		idx = rcd->rxdIdx;
 		ring_idx = (uint8_t)((rcd->rqID == rxq->qid1) ? 0 : 1);
 		rbi = rxq->cmd_ring[ring_idx].buf_info + idx;
-
+		rxd = (Vmxnet3_RxDesc *)rxq->cmd_ring[ring_idx].base + idx;
 
 		PMD_RX_LOG(DEBUG, "rxd idx: %d ring idx: %d.", idx, ring_idx);
 
-#ifdef RTE_LIBRTE_VMXNET3_DEBUG_DRIVER
-		Vmxnet3_RxDesc *rxd
-			= (Vmxnet3_RxDesc *)rxq->cmd_ring[ring_idx].base + idx;
 		VMXNET3_ASSERT(rcd->len <= rxd->len);
 		VMXNET3_ASSERT(rbi->m);
-#endif
 
 		/* Get the packet buffer pointer from buf_info */
 		rxm = rbi->m;
@@ -652,10 +647,8 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 		 * the last mbuf of the current packet.
 		 */
 		if (rcd->sop) {
-#ifdef RTE_LIBRTE_VMXNET3_DEBUG_DRIVER
 			VMXNET3_ASSERT(!rxq->start_seg);
 			VMXNET3_ASSERT(rxd->btype == VMXNET3_RXD_BTYPE_HEAD);
-#endif
 
 			if (unlikely(rcd->len == 0)) {
 				PMD_RX_LOG(DEBUG,
@@ -670,10 +663,8 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 		} else {
 			struct rte_mbuf *start = rxq->start_seg;
 
-#ifdef RTE_LIBRTE_VMXNET3_DEBUG_DRIVER
 			VMXNET3_ASSERT(rxd->btype == VMXNET3_RXD_BTYPE_BODY);
 			VMXNET3_ASSERT(start != NULL);
-#endif
 
 			start->pkt_len += rxm->data_len;
 			start->nb_segs++;
-- 
2.1.4

Re: [dpdk-dev] [PATCH v3 08/10] vmxnet3: get rid of DEBUG ifdefs

[Yong Wang]

On 3/5/15, 4:10 PM, "Stephen Hemminger" <stephen@networkplumber.org> wrote:

>By defining macro as a stub it is possible to get rid of #ifdef's
>in the actual code. Always evaluate the argument (even in the stub)
>so that there are no extra unused variable errors.
>
>Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

Acked-by: Yong Wang <yongwang@vmware.com>

>---
> lib/librte_pmd_vmxnet3/vmxnet3_ethdev.h |  6 ++++--
> lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c   | 13 ++-----------
> 2 files changed, 6 insertions(+), 13 deletions(-)
>
>diff --git a/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.h
>b/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.h
>index 83182e2..1c0d95f 100644
>--- a/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.h
>+++ b/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.h
>@@ -35,9 +35,11 @@
> #define _VMXNET3_ETHDEV_H_
> 
> #ifdef RTE_LIBRTE_VMXNET3_DEBUG_DRIVER
>-#define VMXNET3_ASSERT(x) do { \
>-	if (!(x)) rte_panic("VMXNET3: x"); \
>+#define VMXNET3_ASSERT(x) do {					\
>+	if (unlikely(!(x))) rte_panic("VMXNET3: %s\n", #x);	\
> } while(0)
>+#else
>+#define VMXNET3_ASSERT(x) do { (void)(x); } while (0)
> #endif
> 
> #define VMXNET3_MAX_MAC_ADDRS 1
>diff --git a/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
>b/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
>index 5cf187a..3bd13ef 100644
>--- a/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
>+++ b/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
>@@ -307,9 +307,7 @@ vmxnet3_tq_tx_complete(vmxnet3_tx_queue_t *txq)
> 
> 	while (tcd->gen == comp_ring->gen) {
> 		/* Release cmd_ring descriptor and free mbuf */
>-#ifdef RTE_LIBRTE_VMXNET3_DEBUG_DRIVER
> 		VMXNET3_ASSERT(txq->cmd_ring.base[tcd->txdIdx].txd.eop == 1);
>-#endif
> 
> 		while (txq->cmd_ring.next2comp != tcd->txdIdx) {
> 			mbuf = txq->cmd_ring.buf_info[txq->cmd_ring.next2comp].m;
>@@ -570,6 +568,7 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf
>**rx_pkts, uint16_t nb_pkts)
> 	uint8_t ring_idx;
> 	vmxnet3_rx_queue_t *rxq;
> 	Vmxnet3_RxCompDesc *rcd;
>+	Vmxnet3_RxDesc *rxd;
> 	vmxnet3_buf_info_t *rbi;
> 	struct rte_mbuf *rxm = NULL;
> 	struct vmxnet3_hw *hw;
>@@ -596,16 +595,12 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf
>**rx_pkts, uint16_t nb_pkts)
> 		idx = rcd->rxdIdx;
> 		ring_idx = (uint8_t)((rcd->rqID == rxq->qid1) ? 0 : 1);
> 		rbi = rxq->cmd_ring[ring_idx].buf_info + idx;
>-
>+		rxd = (Vmxnet3_RxDesc *)rxq->cmd_ring[ring_idx].base + idx;
> 
> 		PMD_RX_LOG(DEBUG, "rxd idx: %d ring idx: %d.", idx, ring_idx);
> 
>-#ifdef RTE_LIBRTE_VMXNET3_DEBUG_DRIVER
>-		Vmxnet3_RxDesc *rxd
>-			= (Vmxnet3_RxDesc *)rxq->cmd_ring[ring_idx].base + idx;
> 		VMXNET3_ASSERT(rcd->len <= rxd->len);
> 		VMXNET3_ASSERT(rbi->m);
>-#endif
> 
> 		/* Get the packet buffer pointer from buf_info */
> 		rxm = rbi->m;
>@@ -652,10 +647,8 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf
>**rx_pkts, uint16_t nb_pkts)
> 		 * the last mbuf of the current packet.
> 		 */
> 		if (rcd->sop) {
>-#ifdef RTE_LIBRTE_VMXNET3_DEBUG_DRIVER
> 			VMXNET3_ASSERT(!rxq->start_seg);
> 			VMXNET3_ASSERT(rxd->btype == VMXNET3_RXD_BTYPE_HEAD);
>-#endif
> 
> 			if (unlikely(rcd->len == 0)) {
> 				PMD_RX_LOG(DEBUG,
>@@ -670,10 +663,8 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf
>**rx_pkts, uint16_t nb_pkts)
> 		} else {
> 			struct rte_mbuf *start = rxq->start_seg;
> 
>-#ifdef RTE_LIBRTE_VMXNET3_DEBUG_DRIVER
> 			VMXNET3_ASSERT(rxd->btype == VMXNET3_RXD_BTYPE_BODY);
> 			VMXNET3_ASSERT(start != NULL);
>-#endif
> 
> 			start->pkt_len += rxm->data_len;
> 			start->nb_segs++;
>-- 
>2.1.4
>

[dpdk-dev] [PATCH v3 09/10] vmxnet3: add check for jumbo segment

[Stephen Hemminger]

From: Stephen Hemminger <shemming@brocade.com>

It is possible that some rogue application might pass a segment
larger than 16K to the vmxnet3 transmit routine. In which case
just drop it and increment counter.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_pmd_vmxnet3/vmxnet3_ring.h |  1 +
 lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/lib/librte_pmd_vmxnet3/vmxnet3_ring.h b/lib/librte_pmd_vmxnet3/vmxnet3_ring.h
index 55ceadf..5cdcb23 100644
--- a/lib/librte_pmd_vmxnet3/vmxnet3_ring.h
+++ b/lib/librte_pmd_vmxnet3/vmxnet3_ring.h
@@ -126,6 +126,7 @@ struct vmxnet3_txq_stats {
 				     * different reasons
 				     */
 	uint64_t	drop_too_many_segs;
+	uint64_t	drop_too_big;
 	uint64_t	drop_tso;
 	uint64_t	tx_ring_full;
 };
diff --git a/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c b/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
index 3bd13ef..f6c3452 100644
--- a/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
+++ b/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
@@ -327,6 +327,17 @@ vmxnet3_tq_tx_complete(vmxnet3_tx_queue_t *txq)
 	PMD_TX_LOG(DEBUG, "Processed %d tx comps & command descs.", completed);
 }
 
+static inline int
+vmxnet3_seg_too_big(const struct rte_mbuf *m)
+{
+	do {
+		if (m->data_len > VMXNET3_MAX_TX_BUF_SIZE)
+			return 1;
+	} while  ((m = m->next) != NULL);
+
+	return 0;
+}
+
 uint16_t
 vmxnet3_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 		  uint16_t nb_pkts)
@@ -353,6 +364,13 @@ vmxnet3_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 		/* Is this packet execessively fragmented, then drop */
 		if (unlikely(txm->nb_segs > VMXNET3_MAX_TXD_PER_PKT)) {
 			++txq->stats.drop_too_many_segs;
+			goto drop;
+		}
+
+		/* Check for case of monster segment */
+		if (unlikely(vmxnet3_seg_too_big(txm))) {
+			++txq->stats.drop_too_big;
+		drop:
 			++txq->stats.drop_total;
 			rte_pktmbuf_free(txm);
 			++nb_tx;
-- 
2.1.4

Re: [dpdk-dev] [PATCH v3 09/10] vmxnet3: add check for jumbo segment

[Yong Wang]

On 3/5/15, 4:10 PM, "Stephen Hemminger" <stephen@networkplumber.org> wrote:

>From: Stephen Hemminger <shemming@brocade.com>
>
>It is possible that some rogue application might pass a segment
>larger than 16K to the vmxnet3 transmit routine. In which case
>just drop it and increment counter.
>
>Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

One minor comment below. Otherwise looks good to me.


Acked-by: Yong Wang <yongwang@vmware.com>

>---
> lib/librte_pmd_vmxnet3/vmxnet3_ring.h |  1 +
> lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c | 18 ++++++++++++++++++
> 2 files changed, 19 insertions(+)
>
>diff --git a/lib/librte_pmd_vmxnet3/vmxnet3_ring.h
>b/lib/librte_pmd_vmxnet3/vmxnet3_ring.h
>index 55ceadf..5cdcb23 100644
>--- a/lib/librte_pmd_vmxnet3/vmxnet3_ring.h
>+++ b/lib/librte_pmd_vmxnet3/vmxnet3_ring.h
>@@ -126,6 +126,7 @@ struct vmxnet3_txq_stats {
> 				     * different reasons
> 				     */
> 	uint64_t	drop_too_many_segs;
>+	uint64_t	drop_too_big;
> 	uint64_t	drop_tso;
> 	uint64_t	tx_ring_full;
> };
>diff --git a/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
>b/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
>index 3bd13ef..f6c3452 100644
>--- a/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
>+++ b/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
>@@ -327,6 +327,17 @@ vmxnet3_tq_tx_complete(vmxnet3_tx_queue_t *txq)
> 	PMD_TX_LOG(DEBUG, "Processed %d tx comps & command descs.", completed);
> }
> 
>+static inline int
>+vmxnet3_seg_too_big(const struct rte_mbuf *m)
>+{
>+	do {
>+		if (m->data_len > VMXNET3_MAX_TX_BUF_SIZE)
>+			return 1;
>+	} while  ((m = m->next) != NULL);

nit: extra space after "while”.

>+
>+	return 0;
>+}
>+
> uint16_t
> vmxnet3_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
> 		  uint16_t nb_pkts)
>@@ -353,6 +364,13 @@ vmxnet3_xmit_pkts(void *tx_queue, struct rte_mbuf
>**tx_pkts,
> 		/* Is this packet execessively fragmented, then drop */
> 		if (unlikely(txm->nb_segs > VMXNET3_MAX_TXD_PER_PKT)) {
> 			++txq->stats.drop_too_many_segs;
>+			goto drop;
>+		}
>+
>+		/* Check for case of monster segment */
>+		if (unlikely(vmxnet3_seg_too_big(txm))) {
>+			++txq->stats.drop_too_big;
>+		drop:
> 			++txq->stats.drop_total;
> 			rte_pktmbuf_free(txm);
> 			++nb_tx;
>-- 
>2.1.4
>

[dpdk-dev] [PATCH v3 10/10] vmxnet3: remove excess inlining

[Stephen Hemminger]

From: Stephen Hemminger <shemming@brocade.com>

No reason to inline large functions. Compiler will decide already
based on optimization level.

Also register array should be const.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c b/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
index f6c3452..cabb505 100644
--- a/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
+++ b/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
@@ -84,10 +84,8 @@
 #define RTE_MBUF_DATA_DMA_ADDR_DEFAULT(mb) \
 	(uint64_t) ((mb)->buf_physaddr + RTE_PKTMBUF_HEADROOM)
 
-static uint32_t rxprod_reg[2] = {VMXNET3_REG_RXPROD, VMXNET3_REG_RXPROD2};
+static const uint32_t rxprod_reg[2] = {VMXNET3_REG_RXPROD, VMXNET3_REG_RXPROD2};
 
-static inline int vmxnet3_post_rx_bufs(vmxnet3_rx_queue_t* , uint8_t);
-static inline void vmxnet3_tq_tx_complete(vmxnet3_tx_queue_t *);
 #ifdef RTE_LIBRTE_VMXNET3_DEBUG_DRIVER_NOT_USED
 static void vmxnet3_rxq_dump(struct vmxnet3_rx_queue *);
 static void vmxnet3_txq_dump(struct vmxnet3_tx_queue *);
@@ -157,7 +155,7 @@ vmxnet3_txq_dump(struct vmxnet3_tx_queue *txq)
 }
 #endif
 
-static inline void
+static void
 vmxnet3_cmd_ring_release_mbufs(vmxnet3_cmd_ring_t *ring)
 {
 	while (ring->next2comp != ring->next2fill) {
@@ -296,7 +294,7 @@ vmxnet3_dev_clear_queues(struct rte_eth_dev *dev)
 	}
 }
 
-static inline void
+static void
 vmxnet3_tq_tx_complete(vmxnet3_tx_queue_t *txq)
 {
 	int completed = 0;
@@ -472,7 +470,7 @@ vmxnet3_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
  *      only for LRO.
  *
  */
-static inline int
+static int
 vmxnet3_post_rx_bufs(vmxnet3_rx_queue_t *rxq, uint8_t ring_id)
 {
 	int err = 0;
-- 
2.1.4

Re: [dpdk-dev] [PATCH v3 10/10] vmxnet3: remove excess inlining

[Yong Wang]

On 3/5/15, 4:10 PM, "Stephen Hemminger" <stephen@networkplumber.org> wrote:

>From: Stephen Hemminger <shemming@brocade.com>
>
>No reason to inline large functions. Compiler will decide already
>based on optimization level.
>
>Also register array should be const.
>
>Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

One comment below.

Acked-by: Yong Wang <yongwang@vmware.com>

>---
> lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
>diff --git a/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
>b/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
>index f6c3452..cabb505 100644
>--- a/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
>+++ b/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
>@@ -84,10 +84,8 @@
> #define RTE_MBUF_DATA_DMA_ADDR_DEFAULT(mb) \
> 	(uint64_t) ((mb)->buf_physaddr + RTE_PKTMBUF_HEADROOM)
> 
>-static uint32_t rxprod_reg[2] = {VMXNET3_REG_RXPROD,
>VMXNET3_REG_RXPROD2};
>+static const uint32_t rxprod_reg[2] = {VMXNET3_REG_RXPROD,
>VMXNET3_REG_RXPROD2};
> 
>-static inline int vmxnet3_post_rx_bufs(vmxnet3_rx_queue_t* , uint8_t);
>-static inline void vmxnet3_tq_tx_complete(vmxnet3_tx_queue_t *);
> #ifdef RTE_LIBRTE_VMXNET3_DEBUG_DRIVER_NOT_USED
> static void vmxnet3_rxq_dump(struct vmxnet3_rx_queue *);
> static void vmxnet3_txq_dump(struct vmxnet3_tx_queue *);
>@@ -157,7 +155,7 @@ vmxnet3_txq_dump(struct vmxnet3_tx_queue *txq)
> }
> #endif
> 
>-static inline void
>+static void
> vmxnet3_cmd_ring_release_mbufs(vmxnet3_cmd_ring_t *ring)
> {
> 	while (ring->next2comp != ring->next2fill) {
>@@ -296,7 +294,7 @@ vmxnet3_dev_clear_queues(struct rte_eth_dev *dev)
> 	}
> }
> 
>-static inline void
>+static void
> vmxnet3_tq_tx_complete(vmxnet3_tx_queue_t *txq)

Since there is only one caller of this routine, inlining it should be
fine. But I have no problem with letting the compiler decide (which
probably will inline it anyway for this particular case).

> {
> 	int completed = 0;
>@@ -472,7 +470,7 @@ vmxnet3_xmit_pkts(void *tx_queue, struct rte_mbuf
>**tx_pkts,
>  *      only for LRO.
>  *
>  */
>-static inline int
>+static int
> vmxnet3_post_rx_bufs(vmxnet3_rx_queue_t *rxq, uint8_t ring_id)
> {
> 	int err = 0;
>-- 
>2.1.4
>

Re: [dpdk-dev] [PATCH v3 10/10] vmxnet3: remove excess inlining

[Stephen Hemminger]

On Fri, 6 Mar 2015 23:54:23 +0000
Yong Wang <yongwang@vmware.com> wrote:

> Since there is only one caller of this routine, inlining it should be
> fine. But I have no problem with letting the compiler decide (which
> probably will inline it anyway for this particular case).

Sometimes compiler will not inline because of register pressure issues
especially on 32 bit where there aren't enough registers.

Re: [dpdk-dev] [PATCH v3 00/10] vmxnet3: multisegment and bugfixes

[Thomas Monjalon]

2015-03-05 16:10, Stephen Hemminger:
> From: Stephen Hemminger <shemming@brocade.com>
> 
> Revised version of earlier patches.
> Incorporate the small packet optimization
> Add more cleanups
> 
> Stephen Hemminger (10):
>   vmxnet3: fix link state handling
>   vmxnet3: enable VLAN filtering
>   vmxnet3: remove mtu check
>   vmxnet3: cleanup txq stats
>   vmxnet3: add support for multi-segment transmit
>   vmxnet3: support RSS and refactor offload
>   vmxnet3: support jumbo frames
>   vmxnet3: get rid of DEBUG ifdefs
>   vmxnet3: add check for jumbo segment
>   vmxnet3: remove excess inlining

Fixes and cleanup are mostly OK except minor nits.
Please fix and re-send them without patches 5 and 7 which
are not yet approved (postponed to 2.1).

Re: [dpdk-dev] [PATCH v3 00/10] vmxnet3: multisegment and bugfixes

[Thomas Monjalon]

2015-03-10 14:42, Thomas Monjalon:
> 2015-03-05 16:10, Stephen Hemminger:
> > From: Stephen Hemminger <shemming@brocade.com>
> > 
> > Revised version of earlier patches.
> > Incorporate the small packet optimization
> > Add more cleanups
> > 
> > Stephen Hemminger (10):
> >   vmxnet3: fix link state handling
> >   vmxnet3: enable VLAN filtering
> >   vmxnet3: remove mtu check
> >   vmxnet3: cleanup txq stats
> >   vmxnet3: add support for multi-segment transmit
> >   vmxnet3: support RSS and refactor offload
> >   vmxnet3: support jumbo frames
> >   vmxnet3: get rid of DEBUG ifdefs
> >   vmxnet3: add check for jumbo segment
> >   vmxnet3: remove excess inlining
> 
> Fixes and cleanup are mostly OK except minor nits.
> Please fix and re-send them without patches 5 and 7 which
> are not yet approved (postponed to 2.1).

Guys, any news about patches 5 and 7?

Re: [dpdk-dev] [PATCH v3 00/10] vmxnet3: multisegment and bugfixes

[Thomas Monjalon]

Ping Stephen

2015-06-22 14:22, Thomas Monjalon:
> 2015-03-10 14:42, Thomas Monjalon:
> > 2015-03-05 16:10, Stephen Hemminger:
> > > From: Stephen Hemminger <shemming@brocade.com>
> > > 
> > > Revised version of earlier patches.
> > > Incorporate the small packet optimization
> > > Add more cleanups
> > > 
> > > Stephen Hemminger (10):
> > >   vmxnet3: fix link state handling
> > >   vmxnet3: enable VLAN filtering
> > >   vmxnet3: remove mtu check
> > >   vmxnet3: cleanup txq stats
> > >   vmxnet3: add support for multi-segment transmit
> > >   vmxnet3: support RSS and refactor offload
> > >   vmxnet3: support jumbo frames
> > >   vmxnet3: get rid of DEBUG ifdefs
> > >   vmxnet3: add check for jumbo segment
> > >   vmxnet3: remove excess inlining
> > 
> > Fixes and cleanup are mostly OK except minor nits.
> > Please fix and re-send them without patches 5 and 7 which
> > are not yet approved (postponed to 2.1).
> 
> Guys, any news about patches 5 and 7?