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)?
Maxime
-----Original Message-----
From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Yao, Lei A
Sent: Monday, October 16, 2017 2:00 PM
To: 'Maxime Coquelin' <maxime.coque...@redhat.com>; 'dev@dpdk.org'
<dev@dpdk.org>; Horton, Remy <remy.hor...@intel.com>; Bie, Tiwei
<tiwei....@intel.com>; 'y...@fridaylinux.org' <y...@fridaylinux.org>
Cc: 'm...@redhat.com' <m...@redhat.com>; 'jfrei...@redhat.com'
<jfrei...@redhat.com>; 'vkapl...@redhat.com' <vkapl...@redhat.com>;
'jasow...@redhat.com' <jasow...@redhat.com>
Subject: Re: [dpdk-dev] [PATCH v3 15/19] vhost: postpone rings addresses
translation
Hi, Maxime
-----Original Message-----
From: Yao, Lei A
Sent: Friday, October 13, 2017 3:55 PM
To: Maxime Coquelin <maxime.coque...@redhat.com>; dev@dpdk.org;
Horton, Remy <remy.hor...@intel.com>; Bie, Tiwei <tiwei....@intel.com>;
y...@fridaylinux.org
Cc: m...@redhat.com; jfrei...@redhat.com; vkapl...@redhat.com;
jasow...@redhat.com
Subject: RE: [dpdk-dev] [PATCH v3 15/19] vhost: postpone rings addresses
translation
Hi, Maxime
-----Original Message-----
From: Maxime Coquelin [mailto:maxime.coque...@redhat.com]
Sent: Friday, October 13, 2017 3:32 PM
To: Yao, Lei A <lei.a....@intel.com>; dev@dpdk.org; Horton, Remy
<remy.hor...@intel.com>; Bie, Tiwei <tiwei....@intel.com>;
y...@fridaylinux.org
Cc: m...@redhat.com; jfrei...@redhat.com; vkapl...@redhat.com;
jasow...@redhat.com
Subject: Re: [dpdk-dev] [PATCH v3 15/19] vhost: postpone rings
addresses
translation
Hi Lei,
On 10/13/2017 03:47 AM, Yao, Lei A wrote:
Hi, Maxime
After this commit, vhost/virtio loopback test will fail when
use the CPU on socket 1.
Error message like following during initialize:
VHOST_CONFIG: vring kick idx:0 file:20
VHOST_CONFIG: reallocate vq from 0 to 1 node
VHOST_CONFIG: reallocate dev from 0 to 1 node
VHOST_CONFIG: (0) failed to find avail ring address.
VHOST_CONFIG: read message VHOST_USER_SET_VRING_NUM
VHOST_CONFIG: read message VHOST_USER_SET_VRING_BASE
VHOST_CONFIG: read message VHOST_USER_SET_VRING_ADDR
VHOST_CONFIG: read message VHOST_USER_SET_VRING_KICK
VHOST_CONFIG: vring kick idx:1 file:21
VHOST_CONFIG: reallocate vq from 0 to 1 node
VHOST_CONFIG: (0) failed to find avail ring address.
But if use CPU on socket 0. It still can work. Could you have a check on
this? Thanks a lot!
Thanks for reporting the issue. It seems addr pointer still points to
the old structure after the reallocation. This patch should fix the
issue:
diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index 9acac6125..2416a0061 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -417,6 +417,7 @@ translate_ring_addresses(struct virtio_net *dev,
int
vq_index)
dev = numa_realloc(dev, vq_index);
vq = dev->virtqueue[vq_index];
+ addr = &vq->ring_addrs;
vq->avail = (struct vring_avail *)(uintptr_t)ring_addr_to_vva(dev,
vq, addr->avail_user_addr, sizeof(struct
vring_avail));
I only have access to single-socket machines at the moment, so I cannot
reproduce the issue.
Can you have a try?
With your new patch, it can work on socket 1 now. Thanks.
I find another issue with this patch when I use V17.11-rc1.
It will break the connection between vhost and virtio-net in VM.
The link status of vhost-user port is always down.
But virtio-pmd driver is still ok.
My qemu version: 2.5
Guest OS: Ubuntu 16.04
Guest kernel: 4.4.0
Could you have a check on this issue? Thanks a lot!
BRs
Lei
Thanks,
Maxime
Following is my cmd:
Vhost: testpmd -n 4 -c 0x3000000 --socket-mem 1024,1024 --file-
prefix=vhost \
--vdev 'net_vhost0,iface=vhost-net,queues=1,client=0' -- -i
Virtio-user: testpmd -n 4 -c 0xc00000 --socket-mem 1024,1024 --no-pci -
-
file-prefix=virtio \
--
vdev=net_virtio_user0,mac=00:01:02:03:04:05,path=./vhost-
net \
-- -i --txqflags=0xf01 --disable-hw-vlan-filter
BRs
Lei
-----Original Message-----
From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Maxime
Coquelin
Sent: Thursday, October 5, 2017 4:36 PM
To: dev@dpdk.org; Horton, Remy <remy.hor...@intel.com>; Bie,
Tiwei
<tiwei....@intel.com>; y...@fridaylinux.org
Cc: m...@redhat.com; jfrei...@redhat.com; vkapl...@redhat.com;
jasow...@redhat.com; Maxime Coquelin
<maxime.coque...@redhat.com>
Subject: [dpdk-dev] [PATCH v3 15/19] vhost: postpone rings
addresses
translation
This patch postpones rings addresses translations and checks, as
addresses sent by the master shuld not be interpreted as long as
ring is not started and enabled[0].
When protocol features aren't negotiated, the ring is started in
enabled state, so the addresses translations are postponed to
vhost_user_set_vring_kick().
Otherwise, it is postponed to when ring is enabled, in
vhost_user_set_vring_enable().
[0]: http://lists.nongnu.org/archive/html/qemu-devel/2017-
05/msg04355.html
Signed-off-by: Maxime Coquelin <maxime.coque...@redhat.com>
---
lib/librte_vhost/vhost.h | 1 +
lib/librte_vhost/vhost_user.c | 69
++++++++++++++++++++++++++++++++++---------
2 files changed, 56 insertions(+), 14 deletions(-)
diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h
index 79351c66f..903da5db5 100644
--- a/lib/librte_vhost/vhost.h
+++ b/lib/librte_vhost/vhost.h
@@ -125,6 +125,7 @@ struct vhost_virtqueue {
struct vring_used_elem *shadow_used_ring;
uint16_t shadow_used_idx;
+ struct vhost_vring_addr ring_addrs;
struct batch_copy_elem *batch_copy_elems;
uint16_t batch_copy_nb_elems;
diff --git a/lib/librte_vhost/vhost_user.c
b/lib/librte_vhost/vhost_user.c
index f495dd36e..319867c65 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -356,6 +356,7 @@ 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;
@@ -363,35 +364,50 @@ vhost_user_set_vring_addr(struct virtio_net
*dev,
VhostUserMsg *msg)
/* 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));
+
+ return 0;
+}
+
+static struct virtio_net *translate_ring_addresses(struct virtio_net
*dev,
+ int vq_index)
+{
+ struct vhost_virtqueue *vq = dev->virtqueue[vq_index];
+ struct vhost_vring_addr *addr = &vq->ring_addrs;
+
/* The addresses are converted from QEMU virtual to Vhost
virtual.
*/
vq->desc = (struct vring_desc *)(uintptr_t)qva_to_vva(dev,
- msg->payload.addr.desc_user_addr);
+ addr->desc_user_addr);
if (vq->desc == 0) {
RTE_LOG(ERR, VHOST_CONFIG,
"(%d) failed to find desc ring address.\n",
dev->vid);
- return -1;
+ return NULL;
}
- dev = numa_realloc(dev, msg->payload.addr.index);
- vq = dev->virtqueue[msg->payload.addr.index];
+ dev = numa_realloc(dev, vq_index);
+ vq = dev->virtqueue[vq_index];
vq->avail = (struct vring_avail *)(uintptr_t)qva_to_vva(dev,
- msg->payload.addr.avail_user_addr);
+ addr->avail_user_addr);
if (vq->avail == 0) {
RTE_LOG(ERR, VHOST_CONFIG,
"(%d) failed to find avail ring address.\n",
dev->vid);
- return -1;
+ return NULL;
}
vq->used = (struct vring_used *)(uintptr_t)qva_to_vva(dev,
- msg->payload.addr.used_user_addr);
+ addr->used_user_addr);
if (vq->used == 0) {
RTE_LOG(ERR, VHOST_CONFIG,
"(%d) failed to find used ring address.\n",
dev->vid);
- return -1;
+ return NULL;
}
if (vq->last_used_idx != vq->used->idx) {
@@ -403,7 +419,7 @@ vhost_user_set_vring_addr(struct virtio_net
*dev,
VhostUserMsg *msg)
vq->last_avail_idx = vq->used->idx;
}
- vq->log_guest_addr = msg->payload.addr.log_guest_addr;
+ vq->log_guest_addr = addr->log_guest_addr;
LOG_DEBUG(VHOST_CONFIG, "(%d) mapped address
desc: %p\n",
dev->vid, vq->desc);
@@ -414,7 +430,7 @@ vhost_user_set_vring_addr(struct virtio_net
*dev,
VhostUserMsg *msg)
LOG_DEBUG(VHOST_CONFIG, "(%d) log_guest_addr: %"
PRIx64 "\n",
dev->vid, vq->log_guest_addr);
- return 0;
+ return dev;
}
/*
@@ -685,10 +701,11 @@ vhost_user_set_vring_call(struct virtio_net
*dev,
struct VhostUserMsg *pmsg)
}
static void
-vhost_user_set_vring_kick(struct virtio_net *dev, struct
VhostUserMsg
*pmsg)
+vhost_user_set_vring_kick(struct virtio_net **pdev, struct
VhostUserMsg
*pmsg)
{
struct vhost_vring_file file;
struct vhost_virtqueue *vq;
+ struct virtio_net *dev = *pdev;
file.index = pmsg->payload.u64 &
VHOST_USER_VRING_IDX_MASK;
if (pmsg->payload.u64 & VHOST_USER_VRING_NOFD_MASK)
@@ -698,6 +715,16 @@ vhost_user_set_vring_kick(struct virtio_net
*dev,
struct VhostUserMsg *pmsg)
RTE_LOG(INFO, VHOST_CONFIG,
"vring kick idx:%d file:%d\n", file.index, file.fd);
+ /*
+ * Interpret ring addresses only when ring is started and
enabled.
+ * This is now if protocol features aren't supported.
+ */
+ if (!(dev->features & (1ULL <<
VHOST_USER_F_PROTOCOL_FEATURES))) {
+ *pdev = dev = translate_ring_addresses(dev,
file.index);
+ if (!dev)
+ return;
+ }
+
vq = dev->virtqueue[file.index];
/*
@@ -778,15 +805,29 @@ vhost_user_get_vring_base(struct
virtio_net
*dev,
* enable the virtio queue pair.
*/
static int
-vhost_user_set_vring_enable(struct virtio_net *dev,
+vhost_user_set_vring_enable(struct virtio_net **pdev,
VhostUserMsg *msg)
{
+ struct virtio_net *dev = *pdev;
int enable = (int)msg->payload.state.num;
RTE_LOG(INFO, VHOST_CONFIG,
"set queue enable: %d to qp idx: %d\n",
enable, msg->payload.state.index);
+ /*
+ * Interpret ring addresses only when ring is started and
enabled.
+ * This is now if protocol features are supported.
+ */
+ if (enable && (dev->features &
+ (1ULL <<
VHOST_USER_F_PROTOCOL_FEATURES))) {
+ dev = translate_ring_addresses(dev, msg-
payload.state.index);
+ if (!dev)
+ return -1;
+
+ *pdev = dev;
+ }
+
if (dev->notify_ops->vring_state_changed)
dev->notify_ops->vring_state_changed(dev->vid,
msg->payload.state.index, enable);
@@ -1155,7 +1196,7 @@ vhost_user_msg_handler(int vid, int fd)
break;
case VHOST_USER_SET_VRING_KICK:
- vhost_user_set_vring_kick(dev, &msg);
+ vhost_user_set_vring_kick(&dev, &msg);
break;
case VHOST_USER_SET_VRING_CALL:
vhost_user_set_vring_call(dev, &msg);
@@ -1174,7 +1215,7 @@ vhost_user_msg_handler(int vid, int fd)
break;
case VHOST_USER_SET_VRING_ENABLE:
- vhost_user_set_vring_enable(dev, &msg);
+ vhost_user_set_vring_enable(&dev, &msg);
break;
case VHOST_USER_SEND_RARP:
vhost_user_send_rarp(dev, &msg);
--
2.13.6
