Skip to contentSkip to conversation

net/vhost: set the vq consistently

PATCH Applied Chas Williams 3 participants 7 messages Mar 29, 2018 – May 8, 2018 Network drivers

lore
Share & tools
lore
#0 lore raw

PATCH 1/2 net/vhost: set the vq consistently

From: Robert Shearman <[email protected]>

The vid and port of the vq were only set on queues created during initial device setup.

Fixes ee584e9710b9 ("vhost: add driver on top of the library")
---
 drivers/net/vhost/rte_eth_vhost.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
drivers/net/vhost/rte_eth_vhost.c +15 -0
index 3aae01c39..453d9bee1 100644
@@ -118,6 +118,7 @@ struct pmd_internal {
 	char *iface_name;
 	uint16_t max_queues;
 	rte_atomic32_t started;
+	int vid;
 };
 
 struct internal_list {
@@ -573,6 +574,7 @@ new_device(int vid)
 
 	eth_dev = list->eth_dev;
 	internal = eth_dev->data->dev_private;
+	internal->vid = vid;
 
 #ifdef RTE_LIBRTE_VHOST_NUMA
 	newnode = rte_vhost_get_numa_node(vid);
@@ -633,6 +635,7 @@ destroy_device(int vid)
 	}
 	eth_dev = list->eth_dev;
 	internal = eth_dev->data->dev_private;
+	internal->vid = -1;
 
 	rte_atomic32_set(&internal->dev_attached, 0);
 	update_queuing_status(eth_dev);
