vhost: zero_copy incompatible with client mode

Message ID 20200429025946.48096-1-xuan.ding@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Maxime Coquelin
Headers
Series vhost: zero_copy incompatible with client mode |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-nxp-Performance success Performance Testing PASS
ci/travis-robot warning Travis build: failed
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-testing success Testing PASS
ci/Intel-compilation fail Compilation issues

Commit Message

Ding, Xuan April 29, 2020, 2:59 a.m. UTC
  In server mode, virtio-user inits under the assumption that vhost-user
supports a list of features. However, this could be problematic when
in_order feature is negotiated but not supported by vhost-user when
enables dequeue_zero_copy later.

Add handling when vhost-user enables dequeue_zero_copy as client.

Signed-off-by: Xuan Ding <xuan.ding@intel.com>
---
 lib/librte_vhost/socket.c | 6 ++++++
 1 file changed, 6 insertions(+)
  

Comments

Wang, Yinan April 29, 2020, 8:33 a.m. UTC | #1
Tested-by: Yinan Wang <yinan.wang@intel.com>

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Xuan Ding
> Sent: 2020年4月29日 11:00
> To: maxime.coquelin@redhat.com; Wang, Zhihong <zhihong.wang@intel.com>;
> Ye, Xiaolong <xiaolong.ye@intel.com>
> Cc: dev@dpdk.org; Ding, Xuan <xuan.ding@intel.com>
> Subject: [dpdk-dev] [PATCH] vhost: zero_copy incompatible with client mode
> 
> In server mode, virtio-user inits under the assumption that vhost-user supports a
> list of features. However, this could be problematic when in_order feature is
> negotiated but not supported by vhost-user when enables dequeue_zero_copy
> later.
> 
> Add handling when vhost-user enables dequeue_zero_copy as client.
> 
> Signed-off-by: Xuan Ding <xuan.ding@intel.com>
> ---
>  lib/librte_vhost/socket.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c index
> 7c8012179..bb8d0d780 100644
> --- a/lib/librte_vhost/socket.c
> +++ b/lib/librte_vhost/socket.c
> @@ -926,6 +926,12 @@ rte_vhost_driver_register(const char *path, uint64_t
> flags)
>  			ret = -1;
>  			goto out_mutex;
>  		}
> +		if (!vsocket->is_server) {
> +			VHOST_LOG_CONFIG(ERR,
> +			"error: zero copy is incompatible with vhost client
> mode\n");
> +			ret = -1;
> +			goto out_mutex;
> +		}
>  		vsocket->supported_features &= ~(1ULL <<
> VIRTIO_F_IN_ORDER);
>  		vsocket->features &= ~(1ULL << VIRTIO_F_IN_ORDER);
> 
> --
> 2.17.1
  
Maxime Coquelin April 29, 2020, 8:46 a.m. UTC | #2
Hi Yinan,

On 4/29/20 10:33 AM, Wang, Yinan wrote:
> Tested-by: Yinan Wang <yinan.wang@intel.com>

Thanks for running tests on Vhost patches, that's much appreciated.

Could you provide more information on the test run?
Does it consist in reproducing the issue the patch is aiming to fix,
apply patch and check issue is fixed?
Or does it consist in running a generic Vhost/Virtio test suite?

Regards,
Maxime

>> -----Original Message-----
>> From: dev <dev-bounces@dpdk.org> On Behalf Of Xuan Ding
>> Sent: 2020年4月29日 11:00
>> To: maxime.coquelin@redhat.com; Wang, Zhihong <zhihong.wang@intel.com>;
>> Ye, Xiaolong <xiaolong.ye@intel.com>
>> Cc: dev@dpdk.org; Ding, Xuan <xuan.ding@intel.com>
>> Subject: [dpdk-dev] [PATCH] vhost: zero_copy incompatible with client mode
>>
>> In server mode, virtio-user inits under the assumption that vhost-user supports a
>> list of features. However, this could be problematic when in_order feature is
>> negotiated but not supported by vhost-user when enables dequeue_zero_copy
>> later.
>>
>> Add handling when vhost-user enables dequeue_zero_copy as client.
>>
>> Signed-off-by: Xuan Ding <xuan.ding@intel.com>
>> ---
>>  lib/librte_vhost/socket.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c index
>> 7c8012179..bb8d0d780 100644
>> --- a/lib/librte_vhost/socket.c
>> +++ b/lib/librte_vhost/socket.c
>> @@ -926,6 +926,12 @@ rte_vhost_driver_register(const char *path, uint64_t
>> flags)
>>  			ret = -1;
>>  			goto out_mutex;
>>  		}
>> +		if (!vsocket->is_server) {
>> +			VHOST_LOG_CONFIG(ERR,
>> +			"error: zero copy is incompatible with vhost client
>> mode\n");
>> +			ret = -1;
>> +			goto out_mutex;
>> +		}
>>  		vsocket->supported_features &= ~(1ULL <<
>> VIRTIO_F_IN_ORDER);
>>  		vsocket->features &= ~(1ULL << VIRTIO_F_IN_ORDER);
>>
>> --
>> 2.17.1
>
  
Maxime Coquelin April 29, 2020, 11:48 a.m. UTC | #3
On 4/29/20 4:59 AM, Xuan Ding wrote:
> In server mode, virtio-user inits under the assumption that vhost-user
> supports a list of features. However, this could be problematic when
> in_order feature is negotiated but not supported by vhost-user when
> enables dequeue_zero_copy later.
> 
> Add handling when vhost-user enables dequeue_zero_copy as client.
> 
> Signed-off-by: Xuan Ding <xuan.ding@intel.com>
> ---
>  lib/librte_vhost/socket.c | 6 ++++++
>  1 file changed, 6 insertions(+)

Reiewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime
  
Maxime Coquelin April 30, 2020, 8:13 a.m. UTC | #4
On 4/29/20 4:59 AM, Xuan Ding wrote:
> In server mode, virtio-user inits under the assumption that vhost-user
> supports a list of features. However, this could be problematic when
> in_order feature is negotiated but not supported by vhost-user when
> enables dequeue_zero_copy later.
> 
> Add handling when vhost-user enables dequeue_zero_copy as client.
> 
> Signed-off-by: Xuan Ding <xuan.ding@intel.com>
> ---
>  lib/librte_vhost/socket.c | 6 ++++++
>  1 file changed, 6 insertions(+)

Applied to dpdk-next-virtio/master.

Thanks,
Maxime
  

Patch

diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
index 7c8012179..bb8d0d780 100644
--- a/lib/librte_vhost/socket.c
+++ b/lib/librte_vhost/socket.c
@@ -926,6 +926,12 @@  rte_vhost_driver_register(const char *path, uint64_t flags)
 			ret = -1;
 			goto out_mutex;
 		}
+		if (!vsocket->is_server) {
+			VHOST_LOG_CONFIG(ERR,
+			"error: zero copy is incompatible with vhost client mode\n");
+			ret = -1;
+			goto out_mutex;
+		}
 		vsocket->supported_features &= ~(1ULL << VIRTIO_F_IN_ORDER);
 		vsocket->features &= ~(1ULL << VIRTIO_F_IN_ORDER);