diff mbox series

vhost: log Virtio and Vhost-user negotiated features

Message ID 20190620200712.9187-1-maxime.coquelin@redhat.com (mailing list archive)
State Accepted, archived
Delegated to: Maxime Coquelin
Headers
Series vhost: log Virtio and Vhost-user negotiated features |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/intel-Performance-Testing success Performance Testing PASS
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/Intel-compilation fail Compilation issues

Commit Message

Maxime Coquelin June 20, 2019, 8:07 p.m. UTC 
  Having this info logged by default when analysing bug reports
has proved to be useful.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/librte_vhost/vhost_user.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Tiwei Bie June 21, 2019, 4:51 a.m. UTC | #1 
On Thu, Jun 20, 2019 at 10:07:12PM +0200, Maxime Coquelin wrote:
> Having this info logged by default when analysing bug reports
> has proved to be useful.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  lib/librte_vhost/vhost_user.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
> index c9e29ece8..370864865 100644
> --- a/lib/librte_vhost/vhost_user.c
> +++ b/lib/librte_vhost/vhost_user.c
> @@ -284,6 +284,8 @@ vhost_user_set_features(struct virtio_net **pdev, struct VhostUserMsg *msg,
>  	} else {
>  		dev->vhost_hlen = sizeof(struct virtio_net_hdr);
>  	}
> +	RTE_LOG(INFO, VHOST_CONFIG,
> +		"Negotiated Virtio features: 0x%" PRIx64 "\n", dev->features);

Might be better to use lowercase for "Negotiated", and we will
get more consistent messages like this:

VHOST_CONFIG: new vhost user connection is 278
VHOST_CONFIG: new device, handle is 0
VHOST_CONFIG: read message VHOST_USER_SET_OWNER
VHOST_CONFIG: read message VHOST_USER_GET_FEATURES
VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
VHOST_CONFIG: vring call idx:0 file:279
VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
VHOST_CONFIG: vring call idx:1 file:280
VHOST_CONFIG: read message VHOST_USER_SET_FEATURES
VHOST_CONFIG: negotiated Virtio features: 0xd10008000
VHOST_CONFIG: read message VHOST_USER_SET_MEM_TABLE
VHOST_CONFIG: guest memory region 0, size: 0x20000000
......

instead of:

VHOST_CONFIG: new vhost user connection is 278
VHOST_CONFIG: new device, handle is 0
VHOST_CONFIG: read message VHOST_USER_SET_OWNER
VHOST_CONFIG: read message VHOST_USER_GET_FEATURES
VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
VHOST_CONFIG: vring call idx:0 file:279
VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
VHOST_CONFIG: vring call idx:1 file:280
VHOST_CONFIG: read message VHOST_USER_SET_FEATURES
VHOST_CONFIG: Negotiated Virtio features: 0xd10008000
VHOST_CONFIG: read message VHOST_USER_SET_MEM_TABLE
VHOST_CONFIG: guest memory region 0, size: 0x20000000
......

