diff mbox series

[3/5] net/virtio: add VIRTIO_SET_STATUS support to Virtio-user

Message ID 20200715171828.95887-4-amorenoz@redhat.com (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers
Series Add support for GET/SET_STATUS on virtio-user pmd |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation success Compilation OK

Commit Message

Adrian Moreno July 15, 2020, 5:18 p.m. UTC 
  From: Maxime Coquelin <maxime.coquelin@redhat.com>

This patch adds support for VHOST_USER_SET_STATUS
request. It is used to make the backend aware of
Virtio devices status update.

It is useful for the backend to know when the Virtio
driver is done with the Virtio device configuration.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
---
 drivers/net/virtio/virtio_user/vhost.h        |  6 +++++
 drivers/net/virtio/virtio_user/vhost_user.c   | 10 ++++++++
 .../net/virtio/virtio_user/virtio_user_dev.c  | 23 ++++++++++++++++++-
 .../net/virtio/virtio_user/virtio_user_dev.h  |  1 +
 drivers/net/virtio/virtio_user_ethdev.c       |  1 +
 5 files changed, 40 insertions(+), 1 deletion(-)

Comments

Chenbo Xia July 16, 2020, 3:15 a.m. UTC | #1 
Hi Adrian,

> -----Original Message-----
> From: Adrian Moreno <amorenoz@redhat.com>
> Sent: Thursday, July 16, 2020 1:18 AM
> To: dev@dpdk.org
> Cc: maxime.coquelin@redhat.com; Wang, Zhihong <zhihong.wang@intel.com>;
> amorenoz@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>
> Subject: [PATCH 3/5] net/virtio: add VIRTIO_SET_STATUS support to Virtio-user
> 
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> 
> This patch adds support for VHOST_USER_SET_STATUS request. It is used to
> make the backend aware of Virtio devices status update.
> 
> It is useful for the backend to know when the Virtio driver is done with the Virtio
> device configuration.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
> ---
>  drivers/net/virtio/virtio_user/vhost.h        |  6 +++++
>  drivers/net/virtio/virtio_user/vhost_user.c   | 10 ++++++++
>  .../net/virtio/virtio_user/virtio_user_dev.c  | 23 ++++++++++++++++++-
>   .../net/virtio/virtio_user/virtio_user_dev.h  |  1 +
>  drivers/net/virtio/virtio_user_ethdev.c       |  1 +
>  5 files changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/virtio/virtio_user/vhost.h
> b/drivers/net/virtio/virtio_user/vhost.h
> index 260e1c308..8f49ef457 100644
> --- a/drivers/net/virtio/virtio_user/vhost.h
> +++ b/drivers/net/virtio/virtio_user/vhost.h
> @@ -57,6 +57,10 @@ struct vhost_vring_addr {  #define
> VHOST_USER_PROTOCOL_F_REPLY_ACK 3  #endif
> 
> +#ifndef VHOST_USER_PROTOCOL_F_STATUS
> +#define VHOST_USER_PROTOCOL_F_STATUS 16 #endif
> +
>  enum vhost_user_request {
>  	VHOST_USER_NONE = 0,
>  	VHOST_USER_GET_FEATURES = 1,
> @@ -77,6 +81,8 @@ enum vhost_user_request {
>  	VHOST_USER_SET_PROTOCOL_FEATURES = 16,
>  	VHOST_USER_GET_QUEUE_NUM = 17,
>  	VHOST_USER_SET_VRING_ENABLE = 18,
> +	VHOST_USER_SET_STATUS = 39,
> +	VHOST_USER_GET_STATUS = 40,
>  	VHOST_USER_MAX
>  };
> 
> diff --git a/drivers/net/virtio/virtio_user/vhost_user.c
> b/drivers/net/virtio/virtio_user/vhost_user.c
> index 631d0e353..2332e01d1 100644
> --- a/drivers/net/virtio/virtio_user/vhost_user.c
> +++ b/drivers/net/virtio/virtio_user/vhost_user.c
> @@ -244,6 +244,8 @@ const char * const vhost_msg_strings[] = {
>  	[VHOST_USER_SET_VRING_ENABLE] = "VHOST_SET_VRING_ENABLE",
>  	[VHOST_USER_GET_PROTOCOL_FEATURES] =
> "VHOST_USER_GET_PROTOCOL_FEATURES",
>  	[VHOST_USER_SET_PROTOCOL_FEATURES] =
> "VHOST_USER_SET_PROTOCOL_FEATURES",
> +	[VHOST_USER_SET_STATUS] = "VHOST_SET_STATUS",
> +	[VHOST_USER_GET_STATUS] = "VHOST_GET_STATUS",
>  };
> 
>  static int
> @@ -280,6 +282,14 @@ vhost_user_sock(struct virtio_user_dev *dev,
>  		need_reply = 1;
>  		break;
> 
> +	case VHOST_USER_SET_STATUS:
> +		if (!(dev->protocol_features &
> +				(1ULL <<
> VHOST_USER_PROTOCOL_F_STATUS)))
> +			return 0;
> +
> +		if (has_reply_ack)
> +			msg.flags |= VHOST_USER_NEED_REPLY_MASK;
> +		/* Fallthrough */

There's a coding style issue here:
WARNING:PREFER_FALLTHROUGH: Prefer 'fallthrough;' over fallthrough comment.
Could you fix this?

>  	case VHOST_USER_SET_FEATURES:
>  	case VHOST_USER_SET_PROTOCOL_FEATURES:
>  	case VHOST_USER_SET_LOG_BASE:
> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> index 0a6991bcc..2c400a448 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> @@ -424,7 +424,8 @@ virtio_user_dev_setup(struct virtio_user_dev *dev)
> 
>  #define VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES		\
>  	(1ULL << VHOST_USER_PROTOCOL_F_MQ |		\
> -	 1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK)
> +	 1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK |	\
> +	 1ULL << VHOST_USER_PROTOCOL_F_STATUS)
> 
>  int
>  virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues, @@ -
> 783,3 +784,23 @@ virtio_user_handle_cq(struct virtio_user_dev *dev, uint16_t
> queue_idx)
>  		__atomic_add_fetch(&vring->used->idx, 1,
> __ATOMIC_RELAXED);
>  	}
>  }
> +
> +int
> +virtio_user_send_status_update(struct virtio_user_dev *dev, uint8_t
> +status) {
> +	int ret;
> +	uint64_t arg = status;
> +
> +	/* Vhost-user only for now */
> +	if (!is_vhost_user_by_type(dev->path))
> +		return 0;
> +
> +	if (!(dev->protocol_features & (1ULL <<
> VHOST_USER_PROTOCOL_F_STATUS)))
> +		return 0;
> +
> +	ret = dev->ops->send_request(dev, VHOST_USER_SET_STATUS, &arg);
> +	if (ret)
> +		return -1;

Do you think we should add a log here to show if SET_STAUTS failed?

Thanks!
Chenbo

> +
> +	return 0;
> +}
> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> index 10b274d19..a76d7cfaf 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> @@ -74,4 +74,5 @@ void virtio_user_handle_cq(struct virtio_user_dev *dev,
> uint16_t queue_idx);  void virtio_user_handle_cq_packed(struct virtio_user_dev
> *dev,
>  				  uint16_t queue_idx);
>  uint8_t virtio_user_handle_mq(struct virtio_user_dev *dev, uint16_t q_pairs);
> +int virtio_user_send_status_update(struct virtio_user_dev *dev, uint8_t
> +status);
>  #endif
> diff --git a/drivers/net/virtio/virtio_user_ethdev.c
> b/drivers/net/virtio/virtio_user_ethdev.c
> index 5b06d8e89..e52f11341 100644
> --- a/drivers/net/virtio/virtio_user_ethdev.c
> +++ b/drivers/net/virtio/virtio_user_ethdev.c
> @@ -273,6 +273,7 @@ virtio_user_set_status(struct virtio_hw *hw, uint8_t
> status)
>  	else if (status == VIRTIO_CONFIG_STATUS_RESET)
>  		virtio_user_reset(hw);
>  	dev->status = status;
> +	virtio_user_send_status_update(dev, status);
>  }
> 
>  static uint8_t
> --
> 2.26.2
Adrian Moreno July 16, 2020, 7:43 a.m. UTC | #2 
On 7/16/20 5:15 AM, Xia, Chenbo wrote:
> Hi Adrian,
> 
>> -----Original Message-----
>> From: Adrian Moreno <amorenoz@redhat.com>
>> Sent: Thursday, July 16, 2020 1:18 AM
>> To: dev@dpdk.org
>> Cc: maxime.coquelin@redhat.com; Wang, Zhihong <zhihong.wang@intel.com>;
>> amorenoz@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>
>> Subject: [PATCH 3/5] net/virtio: add VIRTIO_SET_STATUS support to Virtio-user
>>
>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>>
>> This patch adds support for VHOST_USER_SET_STATUS request. It is used to
>> make the backend aware of Virtio devices status update.
>>
>> It is useful for the backend to know when the Virtio driver is done with the Virtio
>> device configuration.
>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
>> ---
>>  drivers/net/virtio/virtio_user/vhost.h        |  6 +++++
>>  drivers/net/virtio/virtio_user/vhost_user.c   | 10 ++++++++
>>  .../net/virtio/virtio_user/virtio_user_dev.c  | 23 ++++++++++++++++++-
>>   .../net/virtio/virtio_user/virtio_user_dev.h  |  1 +
>>  drivers/net/virtio/virtio_user_ethdev.c       |  1 +
>>  5 files changed, 40 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/virtio/virtio_user/vhost.h
>> b/drivers/net/virtio/virtio_user/vhost.h
>> index 260e1c308..8f49ef457 100644
>> --- a/drivers/net/virtio/virtio_user/vhost.h
>> +++ b/drivers/net/virtio/virtio_user/vhost.h
>> @@ -57,6 +57,10 @@ struct vhost_vring_addr {  #define
>> VHOST_USER_PROTOCOL_F_REPLY_ACK 3  #endif
>>
>> +#ifndef VHOST_USER_PROTOCOL_F_STATUS
>> +#define VHOST_USER_PROTOCOL_F_STATUS 16 #endif
>> +
>>  enum vhost_user_request {
>>  	VHOST_USER_NONE = 0,
>>  	VHOST_USER_GET_FEATURES = 1,
>> @@ -77,6 +81,8 @@ enum vhost_user_request {
>>  	VHOST_USER_SET_PROTOCOL_FEATURES = 16,
>>  	VHOST_USER_GET_QUEUE_NUM = 17,
>>  	VHOST_USER_SET_VRING_ENABLE = 18,
>> +	VHOST_USER_SET_STATUS = 39,
>> +	VHOST_USER_GET_STATUS = 40,
>>  	VHOST_USER_MAX
>>  };
>>
>> diff --git a/drivers/net/virtio/virtio_user/vhost_user.c
>> b/drivers/net/virtio/virtio_user/vhost_user.c
>> index 631d0e353..2332e01d1 100644
>> --- a/drivers/net/virtio/virtio_user/vhost_user.c
>> +++ b/drivers/net/virtio/virtio_user/vhost_user.c
>> @@ -244,6 +244,8 @@ const char * const vhost_msg_strings[] = {
>>  	[VHOST_USER_SET_VRING_ENABLE] = "VHOST_SET_VRING_ENABLE",
>>  	[VHOST_USER_GET_PROTOCOL_FEATURES] =
>> "VHOST_USER_GET_PROTOCOL_FEATURES",
>>  	[VHOST_USER_SET_PROTOCOL_FEATURES] =
>> "VHOST_USER_SET_PROTOCOL_FEATURES",
>> +	[VHOST_USER_SET_STATUS] = "VHOST_SET_STATUS",
>> +	[VHOST_USER_GET_STATUS] = "VHOST_GET_STATUS",
>>  };
>>
>>  static int
>> @@ -280,6 +282,14 @@ vhost_user_sock(struct virtio_user_dev *dev,
>>  		need_reply = 1;
>>  		break;
>>
>> +	case VHOST_USER_SET_STATUS:
>> +		if (!(dev->protocol_features &
>> +				(1ULL <<
>> VHOST_USER_PROTOCOL_F_STATUS)))
>> +			return 0;
>> +
>> +		if (has_reply_ack)
>> +			msg.flags |= VHOST_USER_NEED_REPLY_MASK;
>> +		/* Fallthrough */
> 
> There's a coding style issue here:
> WARNING:PREFER_FALLTHROUGH: Prefer 'fallthrough;' over fallthrough comment.
> Could you fix this?
> 
"fallthrough" is not defined. I think this macro is defined in the linux kernel
(where checkpatches.pl comes from?). We could define the same macro that would
depend in compiler support for __falthrough__ attribute.

In any case, this is not the only case:

$ find -name *.c | xargs grep -i "/\* fallthrough \*/" | wc -l
62

So maybe this warning is new?
Should I change all of them together in another patch?

>>  	case VHOST_USER_SET_FEATURES:
>>  	case VHOST_USER_SET_PROTOCOL_FEATURES:
>>  	case VHOST_USER_SET_LOG_BASE:
>> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
>> b/drivers/net/virtio/virtio_user/virtio_user_dev.c
>> index 0a6991bcc..2c400a448 100644
>> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
>> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
>> @@ -424,7 +424,8 @@ virtio_user_dev_setup(struct virtio_user_dev *dev)
>>
>>  #define VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES		\
>>  	(1ULL << VHOST_USER_PROTOCOL_F_MQ |		\
>> -	 1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK)
>> +	 1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK |	\
>> +	 1ULL << VHOST_USER_PROTOCOL_F_STATUS)
>>
>>  int
>>  virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues, @@ -
>> 783,3 +784,23 @@ virtio_user_handle_cq(struct virtio_user_dev *dev, uint16_t
>> queue_idx)
>>  		__atomic_add_fetch(&vring->used->idx, 1,
>> __ATOMIC_RELAXED);
>>  	}
>>  }
>> +
>> +int
>> +virtio_user_send_status_update(struct virtio_user_dev *dev, uint8_t
>> +status) {
>> +	int ret;
>> +	uint64_t arg = status;
>> +
>> +	/* Vhost-user only for now */
>> +	if (!is_vhost_user_by_type(dev->path))
>> +		return 0;
>> +
>> +	if (!(dev->protocol_features & (1ULL <<
>> VHOST_USER_PROTOCOL_F_STATUS)))
>> +		return 0;
>> +
>> +	ret = dev->ops->send_request(dev, VHOST_USER_SET_STATUS, &arg);
>> +	if (ret)
>> +		return -1;
> 
> Do you think we should add a log here to show if SET_STAUTS failed?
> 
Good idea! Will do in the next version. Thanks

> Thanks!
> Chenbo
> 
>> +
>> +	return 0;
>> +}
>> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h
>> b/drivers/net/virtio/virtio_user/virtio_user_dev.h
>> index 10b274d19..a76d7cfaf 100644
>> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
>> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
>> @@ -74,4 +74,5 @@ void virtio_user_handle_cq(struct virtio_user_dev *dev,
>> uint16_t queue_idx);  void virtio_user_handle_cq_packed(struct virtio_user_dev
>> *dev,
>>  				  uint16_t queue_idx);
>>  uint8_t virtio_user_handle_mq(struct virtio_user_dev *dev, uint16_t q_pairs);
>> +int virtio_user_send_status_update(struct virtio_user_dev *dev, uint8_t
>> +status);
>>  #endif
>> diff --git a/drivers/net/virtio/virtio_user_ethdev.c
>> b/drivers/net/virtio/virtio_user_ethdev.c
>> index 5b06d8e89..e52f11341 100644
>> --- a/drivers/net/virtio/virtio_user_ethdev.c
>> +++ b/drivers/net/virtio/virtio_user_ethdev.c
>> @@ -273,6 +273,7 @@ virtio_user_set_status(struct virtio_hw *hw, uint8_t
>> status)
>>  	else if (status == VIRTIO_CONFIG_STATUS_RESET)
>>  		virtio_user_reset(hw);
>>  	dev->status = status;
>> +	virtio_user_send_status_update(dev, status);
>>  }
>>
>>  static uint8_t
>> --
>> 2.26.2
>
Chenbo Xia July 16, 2020, 8:58 a.m. UTC | #3 
Hi Adrian,

