Re: [dpdk-dev] [PATCH 2/3] net/virtio: add reply-ack support to Virtio-user

[Maxime Coquelin <maxime.coquelin@redhat.com>]

Hi Chenbo,

On 6/17/20 5:22 AM, Xia, Chenbo wrote:
> Hi Maxime,
> 
>> -----Original Message-----
>> From: dev <dev-bounces@dpdk.org> On Behalf Of Maxime Coquelin
>> Sent: Thursday, May 28, 2020 3:46 PM
>> To: dev@dpdk.org; amorenoz@redhat.com; Ye, Xiaolong
>> <xiaolong.ye@intel.com>; shahafs@mellanox.com; matan@mellanox.com
>> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Subject: [dpdk-dev] [PATCH 2/3] net/virtio: add reply-ack support to Virtio-user
>>
>> This patch adds support reply-ack vhost-user protocol feature, which is for now
>> only used to ensure VHOST_USER_SET_MEM_TABLE requests are handled by the
>> slave, but later will be used for VHOST_USER_SET_STATUS.
>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> ---
>>  drivers/net/virtio/virtio_user/vhost.h        |  6 ++++-
>>  drivers/net/virtio/virtio_user/vhost_user.c   | 24 ++++++++++++++++---
>>  .../net/virtio/virtio_user/virtio_user_dev.c  |  3 ++-
>>  3 files changed, 28 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/net/virtio/virtio_user/vhost.h
>> b/drivers/net/virtio/virtio_user/vhost.h
>> index 9ace1a90c3..260e1c3081 100644
>> --- a/drivers/net/virtio/virtio_user/vhost.h
>> +++ b/drivers/net/virtio/virtio_user/vhost.h
>> @@ -50,7 +50,11 @@ struct vhost_vring_addr {
>>
>>  /** Protocol features. */
>>  #ifndef VHOST_USER_PROTOCOL_F_MQ
>> -#define VHOST_USER_PROTOCOL_F_MQ	0
>> +#define VHOST_USER_PROTOCOL_F_MQ 0
>> +#endif
>> +
>> +#ifndef VHOST_USER_PROTOCOL_F_REPLY_ACK #define
>> +VHOST_USER_PROTOCOL_F_REPLY_ACK 3
>>  #endif
>>
>>  enum vhost_user_request {
>> diff --git a/drivers/net/virtio/virtio_user/vhost_user.c
>> b/drivers/net/virtio/virtio_user/vhost_user.c
>> index b687665042..f8d751c98e 100644
>> --- a/drivers/net/virtio/virtio_user/vhost_user.c
>> +++ b/drivers/net/virtio/virtio_user/vhost_user.c
>> @@ -32,6 +32,7 @@ struct vhost_user_msg {
>>
>>  #define VHOST_USER_VERSION_MASK     0x3
>>  #define VHOST_USER_REPLY_MASK       (0x1 << 2)
>> +#define VHOST_USER_NEED_REPLY_MASK  (0x1 << 3)
>>  	uint32_t flags;
>>  	uint32_t size; /* the following payload size */
>>  	union {
>> @@ -251,6 +252,7 @@ vhost_user_sock(struct virtio_user_dev *dev,
>>  	struct vhost_user_msg msg;
>>  	struct vhost_vring_file *file = 0;
>>  	int need_reply = 0;
>> +	int has_reply_ack;
>>  	int fds[VHOST_MEMORY_MAX_NREGIONS];
>>  	int fd_num = 0;
>>  	int len;
>> @@ -263,6 +265,9 @@ vhost_user_sock(struct virtio_user_dev *dev,
>>  	if (dev->is_server && vhostfd < 0)
>>  		return -1;
>>
>> +	if (dev->protocol_features & (1ULL <<
>> VHOST_USER_PROTOCOL_F_REPLY_ACK))
>> +		has_reply_ack = 1;
>> +
>>  	msg.request = req;
>>  	msg.flags = VHOST_USER_VERSION;
>>  	msg.size = 0;
>> @@ -291,6 +296,9 @@ vhost_user_sock(struct virtio_user_dev *dev,
>>  		msg.size = sizeof(m.payload.memory.nregions);
>>  		msg.size += sizeof(m.payload.memory.padding);
>>  		msg.size += fd_num * sizeof(struct vhost_memory_region);
>> +
>> +		if (has_reply_ack)
>> +			msg.flags |= VHOST_USER_NEED_REPLY_MASK;
> 
> Do we have counterpart in vhost-user to handle such case as VHOST_USER_SET_MEM_TABLE may have
> a need-reply mask? Do I miss something?
> 

That's actually already supported on Vhost-user backend side, and is
handled for all the requests in a generic way.

http://code.dpdk.org/dpdk/latest/source/lib/librte_vhost/vhost_user.c#L2792

Regards,
Maxime