DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH v2 0/2] vhost: support VIRTIO_F_RING_RESET for vhost-user
@ 2022-09-05  3:48 Kangjie Xu
  2022-09-05  3:48 ` [PATCH v2 1/2] " Kangjie Xu
  2022-09-05  3:48 ` [PATCH v2 2/2] vhost: introduce VHOST_USER_RESET_VRING Kangjie Xu
  0 siblings, 2 replies; 6+ messages in thread
From: Kangjie Xu @ 2022-09-05  3:48 UTC (permalink / raw)
  To: maxime.coquelin, chenbo.xia; +Cc: dev, xuanzhuo, hengqi, jasonwang, mst

Add VIRTIO_F_RING_RESET, which indicates that the driver can reset a
virtqueue individually.

VIRTIO_F_RING_RESET feature is added to virtio-spec 1.2. The relevant
information is in
    https://github.com/oasis-tcs/virtio-spec/issues/124
    https://github.com/oasis-tcs/virtio-spec/issues/139

The implementation only adds the feature bit in supported features.
It also adds a new vhost-user message VHOST_USER_RESET_VRING. The
related definition is defined in the related QEMU patch set:
  https://lore.kernel.org/qemu-devel/cover.1661510725.git.kangjie.xu@linux.alibaba.com/T/#t

The virtqueue reset process can be concluded as two parts:
1. The driver can reset a virtqueue. It will send VHOST_USER_RESET_VRING
to DPDK. After received the message, DPDK will reset the virtqueue. The
new message VHOST_USER_RESET_VRING has been acked by the QEMU virtio
maintainer:
    https://lore.kernel.org/qemu-devel/57362274-b2fb-a47d-fea7-d3ebcfad967b@redhat.com/

2. After the virtqueue is disabled, the driver may optionally re-enable
it. To avoid confusion with VHOST_USER_SET_VRING_ENABLE, we call this
part as "restart". The virtqueue's information may be changed when
restarting it. Thus, the information of the reset virtqueue should be
updated. This part is basically similar to when the virtqueue is started
for the first time, except that the restart process does not need to set
features and set mem table since they do not change. QEMU will send
messages containing size, base, addr, kickfd and callfd of the virtqueue
in order. Specifically, the DPDK will receive these messages in order:
    a. VHOST_USER_SET_VRING_NUM
    b. VHOST_USER_SET_VRING_BASE
    c. VHOST_USER_SET_VRING_ADDR
    d. VHOST_USER_SET_VRING_KICK
    e. VHOST_USER_SET_VRING_CALL
    f. VHOST_USER_SET_VRING_ENABLE
The last VHOST_USER_SET_VRING_ENABLE message with "payload.state.num" set
to 1, will be sent to enable the virtqueue and the restart process is
finished.

Test environment:
    Host: 5.4.189
    Qemu: QEMU emulator version 7.0.50 (With vq reset support)
    Guest: 5.19.0-rc3 (With vq reset support)
    DPDK: 22.07-rc1
    Test Cmd: ethtool -g eth1; ethtool -G eth1 rx $1 tx $2;
            ethtool -g eth1;

    The driver can resize the virtio queue, then virtio queue reset
    function should be triggered.

Guest Kernel Patch:
    https://lore.kernel.org/bpf/20220801063902.129329-1-xuanzhuo@linux.alibaba.com/

QEMU Patch:
    https://lore.kernel.org/qemu-devel/cover.1661510725.git.kangjie.xu@linux.alibaba.com/T/
    https://lore.kernel.org/qemu-devel/cover.1661414345.git.kangjie.xu@linux.alibaba.com/T/

Looking forward to your review and comments.

Kangjie Xu (2):
  vhost: support VIRTIO_F_RING_RESET for vhost-user
  vhost: introduce VHOST_USER_RESET_VRING

 lib/vhost/vhost.c      |  2 +-
 lib/vhost/vhost.h      |  9 ++++++++-
 lib/vhost/vhost_user.c | 27 ++++++++++++++++++++++++++-
 lib/vhost/vhost_user.h |  1 +
 4 files changed, 36 insertions(+), 3 deletions(-)

-- 
2.32.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 1/2] vhost: support VIRTIO_F_RING_RESET for vhost-user
  2022-09-05  3:48 [PATCH v2 0/2] vhost: support VIRTIO_F_RING_RESET for vhost-user Kangjie Xu