> -----Original Message-----
> From: Adrian Moreno <amorenoz@redhat.com>
> Sent: Thursday, July 16, 2020 3:43 PM
> To: Xia, Chenbo <chenbo.xia@intel.com>; dev@dpdk.org
> Cc: maxime.coquelin@redhat.com; Wang, Zhihong <zhihong.wang@intel.com>
> Subject: Re: [PATCH 3/5] net/virtio: add VIRTIO_SET_STATUS support to Virtio-
> user
> 
> 
> 
> On 7/16/20 5:15 AM, Xia, Chenbo wrote:
> > Hi Adrian,
> >
> >> -----Original Message-----
> >> From: Adrian Moreno <amorenoz@redhat.com>
> >> Sent: Thursday, July 16, 2020 1:18 AM
> >> To: dev@dpdk.org
> >> Cc: maxime.coquelin@redhat.com; Wang, Zhihong
> >> <zhihong.wang@intel.com>; amorenoz@redhat.com; Xia, Chenbo
> >> <chenbo.xia@intel.com>
> >> Subject: [PATCH 3/5] net/virtio: add VIRTIO_SET_STATUS support to
> >> Virtio-user
> >>
> >> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> >>
> >> This patch adds support for VHOST_USER_SET_STATUS request. It is used
> >> to make the backend aware of Virtio devices status update.
> >>
> >> It is useful for the backend to know when the Virtio driver is done
> >> with the Virtio device configuration.
> >>
> >> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> >> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
> >> ---
> >>  drivers/net/virtio/virtio_user/vhost.h        |  6 +++++
> >>  drivers/net/virtio/virtio_user/vhost_user.c   | 10 ++++++++
> >>  .../net/virtio/virtio_user/virtio_user_dev.c  | 23 ++++++++++++++++++-
> >>   .../net/virtio/virtio_user/virtio_user_dev.h  |  1 +
> >>  drivers/net/virtio/virtio_user_ethdev.c       |  1 +
> >>  5 files changed, 40 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/net/virtio/virtio_user/vhost.h
> >> b/drivers/net/virtio/virtio_user/vhost.h
> >> index 260e1c308..8f49ef457 100644
> >> --- a/drivers/net/virtio/virtio_user/vhost.h
> >> +++ b/drivers/net/virtio/virtio_user/vhost.h
> >> @@ -57,6 +57,10 @@ struct vhost_vring_addr {  #define
> >> VHOST_USER_PROTOCOL_F_REPLY_ACK 3  #endif
> >>
> >> +#ifndef VHOST_USER_PROTOCOL_F_STATUS #define
> >> +VHOST_USER_PROTOCOL_F_STATUS 16 #endif
> >> +
> >>  enum vhost_user_request {
> >>  	VHOST_USER_NONE = 0,
> >>  	VHOST_USER_GET_FEATURES = 1,
> >> @@ -77,6 +81,8 @@ enum vhost_user_request {
> >>  	VHOST_USER_SET_PROTOCOL_FEATURES = 16,
> >>  	VHOST_USER_GET_QUEUE_NUM = 17,
> >>  	VHOST_USER_SET_VRING_ENABLE = 18,
> >> +	VHOST_USER_SET_STATUS = 39,
> >> +	VHOST_USER_GET_STATUS = 40,
> >>  	VHOST_USER_MAX
> >>  };
> >>
> >> diff --git a/drivers/net/virtio/virtio_user/vhost_user.c
> >> b/drivers/net/virtio/virtio_user/vhost_user.c
> >> index 631d0e353..2332e01d1 100644
> >> --- a/drivers/net/virtio/virtio_user/vhost_user.c
> >> +++ b/drivers/net/virtio/virtio_user/vhost_user.c
> >> @@ -244,6 +244,8 @@ const char * const vhost_msg_strings[] = {
> >>  	[VHOST_USER_SET_VRING_ENABLE] = "VHOST_SET_VRING_ENABLE",
> >>  	[VHOST_USER_GET_PROTOCOL_FEATURES] =
> >> "VHOST_USER_GET_PROTOCOL_FEATURES",
> >>  	[VHOST_USER_SET_PROTOCOL_FEATURES] =
> >> "VHOST_USER_SET_PROTOCOL_FEATURES",
> >> +	[VHOST_USER_SET_STATUS] = "VHOST_SET_STATUS",
> >> +	[VHOST_USER_GET_STATUS] = "VHOST_GET_STATUS",
> >>  };
> >>
> >>  static int
> >> @@ -280,6 +282,14 @@ vhost_user_sock(struct virtio_user_dev *dev,
> >>  		need_reply = 1;
> >>  		break;
> >>
> >> +	case VHOST_USER_SET_STATUS:
> >> +		if (!(dev->protocol_features &
> >> +				(1ULL <<
> >> VHOST_USER_PROTOCOL_F_STATUS)))
> >> +			return 0;
> >> +
> >> +		if (has_reply_ack)
> >> +			msg.flags |= VHOST_USER_NEED_REPLY_MASK;
> >> +		/* Fallthrough */
> >
> > There's a coding style issue here:
> > WARNING:PREFER_FALLTHROUGH: Prefer 'fallthrough;' over fallthrough
> comment.
> > Could you fix this?
> >
> "fallthrough" is not defined. I think this macro is defined in the linux kernel
> (where checkpatches.pl comes from?). We could define the same macro that
> would depend in compiler support for __falthrough__ attribute.
> 
> In any case, this is not the only case:
> 
> $ find -name *.c | xargs grep -i "/\* fallthrough \*/" | wc -l
> 62
> 
> So maybe this warning is new?
> Should I change all of them together in another patch?

checkpatches.pl is in linux kernel. I think it prefers 'fallthrough' rather than
'Fallthrough'. I think it's a linux style, right?

Thanks!
Chenbo

> 
> >>  	case VHOST_USER_SET_FEATURES:
> >>  	case VHOST_USER_SET_PROTOCOL_FEATURES:
> >>  	case VHOST_USER_SET_LOG_BASE:
> >> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> >> b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> >> index 0a6991bcc..2c400a448 100644
> >> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> >> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> >> @@ -424,7 +424,8 @@ virtio_user_dev_setup(struct virtio_user_dev
> >> *dev)
> >>
> >>  #define VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES		\
> >>  	(1ULL << VHOST_USER_PROTOCOL_F_MQ |		\
> >> -	 1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK)
> >> +	 1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK |	\
> >> +	 1ULL << VHOST_USER_PROTOCOL_F_STATUS)
> >>
> >>  int
> >>  virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int
> >> queues, @@ -
> >> 783,3 +784,23 @@ virtio_user_handle_cq(struct virtio_user_dev *dev,
> >> uint16_t
> >> queue_idx)
> >>  		__atomic_add_fetch(&vring->used->idx, 1,
> __ATOMIC_RELAXED);
> >>  	}
> >>  }
> >> +
> >> +int
> >> +virtio_user_send_status_update(struct virtio_user_dev *dev, uint8_t
> >> +status) {
> >> +	int ret;
> >> +	uint64_t arg = status;
> >> +
> >> +	/* Vhost-user only for now */
> >> +	if (!is_vhost_user_by_type(dev->path))
> >> +		return 0;
> >> +
> >> +	if (!(dev->protocol_features & (1ULL <<
> >> VHOST_USER_PROTOCOL_F_STATUS)))
> >> +		return 0;
> >> +
> >> +	ret = dev->ops->send_request(dev, VHOST_USER_SET_STATUS, &arg);
> >> +	if (ret)
> >> +		return -1;
> >
> > Do you think we should add a log here to show if SET_STAUTS failed?
> >
> Good idea! Will do in the next version. Thanks
> 
> > Thanks!
> > Chenbo
> >
> >> +
> >> +	return 0;
> >> +}
> >> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> >> b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> >> index 10b274d19..a76d7cfaf 100644
> >> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> >> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> >> @@ -74,4 +74,5 @@ void virtio_user_handle_cq(struct virtio_user_dev
> >> *dev, uint16_t queue_idx);  void virtio_user_handle_cq_packed(struct
> >> virtio_user_dev *dev,
> >>  				  uint16_t queue_idx);
> >>  uint8_t virtio_user_handle_mq(struct virtio_user_dev *dev, uint16_t
> >> q_pairs);
> >> +int virtio_user_send_status_update(struct virtio_user_dev *dev,
> >> +uint8_t status);
> >>  #endif
> >> diff --git a/drivers/net/virtio/virtio_user_ethdev.c
> >> b/drivers/net/virtio/virtio_user_ethdev.c
> >> index 5b06d8e89..e52f11341 100644
> >> --- a/drivers/net/virtio/virtio_user_ethdev.c
> >> +++ b/drivers/net/virtio/virtio_user_ethdev.c
> >> @@ -273,6 +273,7 @@ virtio_user_set_status(struct virtio_hw *hw,
> >> uint8_t
> >> status)
> >>  	else if (status == VIRTIO_CONFIG_STATUS_RESET)
> >>  		virtio_user_reset(hw);
> >>  	dev->status = status;
> >> +	virtio_user_send_status_update(dev, status);
> >>  }
> >>
> >>  static uint8_t
> >> --
> >> 2.26.2
> >
> 
> --
> Adrián Moreno
Adrian Moreno July 16, 2020, 9:51 a.m. UTC | #4 
On 7/16/20 10:58 AM, Xia, Chenbo wrote:
> Hi Adrian,
> 
>> -----Original Message-----
>> From: Adrian Moreno <amorenoz@redhat.com>
>> Sent: Thursday, July 16, 2020 3:43 PM
>> To: Xia, Chenbo <chenbo.xia@intel.com>; dev@dpdk.org
>> Cc: maxime.coquelin@redhat.com; Wang, Zhihong <zhihong.wang@intel.com>
>> Subject: Re: [PATCH 3/5] net/virtio: add VIRTIO_SET_STATUS support to Virtio-
>> user
>>
>>
>>
>> On 7/16/20 5:15 AM, Xia, Chenbo wrote:
>>> Hi Adrian,
>>>
>>>> -----Original Message-----
>>>> From: Adrian Moreno <amorenoz@redhat.com>
>>>> Sent: Thursday, July 16, 2020 1:18 AM
>>>> To: dev@dpdk.org
>>>> Cc: maxime.coquelin@redhat.com; Wang, Zhihong
>>>> <zhihong.wang@intel.com>; amorenoz@redhat.com; Xia, Chenbo
>>>> <chenbo.xia@intel.com>
>>>> Subject: [PATCH 3/5] net/virtio: add VIRTIO_SET_STATUS support to
>>>> Virtio-user
>>>>
>>>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>>>>
>>>> This patch adds support for VHOST_USER_SET_STATUS request. It is used
>>>> to make the backend aware of Virtio devices status update.
>>>>
>>>> It is useful for the backend to know when the Virtio driver is done
>>>> with the Virtio device configuration.
>>>>
>>>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>>>> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
>>>> ---
>>>>  drivers/net/virtio/virtio_user/vhost.h        |  6 +++++
>>>>  drivers/net/virtio/virtio_user/vhost_user.c   | 10 ++++++++
>>>>  .../net/virtio/virtio_user/virtio_user_dev.c  | 23 ++++++++++++++++++-
>>>>   .../net/virtio/virtio_user/virtio_user_dev.h  |  1 +
>>>>  drivers/net/virtio/virtio_user_ethdev.c       |  1 +
>>>>  5 files changed, 40 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/net/virtio/virtio_user/vhost.h
>>>> b/drivers/net/virtio/virtio_user/vhost.h
>>>> index 260e1c308..8f49ef457 100644
>>>> --- a/drivers/net/virtio/virtio_user/vhost.h
>>>> +++ b/drivers/net/virtio/virtio_user/vhost.h
>>>> @@ -57,6 +57,10 @@ struct vhost_vring_addr {  #define
>>>> VHOST_USER_PROTOCOL_F_REPLY_ACK 3  #endif
>>>>
>>>> +#ifndef VHOST_USER_PROTOCOL_F_STATUS #define
>>>> +VHOST_USER_PROTOCOL_F_STATUS 16 #endif
>>>> +
>>>>  enum vhost_user_request {
>>>>  	VHOST_USER_NONE = 0,
>>>>  	VHOST_USER_GET_FEATURES = 1,
>>>> @@ -77,6 +81,8 @@ enum vhost_user_request {
>>>>  	VHOST_USER_SET_PROTOCOL_FEATURES = 16,
>>>>  	VHOST_USER_GET_QUEUE_NUM = 17,
>>>>  	VHOST_USER_SET_VRING_ENABLE = 18,
>>>> +	VHOST_USER_SET_STATUS = 39,
>>>> +	VHOST_USER_GET_STATUS = 40,
>>>>  	VHOST_USER_MAX
>>>>  };
>>>>
>>>> diff --git a/drivers/net/virtio/virtio_user/vhost_user.c
>>>> b/drivers/net/virtio/virtio_user/vhost_user.c
>>>> index 631d0e353..2332e01d1 100644
>>>> --- a/drivers/net/virtio/virtio_user/vhost_user.c
>>>> +++ b/drivers/net/virtio/virtio_user/vhost_user.c
>>>> @@ -244,6 +244,8 @@ const char * const vhost_msg_strings[] = {
>>>>  	[VHOST_USER_SET_VRING_ENABLE] = "VHOST_SET_VRING_ENABLE",
>>>>  	[VHOST_USER_GET_PROTOCOL_FEATURES] =
>>>> "VHOST_USER_GET_PROTOCOL_FEATURES",
>>>>  	[VHOST_USER_SET_PROTOCOL_FEATURES] =
>>>> "VHOST_USER_SET_PROTOCOL_FEATURES",
>>>> +	[VHOST_USER_SET_STATUS] = "VHOST_SET_STATUS",
>>>> +	[VHOST_USER_GET_STATUS] = "VHOST_GET_STATUS",
>>>>  };
>>>>
>>>>  static int
>>>> @@ -280,6 +282,14 @@ vhost_user_sock(struct virtio_user_dev *dev,
>>>>  		need_reply = 1;
>>>>  		break;
>>>>
>>>> +	case VHOST_USER_SET_STATUS:
>>>> +		if (!(dev->protocol_features &
>>>> +				(1ULL <<
>>>> VHOST_USER_PROTOCOL_F_STATUS)))
>>>> +			return 0;
>>>> +
>>>> +		if (has_reply_ack)
>>>> +			msg.flags |= VHOST_USER_NEED_REPLY_MASK;
>>>> +		/* Fallthrough */
>>>
>>> There's a coding style issue here:
>>> WARNING:PREFER_FALLTHROUGH: Prefer 'fallthrough;' over fallthrough
>> comment.
>>> Could you fix this?
>>>
>> "fallthrough" is not defined. I think this macro is defined in the linux kernel
>> (where checkpatches.pl comes from?). We could define the same macro that
>> would depend in compiler support for __falthrough__ attribute.
>>
>> In any case, this is not the only case:
>>
>> $ find -name *.c | xargs grep -i "/\* fallthrough \*/" | wc -l
>> 62
>>
>> So maybe this warning is new?
>> Should I change all of them together in another patch?
> 
> checkpatches.pl is in linux kernel. I think it prefers 'fallthrough' rather than
> 'Fallthrough'. I think it's a linux style, right?
> 
It refers to this:
https://github.com/torvalds/linux/blob/91a9a90d040e8b9ff63d48ea71468e0f4db764ff/include/linux/compiler_attributes.h#L201

Since we don't have that pseudo-keyword, we should see if we can disable this in
checkpatches.


> Thanks!
> Chenbo
> 
>>
>>>>  	case VHOST_USER_SET_FEATURES:
>>>>  	case VHOST_USER_SET_PROTOCOL_FEATURES:
>>>>  	case VHOST_USER_SET_LOG_BASE:
>>>> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
>>>> b/drivers/net/virtio/virtio_user/virtio_user_dev.c
>>>> index 0a6991bcc..2c400a448 100644
>>>> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
>>>> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
>>>> @@ -424,7 +424,8 @@ virtio_user_dev_setup(struct virtio_user_dev
>>>> *dev)
>>>>
>>>>  #define VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES		\
>>>>  	(1ULL << VHOST_USER_PROTOCOL_F_MQ |		\
>>>> -	 1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK)
>>>> +	 1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK |	\
>>>> +	 1ULL << VHOST_USER_PROTOCOL_F_STATUS)
>>>>
>>>>  int
>>>>  virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int
>>>> queues, @@ -
>>>> 783,3 +784,23 @@ virtio_user_handle_cq(struct virtio_user_dev *dev,
>>>> uint16_t
>>>> queue_idx)
>>>>  		__atomic_add_fetch(&vring->used->idx, 1,
>> __ATOMIC_RELAXED);
>>>>  	}
>>>>  }
>>>> +
>>>> +int
>>>> +virtio_user_send_status_update(struct virtio_user_dev *dev, uint8_t
>>>> +status) {
>>>> +	int ret;
>>>> +	uint64_t arg = status;
>>>> +
>>>> +	/* Vhost-user only for now */
>>>> +	if (!is_vhost_user_by_type(dev->path))
>>>> +		return 0;
>>>> +
>>>> +	if (!(dev->protocol_features & (1ULL <<
>>>> VHOST_USER_PROTOCOL_F_STATUS)))
>>>> +		return 0;
>>>> +
>>>> +	ret = dev->ops->send_request(dev, VHOST_USER_SET_STATUS, &arg);
>>>> +	if (ret)
>>>> +		return -1;
>>>
>>> Do you think we should add a log here to show if SET_STAUTS failed?
>>>
>> Good idea! Will do in the next version. Thanks
>>
>>> Thanks!
>>> Chenbo
>>>
>>>> +
>>>> +	return 0;
>>>> +}
>>>> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h
>>>> b/drivers/net/virtio/virtio_user/virtio_user_dev.h
>>>> index 10b274d19..a76d7cfaf 100644
>>>> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
>>>> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
>>>> @@ -74,4 +74,5 @@ void virtio_user_handle_cq(struct virtio_user_dev
>>>> *dev, uint16_t queue_idx);  void virtio_user_handle_cq_packed(struct
>>>> virtio_user_dev *dev,
>>>>  				  uint16_t queue_idx);
>>>>  uint8_t virtio_user_handle_mq(struct virtio_user_dev *dev, uint16_t
>>>> q_pairs);
>>>> +int virtio_user_send_status_update(struct virtio_user_dev *dev,
>>>> +uint8_t status);
>>>>  #endif
>>>> diff --git a/drivers/net/virtio/virtio_user_ethdev.c
>>>> b/drivers/net/virtio/virtio_user_ethdev.c
>>>> index 5b06d8e89..e52f11341 100644
>>>> --- a/drivers/net/virtio/virtio_user_ethdev.c
>>>> +++ b/drivers/net/virtio/virtio_user_ethdev.c
>>>> @@ -273,6 +273,7 @@ virtio_user_set_status(struct virtio_hw *hw,
>>>> uint8_t
>>>> status)
>>>>  	else if (status == VIRTIO_CONFIG_STATUS_RESET)
>>>>  		virtio_user_reset(hw);
>>>>  	dev->status = status;
>>>> +	virtio_user_send_status_update(dev, status);
>>>>  }
>>>>
>>>>  static uint8_t
>>>> --
>>>> 2.26.2
>>>
>>
>> --
>> Adrián Moreno
>
Chenbo Xia July 16, 2020, 11:18 a.m. UTC | #5 
Hi Adrian,

