[PATCH] [PMD] [VHOST] Revert unnecessary definition and fix wrong referring in user space vhost zero copy patches

[PATCH] [PMD] [VHOST] Revert unnecessary definition and fix wrong referring in user space vhost zero copy patches

[Ouyang Changchun]

 1. Revert the change of metadata macro definition for referring to headroom space in mbuf;
 2. Fix wrongly referring to RX queues number in TX queues start/stop function.

Signed-off-by: Ouyang Changchun <changchun.ouyang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 examples/vhost/main.c         | 15 +++++++++------
 lib/librte_ether/rte_ethdev.c |  8 ++++----
 lib/librte_mbuf/rte_mbuf.h    | 17 -----------------
 3 files changed, 13 insertions(+), 27 deletions(-)

diff --git a/examples/vhost/main.c b/examples/vhost/main.c
index 21704f1..674608c 100644
--- a/examples/vhost/main.c
+++ b/examples/vhost/main.c
@@ -129,6 +129,9 @@
 #define RTE_TEST_RX_DESC_DEFAULT_ZCP 32   /* legacy: 32, DPDK virt FE: 128. */
 #define RTE_TEST_TX_DESC_DEFAULT_ZCP 64   /* legacy: 64, DPDK virt FE: 64.  */
 
+/* Get first 4 bytes in mbuf headroom. */
+#define MBUF_HEADROOM_UINT32(mbuf) (*(uint32_t*)((uint8_t*)(mbuf) + sizeof(struct rte_mbuf)))
+
 /* true if x is a power of 2 */
 #define POWEROF2(x) ((((x)-1) & (x)) == 0)
 
@@ -1638,7 +1641,7 @@ attach_rxmbuf_zcp(struct virtio_net *dev)
 	mbuf->pkt.data = (void*)(uintptr_t)(buff_addr);
 	mbuf->buf_physaddr = phys_addr - RTE_PKTMBUF_HEADROOM;
 	mbuf->pkt.data_len = desc->len;
-	RTE_MBUF_METADATA_UINT32(mbuf, 0) = (uint32_t)desc_idx;
+	MBUF_HEADROOM_UINT32(mbuf) = (uint32_t)desc_idx;
 
 	LOG_DEBUG(DATA, "(%"PRIu64") in attach_rxmbuf_zcp: res base idx:%d, descriptor idx:%d\n",
 		dev->device_fh, res_base_idx, desc_idx);
@@ -1700,7 +1703,7 @@ txmbuf_clean_zcp(struct virtio_net* dev, struct vpool* vpool)
 		rte_ring_sp_enqueue(vpool->ring, mbuf);
 
 		/* Update used index buffer information. */
-		vq->used->ring[used_idx].id = RTE_MBUF_METADATA_UINT32(mbuf, 0);
+		vq->used->ring[used_idx].id = MBUF_HEADROOM_UINT32(mbuf);
 		vq->used->ring[used_idx].len = 0;
 
 		used_idx = (used_idx + 1) & (vq->size - 1);
