[dpdk-dev] vhost: Introduce vhost-user's REPLY_ACK feature
Checks
Commit Message
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(-)
Comments
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
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
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
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
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
@@ -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;
}
@@ -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 {