> -----Original Message-----
> From: Adrian Moreno <amorenoz@redhat.com>
> Sent: Thursday, July 16, 2020 5:51 PM
> To: Xia, Chenbo <chenbo.xia@intel.com>; dev@dpdk.org
> Cc: maxime.coquelin@redhat.com; Wang, Zhihong <zhihong.wang@intel.com>
> Subject: Re: [PATCH 3/5] net/virtio: add VIRTIO_SET_STATUS support to Virtio-
> user
> 
> 
> 
> On 7/16/20 10:58 AM, Xia, Chenbo wrote:
> > Hi Adrian,
> >
> >> -----Original Message-----
> >> From: Adrian Moreno <amorenoz@redhat.com>
> >> Sent: Thursday, July 16, 2020 3:43 PM
> >> To: Xia, Chenbo <chenbo.xia@intel.com>; dev@dpdk.org
> >> Cc: maxime.coquelin@redhat.com; Wang, Zhihong
> >> <zhihong.wang@intel.com>
> >> Subject: Re: [PATCH 3/5] net/virtio: add VIRTIO_SET_STATUS support to
> >> Virtio- user
> >>
> >>
> >>
> >> On 7/16/20 5:15 AM, Xia, Chenbo wrote:
> >>> Hi Adrian,
> >>>
> >>>> -----Original Message-----
> >>>> From: Adrian Moreno <amorenoz@redhat.com>
> >>>> Sent: Thursday, July 16, 2020 1:18 AM
> >>>> To: dev@dpdk.org
> >>>> Cc: maxime.coquelin@redhat.com; Wang, Zhihong
> >>>> <zhihong.wang@intel.com>; amorenoz@redhat.com; Xia, Chenbo
> >>>> <chenbo.xia@intel.com>
> >>>> Subject: [PATCH 3/5] net/virtio: add VIRTIO_SET_STATUS support to
> >>>> Virtio-user
> >>>>
> >>>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> >>>>
> >>>> This patch adds support for VHOST_USER_SET_STATUS request. It is
> >>>> used to make the backend aware of Virtio devices status update.
> >>>>
> >>>> It is useful for the backend to know when the Virtio driver is done
> >>>> with the Virtio device configuration.
> >>>>
> >>>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> >>>> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
> >>>> ---
> >>>>  drivers/net/virtio/virtio_user/vhost.h        |  6 +++++
> >>>>  drivers/net/virtio/virtio_user/vhost_user.c   | 10 ++++++++
> >>>>  .../net/virtio/virtio_user/virtio_user_dev.c  | 23 ++++++++++++++++++-
> >>>>   .../net/virtio/virtio_user/virtio_user_dev.h  |  1 +
> >>>>  drivers/net/virtio/virtio_user_ethdev.c       |  1 +
> >>>>  5 files changed, 40 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/drivers/net/virtio/virtio_user/vhost.h
> >>>> b/drivers/net/virtio/virtio_user/vhost.h
> >>>> index 260e1c308..8f49ef457 100644
> >>>> --- a/drivers/net/virtio/virtio_user/vhost.h
> >>>> +++ b/drivers/net/virtio/virtio_user/vhost.h
> >>>> @@ -57,6 +57,10 @@ struct vhost_vring_addr {  #define
> >>>> VHOST_USER_PROTOCOL_F_REPLY_ACK 3  #endif
> >>>>
> >>>> +#ifndef VHOST_USER_PROTOCOL_F_STATUS #define
> >>>> +VHOST_USER_PROTOCOL_F_STATUS 16 #endif
> >>>> +
> >>>>  enum vhost_user_request {
> >>>>  	VHOST_USER_NONE = 0,
> >>>>  	VHOST_USER_GET_FEATURES = 1,
> >>>> @@ -77,6 +81,8 @@ enum vhost_user_request {
> >>>>  	VHOST_USER_SET_PROTOCOL_FEATURES = 16,
> >>>>  	VHOST_USER_GET_QUEUE_NUM = 17,
> >>>>  	VHOST_USER_SET_VRING_ENABLE = 18,
> >>>> +	VHOST_USER_SET_STATUS = 39,
> >>>> +	VHOST_USER_GET_STATUS = 40,
> >>>>  	VHOST_USER_MAX
> >>>>  };
> >>>>
> >>>> diff --git a/drivers/net/virtio/virtio_user/vhost_user.c
> >>>> b/drivers/net/virtio/virtio_user/vhost_user.c
> >>>> index 631d0e353..2332e01d1 100644
> >>>> --- a/drivers/net/virtio/virtio_user/vhost_user.c
> >>>> +++ b/drivers/net/virtio/virtio_user/vhost_user.c
> >>>> @@ -244,6 +244,8 @@ const char * const vhost_msg_strings[] = {
> >>>>  	[VHOST_USER_SET_VRING_ENABLE] = "VHOST_SET_VRING_ENABLE",
> >>>>  	[VHOST_USER_GET_PROTOCOL_FEATURES] =
> >>>> "VHOST_USER_GET_PROTOCOL_FEATURES",
> >>>>  	[VHOST_USER_SET_PROTOCOL_FEATURES] =
> >>>> "VHOST_USER_SET_PROTOCOL_FEATURES",
> >>>> +	[VHOST_USER_SET_STATUS] = "VHOST_SET_STATUS",
> >>>> +	[VHOST_USER_GET_STATUS] = "VHOST_GET_STATUS",
> >>>>  };
> >>>>
> >>>>  static int
> >>>> @@ -280,6 +282,14 @@ vhost_user_sock(struct virtio_user_dev *dev,
> >>>>  		need_reply = 1;
> >>>>  		break;
> >>>>
> >>>> +	case VHOST_USER_SET_STATUS:
> >>>> +		if (!(dev->protocol_features &
> >>>> +				(1ULL <<
> >>>> VHOST_USER_PROTOCOL_F_STATUS)))
> >>>> +			return 0;
> >>>> +
> >>>> +		if (has_reply_ack)
> >>>> +			msg.flags |= VHOST_USER_NEED_REPLY_MASK;
> >>>> +		/* Fallthrough */
> >>>
> >>> There's a coding style issue here:
> >>> WARNING:PREFER_FALLTHROUGH: Prefer 'fallthrough;' over fallthrough
> >> comment.
> >>> Could you fix this?
> >>>
> >> "fallthrough" is not defined. I think this macro is defined in the
> >> linux kernel (where checkpatches.pl comes from?). We could define the
> >> same macro that would depend in compiler support for __falthrough__
> attribute.
> >>
> >> In any case, this is not the only case:
> >>
> >> $ find -name *.c | xargs grep -i "/\* fallthrough \*/" | wc -l
> >> 62
> >>
> >> So maybe this warning is new?
> >> Should I change all of them together in another patch?
> >
> > checkpatches.pl is in linux kernel. I think it prefers 'fallthrough'
> > rather than 'Fallthrough'. I think it's a linux style, right?
> >
> It refers to this:
> https://github.com/torvalds/linux/blob/91a9a90d040e8b9ff63d48ea71468e0f4
> db764ff/include/linux/compiler_attributes.h#L201
> 
> Since we don't have that pseudo-keyword, we should see if we can disable this
> in checkpatches.

Yeah, I confirmed this could be ignored. So you could change it or keep it.
Both fine.

Cheers,
Chenbo

> 
> 
> > Thanks!
> > Chenbo
> >
> >>
> >>>>  	case VHOST_USER_SET_FEATURES:
> >>>>  	case VHOST_USER_SET_PROTOCOL_FEATURES:
> >>>>  	case VHOST_USER_SET_LOG_BASE:
> >>>> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> >>>> b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> >>>> index 0a6991bcc..2c400a448 100644
> >>>> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> >>>> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> >>>> @@ -424,7 +424,8 @@ virtio_user_dev_setup(struct virtio_user_dev
> >>>> *dev)
> >>>>
> >>>>  #define VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES		\
> >>>>  	(1ULL << VHOST_USER_PROTOCOL_F_MQ |		\
> >>>> -	 1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK)
> >>>> +	 1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK |	\
> >>>> +	 1ULL << VHOST_USER_PROTOCOL_F_STATUS)
> >>>>
> >>>>  int
> >>>>  virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int
> >>>> queues, @@ -
> >>>> 783,3 +784,23 @@ virtio_user_handle_cq(struct virtio_user_dev *dev,
> >>>> uint16_t
> >>>> queue_idx)
> >>>>  		__atomic_add_fetch(&vring->used->idx, 1,
> >> __ATOMIC_RELAXED);
> >>>>  	}
> >>>>  }
> >>>> +
> >>>> +int
> >>>> +virtio_user_send_status_update(struct virtio_user_dev *dev,
> >>>> +uint8_t
> >>>> +status) {
> >>>> +	int ret;
> >>>> +	uint64_t arg = status;
> >>>> +
> >>>> +	/* Vhost-user only for now */
> >>>> +	if (!is_vhost_user_by_type(dev->path))
> >>>> +		return 0;
> >>>> +
> >>>> +	if (!(dev->protocol_features & (1ULL <<
> >>>> VHOST_USER_PROTOCOL_F_STATUS)))
> >>>> +		return 0;
> >>>> +
> >>>> +	ret = dev->ops->send_request(dev, VHOST_USER_SET_STATUS, &arg);
> >>>> +	if (ret)
> >>>> +		return -1;
> >>>
> >>> Do you think we should add a log here to show if SET_STAUTS failed?
> >>>
> >> Good idea! Will do in the next version. Thanks
> >>
> >>> Thanks!
> >>> Chenbo
> >>>
> >>>> +
> >>>> +	return 0;
> >>>> +}
> >>>> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> >>>> b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> >>>> index 10b274d19..a76d7cfaf 100644
> >>>> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> >>>> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> >>>> @@ -74,4 +74,5 @@ void virtio_user_handle_cq(struct virtio_user_dev
> >>>> *dev, uint16_t queue_idx);  void
> >>>> virtio_user_handle_cq_packed(struct
> >>>> virtio_user_dev *dev,
> >>>>  				  uint16_t queue_idx);
> >>>>  uint8_t virtio_user_handle_mq(struct virtio_user_dev *dev,
> >>>> uint16_t q_pairs);
> >>>> +int virtio_user_send_status_update(struct virtio_user_dev *dev,
> >>>> +uint8_t status);
> >>>>  #endif
> >>>> diff --git a/drivers/net/virtio/virtio_user_ethdev.c
> >>>> b/drivers/net/virtio/virtio_user_ethdev.c
> >>>> index 5b06d8e89..e52f11341 100644
> >>>> --- a/drivers/net/virtio/virtio_user_ethdev.c
> >>>> +++ b/drivers/net/virtio/virtio_user_ethdev.c
> >>>> @@ -273,6 +273,7 @@ virtio_user_set_status(struct virtio_hw *hw,
> >>>> uint8_t
> >>>> status)
> >>>>  	else if (status == VIRTIO_CONFIG_STATUS_RESET)
> >>>>  		virtio_user_reset(hw);
> >>>>  	dev->status = status;
> >>>> +	virtio_user_send_status_update(dev, status);
> >>>>  }
> >>>>
> >>>>  static uint8_t
> >>>> --
> >>>> 2.26.2
> >>>
> >>
> >> --
> >> Adrián Moreno
> >
> 
> --
> Adrián Moreno
diff mbox series