>  	VHOST_LOG_DEBUG(VHOST_CONFIG,
>  		"(%d) mergeable RX buffers %s, virtio 1 %s\n",
>  		dev->vid,
> @@ -1406,6 +1408,9 @@ vhost_user_set_protocol_features(struct virtio_net **pdev,
>  	}
>  
>  	dev->protocol_features = protocol_features;
> +	RTE_LOG(INFO, VHOST_CONFIG,
> +		"Negotiated Vhost-user protocol features: 0x%" PRIx64 "\n",
> +		dev->protocol_features);

Ditto.

Other than that,
Reviewed-by: Tiwei Bie <tiwei.bie@intel.com>

Thanks,
Tiwei

>  
>  	return RTE_VHOST_MSG_RESULT_OK;
>  }
> -- 
> 2.21.0
>
Maxime Coquelin June 21, 2019, 7:30 a.m. UTC | #2 
On 6/21/19 6:51 AM, Tiwei Bie wrote:
> On Thu, Jun 20, 2019 at 10:07:12PM +0200, Maxime Coquelin wrote:
>> Having this info logged by default when analysing bug reports
>> has proved to be useful.
>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> ---
>>   lib/librte_vhost/vhost_user.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
>> index c9e29ece8..370864865 100644
>> --- a/lib/librte_vhost/vhost_user.c
>> +++ b/lib/librte_vhost/vhost_user.c
>> @@ -284,6 +284,8 @@ vhost_user_set_features(struct virtio_net **pdev, struct VhostUserMsg *msg,
>>   	} else {
>>   		dev->vhost_hlen = sizeof(struct virtio_net_hdr);
>>   	}
>> +	RTE_LOG(INFO, VHOST_CONFIG,
>> +		"Negotiated Virtio features: 0x%" PRIx64 "\n", dev->features);
> 
> Might be better to use lowercase for "Negotiated", and we will
> get more consistent messages like this:
> 
> VHOST_CONFIG: new vhost user connection is 278
> VHOST_CONFIG: new device, handle is 0
> VHOST_CONFIG: read message VHOST_USER_SET_OWNER
> VHOST_CONFIG: read message VHOST_USER_GET_FEATURES
> VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
> VHOST_CONFIG: vring call idx:0 file:279
> VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
> VHOST_CONFIG: vring call idx:1 file:280
> VHOST_CONFIG: read message VHOST_USER_SET_FEATURES
> VHOST_CONFIG: negotiated Virtio features: 0xd10008000
> VHOST_CONFIG: read message VHOST_USER_SET_MEM_TABLE
> VHOST_CONFIG: guest memory region 0, size: 0x20000000
> ......
> 
> instead of:
> 
> VHOST_CONFIG: new vhost user connection is 278
> VHOST_CONFIG: new device, handle is 0
> VHOST_CONFIG: read message VHOST_USER_SET_OWNER
> VHOST_CONFIG: read message VHOST_USER_GET_FEATURES
> VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
> VHOST_CONFIG: vring call idx:0 file:279
> VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
> VHOST_CONFIG: vring call idx:1 file:280
> VHOST_CONFIG: read message VHOST_USER_SET_FEATURES
> VHOST_CONFIG: Negotiated Virtio features: 0xd10008000
> VHOST_CONFIG: read message VHOST_USER_SET_MEM_TABLE
> VHOST_CONFIG: guest memory region 0, size: 0x20000000
> ......
> 

Right, I'll fix it while applying.

>>   	VHOST_LOG_DEBUG(VHOST_CONFIG,
>>   		"(%d) mergeable RX buffers %s, virtio 1 %s\n",
>>   		dev->vid,
>> @@ -1406,6 +1408,9 @@ vhost_user_set_protocol_features(struct virtio_net **pdev,
>>   	}
>>   
>>   	dev->protocol_features = protocol_features;
>> +	RTE_LOG(INFO, VHOST_CONFIG,
>> +		"Negotiated Vhost-user protocol features: 0x%" PRIx64 "\n",
>> +		dev->protocol_features);
> 
> Ditto.
> 
> Other than that,
> Reviewed-by: Tiwei Bie <tiwei.bie@intel.com>


Thanks,
Maxime

> Thanks,
> Tiwei
> 
>>   
>>   	return RTE_VHOST_MSG_RESULT_OK;
>>   }
>> -- 
>> 2.21.0
>>
Tiwei Bie July 5, 2019, 9:01 a.m. UTC | #3 
On Thu, Jun 20, 2019 at 10:07:12PM +0200, Maxime Coquelin wrote:
> Having this info logged by default when analysing bug reports
> has proved to be useful.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  lib/librte_vhost/vhost_user.c | 5 +++++
>  1 file changed, 5 insertions(+)

Applied to dpdk-next-virtio/master with the s/Negotiated/negotiated/
change in the log message.

Thanks!
Tiwei
diff mbox series

Patch

diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index c9e29ece8..370864865 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -284,6 +284,8 @@  vhost_user_set_features(struct virtio_net **pdev, struct VhostUserMsg *msg,
 	} else {
 		dev->vhost_hlen = sizeof(struct virtio_net_hdr);
 	}
+	RTE_LOG(INFO, VHOST_CONFIG,
+		"Negotiated Virtio features: 0x%" PRIx64 "\n", dev->features);
 	VHOST_LOG_DEBUG(VHOST_CONFIG,
 		"(%d) mergeable RX buffers %s, virtio 1 %s\n",
 		dev->vid,
@@ -1406,6 +1408,9 @@  vhost_user_set_protocol_features(struct virtio_net **pdev,
 	}
 
 	dev->protocol_features = protocol_features;
+	RTE_LOG(INFO, VHOST_CONFIG,
+		"Negotiated Vhost-user protocol features: 0x%" PRIx64 "\n",
+		dev->protocol_features);
 
 	return RTE_VHOST_MSG_RESULT_OK;
 }