@@ -1788,7 +1791,7 @@ virtio_dev_rx_zcp(struct virtio_net *dev, struct rte_mbuf **pkts, uint32_t count
 
 	/* Retrieve all of the head indexes first to avoid caching issues. */
 	for (head_idx = 0; head_idx < count; head_idx++)
-		head[head_idx] = RTE_MBUF_METADATA_UINT32((pkts[head_idx]), 0);
+		head[head_idx] = MBUF_HEADROOM_UINT32(pkts[head_idx]);
 
 	/*Prefetch descriptor index. */
 	rte_prefetch0(&vq->desc[head[packet_success]]);
@@ -1799,7 +1802,7 @@ virtio_dev_rx_zcp(struct virtio_net *dev, struct rte_mbuf **pkts, uint32_t count
 
 		buff = pkts[packet_success];
 		LOG_DEBUG(DATA, "(%"PRIu64") in dev_rx_zcp: update the used idx for pkt[%d] descriptor idx: %d\n", 
-			dev->device_fh, packet_success, RTE_MBUF_METADATA_UINT32(buff, 0));
+			dev->device_fh, packet_success, MBUF_HEADROOM_UINT32(buff));
 
 		PRINT_PACKET(dev, (uintptr_t)(((uint64_t)(uintptr_t)buff->buf_addr) + RTE_PKTMBUF_HEADROOM), 
 				rte_pktmbuf_data_len(buff), 0);
@@ -1901,7 +1904,7 @@ virtio_tx_route_zcp(struct virtio_net* dev, struct rte_mbuf *m, uint32_t desc_id
 				if (unlikely(dev_ll->dev->device_fh == dev->device_fh)) {
 					LOG_DEBUG(DATA, "(%"PRIu64") TX: Source and destination MAC addresses are the same. Dropping packet.\n",
 							dev_ll->dev->device_fh);
-					RTE_MBUF_METADATA_UINT32(mbuf, 0) = (uint32_t)desc_idx;
+					MBUF_HEADROOM_UINT32(mbuf) = (uint32_t)desc_idx;
 					__rte_mbuf_raw_free(mbuf);
 					return ;
 				}
@@ -1936,7 +1939,7 @@ virtio_tx_route_zcp(struct virtio_net* dev, struct rte_mbuf *m, uint32_t desc_id
 	mbuf->pkt.vlan_macip.f.vlan_tci = vlan_tag;
 	mbuf->pkt.vlan_macip.f.l2_len = sizeof(struct ether_hdr);
 	mbuf->pkt.vlan_macip.f.l3_len = sizeof(struct ipv4_hdr);
-	RTE_MBUF_METADATA_UINT32(mbuf, 0) = (uint32_t)desc_idx;
+	MBUF_HEADROOM_UINT32(mbuf) = (uint32_t)desc_idx;
 
 	tx_q->m_table[len] = mbuf;
 	len++;
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 7faeeff..0008755 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -360,8 +360,8 @@ rte_eth_dev_tx_queue_start(uint8_t port_id, uint16_t tx_queue_id)
 	}
 
 	dev = &rte_eth_devices[port_id];
-	if (tx_queue_id >= dev->data->nb_rx_queues) {
-		PMD_DEBUG_TRACE("Invalid RX queue_id=%d\n", tx_queue_id);
+	if (tx_queue_id >= dev->data->nb_tx_queues) {
+		PMD_DEBUG_TRACE("Invalid TX queue_id=%d\n", tx_queue_id);
 		return (-EINVAL);
 	}
 
@@ -386,8 +386,8 @@ rte_eth_dev_tx_queue_stop(uint8_t port_id, uint16_t tx_queue_id)
 	}
 
 	dev = &rte_eth_devices[port_id];
-	if (tx_queue_id >= dev->data->nb_rx_queues) {
-		PMD_DEBUG_TRACE("Invalid RX queue_id=%d\n", tx_queue_id);
+	if (tx_queue_id >= dev->data->nb_tx_queues) {
+		PMD_DEBUG_TRACE("Invalid TX queue_id=%d\n", tx_queue_id);
 		return (-EINVAL);
 	}
 
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index baf3ca4..edffc2c 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -201,25 +201,8 @@ struct rte_mbuf {
 		struct rte_ctrlmbuf ctrl;
 		struct rte_pktmbuf pkt;
 	};
-
-	union {
-		uint8_t metadata[0];
-		uint16_t metadata16[0];
-		uint32_t metadata32[0];
-		uint64_t metadata64[0];
-	};
 } __rte_cache_aligned;
 
-#define RTE_MBUF_METADATA_UINT8(mbuf, offset)       (mbuf->metadata[offset])
-#define RTE_MBUF_METADATA_UINT16(mbuf, offset)      (mbuf->metadata16[offset/sizeof(uint16_t)])
-#define RTE_MBUF_METADATA_UINT32(mbuf, offset)      (mbuf->metadata32[offset/sizeof(uint32_t)])
-#define RTE_MBUF_METADATA_UINT64(mbuf, offset)      (mbuf->metadata64[offset/sizeof(uint64_t)])
-
-#define RTE_MBUF_METADATA_UINT8_PTR(mbuf, offset)   (&mbuf->metadata[offset])
-#define RTE_MBUF_METADATA_UINT16_PTR(mbuf, offset)  (&mbuf->metadata16[offset/sizeof(uint16_t)])
-#define RTE_MBUF_METADATA_UINT32_PTR(mbuf, offset)  (&mbuf->metadata32[offset/sizeof(uint32_t)])
-#define RTE_MBUF_METADATA_UINT64_PTR(mbuf, offset)  (&mbuf->metadata64[offset/sizeof(uint64_t)])
-
 /**
  * Given the buf_addr returns the pointer to corresponding mbuf.
  */
-- 
1.9.0

Re: [PATCH] [PMD] [VHOST] Revert unnecessary definition and fix wrong referring in user space vhost zero copy patches

[Thomas Monjalon]

Hi Changchun,

2014-05-19 23:09, Ouyang Changchun:
> 1. Revert the change of metadata macro definition for referring to headroom
> space in mbuf;
> 2. Fix wrongly referring to RX queues number in TX queues
> start/stop function.
> 
> Signed-off-by: Ouyang Changchun <changchun.ouyang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

You are fixing commits which are not yet applied.
Please merge and re-send the whole serie by suffixing with "v2".

The title was "[PATCH 0/3] [PMD] [VHOST] *** Support zero copy RX/TX in user 
space vhost ***"
It should be "[PATCH v2 0/3] Support zero copy RX/TX in user space vhost"

Other notes:
- please split API and ixgbe changes
- set a significant title to each patch
- use prefixes like "ethdev:", "ixgbe:" or "examples/vhost:"

In general, this page is a good help:
	http://dpdk.org/dev#send

Thanks
-- 
Thomas

Re: [PATCH] [PMD] [VHOST] Revert unnecessary definition and fix wrong referring in user space vhost zero copy patches

[Ouyang, Changchun]

Hi Thomas,

Fine, I will do it.

One more question:

You have comments as follow:
The title was "[PATCH 0/3] [PMD] [VHOST] *** Support zero copy RX/TX in user space vhost ***"
It should be "[PATCH v2 0/3] Support zero copy RX/TX in user space vhost"

So "[PMD] [VHOST]" in the title should be removed in the cover letter, right?
And in each separate patch letter, it could use "ixgbe:"  or "examples/vhost:", instead of "[PMD] [VHOST]"             
Is it right?


Thanks 
Changchun


-----Original Message-----
From: Thomas Monjalon [mailto:thomas.monjalon-pdR9zngts4EAvxtiuMwx3w@public.gmane.org] 
Sent: Tuesday, May 20, 2014 12:00 AM
To: Ouyang, Changchun
Cc: dev-VfR2kkLFssw@public.gmane.org
Subject: Re: [dpdk-dev] [PATCH] [PMD] [VHOST] Revert unnecessary definition and fix wrong referring in user space vhost zero copy patches

Hi Changchun,

2014-05-19 23:09, Ouyang Changchun:
> 1. Revert the change of metadata macro definition for referring to 
> headroom space in mbuf; 2. Fix wrongly referring to RX queues number 
> in TX queues start/stop function.
> 
> Signed-off-by: Ouyang Changchun <changchun.ouyang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

You are fixing commits which are not yet applied.
Please merge and re-send the whole serie by suffixing with "v2".

The title was "[PATCH 0/3] [PMD] [VHOST] *** Support zero copy RX/TX in user space vhost ***"
It should be "[PATCH v2 0/3] Support zero copy RX/TX in user space vhost"

Other notes:
- please split API and ixgbe changes
- set a significant title to each patch
- use prefixes like "ethdev:", "ixgbe:" or "examples/vhost:"

In general, this page is a good help:
	http://dpdk.org/dev#send

Thanks
--
Thomas

Re: [PATCH] [PMD] [VHOST] Revert unnecessary definition and fix wrong referring in user space vhost zero copy patches

[Thomas Monjalon]

Hi Changchun,

Please, it is preferred to answer below the question.

2014-05-20 01:14, Ouyang, Changchun:
> So "[PMD] [VHOST]" in the title should be removed in the cover letter,
> right? And in each separate patch letter, it could use "ixgbe:"  or
> "examples/vhost:", instead of "[PMD] [VHOST]" Is it right?

Yes, you did right in the v2.

Thanks
-- 
Thomas


> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon-pdR9zngts4EAvxtiuMwx3w@public.gmane.org]
> Sent: Tuesday, May 20, 2014 12:00 AM
> To: Ouyang, Changchun
> Cc: dev-VfR2kkLFssw@public.gmane.org
> Subject: Re: [dpdk-dev] [PATCH] [PMD] [VHOST] Revert unnecessary definition
> and fix wrong referring in user space vhost zero copy patches
> 
> Hi Changchun,
> 
> 2014-05-19 23:09, Ouyang Changchun:
> > 1. Revert the change of metadata macro definition for referring to
> > headroom space in mbuf; 2. Fix wrongly referring to RX queues number
> > in TX queues start/stop function.
> > 
> > Signed-off-by: Ouyang Changchun <changchun.ouyang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> 
> You are fixing commits which are not yet applied.
> Please merge and re-send the whole serie by suffixing with "v2".
> 
> The title was "[PATCH 0/3] [PMD] [VHOST] *** Support zero copy RX/TX in user
> space vhost ***" It should be "[PATCH v2 0/3] Support zero copy RX/TX in
> user space vhost"
> 
> Other notes:
> - please split API and ixgbe changes
> - set a significant title to each patch
> - use prefixes like "ethdev:", "ixgbe:" or "examples/vhost:"
> 
> In general, this page is a good help:
> 	http://dpdk.org/dev#send
> 
> Thanks
> --
> Thomas

Re: [PATCH] [PMD] [VHOST] Revert unnecessary definition and fix wrong referring in user space vhost zero copy patches

[Ouyang, Changchun]

Hi, Thomas
Thanks very much for your guiding!
Best regards,
Changchun

-----Original Message-----
From: Thomas Monjalon [mailto:thomas.monjalon-pdR9zngts4EAvxtiuMwx3w@public.gmane.org] 
Sent: Thursday, May 22, 2014 11:29 PM
To: Ouyang, Changchun
Cc: dev-VfR2kkLFssw@public.gmane.org
Subject: Re: [dpdk-dev] [PATCH] [PMD] [VHOST] Revert unnecessary definition and fix wrong referring in user space vhost zero copy patches

Hi Changchun,

Please, it is preferred to answer below the question.

2014-05-20 01:14, Ouyang, Changchun:
> So "[PMD] [VHOST]" in the title should be removed in the cover letter, 
> right? And in each separate patch letter, it could use "ixgbe:"  or 
> "examples/vhost:", instead of "[PMD] [VHOST]" Is it right?

Yes, you did right in the v2.

Thanks
--
Thomas


> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon-pdR9zngts4EAvxtiuMwx3w@public.gmane.org]
> Sent: Tuesday, May 20, 2014 12:00 AM
> To: Ouyang, Changchun
> Cc: dev-VfR2kkLFssw@public.gmane.org
> Subject: Re: [dpdk-dev] [PATCH] [PMD] [VHOST] Revert unnecessary definition
> and fix wrong referring in user space vhost zero copy patches
> 
> Hi Changchun,
> 
> 2014-05-19 23:09, Ouyang Changchun:
> > 1. Revert the change of metadata macro definition for referring to
> > headroom space in mbuf; 2. Fix wrongly referring to RX queues number
> > in TX queues start/stop function.
> > 
> > Signed-off-by: Ouyang Changchun <changchun.ouyang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> 
> You are fixing commits which are not yet applied.
> Please merge and re-send the whole serie by suffixing with "v2".
> 
> The title was "[PATCH 0/3] [PMD] [VHOST] *** Support zero copy RX/TX in user
> space vhost ***" It should be "[PATCH v2 0/3] Support zero copy RX/TX in
> user space vhost"
> 
> Other notes:
> - please split API and ixgbe changes
> - set a significant title to each patch
> - use prefixes like "ethdev:", "ixgbe:" or "examples/vhost:"
> 
> In general, this page is a good help:
> 	http://dpdk.org/dev#send
> 
> Thanks
> --
> Thomas