Patch

diff --git a/drivers/net/virtio/virtio_user/vhost.h b/drivers/net/virtio/virtio_user/vhost.h
index 260e1c308..8f49ef457 100644
--- a/drivers/net/virtio/virtio_user/vhost.h
+++ b/drivers/net/virtio/virtio_user/vhost.h
@@ -57,6 +57,10 @@  struct vhost_vring_addr {
 #define VHOST_USER_PROTOCOL_F_REPLY_ACK 3
 #endif
 
+#ifndef VHOST_USER_PROTOCOL_F_STATUS
+#define VHOST_USER_PROTOCOL_F_STATUS 16
+#endif
+
 enum vhost_user_request {
 	VHOST_USER_NONE = 0,
 	VHOST_USER_GET_FEATURES = 1,
@@ -77,6 +81,8 @@  enum vhost_user_request {
 	VHOST_USER_SET_PROTOCOL_FEATURES = 16,
 	VHOST_USER_GET_QUEUE_NUM = 17,
 	VHOST_USER_SET_VRING_ENABLE = 18,
+	VHOST_USER_SET_STATUS = 39,
+	VHOST_USER_GET_STATUS = 40,
 	VHOST_USER_MAX
 };
 
diff --git a/drivers/net/virtio/virtio_user/vhost_user.c b/drivers/net/virtio/virtio_user/vhost_user.c
index 631d0e353..2332e01d1 100644
--- a/drivers/net/virtio/virtio_user/vhost_user.c
+++ b/drivers/net/virtio/virtio_user/vhost_user.c
@@ -244,6 +244,8 @@  const char * const vhost_msg_strings[] = {
 	[VHOST_USER_SET_VRING_ENABLE] = "VHOST_SET_VRING_ENABLE",
 	[VHOST_USER_GET_PROTOCOL_FEATURES] = "VHOST_USER_GET_PROTOCOL_FEATURES",
 	[VHOST_USER_SET_PROTOCOL_FEATURES] = "VHOST_USER_SET_PROTOCOL_FEATURES",
+	[VHOST_USER_SET_STATUS] = "VHOST_SET_STATUS",
+	[VHOST_USER_GET_STATUS] = "VHOST_GET_STATUS",
 };
 
 static int
@@ -280,6 +282,14 @@  vhost_user_sock(struct virtio_user_dev *dev,
 		need_reply = 1;
 		break;
 
+	case VHOST_USER_SET_STATUS:
+		if (!(dev->protocol_features &
+				(1ULL << VHOST_USER_PROTOCOL_F_STATUS)))
+			return 0;
+
+		if (has_reply_ack)
+			msg.flags |= VHOST_USER_NEED_REPLY_MASK;
+		/* Fallthrough */
 	case VHOST_USER_SET_FEATURES:
 	case VHOST_USER_SET_PROTOCOL_FEATURES:
 	case VHOST_USER_SET_LOG_BASE:
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 0a6991bcc..2c400a448 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -424,7 +424,8 @@  virtio_user_dev_setup(struct virtio_user_dev *dev)
 
 #define VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES		\
 	(1ULL << VHOST_USER_PROTOCOL_F_MQ |		\
-	 1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK)
+	 1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK |	\
+	 1ULL << VHOST_USER_PROTOCOL_F_STATUS)
 
 int
 virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
@@ -783,3 +784,23 @@  virtio_user_handle_cq(struct virtio_user_dev *dev, uint16_t queue_idx)
 		__atomic_add_fetch(&vring->used->idx, 1, __ATOMIC_RELAXED);
 	}
 }
