From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id E75B142DDA; Wed, 5 Jul 2023 19:02:20 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 8197B406B5; Wed, 5 Jul 2023 19:02:20 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id C24D24021F for ; Wed, 5 Jul 2023 19:02:18 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1688576538; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ixoyLDOo45Zuea327FKGgmWLAm5C1qM1gf6/Kp84UpI=; b=Rj4YrZqo6MHN/OOroWmePmKSgyl1owb/AVS8MF/AYmO3xEsRGIyO0w1R9moXna79JdcMIE pW8D3EaPCGFur6zj/6QIIyejcH8EMCQofZvzwuCN2TPjtCKVfYT3s4Ix6vrvktuYL3tSgf uMRGdo+R1kk4QFlrGcnWlcZdu6re2C8= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-650-d9knoa11NV-5LZhLQ2b_rA-1; Wed, 05 Jul 2023 13:02:11 -0400 X-MC-Unique: d9knoa11NV-5LZhLQ2b_rA-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.rdu2.redhat.com [10.11.54.1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id C11518564EF; Wed, 5 Jul 2023 17:02:10 +0000 (UTC) Received: from [10.39.208.34] (unknown [10.39.208.34]) by smtp.corp.redhat.com (Postfix) with ESMTPS id DDCF540C2070; Wed, 5 Jul 2023 17:02:09 +0000 (UTC) Message-ID: <3cc8e20a-70d7-871c-c799-5adf37306167@redhat.com> Date: Wed, 5 Jul 2023 19:02:07 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.12.0 To: David Marchand Cc: dev@dpdk.org, chenbo.xia@intel.com References: <20230705132232.114266-1-maxime.coquelin@redhat.com> <20230705132232.114266-2-maxime.coquelin@redhat.com> From: Maxime Coquelin Subject: Re: [PATCH 1/2] vhost: fix vduse features negotiation In-Reply-To: X-Scanned-By: MIMEDefang 3.1 on 10.11.54.1 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Hi David, On 7/5/23 15:36, David Marchand wrote: > On Wed, Jul 5, 2023 at 3:22 PM Maxime Coquelin > wrote: >> @@ -950,9 +954,14 @@ rte_vhost_driver_register(const char *path, uint64_t flags) >> * two values. >> */ >> vsocket->use_builtin_virtio_net = true; >> - vsocket->supported_features = VIRTIO_NET_SUPPORTED_FEATURES; >> - vsocket->features = VIRTIO_NET_SUPPORTED_FEATURES; >> - vsocket->protocol_features = VHOST_USER_PROTOCOL_FEATURES; >> + if (vsocket->is_vduse) { >> + vsocket->supported_features = VDUSE_NET_SUPPORTED_FEATURES; >> + vsocket->features = VDUSE_NET_SUPPORTED_FEATURES; >> + } else { >> + vsocket->supported_features = VIRTIO_NET_SUPPORTED_FEATURES; >> + vsocket->features = VIRTIO_NET_SUPPORTED_FEATURES; >> + vsocket->protocol_features = VHOST_USER_PROTOCOL_FEATURES; >> + } >> > > Would it make sense to split those features in a set of features > shared by vhost-user and vduse, and one dedicated set for each of > them? > For example, the VIRTIO_NET_F_GUEST/HOST_* features are not bound to > vhost-user / vduse, are they? Most of the features are the same, but there are a few differences: - VIRTIO_F_RING_PACKED: this is not really Vhost or VDUSE specific. The issue is we just lack packed ring support in the control queue implementation, only used by VDUSE for now. I expect to add it in v23.11. - VHOST_USER_F_PROTOCOL_FEATURES: This feature is Vhost-user specific - VHOST_F_LOG_ALL: This is for live-migration, maybe it will be advertised by VDUSE in the future, but we miss live-migration support in Kernel VDUSE at the moment. - VIRTIO_NET_F_CTRL_RX: This is advertised by Vhost-user, but it does not do anything specific. While if we do it in VDUSE, we need to implement its support in CVQ. To summarize, we have some features specific to Vhost-user, but for the other differences, this is just some features are not yet implemented/tested with VDUSE. Maybe we could have something like this, which has the advantage to highlight the gaps we have in VDUSE (not compiled/tested): ------------------------------------------------------------ diff --git a/lib/vhost/socket.c b/lib/vhost/socket.c index a65d301406..f55fb299fd 100644 --- a/lib/vhost/socket.c +++ b/lib/vhost/socket.c @@ -958,8 +958,8 @@ rte_vhost_driver_register(const char *path, uint64_t flags) vsocket->supported_features = VDUSE_NET_SUPPORTED_FEATURES; vsocket->features = VDUSE_NET_SUPPORTED_FEATURES; } else { - vsocket->supported_features = VIRTIO_NET_SUPPORTED_FEATURES; - vsocket->features = VIRTIO_NET_SUPPORTED_FEATURES; + vsocket->supported_features = VHOST_USER_NET_SUPPORTED_FEATURES; + vsocket->features = VHOST_USER_NET_SUPPORTED_FEATURES; vsocket->protocol_features = VHOST_USER_PROTOCOL_FEATURES; } diff --git a/lib/vhost/vduse.h b/lib/vhost/vduse.h index 46753fec73..5a8b37ec67 100644 --- a/lib/vhost/vduse.h +++ b/lib/vhost/vduse.h @@ -7,28 +7,7 @@ #include "vhost.h" -#define VDUSE_NET_SUPPORTED_FEATURES ((1ULL << VIRTIO_NET_F_MRG_RXBUF) | \ - (1ULL << VIRTIO_F_ANY_LAYOUT) | \ - (1ULL << VIRTIO_F_VERSION_1) | \ - (1ULL << VIRTIO_NET_F_GSO) | \ - (1ULL << VIRTIO_NET_F_HOST_TSO4) | \ - (1ULL << VIRTIO_NET_F_HOST_TSO6) | \ - (1ULL << VIRTIO_NET_F_HOST_UFO) | \ - (1ULL << VIRTIO_NET_F_HOST_ECN) | \ - (1ULL << VIRTIO_NET_F_CSUM) | \ - (1ULL << VIRTIO_NET_F_GUEST_CSUM) | \ - (1ULL << VIRTIO_NET_F_GUEST_TSO4) | \ - (1ULL << VIRTIO_NET_F_GUEST_TSO6) | \ - (1ULL << VIRTIO_NET_F_GUEST_UFO) | \ - (1ULL << VIRTIO_NET_F_GUEST_ECN) | \ - (1ULL << VIRTIO_RING_F_INDIRECT_DESC) | \ - (1ULL << VIRTIO_RING_F_EVENT_IDX) | \ - (1ULL << VIRTIO_F_IN_ORDER) | \ - (1ULL << VIRTIO_F_IOMMU_PLATFORM) | \ - (1ULL << VIRTIO_NET_F_CTRL_VQ) | \ - (1ULL << VIRTIO_NET_F_MQ)) - -#ifdef VHOST_HAS_VDUSE +#define VDUSE_NET_SUPPORTED_FEATURES VIRTIO_NET_SUPPORTED_FEATURES int vduse_device_create(const char *path); int vduse_device_destroy(const char *path); diff --git a/lib/vhost/vhost.h b/lib/vhost/vhost.h index 9ecede0f30..f49ce943b0 100644 --- a/lib/vhost/vhost.h +++ b/lib/vhost/vhost.h @@ -438,12 +438,8 @@ struct vring_packed_desc_event { #define VIRTIO_NET_SUPPORTED_FEATURES ((1ULL << VIRTIO_NET_F_MRG_RXBUF) | \ (1ULL << VIRTIO_F_ANY_LAYOUT) | \ (1ULL << VIRTIO_NET_F_CTRL_VQ) | \ - (1ULL << VIRTIO_NET_F_CTRL_RX) | \ - (1ULL << VIRTIO_NET_F_GUEST_ANNOUNCE) | \ (1ULL << VIRTIO_NET_F_MQ) | \ (1ULL << VIRTIO_F_VERSION_1) | \ - (1ULL << VHOST_F_LOG_ALL) | \ - (1ULL << VHOST_USER_F_PROTOCOL_FEATURES) | \ (1ULL << VIRTIO_NET_F_GSO) | \ (1ULL << VIRTIO_NET_F_HOST_TSO4) | \ (1ULL << VIRTIO_NET_F_HOST_TSO6) | \ @@ -457,10 +453,8 @@ struct vring_packed_desc_event { (1ULL << VIRTIO_NET_F_GUEST_ECN) | \ (1ULL << VIRTIO_RING_F_INDIRECT_DESC) | \ (1ULL << VIRTIO_RING_F_EVENT_IDX) | \ - (1ULL << VIRTIO_NET_F_MTU) | \ (1ULL << VIRTIO_F_IN_ORDER) | \ - (1ULL << VIRTIO_F_IOMMU_PLATFORM) | \ - (1ULL << VIRTIO_F_RING_PACKED)) + (1ULL << VIRTIO_F_IOMMU_PLATFORM)) struct guest_page { diff --git a/lib/vhost/vhost_user.h b/lib/vhost/vhost_user.h index 1ffeca92f3..edf7adb3c0 100644 --- a/lib/vhost/vhost_user.h +++ b/lib/vhost/vhost_user.h @@ -13,6 +13,15 @@ #define VHOST_MEMORY_MAX_NREGIONS 8 +#define VHOST_USER_NET_SUPPORTED_FEATURES \ + (VIRTIO_NET_SUPPORTED_FEATURES | \ + (1ULL << VIRTIO_F_RING_PACKED) | \ + (1ULL << VIRTIO_NET_F_MTU) | \ + (1ULL << VHOST_F_LOG_ALL) | \ + (1ULL << VHOST_USER_F_PROTOCOL_FEATURES) | \ + (1ULL << VIRTIO_NET_F_CTRL_RX) | \ + (1ULL << VIRTIO_NET_F_GUEST_ANNOUNCE)) + #define VHOST_USER_PROTOCOL_FEATURES ((1ULL << VHOST_USER_PROTOCOL_F_MQ) | \ (1ULL << VHOST_USER_PROTOCOL_F_LOG_SHMFD) |\ (1ULL << VHOST_USER_PROTOCOL_F_RARP) | \ ------------------------------------------------------------ WDYT? Thanks, Maxime > >