From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 6F7B11B5F3 for ; Fri, 13 Oct 2017 03:47:57 +0200 (CEST) Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Oct 2017 18:47:56 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.43,369,1503385200"; d="scan'208";a="162124409" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by fmsmga005.fm.intel.com with ESMTP; 12 Oct 2017 18:47:56 -0700 Received: from fmsmsx123.amr.corp.intel.com (10.18.125.38) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 12 Oct 2017 18:47:55 -0700 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by fmsmsx123.amr.corp.intel.com (10.18.125.38) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 12 Oct 2017 18:47:55 -0700 Received: from shsmsx151.ccr.corp.intel.com ([169.254.3.98]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.93]) with mapi id 14.03.0319.002; Fri, 13 Oct 2017 09:47:53 +0800 From: "Yao, Lei A" To: Maxime Coquelin , "dev@dpdk.org" , "Horton, Remy" , "Bie, Tiwei" , "yliu@fridaylinux.org" CC: "mst@redhat.com" , "jfreiman@redhat.com" , "vkaplans@redhat.com" , "jasowang@redhat.com" Thread-Topic: [dpdk-dev] [PATCH v3 15/19] vhost: postpone rings addresses translation Thread-Index: AQHTPbYtzV7VpTQpiE69DOn7iGhLCaLhDcYw Date: Fri, 13 Oct 2017 01:47:53 +0000 Message-ID: <2DBBFF226F7CF64BAFCA79B681719D953A2A66FC@SHSMSX151.ccr.corp.intel.com> References: <20171005083627.27828-1-maxime.coquelin@redhat.com> <20171005083627.27828-16-maxime.coquelin@redhat.com> In-Reply-To: <20171005083627.27828-16-maxime.coquelin@redhat.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZTRjMDZjNjgtNzQ4ZC00MDc0LWI4ZTEtYTlmMDI4ZGEzOTI5IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6IjFJS1RUaEJCQThNZTFQY3JYWmlabk9RWXNIdTRzOGJaQml6QVdDaWVjQm89In0= x-ctpclassification: CTP_IC dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v3 15/19] vhost: postpone rings addresses translation X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 13 Oct 2017 01:47:58 -0000 Hi, Maxime After this commit, vhost/virtio loopback test will fail when=20 use the CPU on socket 1.=20 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=20 this? Thanks a lot! Following is my cmd: Vhost: testpmd -n 4 -c 0x3000000 --socket-mem 1024,1024 --file-prefix=3Dvho= st \ --vdev 'net_vhost0,iface=3Dvhost-net,queues=3D1,client=3D0'= -- -i Virtio-user: testpmd -n 4 -c 0xc00000 --socket-mem 1024,1024 --no-pci --fil= e-prefix=3Dvirtio \ --vdev=3Dnet_virtio_user0,mac=3D00:01:02:03:04:05,pa= th=3D./vhost-net \ -- -i --txqflags=3D0xf01 --disable-hw-vlan-filter BRs Lei=20 > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Maxime Coquelin > Sent: Thursday, October 5, 2017 4:36 PM > To: dev@dpdk.org; Horton, Remy ; Bie, Tiwei > ; yliu@fridaylinux.org > Cc: mst@redhat.com; jfreiman@redhat.com; vkaplans@redhat.com; > jasowang@redhat.com; Maxime Coquelin > Subject: [dpdk-dev] [PATCH v3 15/19] vhost: postpone rings addresses > translation >=20 > 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]. >=20 > 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(). >=20 > [0]: http://lists.nongnu.org/archive/html/qemu-devel/2017- > 05/msg04355.html >=20 > Signed-off-by: Maxime Coquelin > --- > lib/librte_vhost/vhost.h | 1 + > lib/librte_vhost/vhost_user.c | 69 > ++++++++++++++++++++++++++++++++++--------- > 2 files changed, 56 insertions(+), 14 deletions(-) >=20 > 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 { >=20 > struct vring_used_elem *shadow_used_ring; > uint16_t shadow_used_idx; > + struct vhost_vring_addr ring_addrs; >=20 > 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 =3D &msg->payload.addr; >=20 > if (dev->mem =3D=3D 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 =3D dev->virtqueue[msg->payload.addr.index]; >=20 > + /* > + * 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 *de= v, > + int vq_index) > +{ > + struct vhost_virtqueue *vq =3D dev->virtqueue[vq_index]; > + struct vhost_vring_addr *addr =3D &vq->ring_addrs; > + > /* The addresses are converted from QEMU virtual to Vhost virtual. > */ > vq->desc =3D (struct vring_desc *)(uintptr_t)qva_to_vva(dev, > - msg->payload.addr.desc_user_addr); > + addr->desc_user_addr); > if (vq->desc =3D=3D 0) { > RTE_LOG(ERR, VHOST_CONFIG, > "(%d) failed to find desc ring address.\n", > dev->vid); > - return -1; > + return NULL; > } >=20 > - dev =3D numa_realloc(dev, msg->payload.addr.index); > - vq =3D dev->virtqueue[msg->payload.addr.index]; > + dev =3D numa_realloc(dev, vq_index); > + vq =3D dev->virtqueue[vq_index]; >=20 > vq->avail =3D (struct vring_avail *)(uintptr_t)qva_to_vva(dev, > - msg->payload.addr.avail_user_addr); > + addr->avail_user_addr); > if (vq->avail =3D=3D 0) { > RTE_LOG(ERR, VHOST_CONFIG, > "(%d) failed to find avail ring address.\n", > dev->vid); > - return -1; > + return NULL; > } >=20 > vq->used =3D (struct vring_used *)(uintptr_t)qva_to_vva(dev, > - msg->payload.addr.used_user_addr); > + addr->used_user_addr); > if (vq->used =3D=3D 0) { > RTE_LOG(ERR, VHOST_CONFIG, > "(%d) failed to find used ring address.\n", > dev->vid); > - return -1; > + return NULL; > } >=20 > if (vq->last_used_idx !=3D vq->used->idx) { > @@ -403,7 +419,7 @@ vhost_user_set_vring_addr(struct virtio_net *dev, > VhostUserMsg *msg) > vq->last_avail_idx =3D vq->used->idx; > } >=20 > - vq->log_guest_addr =3D msg->payload.addr.log_guest_addr; > + vq->log_guest_addr =3D addr->log_guest_addr; >=20 > 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); >=20 > - return 0; > + return dev; > } >=20 > /* > @@ -685,10 +701,11 @@ vhost_user_set_vring_call(struct virtio_net *dev, > struct VhostUserMsg *pmsg) > } >=20 > 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 =3D *pdev; >=20 > file.index =3D 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); >=20 > + /* > + * 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 =3D dev =3D translate_ring_addresses(dev, file.index); > + if (!dev) > + return; > + } > + > vq =3D dev->virtqueue[file.index]; >=20 > /* > @@ -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 =3D *pdev; > int enable =3D (int)msg->payload.state.num; >=20 > RTE_LOG(INFO, VHOST_CONFIG, > "set queue enable: %d to qp idx: %d\n", > enable, msg->payload.state.index); >=20 > + /* > + * 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 =3D translate_ring_addresses(dev, msg- > >payload.state.index); > + if (!dev) > + return -1; > + > + *pdev =3D 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; >=20 > 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; >=20 > 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