+
+int
+virtio_user_send_status_update(struct virtio_user_dev *dev, uint8_t status)
+{
+	int ret;
+	uint64_t arg = status;
+
+	/* Vhost-user only for now */
+	if (!is_vhost_user_by_type(dev->path))
+		return 0;
+
+	if (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_STATUS)))
+		return 0;
+
+	ret = dev->ops->send_request(dev, VHOST_USER_SET_STATUS, &arg);
+	if (ret)
+		return -1;
+
+	return 0;
+}
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h
index 10b274d19..a76d7cfaf 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
@@ -74,4 +74,5 @@  void virtio_user_handle_cq(struct virtio_user_dev *dev, uint16_t queue_idx);
 void virtio_user_handle_cq_packed(struct virtio_user_dev *dev,
 				  uint16_t queue_idx);
 uint8_t virtio_user_handle_mq(struct virtio_user_dev *dev, uint16_t q_pairs);
+int virtio_user_send_status_update(struct virtio_user_dev *dev, uint8_t status);
 #endif
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 5b06d8e89..e52f11341 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -273,6 +273,7 @@  virtio_user_set_status(struct virtio_hw *hw, uint8_t status)
 	else if (status == VIRTIO_CONFIG_STATUS_RESET)
 		virtio_user_reset(hw);
 	dev->status = status;
+	virtio_user_send_status_update(dev, status);
 }
 
 static uint8_t