@@ -834,6 +837,9 @@ eth_rx_queue_setup(struct rte_eth_dev *dev, uint16_t rx_queue_id,
 		   struct rte_mempool *mb_pool)
 {
 	struct vhost_queue *vq;
+	struct pmd_internal *internal;
+
+	internal = dev->data->dev_private;
 
 	vq = rte_zmalloc_socket(NULL, sizeof(struct vhost_queue),
 			RTE_CACHE_LINE_SIZE, socket_id);
@@ -844,6 +850,9 @@ eth_rx_queue_setup(struct rte_eth_dev *dev, uint16_t rx_queue_id,
 
 	vq->mb_pool = mb_pool;
 	vq->virtqueue_id = rx_queue_id * VIRTIO_QNUM + VIRTIO_TXQ;
+	vq->internal = internal;
+	vq->vid = internal->vid;
+	vq->port = dev->data->port_id;
 	dev->data->rx_queues[rx_queue_id] = vq;
 
 	return 0;
@@ -856,6 +865,9 @@ eth_tx_queue_setup(struct rte_eth_dev *dev, uint16_t tx_queue_id,
 		   const struct rte_eth_txconf *tx_conf __rte_unused)
 {
 	struct vhost_queue *vq;
+	struct pmd_internal *internal;
+
+	internal = dev->data->dev_private;
 
 	vq = rte_zmalloc_socket(NULL, sizeof(struct vhost_queue),
 			RTE_CACHE_LINE_SIZE, socket_id);
@@ -865,6 +877,9 @@ eth_tx_queue_setup(struct rte_eth_dev *dev, uint16_t tx_queue_id,
 	}
 
 	vq->virtqueue_id = tx_queue_id * VIRTIO_QNUM + VIRTIO_RXQ;
+	vq->internal = internal;
+	vq->vid = internal->vid;
+	vq->port = dev->data->port_id;
 	dev->data->tx_queues[tx_queue_id] = vq;
 
 	return 0;
Signature · 1 line
2.13.6
#1 lore raw

PATCH 2/2 net/vhost: insert/strip VLAN header in software

From: Jan Blunck <[email protected]>

This lets the vhost driver handle the VLAN header like the virtio driver in software.

---
 drivers/net/vhost/rte_eth_vhost.c | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)
drivers/net/vhost/rte_eth_vhost.c +34 -1
index 453d9bee1..0beb28e94 100644
@@ -119,6 +119,7 @@ struct pmd_internal {
 	uint16_t max_queues;
 	rte_atomic32_t started;
 	int vid;
+	uint8_t vlan_strip;
 };
 
 struct internal_list {
@@ -422,6 +423,12 @@ eth_vhost_rx(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs)
 
 	for (i = 0; likely(i < nb_rx); i++) {
 		bufs[i]->port = r->port;
+		bufs[i]->ol_flags = 0;
+		bufs[i]->vlan_tci = 0;
+
+		if (r->internal->vlan_strip)
+			rte_vlan_strip(bufs[i]);
+
 		r->stats.bytes += bufs[i]->pkt_len;
 	}
 
@@ -438,7 +445,7 @@ eth_vhost_tx(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs)
 {
 	struct vhost_queue *r = q;
 	uint16_t i, nb_tx = 0;
-	uint16_t nb_send = nb_bufs;
+	uint16_t nb_send = 0;
 
 	if (unlikely(rte_atomic32_read(&r->allow_queuing) == 0))
 		return 0;
@@ -448,6 +455,22 @@ eth_vhost_tx(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs)
 	if (unlikely(rte_atomic32_read(&r->allow_queuing) == 0))
 		goto out;
 
+	for (i = 0; i < nb_bufs; i++) {
+		struct rte_mbuf *m = bufs[i];
+
+		/* Do VLAN tag insertion */
+		if (m->ol_flags & PKT_TX_VLAN_PKT) {
+			int error = rte_vlan_insert(&m);
+			if (unlikely(error)) {
+				rte_pktmbuf_free(m);
+				continue;
+			}
+		}
+
+		bufs[nb_send] = m;
+		++nb_send;
+	}
+
 	/* Enqueue packets to guest RX queue */
 	while (nb_send) {
 		uint16_t nb_pkts;
@@ -489,6 +512,16 @@ eth_vhost_tx(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs)
 static int
 eth_dev_configure(struct rte_eth_dev *dev __rte_unused)
 {
+	struct pmd_internal *internal = dev->data->dev_private;
+	const struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
+
+	internal->vlan_strip = rxmode->hw_vlan_strip;
+
+	if (rxmode->hw_vlan_filter)
+		RTE_LOG(WARNING, PMD,
+			"vhost(%s): vlan filtering not available\n",
+			internal->dev_name);
+
 	return 0;
 }
 
Signature · 1 line
2.13.6
Reviewed-by on v1 2/2
#2 lore raw

Re: [PATCH 2/2] net/vhost: insert/strip VLAN header in software

Quoted 10 lines · Chas Williams

From: Jan Blunck <[email protected]>

This lets the vhost driver handle the VLAN header like the virtio driver in software.

---
  drivers/net/vhost/rte_eth_vhost.c | 35 ++++++++++++++++++++++++++++++++++-
  1 file changed, 34 insertions(+), 1 deletion(-)

Thanks!
Maxime

#3 lore raw

Re: [PATCH 2/2] net/vhost: insert/strip VLAN header in software

In reply to Chas Williams
Quoted 9 lines · Chas Williams

From: Jan Blunck <[email protected]>

This lets the vhost driver handle the VLAN header like the virtio driver in software.

---
  drivers/net/vhost/rte_eth_vhost.c | 35 ++++++++++++++++++++++++++++++++++-
  1 file changed, 34 insertions(+), 1 deletion(-)

Applied to dpdk-next-virtio/master.

Thanks!
Maxime

#4 lore raw

Re: [PATCH 2/2] net/vhost: insert/strip VLAN header in software

In reply to Chas Williams

Hi, Jan

For this patch, I find it will break the VM2VM Iperf test as it clear the of_flags. "bufs[i]->ol_flags = 0;"
Could you have a check on this?

The test step to reproduce this issue:
1. Lauch testpmd with two vhost-user port, using IO fwd mode testpmd -c 0xe -n 4 --socket-mem 1024,1024 \
--legacy-mem --vdev 'eth_vhost0,iface=vhost-net,queues=1' \
--vdev 'eth_vhost1,iface=vhost-net1,queues=1' --nb-cores=1 \
--tx-offloads=0x802a
2. Lauch 2 vms with virtio deivce
3. In the 2 vms, up the virtio device and run Iperf test between VMs The test result show that the Iperf traffic is broken but ping test can pass.

Quoted 102 lines

-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Chas Williams
Sent: Friday, March 30, 2018 12:05 AM
To: [email protected]
Cc: [email protected]; [email protected]; [email protected]; Jan Blunck <[email protected]>
Subject: [dpdk-dev] [PATCH 2/2] net/vhost: insert/strip VLAN header in software

From: Jan Blunck <[email protected]>

This lets the vhost driver handle the VLAN header like the virtio driver in software.

---
 drivers/net/vhost/rte_eth_vhost.c | 35
++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)
drivers/net/vhost/rte_eth_vhost.c +8 -1
index 453d9bee1..0beb28e94 100644
@@ -119,6 +119,7 @@ struct pmd_internal {
 	uint16_t max_queues;
 	rte_atomic32_t started;
 	int vid;
+	uint8_t vlan_strip;
 };
 
 struct internal_list {
@@ -422,6 +423,12 @@ eth_vhost_rx(void *q, struct rte_mbuf **bufs,
 uint16_t nb_bufs)
 
 	for (i = 0; likely(i < nb_rx); i++) {
 		bufs[i]->port = r->port;
+		bufs[i]->ol_flags = 0;
+		bufs[i]->vlan_tci = 0;
+
+		if (r->internal->vlan_strip)
+			rte_vlan_strip(bufs[i]);
+
 		r->stats.bytes += bufs[i]->pkt_len;
 	}
@@ -438,7 +445,7 @@ eth_vhost_tx(void *q, struct rte_mbuf **bufs,
 uint16_t nb_bufs)
 {
 	struct vhost_queue *r = q;
 	uint16_t i, nb_tx = 0;
-	uint16_t nb_send = nb_bufs;
+	uint16_t nb_send = 0;
 
 	if (unlikely(rte_atomic32_read(&r->allow_queuing) == 0))
drivers/net/vhost/rte_eth_vhost.c +16 -0
 		return 0;
@@ -448,6 +455,22 @@ eth_vhost_tx(void *q, struct rte_mbuf **bufs,
 uint16_t nb_bufs)
 	if (unlikely(rte_atomic32_read(&r->allow_queuing) == 0))
 		goto out;
 
+	for (i = 0; i < nb_bufs; i++) {
+		struct rte_mbuf *m = bufs[i];
+
+		/* Do VLAN tag insertion */
+		if (m->ol_flags & PKT_TX_VLAN_PKT) {
+			int error = rte_vlan_insert(&m);
+			if (unlikely(error)) {
+				rte_pktmbuf_free(m);
+				continue;
+			}
+		}
+
+		bufs[nb_send] = m;
+		++nb_send;
+	}
+
 	/* Enqueue packets to guest RX queue */
 	while (nb_send) {
drivers/net/vhost/rte_eth_vhost.c +10 -0
 		uint16_t nb_pkts;
@@ -489,6 +512,16 @@ eth_vhost_tx(void *q, struct rte_mbuf **bufs,
 uint16_t nb_bufs)
 static int
 eth_dev_configure(struct rte_eth_dev *dev __rte_unused)
 {
+	struct pmd_internal *internal = dev->data->dev_private;
+	const struct rte_eth_rxmode *rxmode = &dev->data-
 >dev_conf.rxmode;
+
+	internal->vlan_strip = rxmode->hw_vlan_strip;
+
+	if (rxmode->hw_vlan_filter)
+		RTE_LOG(WARNING, PMD,
+			"vhost(%s): vlan filtering not available\n",
+			internal->dev_name);
+
 	return 0;

}

--
2.13.6

Reviewed-by on v1 1/2
#5 lore raw
In reply to Chas Williams
Quoted 5 lines · Chas Williams

From: Robert Shearman <[email protected]>

The vid and port of the vq were only set on queues created during initial device setup.

Quoted 9 lines
Fixes ee584e9710b9 ("vhost: add driver on top of the library")
---
  drivers/net/vhost/rte_eth_vhost.c | 15 +++++++++++++++
  1 file changed, 15 insertions(+)

Thanks,
Maxime

#6 lore raw
Quoted 11 lines · Maxime Coquelin
Quoted 8 lines · Chas Williams

From: Robert Shearman <[email protected]>

The vid and port of the vq were only set on queues created during initial device setup.

Fixes ee584e9710b9 ("vhost: add driver on top of the library")
Quoted 8 lines
Quoted 6 lines

---
  drivers/net/vhost/rte_eth_vhost.c | 15 +++++++++++++++

  1 file changed, 15 insertions(+)

It does not apply because a patch from Junjie seems to address the same problem:

net/vhost: fix crash when creating vdev dynamically

Can you check dpdk-next-virtio/master and confirm this patch is no more required?

Thanks,
Maxime