@ 2022-09-05  3:48 ` Kangjie Xu
  2022-09-05  3:48 ` [PATCH v2 2/2] vhost: introduce VHOST_USER_RESET_VRING Kangjie Xu
  1 sibling, 0 replies; 6+ messages in thread
From: Kangjie Xu @ 2022-09-05  3:48 UTC (permalink / raw)
  To: maxime.coquelin, chenbo.xia; +Cc: dev, xuanzhuo, hengqi, jasonwang, mst

Add VIRTIO_F_RING_RESET, which indicates that the driver can reset a
queue individually.

The feature is added to virtio-spec 1.2. The relevant information is
in https://github.com/oasis-tcs/virtio-spec/issues/124

Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 lib/vhost/vhost.h | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/lib/vhost/vhost.h b/lib/vhost/vhost.h
index 40fac3b7c6..76461a3406 100644
--- a/lib/vhost/vhost.h
+++ b/lib/vhost/vhost.h
@@ -376,6 +376,11 @@ struct vhost_msg {
  #define VIRTIO_F_VERSION_1 32
 #endif
 
+/* This feature indicates that the driver can reset a queue individually. */
+#ifndef VIRTIO_F_RING_RESET
+#define VIRTIO_F_RING_RESET 40
+#endif
+
 /* Declare packed ring related bits for older kernels */
 #ifndef VIRTIO_F_RING_PACKED
 
@@ -438,7 +443,8 @@ struct vring_packed_desc_event {
 				(1ULL << VIRTIO_NET_F_MTU)  | \
 				(1ULL << VIRTIO_F_IN_ORDER) | \
 				(1ULL << VIRTIO_F_IOMMU_PLATFORM) | \
-				(1ULL << VIRTIO_F_RING_PACKED))
+				(1ULL << VIRTIO_F_RING_PACKED)	| \
+				(1ULL << VIRTIO_F_RING_RESET))
 
 
 struct guest_page {
-- 
2.32.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 2/2] vhost: introduce VHOST_USER_RESET_VRING
  2022-09-05  3:48 [PATCH v2 0/2] vhost: support VIRTIO_F_RING_RESET for vhost-user Kangjie Xu
  2022-09-05  3:48 ` [PATCH v2 1/2] " Kangjie Xu
@ 2022-09-05  3:48 ` Kangjie Xu
  2022-09-22  9:35   ` Xia, Chenbo
  1 sibling, 1 reply; 6+ messages in thread
From: Kangjie Xu @ 2022-09-05  3:48 UTC (permalink / raw)
  To: maxime.coquelin, chenbo.xia; +Cc: dev, xuanzhuo, hengqi, jasonwang, mst

To support the reset operation for an individual virtqueue, we
introduce a new message VHOST_USER_RESET_VRING. When the feature
VIRTIO_F_RING_RESET feature has been successfully negotiated, This
message is submitted by the front-end to reset an individual
virtqueue to initial states in the back-end. The reply is needed
to ensure that the reset operation is complete.

Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 lib/vhost/vhost.c      |  2 +-
 lib/vhost/vhost.h      |  1 +
 lib/vhost/vhost_user.c | 27 ++++++++++++++++++++++++++-
 lib/vhost/vhost_user.h |  1 +
 4 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c
index 60cb05a0ff..215a1ca355 100644
--- a/lib/vhost/vhost.c
+++ b/lib/vhost/vhost.c
@@ -610,7 +610,7 @@ init_vring_queue(struct virtio_net *dev, uint32_t vring_idx)
 	vhost_user_iotlb_init(dev, vring_idx);
 }
 
-static void
+void
 reset_vring_queue(struct virtio_net *dev, uint32_t vring_idx)
 {
 	struct vhost_virtqueue *vq;
diff --git a/lib/vhost/vhost.h b/lib/vhost/vhost.h
index 76461a3406..eccb52842d 100644
--- a/lib/vhost/vhost.h
+++ b/lib/vhost/vhost.h
@@ -791,6 +791,7 @@ get_device(int vid)
 
 int vhost_new_device(void);
 void cleanup_device(struct virtio_net *dev, int destroy);
+void reset_vring_queue(struct virtio_net *dev, uint32_t vring_idx);
 void reset_device(struct virtio_net *dev);
 void vhost_destroy_device(int);
 void vhost_destroy_device_notify(struct virtio_net *dev);
diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c
index 4ad28bac45..5f7743d9d9 100644
--- a/lib/vhost/vhost_user.c
+++ b/lib/vhost/vhost_user.c
@@ -2771,6 +2771,30 @@ vhost_user_set_status(struct virtio_net **pdev,
 	return RTE_VHOST_MSG_RESULT_OK;
 }
 
+static int
+vhost_user_reset_vring(struct virtio_net **pdev,
+			struct vhu_msg_context *ctx __rte_unused,
+			int main_fd __rte_unused)
+{
+	struct virtio_net *dev = *pdev;
+	int index = (int)ctx->msg.payload.state.index;
+
+	VHOST_LOG_CONFIG(dev->ifname, INFO, "reset queue: queue idx: %d\n", index);
+
+	if (!(dev->features & (1ULL << VIRTIO_F_RING_RESET))) {
+		return RTE_VHOST_MSG_RESULT_ERR;
+	}
+
+	dev->virtqueue[index]->enabled = false;
+	reset_vring_queue(dev, index);
+
+	ctx->msg.payload.state.num = 0;
+	ctx->msg.size = sizeof(ctx->msg.payload.u64);
+	ctx->fd_num = 0;
+
+	return RTE_VHOST_MSG_RESULT_REPLY;
+}
+
 #define VHOST_MESSAGE_HANDLERS \
 VHOST_MESSAGE_HANDLER(VHOST_USER_NONE, NULL, false) \
 VHOST_MESSAGE_HANDLER(VHOST_USER_GET_FEATURES, vhost_user_get_features, false) \
@@ -2803,7 +2827,8 @@ VHOST_MESSAGE_HANDLER(VHOST_USER_POSTCOPY_END, vhost_user_postcopy_end, false) \
 VHOST_MESSAGE_HANDLER(VHOST_USER_GET_INFLIGHT_FD, vhost_user_get_inflight_fd, false) \
 VHOST_MESSAGE_HANDLER(VHOST_USER_SET_INFLIGHT_FD, vhost_user_set_inflight_fd, true) \
 VHOST_MESSAGE_HANDLER(VHOST_USER_SET_STATUS, vhost_user_set_status, false) \
-VHOST_MESSAGE_HANDLER(VHOST_USER_GET_STATUS, vhost_user_get_status, false)
+VHOST_MESSAGE_HANDLER(VHOST_USER_GET_STATUS, vhost_user_get_status, false) \
+VHOST_MESSAGE_HANDLER(VHOST_USER_RESET_VRING, vhost_user_reset_vring, false)
 
 #define VHOST_MESSAGE_HANDLER(id, handler, accepts_fd) \
 	[id] = { #id, handler, accepts_fd },
diff --git a/lib/vhost/vhost_user.h b/lib/vhost/vhost_user.h
index 8ecca68597..51cb2fc74a 100644
--- a/lib/vhost/vhost_user.h
+++ b/lib/vhost/vhost_user.h
@@ -60,6 +60,7 @@ typedef enum VhostUserRequest {
 	VHOST_USER_SET_INFLIGHT_FD = 32,
 	VHOST_USER_SET_STATUS = 39,
 	VHOST_USER_GET_STATUS = 40,
+	VHOST_USER_RESET_VRING = 41
 } VhostUserRequest;
 
 typedef enum VhostUserSlaveRequest {
-- 
2.32.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH v2 2/2] vhost: introduce VHOST_USER_RESET_VRING
  2022-09-05  3:48 ` [PATCH v2 2/2] vhost: introduce VHOST_USER_RESET_VRING Kangjie Xu
