DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] vhost: Introduce vhost-user's REPLY_ACK feature
@ 2016-12-12 17:54 Maxime Coquelin
  2016-12-13  9:56 ` Yuanhan Liu
  2017-01-12  4:03 ` Yuanhan Liu
  0 siblings, 2 replies; 6+ messages in thread
From: Maxime Coquelin @ 2016-12-12 17:54 UTC (permalink / raw)
  To: yuanhan.liu, dev; +Cc: Maxime Coquelin

REPLY_ACK features provide a generic way for QEMU to ensure both
completion and success of a request.

As described in vhost-user spec in QEMU repository, QEMU sets
VHOST_USER_NEED_REPLY flag (bit 3) when expecting a reply_ack from
the backend. Backend must reply with 0 for success or non-zero
otherwise when flag is set.

Currently, only VHOST_USER_SET_MEM_TABLE request implements reply_ack,
in order to synchronize mapping updates.

This patch enables REPLY_ACK feature generally, but only checks error
code for VHOST_USER_SET_MEM_TABLE.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
Hi,

The intend of this patch is not to fix a known issue, but it is
nice to have this feature, and it will be used by upcoming MTU
feature if it remains in its current form.

Thanks,
Maxime

 lib/librte_vhost/vhost_user.c | 11 ++++++++++-
 lib/librte_vhost/vhost_user.h |  5 ++++-
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index 6b83c15..9ce80cb 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -903,6 +903,7 @@ send_vhost_message(int sockfd, struct VhostUserMsg *msg)
 		return 0;
 
 	msg->flags &= ~VHOST_USER_VERSION_MASK;
+	msg->flags &= ~VHOST_USER_NEED_REPLY;
 	msg->flags |= VHOST_USER_VERSION;
 	msg->flags |= VHOST_USER_REPLY_MASK;
 
@@ -938,6 +939,7 @@ vhost_user_msg_handler(int vid, int fd)
 		return -1;
 	}
 
+	ret = 0;
 	RTE_LOG(INFO, VHOST_CONFIG, "read message %s\n",
 		vhost_message_str[msg.request]);
 	switch (msg.request) {
@@ -967,7 +969,7 @@ vhost_user_msg_handler(int vid, int fd)
 		break;
 
 	case VHOST_USER_SET_MEM_TABLE:
-		vhost_user_set_mem_table(dev, &msg);
+		ret = vhost_user_set_mem_table(dev, &msg);
 		break;
 
 	case VHOST_USER_SET_LOG_BASE:
@@ -1025,9 +1027,16 @@ vhost_user_msg_handler(int vid, int fd)
 		break;
 
 	default:
+		ret = -1;
 		break;
 
 	}
 
+	if (msg.flags & VHOST_USER_NEED_REPLY) {
+		msg.payload.u64 = !!ret;
+		msg.size = sizeof(msg.payload.u64);
+		send_vhost_message(fd, &msg);
+	}
+
 	return 0;
 }
diff --git a/lib/librte_vhost/vhost_user.h b/lib/librte_vhost/vhost_user.h
index ba78d32..179e441 100644
--- a/lib/librte_vhost/vhost_user.h
+++ b/lib/librte_vhost/vhost_user.h
@@ -46,10 +46,12 @@
 #define VHOST_USER_PROTOCOL_F_MQ	0
 #define VHOST_USER_PROTOCOL_F_LOG_SHMFD	1
 #define VHOST_USER_PROTOCOL_F_RARP	2
+#define VHOST_USER_PROTOCOL_F_REPLY_ACK	3
 
 #define VHOST_USER_PROTOCOL_FEATURES	((1ULL << VHOST_USER_PROTOCOL_F_MQ) | \
 					 (1ULL << VHOST_USER_PROTOCOL_F_LOG_SHMFD) |\
