DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] virtio-user: fix backend selection if stat fails
@ 2020-10-20  7:16 Adrian Moreno
  2020-10-20  8:43 ` Jiang, YuX
  2020-10-20  9:01 ` Kevin Traynor
  0 siblings, 2 replies; 7+ messages in thread
From: Adrian Moreno @ 2020-10-20  7:16 UTC (permalink / raw)
  To: dev
  Cc: yinan.wang, patrick.fu, amorenoz, stable, Maxime Coquelin,
	Chenbo Xia, Zhihong Wang

If stat fails it means the backend must be vhost-user in server mode

Bugzilla ID: 559
Fixes: f908b22ea47a ("net/virtio: move backend type selection to ethdev")
Cc: stable@dpdk.org

Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
---
 drivers/net/virtio/virtio_user_ethdev.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 042665bc0..ce74d08ab 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -560,9 +560,10 @@ virtio_user_backend_type(const char *path)
 	struct stat sb;
 
 	if (stat(path, &sb) == -1) {
-		PMD_INIT_LOG(ERR, "Stat fails: %s (%s)\n", path,
+		PMD_INIT_LOG(INFO, "Stat fails: %s (%s)\n", path,
 			     strerror(errno));
-		return VIRTIO_USER_BACKEND_UNKNOWN;
+		/* Must be vhost-user in server mode */
+		return VIRTIO_USER_BACKEND_VHOST_USER;
 	}
 
 	if (S_ISSOCK(sb.st_mode)) {
-- 
2.26.2


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

* Re: [dpdk-dev] [PATCH] virtio-user: fix backend selection if stat fails
  2020-10-20  7:16 [dpdk-dev] [PATCH] virtio-user: fix backend selection if stat fails Adrian Moreno
@ 2020-10-20  8:43 ` Jiang, YuX
  2020-10-20  9:01 ` Kevin Traynor
  1 sibling, 0 replies; 7+ messages in thread
From: Jiang, YuX @ 2020-10-20  8:43 UTC (permalink / raw)
  To: Adrian Moreno, dev
  Cc: Wang, Yinan, Fu, Patrick, stable, Maxime Coquelin, Xia, Chenbo,
	Wang, Zhihong, Jiang, YuX, Zhang, XiX

Tested-by: JiangYuX <yux.jiang@intel.com>

    Best Regards
    Jiang yu


> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Adrian Moreno
> Sent: Tuesday, October 20, 2020 3:16 PM
> To: dev@dpdk.org
> Cc: Wang, Yinan <yinan.wang@intel.com>; Fu, Patrick
> <patrick.fu@intel.com>; amorenoz@redhat.com; stable@dpdk.org; Maxime
> Coquelin <maxime.coquelin@redhat.com>; Xia, Chenbo
> <chenbo.xia@intel.com>; Wang, Zhihong <zhihong.wang@intel.com>
> Subject: [dpdk-dev] [PATCH] virtio-user: fix backend selection if stat fails
> 
> If stat fails it means the backend must be vhost-user in server mode
> 
> Bugzilla ID: 559
> Fixes: f908b22ea47a ("net/virtio: move backend type selection to ethdev")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
> ---
>  drivers/net/virtio/virtio_user_ethdev.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_user_ethdev.c
> b/drivers/net/virtio/virtio_user_ethdev.c
> index 042665bc0..ce74d08ab 100644
> --- a/drivers/net/virtio/virtio_user_ethdev.c
> +++ b/drivers/net/virtio/virtio_user_ethdev.c
> @@ -560,9 +560,10 @@ virtio_user_backend_type(const char *path)
>  	struct stat sb;
> 
>  	if (stat(path, &sb) == -1) {
> -		PMD_INIT_LOG(ERR, "Stat fails: %s (%s)\n", path,
> +		PMD_INIT_LOG(INFO, "Stat fails: %s (%s)\n", path,
>  			     strerror(errno));
> -		return VIRTIO_USER_BACKEND_UNKNOWN;
> +		/* Must be vhost-user in server mode */
> +		return VIRTIO_USER_BACKEND_VHOST_USER;
>  	}
> 
>  	if (S_ISSOCK(sb.st_mode)) {
> --
> 2.26.2


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

* Re: [dpdk-dev] [PATCH] virtio-user: fix backend selection if stat fails
  2020-10-20  7:16 [dpdk-dev] [PATCH] virtio-user: fix backend selection if stat fails Adrian Moreno
  2020-10-20  8:43 ` Jiang, YuX
@ 2020-10-20  9:01 ` Kevin Traynor
  2020-10-20  9:11   ` Maxime Coquelin
  1 sibling, 1 reply; 7+ messages in thread
From: Kevin Traynor @ 2020-10-20  9:01 UTC (permalink / raw)
  To: Adrian Moreno, dev
  Cc: yinan.wang, patrick.fu, stable, Maxime Coquelin, Chenbo Xia,
	Zhihong Wang

On 20/10/2020 08:16, Adrian Moreno wrote:
> If stat fails it means the backend must be vhost-user in server mode
> 
> Bugzilla ID: 559
> Fixes: f908b22ea47a ("net/virtio: move backend type selection to ethdev")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
> ---
>  drivers/net/virtio/virtio_user_ethdev.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
> index 042665bc0..ce74d08ab 100644
> --- a/drivers/net/virtio/virtio_user_ethdev.c
> +++ b/drivers/net/virtio/virtio_user_ethdev.c
> @@ -560,9 +560,10 @@ virtio_user_backend_type(const char *path)
>  	struct stat sb;
>  
>  	if (stat(path, &sb) == -1) {
> -		PMD_INIT_LOG(ERR, "Stat fails: %s (%s)\n", path,
> +		PMD_INIT_LOG(INFO, "Stat fails: %s (%s)\n", path,
>  			     strerror(errno));

It may be accurate, but a 'fail' in the logs can be confusing for users
when it is an INFO log and normal operation. Suggest to reword to
something softer like 'Unable to stat' or 'Not able to get file status'

> -		return VIRTIO_USER_BACKEND_UNKNOWN;
> +		/* Must be vhost-user in server mode */
> +		return VIRTIO_USER_BACKEND_VHOST_USER;
>  	}
>  
>  	if (S_ISSOCK(sb.st_mode)) {
> 


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

* Re: [dpdk-dev] [PATCH] virtio-user: fix backend selection if stat fails
  2020-10-20  9:01 ` Kevin Traynor
@ 2020-10-20  9:11   ` Maxime Coquelin
  2020-10-20  9:38     ` Kevin Traynor
  0 siblings, 1 reply; 7+ messages in thread
From: Maxime Coquelin @ 2020-10-20  9:11 UTC (permalink / raw)
  To: Kevin Traynor, Adrian Moreno, dev
  Cc: yinan.wang, patrick.fu, stable, Chenbo Xia, Zhihong Wang



On 10/20/20 11:01 AM, Kevin Traynor wrote:
> On 20/10/2020 08:16, Adrian Moreno wrote:
>> If stat fails it means the backend must be vhost-user in server mode
>>
>> Bugzilla ID: 559
>> Fixes: f908b22ea47a ("net/virtio: move backend type selection to ethdev")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
>> ---
>>  drivers/net/virtio/virtio_user_ethdev.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
>> index 042665bc0..ce74d08ab 100644
>> --- a/drivers/net/virtio/virtio_user_ethdev.c
>> +++ b/drivers/net/virtio/virtio_user_ethdev.c
>> @@ -560,9 +560,10 @@ virtio_user_backend_type(const char *path)
>>  	struct stat sb;
>>  
>>  	if (stat(path, &sb) == -1) {
>> -		PMD_INIT_LOG(ERR, "Stat fails: %s (%s)\n", path,
>> +		PMD_INIT_LOG(INFO, "Stat fails: %s (%s)\n", path,
>>  			     strerror(errno));
> 
> It may be accurate, but a 'fail' in the logs can be confusing for users
> when it is an INFO log and normal operation. Suggest to reword to
> something softer like 'Unable to stat' or 'Not able to get file status'


We may want to:
- only return VIRTIO_USER_BACKEND_VHOST_USER if -ENOENT, and log that
we assume this is Vhost-user backend in server mode at INFO level.
- return VIRTIO_USER_BACKEND_UNKNOWN otherwise and print an error
message with the strerror(errno).

What do you think?

>> -		return VIRTIO_USER_BACKEND_UNKNOWN;
>> +		/* Must be vhost-user in server mode */
>> +		return VIRTIO_USER_BACKEND_VHOST_USER;
>>  	}
>>  
>>  	if (S_ISSOCK(sb.st_mode)) {
>>
> 


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

* Re: [dpdk-dev] [PATCH] virtio-user: fix backend selection if stat fails
  2020-10-20  9:11   ` Maxime Coquelin
@ 2020-10-20  9:38     ` Kevin Traynor
  2020-10-20  9:55       ` Maxime Coquelin
  0 siblings, 1 reply; 7+ messages in thread
From: Kevin Traynor @ 2020-10-20  9:38 UTC (permalink / raw)
  To: Maxime Coquelin, Adrian Moreno, dev
  Cc: yinan.wang, patrick.fu, stable, Chenbo Xia, Zhihong Wang

On 20/10/2020 10:11, Maxime Coquelin wrote:
> 
> 
> On 10/20/20 11:01 AM, Kevin Traynor wrote:
>> On 20/10/2020 08:16, Adrian Moreno wrote:
>>> If stat fails it means the backend must be vhost-user in server mode
>>>
>>> Bugzilla ID: 559
>>> Fixes: f908b22ea47a ("net/virtio: move backend type selection to ethdev")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
>>> ---
>>>  drivers/net/virtio/virtio_user_ethdev.c | 5 +++--
>>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
>>> index 042665bc0..ce74d08ab 100644
>>> --- a/drivers/net/virtio/virtio_user_ethdev.c
>>> +++ b/drivers/net/virtio/virtio_user_ethdev.c
>>> @@ -560,9 +560,10 @@ virtio_user_backend_type(const char *path)
>>>  	struct stat sb;
>>>  
>>>  	if (stat(path, &sb) == -1) {
>>> -		PMD_INIT_LOG(ERR, "Stat fails: %s (%s)\n", path,
>>> +		PMD_INIT_LOG(INFO, "Stat fails: %s (%s)\n", path,
>>>  			     strerror(errno));
>>
>> It may be accurate, but a 'fail' in the logs can be confusing for users
>> when it is an INFO log and normal operation. Suggest to reword to
>> something softer like 'Unable to stat' or 'Not able to get file status'
> 
> 
> We may want to:
> - only return VIRTIO_USER_BACKEND_VHOST_USER if -ENOENT, and log that
> we assume this is Vhost-user backend in server mode at INFO level.

It will mean that sometimes the backend type is logged and sometimes
not, but maybe you make a distinction because there is an assumption
being made in this case?

> - return VIRTIO_USER_BACKEND_UNKNOWN otherwise and print an error
> message with the strerror(errno).
> 

yes, it seems better like that.

> What do you think?
> 
>>> -		return VIRTIO_USER_BACKEND_UNKNOWN;
>>> +		/* Must be vhost-user in server mode */
>>> +		return VIRTIO_USER_BACKEND_VHOST_USER;
>>>  	}
>>>  
>>>  	if (S_ISSOCK(sb.st_mode)) {
>>>
>>


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

* Re: [dpdk-dev] [PATCH] virtio-user: fix backend selection if stat fails
  2020-10-20  9:38     ` Kevin Traynor
@ 2020-10-20  9:55       ` Maxime Coquelin
  2020-10-20 10:33         ` Adrian Moreno
  0 siblings, 1 reply; 7+ messages in thread
From: Maxime Coquelin @ 2020-10-20  9:55 UTC (permalink / raw)
  To: Kevin Traynor, Adrian Moreno, dev
  Cc: yinan.wang, patrick.fu, stable, Chenbo Xia, Zhihong Wang



On 10/20/20 11:38 AM, Kevin Traynor wrote:
> On 20/10/2020 10:11, Maxime Coquelin wrote:
>>
>>
>> On 10/20/20 11:01 AM, Kevin Traynor wrote:
>>> On 20/10/2020 08:16, Adrian Moreno wrote:
>>>> If stat fails it means the backend must be vhost-user in server mode
>>>>
>>>> Bugzilla ID: 559
>>>> Fixes: f908b22ea47a ("net/virtio: move backend type selection to ethdev")
>>>> Cc: stable@dpdk.org
>>>>
>>>> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
>>>> ---
>>>>  drivers/net/virtio/virtio_user_ethdev.c | 5 +++--
>>>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
>>>> index 042665bc0..ce74d08ab 100644
>>>> --- a/drivers/net/virtio/virtio_user_ethdev.c
>>>> +++ b/drivers/net/virtio/virtio_user_ethdev.c
>>>> @@ -560,9 +560,10 @@ virtio_user_backend_type(const char *path)
>>>>  	struct stat sb;
>>>>  
>>>>  	if (stat(path, &sb) == -1) {
>>>> -		PMD_INIT_LOG(ERR, "Stat fails: %s (%s)\n", path,
>>>> +		PMD_INIT_LOG(INFO, "Stat fails: %s (%s)\n", path,
>>>>  			     strerror(errno));
>>>
>>> It may be accurate, but a 'fail' in the logs can be confusing for users
>>> when it is an INFO log and normal operation. Suggest to reword to
>>> something softer like 'Unable to stat' or 'Not able to get file status'
>>
>>
>> We may want to:
>> - only return VIRTIO_USER_BACKEND_VHOST_USER if -ENOENT, and log that
>> we assume this is Vhost-user backend in server mode at INFO level.
> 
> It will mean that sometimes the backend type is logged and sometimes
> not, but maybe you make a distinction because there is an assumption
> being made in this case?

I agree it would  make sense to log at INFO level for all backend types.

>> - return VIRTIO_USER_BACKEND_UNKNOWN otherwise and print an error
>> message with the strerror(errno).
>>
> 
> yes, it seems better like that.
> 
>> What do you think?
>>
>>>> -		return VIRTIO_USER_BACKEND_UNKNOWN;
>>>> +		/* Must be vhost-user in server mode */
>>>> +		return VIRTIO_USER_BACKEND_VHOST_USER;
>>>>  	}
>>>>  
>>>>  	if (S_ISSOCK(sb.st_mode)) {
>>>>
>>>
> 


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

* Re: [dpdk-dev] [PATCH] virtio-user: fix backend selection if stat fails
  2020-10-20  9:55       ` Maxime Coquelin
@ 2020-10-20 10:33         ` Adrian Moreno
  0 siblings, 0 replies; 7+ messages in thread
From: Adrian Moreno @ 2020-10-20 10:33 UTC (permalink / raw)
  To: Maxime Coquelin, Kevin Traynor, dev
  Cc: yinan.wang, patrick.fu, stable, Chenbo Xia, Zhihong Wang



On 10/20/20 11:55 AM, Maxime Coquelin wrote:
> 
> 
> On 10/20/20 11:38 AM, Kevin Traynor wrote:
>> On 20/10/2020 10:11, Maxime Coquelin wrote:
>>>
>>>
>>> On 10/20/20 11:01 AM, Kevin Traynor wrote:
>>>> On 20/10/2020 08:16, Adrian Moreno wrote:
>>>>> If stat fails it means the backend must be vhost-user in server mode
>>>>>
>>>>> Bugzilla ID: 559
>>>>> Fixes: f908b22ea47a ("net/virtio: move backend type selection to ethdev")
>>>>> Cc: stable@dpdk.org
>>>>>
>>>>> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
>>>>> ---
>>>>>  drivers/net/virtio/virtio_user_ethdev.c | 5 +++--
>>>>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
>>>>> index 042665bc0..ce74d08ab 100644
>>>>> --- a/drivers/net/virtio/virtio_user_ethdev.c
>>>>> +++ b/drivers/net/virtio/virtio_user_ethdev.c
>>>>> @@ -560,9 +560,10 @@ virtio_user_backend_type(const char *path)
>>>>>  	struct stat sb;
>>>>>  
>>>>>  	if (stat(path, &sb) == -1) {
>>>>> -		PMD_INIT_LOG(ERR, "Stat fails: %s (%s)\n", path,
>>>>> +		PMD_INIT_LOG(INFO, "Stat fails: %s (%s)\n", path,
>>>>>  			     strerror(errno));
>>>>
>>>> It may be accurate, but a 'fail' in the logs can be confusing for users
>>>> when it is an INFO log and normal operation. Suggest to reword to
>>>> something softer like 'Unable to stat' or 'Not able to get file status'
>>>
>>>
>>> We may want to:
>>> - only return VIRTIO_USER_BACKEND_VHOST_USER if -ENOENT, and log that
>>> we assume this is Vhost-user backend in server mode at INFO level.
>>
>> It will mean that sometimes the backend type is logged and sometimes
>> not, but maybe you make a distinction because there is an assumption
>> being made in this case?
> 
> I agree it would  make sense to log at INFO level for all backend types.
> 
>>> - return VIRTIO_USER_BACKEND_UNKNOWN otherwise and print an error
>>> message with the strerror(errno).
>>>
>>
>> yes, it seems better like that.
>>
Thanks Maxime and Kevin for your feedback and Jiang for testing. I'll send a v2
addressing the comments.


>>> What do you think?
>>>
>>>>> -		return VIRTIO_USER_BACKEND_UNKNOWN;
>>>>> +		/* Must be vhost-user in server mode */
>>>>> +		return VIRTIO_USER_BACKEND_VHOST_USER;
>>>>>  	}
>>>>>  
>>>>>  	if (S_ISSOCK(sb.st_mode)) {
>>>>>
>>>>
>>
> 

-- 
Adrián Moreno


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

end of thread, other threads:[~2020-10-20 10:33 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-20  7:16 [dpdk-dev] [PATCH] virtio-user: fix backend selection if stat fails Adrian Moreno
2020-10-20  8:43 ` Jiang, YuX
2020-10-20  9:01 ` Kevin Traynor
2020-10-20  9:11   ` Maxime Coquelin
2020-10-20  9:38     ` Kevin Traynor
2020-10-20  9:55       ` Maxime Coquelin
2020-10-20 10:33         ` Adrian Moreno

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ http://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git