From: Maxime Coquelin <maxime.coquelin@redhat.com>
To: "Yao, Lei A" <lei.a.yao@intel.com>,
"'dev@dpdk.org'" <dev@dpdk.org>,
"Horton, Remy" <remy.horton@intel.com>,
"Bie, Tiwei" <tiwei.bie@intel.com>,
"'yliu@fridaylinux.org'" <yliu@fridaylinux.org>
Cc: "'mst@redhat.com'" <mst@redhat.com>,
"'jfreiman@redhat.com'" <jfreiman@redhat.com>,
"'vkaplans@redhat.com'" <vkaplans@redhat.com>,
"'jasowang@redhat.com'" <jasowang@redhat.com>
Subject: Re: [dpdk-dev] [PATCH v3 15/19] vhost: postpone rings addresses translation
Date: Mon, 16 Oct 2017 12:47:42 +0200 [thread overview]
Message-ID: <3e1f7960-711a-39fd-2a2f-ddea47df36d0@redhat.com> (raw)
In-Reply-To: <b9737645-e771-748d-4b58-4c4229e3fbbf@redhat.com>
On 10/16/2017 11:47 AM, Maxime Coquelin wrote:
> Hi Yao,
>
> On 10/16/2017 08:23 AM, Yao, Lei A wrote:
>> Hi, Maxime
>>
>> Add one comment:
>> This issue with virtio-net only occur when I use CPU on socket 1.
>
> Thanks for the report.
> I fail to reproduce for now.
>
> What is your qemu command line?
> Is it reproducible systematically when there is a NUMA reallocation
> (DPDK on socket 0, QEMU on socket 1)?
Nevermind, I just reproduced the (an?) issue.
The issue I reproduce is not linked to NUMA reallocation, but to
messages sequencing differences between QEMU versions.
So, I'm not 100% this is the same issue, as you mention it works fine
when using CPU socket 0.
The patch "vhost: postpone rings addresses translation" moves rings
addresses translation at either vring kick or enable time, depending
on whether protocol features are enabled or not. This has been done
because we must not interpret ring information as long as the vring is
not fully initialized.
While my patch works fine with recent QEMU version, it breaks with older
ones, like QEMU v2.5. The reason is that on these older versions,
VHOST_USER_SET_VRING_ENABLE is called once and before
VHOST_USER_SET_VRING_ADDR. At that time, the ring adresses aren't
available so the translation is not done. On recent QEMU versions,
we receive VHOST_USER_SET_VRING_ENABLE also after having received
the rings addresses, so it works fine.
The below fix consists in performing the rings addresses translation
also when handling VHOST_USER_SET_VRING_ADDR if ring has already been
enabled.
I'll post a formal patch later today or tomorrow morning after having
tested it more conscientiously. Let me know if it fixes your issue.
Thanks,
Maxime
diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index 76c4eeca5..1f6cba4b9 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -372,33 +372,6 @@ ring_addr_to_vva(struct virtio_net *dev, struct
vhost_virtqueue *vq,
return qva_to_vva(dev, ra);
}
-/*
- * The virtio device sends us the desc, used and avail ring addresses.
- * This function then converts these to our address space.
- */
-static int
-vhost_user_set_vring_addr(struct virtio_net *dev, VhostUserMsg *msg)
-{
- struct vhost_virtqueue *vq;
- struct vhost_vring_addr *addr = &msg->payload.addr;
-
- if (dev->mem == NULL)
- return -1;
-
- /* addr->index refers to the queue index. The txq 1, rxq is 0. */
- vq = dev->virtqueue[msg->payload.addr.index];
-
- /*
- * Rings addresses should not be interpreted as long as the ring
is not
- * started and enabled
- */
- memcpy(&vq->ring_addrs, addr, sizeof(*addr));
-
- vring_invalidate(dev, vq);
-
- return 0;
-}
-
static struct virtio_net *
translate_ring_addresses(struct virtio_net *dev, int vq_index)
{
@@ -464,6 +437,43 @@ translate_ring_addresses(struct virtio_net *dev,
int vq_index)
}
/*
+ * The virtio device sends us the desc, used and avail ring addresses.
+ * This function then converts these to our address space.
+ */
+static int
+vhost_user_set_vring_addr(struct virtio_net **pdev, VhostUserMsg *msg)
+{
+ struct vhost_virtqueue *vq;
+ struct vhost_vring_addr *addr = &msg->payload.addr;
+ struct virtio_net *dev = *pdev;
+
+ if (dev->mem == NULL)
+ return -1;
+
+ /* addr->index refers to the queue index. The txq 1, rxq is 0. */
+ vq = dev->virtqueue[msg->payload.addr.index];
+
+ /*
+ * Rings addresses should not be interpreted as long as the ring
is not
+ * started and enabled
+ */
+ memcpy(&vq->ring_addrs, addr, sizeof(*addr));
+
+ vring_invalidate(dev, vq);
+
+ if (vq->enabled && (dev->features &
+ (1ULL << VHOST_USER_F_PROTOCOL_FEATURES))) {
+ dev = translate_ring_addresses(dev,
msg->payload.state.index);
+ if (!dev)
+ return -1;
+
+ *pdev = dev;
+ }
+
+ return 0;
+}
+
+/*
* The virtio device sends us the available ring last used index.
*/
static int
@@ -1273,7 +1283,7 @@ vhost_user_msg_handler(int vid, int fd)
vhost_user_set_vring_num(dev, &msg);
break;
case VHOST_USER_SET_VRING_ADDR:
- vhost_user_set_vring_addr(dev, &msg);
+ vhost_user_set_vring_addr(&dev, &msg);
break;
case VHOST_USER_SET_VRING_BASE:
vhost_user_set_vring_base(dev, &msg);
next prev parent reply other threads:[~2017-10-16 10:47 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-05 8:36 [dpdk-dev] [PATCH v3 00/19] Vhost-user: Implement device IOTLB support Maxime Coquelin
2017-10-05 8:36 ` [dpdk-dev] [PATCH v3 01/19] Revert "vhost: workaround MQ fails to startup" Maxime Coquelin
2017-11-01 17:11 ` Kavanagh, Mark B
2017-11-02 9:40 ` Maxime Coquelin
2017-11-03 13:05 ` Yuanhan Liu
2017-11-03 14:28 ` Maxime Coquelin
2017-11-06 12:00 ` Yuanhan Liu
2017-11-06 12:07 ` Maxime Coquelin
2017-11-06 12:24 ` Yuanhan Liu
2017-11-06 12:50 ` Maxime Coquelin
2017-11-06 13:36 ` Yuanhan Liu
2017-11-03 15:34 ` Thomas Monjalon
2017-11-03 16:31 ` Kavanagh, Mark B
2017-10-05 8:36 ` [dpdk-dev] [PATCH v3 02/19] vhost: make error handling consistent in rx path Maxime Coquelin
2017-10-05 8:36 ` [dpdk-dev] [PATCH v3 03/19] vhost: prepare send_vhost_message() to slave requests Maxime Coquelin
2017-10-05 8:36 ` [dpdk-dev] [PATCH v3 04/19] vhost: add support to slave requests channel Maxime Coquelin
2017-10-05 8:36 ` [dpdk-dev] [PATCH v3 05/19] vhost: declare missing IOMMU-related definitions for old kernels Maxime Coquelin
2017-10-05 8:36 ` [dpdk-dev] [PATCH v3 06/19] vhost: add iotlb helper functions Maxime Coquelin
2017-10-05 8:36 ` [dpdk-dev] [PATCH v3 07/19] vhost: iotlb: add pending miss request list and helpers Maxime Coquelin
2017-10-05 8:36 ` [dpdk-dev] [PATCH v3 08/19] vhost-user: add support to IOTLB miss slave requests Maxime Coquelin
2017-10-05 8:36 ` [dpdk-dev] [PATCH v3 09/19] vhost: initialize vrings IOTLB caches Maxime Coquelin
2017-10-05 8:36 ` [dpdk-dev] [PATCH v3 10/19] vhost-user: handle IOTLB update and invalidate requests Maxime Coquelin
2017-10-05 8:36 ` [dpdk-dev] [PATCH v3 11/19] vhost: introduce guest IOVA to backend VA helper Maxime Coquelin
2017-10-05 8:36 ` [dpdk-dev] [PATCH v3 12/19] vhost: use the guest IOVA to host " Maxime Coquelin
2017-10-05 8:36 ` [dpdk-dev] [PATCH v3 13/19] vhost: enable rings at the right time Maxime Coquelin
2017-10-05 8:36 ` [dpdk-dev] [PATCH v3 14/19] vhost: don't dereference invalid dev pointer after its reallocation Maxime Coquelin
2017-10-05 8:36 ` [dpdk-dev] [PATCH v3 15/19] vhost: postpone rings addresses translation Maxime Coquelin
2017-10-13 1:47 ` Yao, Lei A
2017-10-13 7:32 ` Maxime Coquelin
2017-10-13 7:55 ` Yao, Lei A
2017-10-13 7:56 ` Maxime Coquelin
2017-10-16 5:59 ` Yao, Lei A
2017-10-16 6:23 ` Yao, Lei A
2017-10-16 9:47 ` Maxime Coquelin
2017-10-16 10:47 ` Maxime Coquelin [this message]
2017-10-17 1:24 ` Yao, Lei A
2017-10-17 8:06 ` Maxime Coquelin
2017-10-05 8:36 ` [dpdk-dev] [PATCH v3 16/19] vhost-user: translate ring addresses when IOMMU enabled Maxime Coquelin
2017-10-05 8:36 ` [dpdk-dev] [PATCH v3 17/19] vhost-user: iommu: postpone device creation until ring are mapped Maxime Coquelin
2017-11-02 7:21 ` Yao, Lei A
2017-11-02 8:21 ` Maxime Coquelin
2017-11-02 16:02 ` Maxime Coquelin
2017-11-03 8:25 ` Maxime Coquelin
2017-11-03 15:15 ` Michael S. Tsirkin
2017-11-03 15:54 ` Maxime Coquelin
2017-10-05 8:36 ` [dpdk-dev] [PATCH v3 18/19] vhost: iommu: Invalidate vring in case of matching IOTLB invalidate Maxime Coquelin
2017-10-05 8:36 ` [dpdk-dev] [PATCH v3 19/19] vhost: enable IOMMU support Maxime Coquelin
2017-10-06 6:24 ` [dpdk-dev] [PATCH v3 00/19] Vhost-user: Implement device IOTLB support Yuanhan Liu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3e1f7960-711a-39fd-2a2f-ddea47df36d0@redhat.com \
--to=maxime.coquelin@redhat.com \
--cc=dev@dpdk.org \
--cc=jasowang@redhat.com \
--cc=jfreiman@redhat.com \
--cc=lei.a.yao@intel.com \
--cc=mst@redhat.com \
--cc=remy.horton@intel.com \
--cc=tiwei.bie@intel.com \
--cc=vkaplans@redhat.com \
--cc=yliu@fridaylinux.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).