[dpdk-dev,v5,resend,03/12] vhost: vring queue setup for multiple queue support

Message ID 20150923034612.GF2339@yliu-dev.sh.intel.com (mailing list archive)
State Not Applicable, archived
Headers

Commit Message

Yuanhan Liu Sept. 23, 2015, 3:46 a.m. UTC
  On Tue, Sep 22, 2015 at 05:51:02PM +0300, Marcel Apfelbaum wrote:
> >It's proved to work after the fix (at least in my testing), but
> >it's late here and I'm gonna send a new version tomorrow, including
> >some other comments addressing. Please do more test then :)
> >

It's unlikely that I will send another version unless I have clear clue
how to address a comment from Michael about vring flush.

But anyway, you still could help me to prove the fix works. You can
apply the attachment on top of my old patchset, and it should work.

	--yliu
> 
> Those are very good news!
> Tomorrow we have holidays but the day after that I'll try it for sure.
  

Comments

Marcel Apfelbaum Sept. 24, 2015, 9:51 a.m. UTC | #1
On 09/23/2015 06:46 AM, Yuanhan Liu wrote:
> On Tue, Sep 22, 2015 at 05:51:02PM +0300, Marcel Apfelbaum wrote:
[...]
>>> It's proved to work after the fix (at least in my testing), but
>>> it's late here and I'm gonna send a new version tomorrow, including
>>> some other comments addressing. Please do more test then :)
>>>
>
> It's unlikely that I will send another version unless I have clear clue
> how to address a comment from Michael about vring flush.

Hi,

I don't pretend to understand how exactly this works in DPDK, but since the objective
is to not have packets in vring you could:

1. Disable the vq processing of new packets. (You introduced the 'enable' flag)
2. Wait a reasonable amount of time until the processing cores
    finish dealing with current packets.
3. Check the vqs that no packets are waiting for processing.

Again, this is only a suggestion and may be incomplete (or naive).

>
> But anyway, you still could help me to prove the fix works. You can
> apply the attachment on top of my old patchset, and it should work.
>

I tested it and it works just fine!

Thanks again,
Marcel

> 	--yliu
>>
>> Those are very good news!
>> Tomorrow we have holidays but the day after that I'll try it for sure.
>
  

Patch

diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c
index 33bdacd..d304ee6 100644
--- a/lib/librte_vhost/virtio-net.c
+++ b/lib/librte_vhost/virtio-net.c
@@ -467,6 +467,8 @@  static int
 set_features(struct vhost_device_ctx ctx, uint64_t *pu)
 {
 	struct virtio_net *dev;
+	uint16_t vhost_hlen;
+	uint16_t i;
 
 	dev = get_device(ctx);
 	if (dev == NULL)
@@ -474,27 +476,26 @@  set_features(struct vhost_device_ctx ctx, uint64_t *pu)
 	if (*pu & ~VHOST_FEATURES)
 		return -1;
 
-	/* Store the negotiated feature list for the device. */
 	dev->features = *pu;
-
-	/* Set the vhost_hlen depending on if VIRTIO_NET_F_MRG_RXBUF is set. */
 	if (dev->features & (1 << VIRTIO_NET_F_MRG_RXBUF)) {
 		LOG_DEBUG(VHOST_CONFIG,
 			"(%"PRIu64") Mergeable RX buffers enabled\n",
 			dev->device_fh);
-		dev->virtqueue[VIRTIO_RXQ]->vhost_hlen =
-			sizeof(struct virtio_net_hdr_mrg_rxbuf);
-		dev->virtqueue[VIRTIO_TXQ]->vhost_hlen =
-			sizeof(struct virtio_net_hdr_mrg_rxbuf);
+		vhost_hlen = sizeof(struct virtio_net_hdr_mrg_rxbuf);
 	} else {
 		LOG_DEBUG(VHOST_CONFIG,
 			"(%"PRIu64") Mergeable RX buffers disabled\n",
 			dev->device_fh);
-		dev->virtqueue[VIRTIO_RXQ]->vhost_hlen =
-			sizeof(struct virtio_net_hdr);
-		dev->virtqueue[VIRTIO_TXQ]->vhost_hlen =
-			sizeof(struct virtio_net_hdr);
+		vhost_hlen = sizeof(struct virtio_net_hdr);
+	}
+
+	for (i = 0; i < dev->virt_qp_nb; i++) {
+		uint16_t base_idx = i * VIRTIO_QNUM;
+
+		dev->virtqueue[base_idx + VIRTIO_RXQ]->vhost_hlen = vhost_hlen;
+		dev->virtqueue[base_idx + VIRTIO_TXQ]->vhost_hlen = vhost_hlen;
 	}
+
 	return 0;
 }