@ 2022-09-22  9:35   ` Xia, Chenbo
  2022-09-23  1:41     ` Xuan Zhuo
  0 siblings, 1 reply; 6+ messages in thread
From: Xia, Chenbo @ 2022-09-22  9:35 UTC (permalink / raw)
  To: Kangjie Xu, maxime.coquelin; +Cc: dev, xuanzhuo, hengqi, jasonwang, mst

> -----Original Message-----
> From: Kangjie Xu <kangjie.xu@linux.alibaba.com>
> Sent: Monday, September 5, 2022 11:48 AM
> To: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>
> Cc: dev@dpdk.org; xuanzhuo@linux.alibaba.com; hengqi@linux.alibaba.com;
> jasonwang@redhat.com; mst@redhat.com
> Subject: [PATCH v2 2/2] vhost: introduce VHOST_USER_RESET_VRING
> 
> To support the reset operation for an individual virtqueue, we
> introduce a new message VHOST_USER_RESET_VRING. When the feature
> VIRTIO_F_RING_RESET feature has been successfully negotiated, This
> message is submitted by the front-end to reset an individual
> virtqueue to initial states in the back-end. The reply is needed
> to ensure that the reset operation is complete.

completed

> 
> Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> ---
>  lib/vhost/vhost.c      |  2 +-
>  lib/vhost/vhost.h      |  1 +
>  lib/vhost/vhost_user.c | 27 ++++++++++++++++++++++++++-
>  lib/vhost/vhost_user.h |  1 +
>  4 files changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c
> index 60cb05a0ff..215a1ca355 100644
> --- a/lib/vhost/vhost.c
> +++ b/lib/vhost/vhost.c
> @@ -610,7 +610,7 @@ init_vring_queue(struct virtio_net *dev, uint32_t
> vring_idx)
>  	vhost_user_iotlb_init(dev, vring_idx);
>  }
> 
> -static void
> +void
>  reset_vring_queue(struct virtio_net *dev, uint32_t vring_idx)
>  {
>  	struct vhost_virtqueue *vq;
> diff --git a/lib/vhost/vhost.h b/lib/vhost/vhost.h
> index 76461a3406..eccb52842d 100644
> --- a/lib/vhost/vhost.h
> +++ b/lib/vhost/vhost.h
> @@ -791,6 +791,7 @@ get_device(int vid)
> 
>  int vhost_new_device(void);
>  void cleanup_device(struct virtio_net *dev, int destroy);
> +void reset_vring_queue(struct virtio_net *dev, uint32_t vring_idx);
>  void reset_device(struct virtio_net *dev);
>  void vhost_destroy_device(int);
>  void vhost_destroy_device_notify(struct virtio_net *dev);
> diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c
> index 4ad28bac45..5f7743d9d9 100644
> --- a/lib/vhost/vhost_user.c
> +++ b/lib/vhost/vhost_user.c
> @@ -2771,6 +2771,30 @@ vhost_user_set_status(struct virtio_net **pdev,
>  	return RTE_VHOST_MSG_RESULT_OK;
>  }
> 
> +static int
> +vhost_user_reset_vring(struct virtio_net **pdev,
> +			struct vhu_msg_context *ctx __rte_unused,
> +			int main_fd __rte_unused)
> +{
> +	struct virtio_net *dev = *pdev;
> +	int index = (int)ctx->msg.payload.state.index;

Why not just use unsigned int?

> +
> +	VHOST_LOG_CONFIG(dev->ifname, INFO, "reset queue: queue idx: %d\n",
> index);
> +
> +	if (!(dev->features & (1ULL << VIRTIO_F_RING_RESET))) {
> +		return RTE_VHOST_MSG_RESULT_ERR;
> +	}

braces {} are not necessary for single statement blocks

> +
> +	dev->virtqueue[index]->enabled = false;
> +	reset_vring_queue(dev, index);
> +
> +	ctx->msg.payload.state.num = 0;
> +	ctx->msg.size = sizeof(ctx->msg.payload.u64);
> +	ctx->fd_num = 0;
> +
> +	return RTE_VHOST_MSG_RESULT_REPLY;
> +}

IIUC, before this handler, we need to lock the queue? Using vhost_user_lock_all_queue_pairs

BTW, is this support merged in QEMU now? I remember for similar cases,
we wait for QEMU to merge first and then merge in DPDK.

Maxime, do I remember this correctly?

Thanks,
Chenbo

> +
>  #define VHOST_MESSAGE_HANDLERS \
>  VHOST_MESSAGE_HANDLER(VHOST_USER_NONE, NULL, false) \
>  VHOST_MESSAGE_HANDLER(VHOST_USER_GET_FEATURES, vhost_user_get_features,
> false) \
> @@ -2803,7 +2827,8 @@ VHOST_MESSAGE_HANDLER(VHOST_USER_POSTCOPY_END,
> vhost_user_postcopy_end, false) \
>  VHOST_MESSAGE_HANDLER(VHOST_USER_GET_INFLIGHT_FD,
> vhost_user_get_inflight_fd, false) \
>  VHOST_MESSAGE_HANDLER(VHOST_USER_SET_INFLIGHT_FD,
> vhost_user_set_inflight_fd, true) \
>  VHOST_MESSAGE_HANDLER(VHOST_USER_SET_STATUS, vhost_user_set_status, false)
> \
> -VHOST_MESSAGE_HANDLER(VHOST_USER_GET_STATUS, vhost_user_get_status, false)
> +VHOST_MESSAGE_HANDLER(VHOST_USER_GET_STATUS, vhost_user_get_status, false)
> \
> +VHOST_MESSAGE_HANDLER(VHOST_USER_RESET_VRING, vhost_user_reset_vring,
> false)
> 
>  #define VHOST_MESSAGE_HANDLER(id, handler, accepts_fd) \
>  	[id] = { #id, handler, accepts_fd },
> diff --git a/lib/vhost/vhost_user.h b/lib/vhost/vhost_user.h
> index 8ecca68597..51cb2fc74a 100644
> --- a/lib/vhost/vhost_user.h
> +++ b/lib/vhost/vhost_user.h
> @@ -60,6 +60,7 @@ typedef enum VhostUserRequest {
>  	VHOST_USER_SET_INFLIGHT_FD = 32,
>  	VHOST_USER_SET_STATUS = 39,
>  	VHOST_USER_GET_STATUS = 40,
> +	VHOST_USER_RESET_VRING = 41
>  } VhostUserRequest;
> 
>  typedef enum VhostUserSlaveRequest {
> --
> 2.32.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: RE: [PATCH v2 2/2] vhost: introduce VHOST_USER_RESET_VRING
  2022-09-22  9:35   ` Xia, Chenbo
@ 2022-09-23  1:41     ` Xuan Zhuo
  2022-09-26  7:28       ` Xia, Chenbo
  0 siblings, 1 reply; 6+ messages in thread
From: Xuan Zhuo @ 2022-09-23  1:41 UTC (permalink / raw)
  To: Xia, Chenbo; +Cc: dev, hengqi, jasonwang, mst, Kangjie Xu, maxime.coquelin

On Thu, 22 Sep 2022 09:35:35 +0000, "Xia, Chenbo" <chenbo.xia@intel.com> wrote:
> > -----Original Message-----
> > From: Kangjie Xu <kangjie.xu@linux.alibaba.com>
> > Sent: Monday, September 5, 2022 11:48 AM
> > To: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>
> > Cc: dev@dpdk.org; xuanzhuo@linux.alibaba.com; hengqi@linux.alibaba.com;
> > jasonwang@redhat.com; mst@redhat.com
> > Subject: [PATCH v2 2/2] vhost: introduce VHOST_USER_RESET_VRING
> >
> > To support the reset operation for an individual virtqueue, we
> > introduce a new message VHOST_USER_RESET_VRING. When the feature
> > VIRTIO_F_RING_RESET feature has been successfully negotiated, This
> > message is submitted by the front-end to reset an individual
> > virtqueue to initial states in the back-end. The reply is needed
> > to ensure that the reset operation is complete.
>
> completed
>
> >
> > Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
> > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > ---
> >  lib/vhost/vhost.c      |  2 +-
> >  lib/vhost/vhost.h      |  1 +
> >  lib/vhost/vhost_user.c | 27 ++++++++++++++++++++++++++-
> >  lib/vhost/vhost_user.h |  1 +
> >  4 files changed, 29 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c
> > index 60cb05a0ff..215a1ca355 100644
> > --- a/lib/vhost/vhost.c
> > +++ b/lib/vhost/vhost.c
> > @@ -610,7 +610,7 @@ init_vring_queue(struct virtio_net *dev, uint32_t
> > vring_idx)
> >  	vhost_user_iotlb_init(dev, vring_idx);
> >  }
> >
> > -static void
> > +void
> >  reset_vring_queue(struct virtio_net *dev, uint32_t vring_idx)
> >  {
> >  	struct vhost_virtqueue *vq;
> > diff --git a/lib/vhost/vhost.h b/lib/vhost/vhost.h
> > index 76461a3406..eccb52842d 100644
> > --- a/lib/vhost/vhost.h
> > +++ b/lib/vhost/vhost.h
> > @@ -791,6 +791,7 @@ get_device(int vid)
> >
> >  int vhost_new_device(void);
> >  void cleanup_device(struct virtio_net *dev, int destroy);
> > +void reset_vring_queue(struct virtio_net *dev, uint32_t vring_idx);
> >  void reset_device(struct virtio_net *dev);
> >  void vhost_destroy_device(int);
> >  void vhost_destroy_device_notify(struct virtio_net *dev);
> > diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c
> > index 4ad28bac45..5f7743d9d9 100644
> > --- a/lib/vhost/vhost_user.c
> > +++ b/lib/vhost/vhost_user.c
> > @@ -2771,6 +2771,30 @@ vhost_user_set_status(struct virtio_net **pdev,
> >  	return RTE_VHOST_MSG_RESULT_OK;
> >  }
> >
> > +static int
> > +vhost_user_reset_vring(struct virtio_net **pdev,
> > +			struct vhu_msg_context *ctx __rte_unused,
> > +			int main_fd __rte_unused)
> > +{
> > +	struct virtio_net *dev = *pdev;
> > +	int index = (int)ctx->msg.payload.state.index;
>
> Why not just use unsigned int?
>
> > +
> > +	VHOST_LOG_CONFIG(dev->ifname, INFO, "reset queue: queue idx: %d\n",
> > index);
> > +
> > +	if (!(dev->features & (1ULL << VIRTIO_F_RING_RESET))) {
> > +		return RTE_VHOST_MSG_RESULT_ERR;
> > +	}
>
> braces {} are not necessary for single statement blocks
>
> > +
> > +	dev->virtqueue[index]->enabled = false;
> > +	reset_vring_queue(dev, index);
> > +
> > +	ctx->msg.payload.state.num = 0;
> > +	ctx->msg.size = sizeof(ctx->msg.payload.u64);
> > +	ctx->fd_num = 0;
> > +
> > +	return RTE_VHOST_MSG_RESULT_REPLY;
> > +}
>
> IIUC, before this handler, we need to lock the queue? Using vhost_user_lock_all_queue_pairs
>
> BTW, is this support merged in QEMU now? I remember for similar cases,
> we wait for QEMU to merge first and then merge in DPDK.
>
> Maxime, do I remember this correctly?


Yes, we are simultaneously pushing this feature to QEMU.

We have a patch for v3, maybe you missed it.

Thanks.


>
> Thanks,
> Chenbo
>
> > +
> >  #define VHOST_MESSAGE_HANDLERS \
> >  VHOST_MESSAGE_HANDLER(VHOST_USER_NONE, NULL, false) \
> >  VHOST_MESSAGE_HANDLER(VHOST_USER_GET_FEATURES, vhost_user_get_features,
> > false) \
> > @@ -2803,7 +2827,8 @@ VHOST_MESSAGE_HANDLER(VHOST_USER_POSTCOPY_END,
> > vhost_user_postcopy_end, false) \
> >  VHOST_MESSAGE_HANDLER(VHOST_USER_GET_INFLIGHT_FD,
> > vhost_user_get_inflight_fd, false) \
> >  VHOST_MESSAGE_HANDLER(VHOST_USER_SET_INFLIGHT_FD,
> > vhost_user_set_inflight_fd, true) \
> >  VHOST_MESSAGE_HANDLER(VHOST_USER_SET_STATUS, vhost_user_set_status, false)
> > \
> > -VHOST_MESSAGE_HANDLER(VHOST_USER_GET_STATUS, vhost_user_get_status, false)
> > +VHOST_MESSAGE_HANDLER(VHOST_USER_GET_STATUS, vhost_user_get_status, false)
> > \
> > +VHOST_MESSAGE_HANDLER(VHOST_USER_RESET_VRING, vhost_user_reset_vring,
> > false)
> >
> >  #define VHOST_MESSAGE_HANDLER(id, handler, accepts_fd) \
> >  	[id] = { #id, handler, accepts_fd },
> > diff --git a/lib/vhost/vhost_user.h b/lib/vhost/vhost_user.h
> > index 8ecca68597..51cb2fc74a 100644
> > --- a/lib/vhost/vhost_user.h
> > +++ b/lib/vhost/vhost_user.h
> > @@ -60,6 +60,7 @@ typedef enum VhostUserRequest {
> >  	VHOST_USER_SET_INFLIGHT_FD = 32,
> >  	VHOST_USER_SET_STATUS = 39,
> >  	VHOST_USER_GET_STATUS = 40,
> > +	VHOST_USER_RESET_VRING = 41
> >  } VhostUserRequest;
> >
> >  typedef enum VhostUserSlaveRequest {
> > --
> > 2.32.0
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: RE: [PATCH v2 2/2] vhost: introduce VHOST_USER_RESET_VRING
  2022-09-23  1:41     ` Xuan Zhuo
@ 2022-09-26  7:28       ` Xia, Chenbo
  0 siblings, 0 replies; 6+ messages in thread
From: Xia, Chenbo @ 2022-09-26  7:28 UTC (permalink / raw)
  To: Xuan Zhuo; +Cc: dev, hengqi, jasonwang, mst, Kangjie Xu, maxime.coquelin

> -----Original Message-----
> From: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> Sent: Friday, September 23, 2022 9:42 AM
> To: Xia, Chenbo <chenbo.xia@intel.com>
> Cc: dev@dpdk.org; hengqi@linux.alibaba.com; jasonwang@redhat.com;
> mst@redhat.com; Kangjie Xu <kangjie.xu@linux.alibaba.com>;
> maxime.coquelin@redhat.com
> Subject: Re: RE: [PATCH v2 2/2] vhost: introduce VHOST_USER_RESET_VRING
> 
> On Thu, 22 Sep 2022 09:35:35 +0000, "Xia, Chenbo" <chenbo.xia@intel.com>
> wrote:
> > > -----Original Message-----
> > > From: Kangjie Xu <kangjie.xu@linux.alibaba.com>
> > > Sent: Monday, September 5, 2022 11:48 AM
> > > To: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>
> > > Cc: dev@dpdk.org; xuanzhuo@linux.alibaba.com; hengqi@linux.alibaba.com;
> > > jasonwang@redhat.com; mst@redhat.com
> > > Subject: [PATCH v2 2/2] vhost: introduce VHOST_USER_RESET_VRING
> > >
> > > To support the reset operation for an individual virtqueue, we
> > > introduce a new message VHOST_USER_RESET_VRING. When the feature
> > > VIRTIO_F_RING_RESET feature has been successfully negotiated, This
> > > message is submitted by the front-end to reset an individual
> > > virtqueue to initial states in the back-end. The reply is needed
> > > to ensure that the reset operation is complete.
> >
> > completed
> >
> > >
> > > Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
> > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > > ---
> > >  lib/vhost/vhost.c      |  2 +-
> > >  lib/vhost/vhost.h      |  1 +
> > >  lib/vhost/vhost_user.c | 27 ++++++++++++++++++++++++++-
> > >  lib/vhost/vhost_user.h |  1 +
> > >  4 files changed, 29 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c
> > > index 60cb05a0ff..215a1ca355 100644
> > > --- a/lib/vhost/vhost.c
> > > +++ b/lib/vhost/vhost.c
> > > @@ -610,7 +610,7 @@ init_vring_queue(struct virtio_net *dev, uint32_t
> > > vring_idx)
> > >  	vhost_user_iotlb_init(dev, vring_idx);
> > >  }
> > >
> > > -static void
> > > +void
> > >  reset_vring_queue(struct virtio_net *dev, uint32_t vring_idx)
> > >  {
> > >  	struct vhost_virtqueue *vq;
> > > diff --git a/lib/vhost/vhost.h b/lib/vhost/vhost.h
> > > index 76461a3406..eccb52842d 100644
> > > --- a/lib/vhost/vhost.h
> > > +++ b/lib/vhost/vhost.h
> > > @@ -791,6 +791,7 @@ get_device(int vid)
> > >
> > >  int vhost_new_device(void);
> > >  void cleanup_device(struct virtio_net *dev, int destroy);
> > > +void reset_vring_queue(struct virtio_net *dev, uint32_t vring_idx);
> > >  void reset_device(struct virtio_net *dev);
> > >  void vhost_destroy_device(int);
> > >  void vhost_destroy_device_notify(struct virtio_net *dev);
> > > diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c
> > > index 4ad28bac45..5f7743d9d9 100644
> > > --- a/lib/vhost/vhost_user.c
> > > +++ b/lib/vhost/vhost_user.c
> > > @@ -2771,6 +2771,30 @@ vhost_user_set_status(struct virtio_net **pdev,
> > >  	return RTE_VHOST_MSG_RESULT_OK;
> > >  }
> > >
> > > +static int
> > > +vhost_user_reset_vring(struct virtio_net **pdev,
> > > +			struct vhu_msg_context *ctx __rte_unused,
> > > +			int main_fd __rte_unused)
> > > +{
> > > +	struct virtio_net *dev = *pdev;
> > > +	int index = (int)ctx->msg.payload.state.index;
> >
> > Why not just use unsigned int?
> >
> > > +
> > > +	VHOST_LOG_CONFIG(dev->ifname, INFO, "reset queue: queue idx: %d\n",
> > > index);
> > > +
> > > +	if (!(dev->features & (1ULL << VIRTIO_F_RING_RESET))) {
> > > +		return RTE_VHOST_MSG_RESULT_ERR;
> > > +	}
> >
> > braces {} are not necessary for single statement blocks
> >
> > > +
> > > +	dev->virtqueue[index]->enabled = false;
> > > +	reset_vring_queue(dev, index);
> > > +
> > > +	ctx->msg.payload.state.num = 0;
> > > +	ctx->msg.size = sizeof(ctx->msg.payload.u64);
> > > +	ctx->fd_num = 0;
> > > +
> > > +	return RTE_VHOST_MSG_RESULT_REPLY;
> > > +}
> >
> > IIUC, before this handler, we need to lock the queue? Using
> vhost_user_lock_all_queue_pairs
> >
> > BTW, is this support merged in QEMU now? I remember for similar cases,
> > we wait for QEMU to merge first and then merge in DPDK.
> >
> > Maxime, do I remember this correctly?
> 
> 
> Yes, we are simultaneously pushing this feature to QEMU.
> 
> We have a patch for v3, maybe you missed it.
> 
> Thanks.

Oops, sorry. I did miss that. And please ping when the QEMU side
is merged.

Thanks,
Chenbo

> 
> 
> >
> > Thanks,
> > Chenbo
> >
> > > +
> > >  #define VHOST_MESSAGE_HANDLERS \
> > >  VHOST_MESSAGE_HANDLER(VHOST_USER_NONE, NULL, false) \
> > >  VHOST_MESSAGE_HANDLER(VHOST_USER_GET_FEATURES,
> vhost_user_get_features,
> > > false) \
> > > @@ -2803,7 +2827,8 @@ VHOST_MESSAGE_HANDLER(VHOST_USER_POSTCOPY_END,
> > > vhost_user_postcopy_end, false) \
> > >  VHOST_MESSAGE_HANDLER(VHOST_USER_GET_INFLIGHT_FD,
> > > vhost_user_get_inflight_fd, false) \
> > >  VHOST_MESSAGE_HANDLER(VHOST_USER_SET_INFLIGHT_FD,
> > > vhost_user_set_inflight_fd, true) \
> > >  VHOST_MESSAGE_HANDLER(VHOST_USER_SET_STATUS, vhost_user_set_status,
> false)
> > > \
> > > -VHOST_MESSAGE_HANDLER(VHOST_USER_GET_STATUS, vhost_user_get_status,
> false)
> > > +VHOST_MESSAGE_HANDLER(VHOST_USER_GET_STATUS, vhost_user_get_status,
> false)
> > > \
> > > +VHOST_MESSAGE_HANDLER(VHOST_USER_RESET_VRING, vhost_user_reset_vring,
> > > false)
> > >
> > >  #define VHOST_MESSAGE_HANDLER(id, handler, accepts_fd) \
> > >  	[id] = { #id, handler, accepts_fd },
> > > diff --git a/lib/vhost/vhost_user.h b/lib/vhost/vhost_user.h
> > > index 8ecca68597..51cb2fc74a 100644
> > > --- a/lib/vhost/vhost_user.h
> > > +++ b/lib/vhost/vhost_user.h
> > > @@ -60,6 +60,7 @@ typedef enum VhostUserRequest {
> > >  	VHOST_USER_SET_INFLIGHT_FD = 32,
> > >  	VHOST_USER_SET_STATUS = 39,
> > >  	VHOST_USER_GET_STATUS = 40,
> > > +	VHOST_USER_RESET_VRING = 41
> > >  } VhostUserRequest;
> > >
> > >  typedef enum VhostUserSlaveRequest {
> > > --
> > > 2.32.0
> >

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-09-26  7:29 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-05  3:48 [PATCH v2 0/2] vhost: support VIRTIO_F_RING_RESET for vhost-user Kangjie Xu
2022-09-05  3:48 ` [PATCH v2 1/2] " Kangjie Xu
2022-09-05  3:48 ` [PATCH v2 2/2] vhost: introduce VHOST_USER_RESET_VRING Kangjie Xu
2022-09-22  9:35   ` Xia, Chenbo
2022-09-23  1:41     ` Xuan Zhuo
2022-09-26  7:28       ` Xia, Chenbo

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).