From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id 482AC1B66C for ; Tue, 6 Feb 2018 11:19:09 +0100 (CET) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 90FA5356E5; Tue, 6 Feb 2018 10:19:08 +0000 (UTC) Received: from [10.36.112.19] (ovpn-112-19.ams2.redhat.com [10.36.112.19]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 7374018205; Tue, 6 Feb 2018 10:19:05 +0000 (UTC) To: Zhihong Wang , dev@dpdk.org Cc: jianfeng.tan@intel.com, tiwei.bie@intel.com, yliu@fridaylinux.org, cunming.liang@intel.com, xiao.w.wang@intel.com, dan.daly@intel.com References: <1517614137-62926-1-git-send-email-zhihong.wang@intel.com> <1517614137-62926-2-git-send-email-zhihong.wang@intel.com> From: Maxime Coquelin Message-ID: <8954f0ba-dfbe-4af5-c7d7-9acfebdd931d@redhat.com> Date: Tue, 6 Feb 2018 11:19:03 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 MIME-Version: 1.0 In-Reply-To: <1517614137-62926-2-git-send-email-zhihong.wang@intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Tue, 06 Feb 2018 10:19:08 +0000 (UTC) Subject: Re: [dpdk-dev] [PATCH 1/7] vhost: make capabilities configurable X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 06 Feb 2018 10:19:09 -0000 Hi Zhihong, On 02/03/2018 12:28 AM, Zhihong Wang wrote: > This patch makes vhost device capabilities configurable to adopt new > devices, since different devices may have different capabilities, like > different combinations of supported features, or different number of > queues. APIs are introduced to let app configure these capabilities. > > Signed-off-by: Zhihong Wang > --- > lib/librte_vhost/rte_vhost.h | 50 ++++++++++++++++++++++++++++ > lib/librte_vhost/socket.c | 77 +++++++++++++++++++++++++++++++++++++++++++ > lib/librte_vhost/vhost_user.c | 48 ++++++++++++++++++++------- > 3 files changed, 164 insertions(+), 11 deletions(-) > ... > diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c > index 6e3857e..e1d0036 100644 > --- a/lib/librte_vhost/socket.c > +++ b/lib/librte_vhost/socket.c > @@ -49,7 +49,10 @@ struct vhost_user_socket { > * features negotiation. > */ > uint64_t supported_features; > + uint64_t supported_protocol_features; > uint64_t features; > + uint64_t protocol_features; > + uint16_t queue_num; > > struct vhost_device_ops const *notify_ops; > }; > @@ -593,6 +596,75 @@ rte_vhost_driver_get_features(const char *path, uint64_t *features) > } > } > > +int rte_vhost_driver_set_protocol_features(const char *path, > + uint64_t protocol_features) > +{ > + struct vhost_user_socket *vsocket; > + > + pthread_mutex_lock(&vhost_user.mutex); > + vsocket = find_vhost_user_socket(path); > + if (vsocket) { > + vsocket->supported_protocol_features = protocol_features; > + vsocket->protocol_features = protocol_features; > + } > + pthread_mutex_unlock(&vhost_user.mutex); > + > + return vsocket ? 0 : -1; > +} > + > +int > +rte_vhost_driver_get_protocol_features(const char *path, > + uint64_t *protocol_features) > +{ > + struct vhost_user_socket *vsocket; > + > + pthread_mutex_lock(&vhost_user.mutex); > + vsocket = find_vhost_user_socket(path); > + if (vsocket) > + *protocol_features = vsocket->protocol_features; > + pthread_mutex_unlock(&vhost_user.mutex); > + > + if (!vsocket) { > + RTE_LOG(ERR, VHOST_CONFIG, > + "socket file %s is not registered yet.\n", path); > + return -1; > + } else { > + return 0; > + } > +} > + > +int rte_vhost_driver_set_queue_num(const char *path, uint16_t queue_num) > +{ > + struct vhost_user_socket *vsocket; > + > + pthread_mutex_lock(&vhost_user.mutex); > + vsocket = find_vhost_user_socket(path); > + if (vsocket) > + vsocket->queue_num = queue_num; Shouldn't be MIN(queue_num, VHOST_MAX_QUEUE_PAIRS) to be sure you can switch from HW offload to SW processing? > + pthread_mutex_unlock(&vhost_user.mutex); > + > + return vsocket ? 0 : -1; > +} > + > +int rte_vhost_driver_get_queue_num(const char *path, uint16_t *queue_num) > +{ > + struct vhost_user_socket *vsocket; > + > + pthread_mutex_lock(&vhost_user.mutex); > + vsocket = find_vhost_user_socket(path); > + if (vsocket) > + *queue_num = vsocket->queue_num; > + pthread_mutex_unlock(&vhost_user.mutex); > + > + if (!vsocket) { > + RTE_LOG(ERR, VHOST_CONFIG, > + "socket file %s is not registered yet.\n", path); > + return -1; > + } else { > + return 0; > + } > +} > + > /* > * Register a new vhost-user socket; here we could act as server > * (the default case), or client (when RTE_VHOST_USER_CLIENT) flag > @@ -655,6 +727,11 @@ rte_vhost_driver_register(const char *path, uint64_t flags) > vsocket->features &= ~(1ULL << VIRTIO_F_IOMMU_PLATFORM); > } > > + vsocket->supported_protocol_features = VHOST_USER_PROTOCOL_FEATURES; > + vsocket->protocol_features = VHOST_USER_PROTOCOL_FEATURES; > + > + vsocket->queue_num = VHOST_MAX_QUEUE_PAIRS; > + > if ((flags & RTE_VHOST_USER_CLIENT) != 0) { > vsocket->reconnect = !(flags & RTE_VHOST_USER_NO_RECONNECT); > if (vsocket->reconnect && reconn_tid == 0) { > diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c > index 1dd1a61..87ba267 100644 > --- a/lib/librte_vhost/vhost_user.c > +++ b/lib/librte_vhost/vhost_user.c > @@ -127,6 +127,18 @@ vhost_user_reset_owner(struct virtio_net *dev) > } > > /* > + * The supported queue num. > + */ > +static uint16_t > +vhost_user_get_queue_num(struct virtio_net *dev) > +{ > + uint16_t queue_num = 0; > + > + rte_vhost_driver_get_queue_num(dev->ifname, &queue_num); > + return queue_num; > +} > + > +/* > * The features that we support are requested. > */ > static uint64_t > @@ -930,13 +942,17 @@ vhost_user_set_vring_enable(struct virtio_net *dev, > return 0; > } > > -static void > -vhost_user_get_protocol_features(struct virtio_net *dev, > - struct VhostUserMsg *msg) > +/* > + * The protocol features that we support are requested. > + */ > + > +static uint64_t > +vhost_user_get_protocol_features(struct virtio_net *dev) > { > - uint64_t features, protocol_features = VHOST_USER_PROTOCOL_FEATURES; > + uint64_t features, protocol_features; > > rte_vhost_driver_get_features(dev->ifname, &features); > + rte_vhost_driver_get_protocol_features(dev->ifname, &protocol_features); > > /* > * REPLY_ACK protocol feature is only mandatory for now > @@ -947,18 +963,27 @@ vhost_user_get_protocol_features(struct virtio_net *dev, > if (!(features & (1ULL << VIRTIO_F_IOMMU_PLATFORM))) > protocol_features &= ~(1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK); > > - msg->payload.u64 = protocol_features; > - msg->size = sizeof(msg->payload.u64); > + return protocol_features; > } > > -static void > +static int > vhost_user_set_protocol_features(struct virtio_net *dev, > uint64_t protocol_features) > { > - if (protocol_features & ~VHOST_USER_PROTOCOL_FEATURES) I think the above check is still necessary, or it should be checked in rte_vhost_driver_set_protocol_features(). Indeed, the application shouldn't set a protocol feature bit that isn't supported by the libvhost-user library. > - return; > + uint64_t vhost_protocol_features = 0; > + > + rte_vhost_driver_get_protocol_features(dev->ifname, > + &vhost_protocol_features); > + if (protocol_features & ~vhost_protocol_features) { > + RTE_LOG(ERR, VHOST_CONFIG, > + "(%d) received invalid negotiated protocol_features.\n", > + dev->vid); > + return -1; > + } > > dev->protocol_features = protocol_features; > + > + return 0; > } > > static int > @@ -1391,7 +1416,8 @@ vhost_user_msg_handler(int vid, int fd) > break; > > case VHOST_USER_GET_PROTOCOL_FEATURES: > - vhost_user_get_protocol_features(dev, &msg); > + msg.payload.u64 = vhost_user_get_protocol_features(dev); > + msg.size = sizeof(msg.payload.u64); > send_vhost_reply(fd, &msg); > break; > case VHOST_USER_SET_PROTOCOL_FEATURES: > @@ -1451,7 +1477,7 @@ vhost_user_msg_handler(int vid, int fd) > break; > > case VHOST_USER_GET_QUEUE_NUM: > - msg.payload.u64 = VHOST_MAX_QUEUE_PAIRS; > + msg.payload.u64 = vhost_user_get_queue_num(dev); > msg.size = sizeof(msg.payload.u64); > send_vhost_reply(fd, &msg); > break; > Maxime