diff mbox series

[v3,8/8] vhost: improve vDPA blk device readiness condition

Message ID 1663308990-621-9-git-send-email-andy.pei@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers
Series vdpa/ifc: add multi queue support |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/github-robot: build success github build: passed
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS

Commit Message

Pei, Andy Sept. 16, 2022, 6:16 a.m. UTC 
  In the virtio blk vDPA live migration use case, for the target VM,
before the live migration process, QEMU will set call fd to all
queues of vDPA back-end. QEMU and vDPA back-end stand by
until live migration starts. During live migration process,
QEMU sets kick fd and new call fd. However, after the kick fd
is set to the vDPA back-end, the vDPA back-end configures device
and data path starts. The new call fd will cause some kind of
"re-configuration", this kind of "re-configuration" cause IO drop.
After this patch, vDPA back-end configures device after kick fd
and call fd are well set and make sure no IO drops.
This patch only impact virtio blk vDPA device and does not impact
net device.

Signed-off-by: Andy Pei <andy.pei@intel.com>
Signed-off-by: Huang Wei <wei.huang@intel.com>
---
 lib/vhost/vhost_user.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

Comments

Chenbo Xia Oct. 12, 2022, 9:35 a.m. UTC | #1 
> -----Original Message-----
> From: Pei, Andy <andy.pei@intel.com>
> Sent: Friday, September 16, 2022 2:17 PM
> To: dev@dpdk.org
> Cc: Xia, Chenbo <chenbo.xia@intel.com>; Xu, Rosen <rosen.xu@intel.com>;
> Huang, Wei <wei.huang@intel.com>; Cao, Gang <gang.cao@intel.com>;
> maxime.coquelin@redhat.com
> Subject: [PATCH v3 8/8] vhost: improve vDPA blk device readiness condition
> 
> In the virtio blk vDPA live migration use case, for the target VM,
> before the live migration process, QEMU will set call fd to all
> queues of vDPA back-end. QEMU and vDPA back-end stand by
> until live migration starts. During live migration process,
> QEMU sets kick fd and new call fd. However, after the kick fd
> is set to the vDPA back-end, the vDPA back-end configures device
> and data path starts. The new call fd will cause some kind of
> "re-configuration", this kind of "re-configuration" cause IO drop.
> After this patch, vDPA back-end configures device after kick fd
> and call fd are well set and make sure no IO drops.
> This patch only impact virtio blk vDPA device and does not impact
> net device.

IIUC, this is an improvement for MQ to make sure all call fds are well-set,
but previously it only makes sure one is well-set. If I am right, the title
and commit message should describe it in better way.

Thanks,
Chenbo