-					 (1ULL << VHOST_USER_PROTOCOL_F_RARP))
+					 (1ULL << VHOST_USER_PROTOCOL_F_RARP) | \
+					 (1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK))
 
 typedef enum VhostUserRequest {
 	VHOST_USER_NONE = 0,
@@ -98,6 +100,7 @@ typedef struct VhostUserMsg {
 
 #define VHOST_USER_VERSION_MASK     0x3
 #define VHOST_USER_REPLY_MASK       (0x1 << 2)
+#define VHOST_USER_NEED_REPLY		(0x1 << 3)
 	uint32_t flags;
 	uint32_t size; /* the following payload size */
 	union {
-- 
2.9.3

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

* Re: [dpdk-dev] [PATCH] vhost: Introduce vhost-user's REPLY_ACK feature
  2016-12-12 17:54 [dpdk-dev] [PATCH] vhost: Introduce vhost-user's REPLY_ACK feature Maxime Coquelin
@ 2016-12-13  9:56 ` Yuanhan Liu
  2016-12-13  9:57   ` Maxime Coquelin
  2017-01-12  4:03 ` Yuanhan Liu
  1 sibling, 1 reply; 6+ messages in thread
From: Yuanhan Liu @ 2016-12-13  9:56 UTC (permalink / raw)
  To: Maxime Coquelin; +Cc: dev

On Mon, Dec 12, 2016 at 06:54:00PM +0100, Maxime Coquelin wrote:
> REPLY_ACK features provide a generic way for QEMU to ensure both
> completion and success of a request.
> 
> As described in vhost-user spec in QEMU repository, QEMU sets
> VHOST_USER_NEED_REPLY flag (bit 3) when expecting a reply_ack from
> the backend. Backend must reply with 0 for success or non-zero
> otherwise when flag is set.
> 
> Currently, only VHOST_USER_SET_MEM_TABLE request implements reply_ack,
> in order to synchronize mapping updates.
> 
> This patch enables REPLY_ACK feature generally, but only checks error
> code for VHOST_USER_SET_MEM_TABLE.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks for the patch: it looks good and straightforward to me.

> ---
> Hi,
> 
> The intend of this patch is not to fix a known issue, but it is
> nice to have this feature, and it will be used by upcoming MTU
> feature if it remains in its current form.

Just asking, when do you plan to send out the patches?

	--yliu

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

* Re: [dpdk-dev] [PATCH] vhost: Introduce vhost-user's REPLY_ACK feature
  2016-12-13  9:56 ` Yuanhan Liu
@ 2016-12-13  9:57   ` Maxime Coquelin
  2016-12-13 10:03     ` Yuanhan Liu
  0 siblings, 1 reply; 6+ messages in thread
From: Maxime Coquelin @ 2016-12-13  9:57 UTC (permalink / raw)
  To: Yuanhan Liu; +Cc: dev



On 12/13/2016 10:56 AM, Yuanhan Liu wrote:
> On Mon, Dec 12, 2016 at 06:54:00PM +0100, Maxime Coquelin wrote:
>> REPLY_ACK features provide a generic way for QEMU to ensure both
>> completion and success of a request.
>>
>> As described in vhost-user spec in QEMU repository, QEMU sets
>> VHOST_USER_NEED_REPLY flag (bit 3) when expecting a reply_ack from
>> the backend. Backend must reply with 0 for success or non-zero
>> otherwise when flag is set.
>>
>> Currently, only VHOST_USER_SET_MEM_TABLE request implements reply_ack,
>> in order to synchronize mapping updates.
>>
>> This patch enables REPLY_ACK feature generally, but only checks error
>> code for VHOST_USER_SET_MEM_TABLE.
>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>
> Thanks for the patch: it looks good and straightforward to me.
Good!

>
>> ---
>> Hi,
>>
>> The intend of this patch is not to fix a known issue, but it is
>> nice to have this feature, and it will be used by upcoming MTU
>> feature if it remains in its current form.
>
> Just asking, when do you plan to send out the patches?
As soon as QEMU part is accepted, because changes in QEMU series
may impact DPDK's one.

Cheers,
Maxime

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

* Re: [dpdk-dev] [PATCH] vhost: Introduce vhost-user's REPLY_ACK feature
  2016-12-13  9:57   ` Maxime Coquelin
@ 2016-12-13 10:03     ` Yuanhan Liu
  2016-12-13 10:14       ` Maxime Coquelin
  0 siblings, 1 reply; 6+ messages in thread
From: Yuanhan Liu @ 2016-12-13 10:03 UTC (permalink / raw)
  To: Maxime Coquelin; +Cc: dev

On Tue, Dec 13, 2016 at 10:57:10AM +0100, Maxime Coquelin wrote:
> 
> 
> On 12/13/2016 10:56 AM, Yuanhan Liu wrote:
> >On Mon, Dec 12, 2016 at 06:54:00PM +0100, Maxime Coquelin wrote:
> >>REPLY_ACK features provide a generic way for QEMU to ensure both
> >>completion and success of a request.
> >>
> >>As described in vhost-user spec in QEMU repository, QEMU sets
> >>VHOST_USER_NEED_REPLY flag (bit 3) when expecting a reply_ack from
> >>the backend. Backend must reply with 0 for success or non-zero
> >>otherwise when flag is set.
> >>
> >>Currently, only VHOST_USER_SET_MEM_TABLE request implements reply_ack,
> >>in order to synchronize mapping updates.
> >>
> >>This patch enables REPLY_ACK feature generally, but only checks error
> >>code for VHOST_USER_SET_MEM_TABLE.
> >>
> >>Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> >
> >Thanks for the patch: it looks good and straightforward to me.
> Good!
> 
> >
> >>---
> >>Hi,
> >>
> >>The intend of this patch is not to fix a known issue, but it is
> >>nice to have this feature, and it will be used by upcoming MTU
> >>feature if it remains in its current form.
> >
> >Just asking, when do you plan to send out the patches?
> As soon as QEMU part is accepted, because changes in QEMU series
> may impact DPDK's one.

But maybe you could send them out earlier, so that we could have
understand it better?

	--yliu

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

* Re: [dpdk-dev] [PATCH] vhost: Introduce vhost-user's REPLY_ACK feature
  2016-12-13 10:03     ` Yuanhan Liu
@ 2016-12-13 10:14       ` Maxime Coquelin
  0 siblings, 0 replies; 6+ messages in thread
From: Maxime Coquelin @ 2016-12-13 10:14 UTC (permalink / raw)
  To: Yuanhan Liu; +Cc: dev



On 12/13/2016 11:03 AM, Yuanhan Liu wrote:
> On Tue, Dec 13, 2016 at 10:57:10AM +0100, Maxime Coquelin wrote:
>>
>>
>> On 12/13/2016 10:56 AM, Yuanhan Liu wrote:
>>> On Mon, Dec 12, 2016 at 06:54:00PM +0100, Maxime Coquelin wrote:
>>>> REPLY_ACK features provide a generic way for QEMU to ensure both
>>>> completion and success of a request.
>>>>
>>>> As described in vhost-user spec in QEMU repository, QEMU sets
>>>> VHOST_USER_NEED_REPLY flag (bit 3) when expecting a reply_ack from
>>>> the backend. Backend must reply with 0 for success or non-zero
>>>> otherwise when flag is set.
>>>>
>>>> Currently, only VHOST_USER_SET_MEM_TABLE request implements reply_ack,
>>>> in order to synchronize mapping updates.
>>>>
>>>> This patch enables REPLY_ACK feature generally, but only checks error
>>>> code for VHOST_USER_SET_MEM_TABLE.
>>>>
>>>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>>>
>>> Thanks for the patch: it looks good and straightforward to me.
>> Good!
>>
>>>
>>>> ---
>>>> Hi,
>>>>
>>>> The intend of this patch is not to fix a known issue, but it is
>>>> nice to have this feature, and it will be used by upcoming MTU
>>>> feature if it remains in its current form.
>>>
>>> Just asking, when do you plan to send out the patches?
>> As soon as QEMU part is accepted, because changes in QEMU series
>> may impact DPDK's one.
>
> But maybe you could send them out earlier, so that we could have
> understand it better?
Sure, makes sense.
I'll try to send a RFC next week, that we could merge only once QEMU
part accepted.

Thanks,
Maxime

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

* Re: [dpdk-dev] [PATCH] vhost: Introduce vhost-user's REPLY_ACK feature
  2016-12-12 17:54 [dpdk-dev] [PATCH] vhost: Introduce vhost-user's REPLY_ACK feature Maxime Coquelin
  2016-12-13  9:56 ` Yuanhan Liu
@ 2017-01-12  4:03 ` Yuanhan Liu
  1 sibling, 0 replies; 6+ messages in thread
From: Yuanhan Liu @ 2017-01-12  4:03 UTC (permalink / raw)
  To: Maxime Coquelin; +Cc: dev

On Mon, Dec 12, 2016 at 06:54:00PM +0100, Maxime Coquelin wrote:
> REPLY_ACK features provide a generic way for QEMU to ensure both
> completion and success of a request.
> 
> As described in vhost-user spec in QEMU repository, QEMU sets
> VHOST_USER_NEED_REPLY flag (bit 3) when expecting a reply_ack from
> the backend. Backend must reply with 0 for success or non-zero
> otherwise when flag is set.
> 
> Currently, only VHOST_USER_SET_MEM_TABLE request implements reply_ack,
> in order to synchronize mapping updates.
> 
> This patch enables REPLY_ACK feature generally, but only checks error
> code for VHOST_USER_SET_MEM_TABLE.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Applied to dpdk-next-virtio.

Thanks.

	--yliu

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

end of thread, other threads:[~2017-01-12  4:01 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-12 17:54 [dpdk-dev] [PATCH] vhost: Introduce vhost-user's REPLY_ACK feature Maxime Coquelin
2016-12-13  9:56 ` Yuanhan Liu
2016-12-13  9:57   ` Maxime Coquelin
2016-12-13 10:03     ` Yuanhan Liu
2016-12-13 10:14       ` Maxime Coquelin
2017-01-12  4:03 ` Yuanhan Liu

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