> 
> Signed-off-by: Andy Pei <andy.pei@intel.com>
> Signed-off-by: Huang Wei <wei.huang@intel.com>
> ---
>  lib/vhost/vhost_user.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c
> index 9169cf5..14ff266 100644
> --- a/lib/vhost/vhost_user.c
> +++ b/lib/vhost/vhost_user.c
> @@ -2983,6 +2983,7 @@ static int is_vring_iotlb(struct virtio_net *dev,
>  	uint32_t vdpa_type = 0;
>  	uint32_t request;
>  	uint32_t i;
> +	uint16_t blk_call_fd;
> 
>  	dev = get_device(vid);
>  	if (dev == NULL)
> @@ -3210,9 +3211,15 @@ static int is_vring_iotlb(struct virtio_net *dev,
>  	if (!vdpa_dev)
>  		goto out;
> 
> -	if (vdpa_type == RTE_VHOST_VDPA_DEVICE_TYPE_BLK
> -		&& request != VHOST_USER_SET_VRING_CALL)
> -		goto out;
> +	if (vdpa_type == RTE_VHOST_VDPA_DEVICE_TYPE_BLK) {
> +		if (request == VHOST_USER_SET_VRING_CALL) {
> +			blk_call_fd = ctx.msg.payload.u64 &
> VHOST_USER_VRING_IDX_MASK;
> +			if (blk_call_fd != dev->nr_vring - 1)
> +				goto out;
> +		} else {
> +			goto out;
> +		}
> +	}
> 
>  	if (!(dev->flags & VIRTIO_DEV_VDPA_CONFIGURED)) {
>  		if (vdpa_dev->ops->dev_conf(dev->vid))
> --
> 1.8.3.1
Pei, Andy Oct. 13, 2022, 7:55 a.m. UTC | #2 
Hi Chenbo,

Thanks for your reply, my reply is inline.

> -----Original Message-----
> From: Xia, Chenbo <chenbo.xia@intel.com>
> Sent: Wednesday, October 12, 2022 5:36 PM
> To: Pei, Andy <andy.pei@intel.com>; dev@dpdk.org
> Cc: Xu, Rosen <rosen.xu@intel.com>; Huang, Wei <wei.huang@intel.com>;
> Cao, Gang <gang.cao@intel.com>; maxime.coquelin@redhat.com
> Subject: RE: [PATCH v3 8/8] vhost: improve vDPA blk device readiness
> condition
> 
> > -----Original Message-----
> > From: Pei, Andy <andy.pei@intel.com>
> > Sent: Friday, September 16, 2022 2:17 PM
> > To: dev@dpdk.org
> > Cc: Xia, Chenbo <chenbo.xia@intel.com>; Xu, Rosen
> > <rosen.xu@intel.com>; Huang, Wei <wei.huang@intel.com>; Cao, Gang
> > <gang.cao@intel.com>; maxime.coquelin@redhat.com
> > Subject: [PATCH v3 8/8] vhost: improve vDPA blk device readiness
> > condition
> >
> > In the virtio blk vDPA live migration use case, for the target VM,
> > before the live migration process, QEMU will set call fd to all queues
> > of vDPA back-end. QEMU and vDPA back-end stand by until live migration
> > starts. During live migration process, QEMU sets kick fd and new call
> > fd. However, after the kick fd is set to the vDPA back-end, the vDPA
> > back-end configures device and data path starts. The new call fd will
> > cause some kind of "re-configuration", this kind of "re-configuration"
> > cause IO drop.
> > After this patch, vDPA back-end configures device after kick fd and
> > call fd are well set and make sure no IO drops.
> > This patch only impact virtio blk vDPA device and does not impact net
> > device.
> 
> IIUC, this is an improvement for MQ to make sure all call fds are well-set, but
> previously it only makes sure one is well-set. If I am right, the title and
> commit message should describe it in better way.
> 
> Thanks,
> Chenbo
> 
In the virtio blk vDPA live migration use case, for the target VM, 
before the live migration process, QEMU will set call fd to all queues of vDPA back-end.
QEMU and vDPA back-end stand by until live migration starts.
During live migration process, QEMU sets kick fd to all queues one by one,
and new call fd all queues one by one.
However, after the kick fd is set to the vDPA back-end,
with the original call fd, the queue is ready.
Then the vDPA  back-end configures device and data path starts.
The new call fd will cause some kind of "re-configuration",
this kind of "re-configuration" cause IO drop.
After this patch, vDPA back-end configures device after kick fd and 
call fd are well set and make sure no IO drops.
This patch only impact virtio blk vDPA device and does not impact net device.

Is this new commit message good?
> >
> > Signed-off-by: Andy Pei <andy.pei@intel.com>
> > Signed-off-by: Huang Wei <wei.huang@intel.com>
> > ---
> >  lib/vhost/vhost_user.c | 13 ++++++++++---
> >  1 file changed, 10 insertions(+), 3 deletions(-)
> >
> > diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c index
> > 9169cf5..14ff266 100644
> > --- a/lib/vhost/vhost_user.c
> > +++ b/lib/vhost/vhost_user.c
> > @@ -2983,6 +2983,7 @@ static int is_vring_iotlb(struct virtio_net *dev,
> >  	uint32_t vdpa_type = 0;
> >  	uint32_t request;
> >  	uint32_t i;
> > +	uint16_t blk_call_fd;
> >
> >  	dev = get_device(vid);
> >  	if (dev == NULL)
> > @@ -3210,9 +3211,15 @@ static int is_vring_iotlb(struct virtio_net *dev,
> >  	if (!vdpa_dev)
> >  		goto out;
> >
> > -	if (vdpa_type == RTE_VHOST_VDPA_DEVICE_TYPE_BLK
> > -		&& request != VHOST_USER_SET_VRING_CALL)
> > -		goto out;
> > +	if (vdpa_type == RTE_VHOST_VDPA_DEVICE_TYPE_BLK) {
> > +		if (request == VHOST_USER_SET_VRING_CALL) {
> > +			blk_call_fd = ctx.msg.payload.u64 &
> > VHOST_USER_VRING_IDX_MASK;
> > +			if (blk_call_fd != dev->nr_vring - 1)
> > +				goto out;
> > +		} else {
> > +			goto out;
> > +		}
> > +	}
> >
> >  	if (!(dev->flags & VIRTIO_DEV_VDPA_CONFIGURED)) {
> >  		if (vdpa_dev->ops->dev_conf(dev->vid))
> > --
> > 1.8.3.1
Pei, Andy Oct. 13, 2022, 8:23 a.m. UTC | #3 
> -----Original Message-----
> From: Pei, Andy <andy.pei@intel.com>
> Sent: Thursday, October 13, 2022 3:56 PM
> To: Xia, Chenbo <Chenbo.Xia@intel.com>; dev@dpdk.org
> Cc: Xu, Rosen <rosen.xu@intel.com>; Huang, Wei <wei.huang@intel.com>;
> Cao, Gang <gang.cao@intel.com>; maxime.coquelin@redhat.com
> Subject: RE: [PATCH v3 8/8] vhost: improve vDPA blk device readiness
> condition
> 
> Hi Chenbo,
> 
> Thanks for your reply, my reply is inline.
> 
> > -----Original Message-----
> > From: Xia, Chenbo <chenbo.xia@intel.com>
> > Sent: Wednesday, October 12, 2022 5:36 PM
> > To: Pei, Andy <andy.pei@intel.com>; dev@dpdk.org
> > Cc: Xu, Rosen <rosen.xu@intel.com>; Huang, Wei <wei.huang@intel.com>;
> > Cao, Gang <gang.cao@intel.com>; maxime.coquelin@redhat.com
> > Subject: RE: [PATCH v3 8/8] vhost: improve vDPA blk device readiness
> > condition
> >
> > > -----Original Message-----
> > > From: Pei, Andy <andy.pei@intel.com>
> > > Sent: Friday, September 16, 2022 2:17 PM
> > > To: dev@dpdk.org
> > > Cc: Xia, Chenbo <chenbo.xia@intel.com>; Xu, Rosen
> > > <rosen.xu@intel.com>; Huang, Wei <wei.huang@intel.com>; Cao, Gang
> > > <gang.cao@intel.com>; maxime.coquelin@redhat.com
> > > Subject: [PATCH v3 8/8] vhost: improve vDPA blk device readiness
> > > condition
> > >
> > > In the virtio blk vDPA live migration use case, for the target VM,
> > > before the live migration process, QEMU will set call fd to all
> > > queues of vDPA back-end. QEMU and vDPA back-end stand by until live
> > > migration starts. During live migration process, QEMU sets kick fd
> > > and new call fd. However, after the kick fd is set to the vDPA
> > > back-end, the vDPA back-end configures device and data path starts.
> > > The new call fd will cause some kind of "re-configuration", this kind of
> "re-configuration"
> > > cause IO drop.
> > > After this patch, vDPA back-end configures device after kick fd and
> > > call fd are well set and make sure no IO drops.
> > > This patch only impact virtio blk vDPA device and does not impact
> > > net device.
> >
> > IIUC, this is an improvement for MQ to make sure all call fds are
> > well-set, but previously it only makes sure one is well-set. If I am
> > right, the title and commit message should describe it in better way.
> >
> > Thanks,
> > Chenbo
> >
> In the virtio blk vDPA live migration use case, for the target VM, before the
> live migration process, QEMU will set call fd to all queues of vDPA back-end.
> QEMU and vDPA back-end stand by until live migration starts.
> During live migration process, QEMU sets kick fd to all queues one by one,
> and new call fd all queues one by one.
> However, after the kick fd is set to the vDPA back-end, with the original call fd,
> the queue is ready.
> Then the vDPA  back-end configures device and data path starts.
> The new call fd will cause some kind of "re-configuration", this kind of "re-
> configuration" cause IO drop.
> After this patch, vDPA back-end configures device after kick fd and call fd are
> well set and make sure no IO drops.
> This patch only impact virtio blk vDPA device and does not impact net device.
> 
> Is this new commit message good?

Explanation seems duplicate with former commit
" vhost: fix virtio block vDPA live migration IO drop".
So I will make it simple here,
"To support multi-queue, configure device after call fd of all queues are set."

I will use " vhost: improve vDPA blk device configure  condition" for title 
To reduce confusion.


> > >
> > > Signed-off-by: Andy Pei <andy.pei@intel.com>
> > > Signed-off-by: Huang Wei <wei.huang@intel.com>
> > > ---
> > >  lib/vhost/vhost_user.c | 13 ++++++++++---
> > >  1 file changed, 10 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c index
> > > 9169cf5..14ff266 100644
> > > --- a/lib/vhost/vhost_user.c
> > > +++ b/lib/vhost/vhost_user.c
> > > @@ -2983,6 +2983,7 @@ static int is_vring_iotlb(struct virtio_net *dev,
> > >  	uint32_t vdpa_type = 0;
> > >  	uint32_t request;
> > >  	uint32_t i;
> > > +	uint16_t blk_call_fd;
> > >
> > >  	dev = get_device(vid);
> > >  	if (dev == NULL)
> > > @@ -3210,9 +3211,15 @@ static int is_vring_iotlb(struct virtio_net *dev,
> > >  	if (!vdpa_dev)
> > >  		goto out;
> > >
> > > -	if (vdpa_type == RTE_VHOST_VDPA_DEVICE_TYPE_BLK
> > > -		&& request != VHOST_USER_SET_VRING_CALL)
> > > -		goto out;
> > > +	if (vdpa_type == RTE_VHOST_VDPA_DEVICE_TYPE_BLK) {
> > > +		if (request == VHOST_USER_SET_VRING_CALL) {
> > > +			blk_call_fd = ctx.msg.payload.u64 &
> > > VHOST_USER_VRING_IDX_MASK;
> > > +			if (blk_call_fd != dev->nr_vring - 1)
> > > +				goto out;
> > > +		} else {
> > > +			goto out;
> > > +		}
> > > +	}
> > >
> > >  	if (!(dev->flags & VIRTIO_DEV_VDPA_CONFIGURED)) {
> > >  		if (vdpa_dev->ops->dev_conf(dev->vid))
> > > --
> > > 1.8.3.1
diff mbox series

Patch

diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c
index 9169cf5..14ff266 100644
--- a/lib/vhost/vhost_user.c
+++ b/lib/vhost/vhost_user.c
@@ -2983,6 +2983,7 @@  static int is_vring_iotlb(struct virtio_net *dev,
 	uint32_t vdpa_type = 0;
 	uint32_t request;
 	uint32_t i;
+	uint16_t blk_call_fd;
 
 	dev = get_device(vid);
 	if (dev == NULL)
@@ -3210,9 +3211,15 @@  static int is_vring_iotlb(struct virtio_net *dev,
 	if (!vdpa_dev)
 		goto out;
 
-	if (vdpa_type == RTE_VHOST_VDPA_DEVICE_TYPE_BLK
-		&& request != VHOST_USER_SET_VRING_CALL)
-		goto out;
+	if (vdpa_type == RTE_VHOST_VDPA_DEVICE_TYPE_BLK) {
+		if (request == VHOST_USER_SET_VRING_CALL) {
+			blk_call_fd = ctx.msg.payload.u64 & VHOST_USER_VRING_IDX_MASK;
+			if (blk_call_fd != dev->nr_vring - 1)
+				goto out;
+		} else {
+			goto out;
+		}
+	}
 
 	if (!(dev->flags & VIRTIO_DEV_VDPA_CONFIGURED)) {
 		if (vdpa_dev->ops->dev_conf(dev->vid))