From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id 915192BF1 for ; Mon, 5 Mar 2018 16:49:48 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 05 Mar 2018 07:49:47 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.47,427,1515484800"; d="scan'208";a="36068265" Received: from unknown (HELO Sent) ([10.103.102.195]) by orsmga001.jf.intel.com with SMTP; 05 Mar 2018 07:49:38 -0800 Received: by Sent (sSMTP sendmail emulation); Mon, 05 Mar 2018 16:49:37 +0100 From: Tomasz Kulasek To: yliu@fridaylinux.org Cc: dev@dpdk.org, Dariusz Stojaczyk Date: Mon, 5 Mar 2018 16:49:09 +0100 Message-Id: <20180305154909.216-1-tomaszx.kulasek@intel.com> X-Mailer: git-send-email 2.16.1 Subject: [dpdk-dev] [PATCH] vhost: maintain separate virtio features field 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: Mon, 05 Mar 2018 15:49:49 -0000 There are two separate abstraction layers: * vsocket - which represents a unix domain socket * virtio_net - which represents a vsocket connection There can be many connections on the same socket. vsocket provides an API to enable/disable particular virtio features on the fly, but it's the virtio_net that uses these features. virtio_net used to rely on vsocket->features during feature negotiation, breaking the layer encapsulation (and yet causing a deadlock - two locks were being locked in a separate order). Now each virtio_net device has it's own copy of vsocket features, created at the time of virtio_net creation. vsocket->features have to be still present, as features can be enabled/disabled while no virtio_net device has been created yet. Signed-off-by: Dariusz Stojaczyk Signed-off-by: Tomasz Kulasek --- lib/librte_vhost/socket.c | 2 +- lib/librte_vhost/vhost.c | 9 +++++---- lib/librte_vhost/vhost.h | 8 +++++--- lib/librte_vhost/vhost_user.c | 33 +++++++++++++++++---------------- 4 files changed, 28 insertions(+), 24 deletions(-) diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c index 83befdced..260e38dbe 100644 --- a/lib/librte_vhost/socket.c +++ b/lib/librte_vhost/socket.c @@ -188,7 +188,7 @@ vhost_user_add_connection(int fd, struct vhost_user_socket *vsocket) return; } - vid = vhost_new_device(); + vid = vhost_new_device(vsocket->features); if (vid == -1) { goto err; } diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c index a407067e2..a307a19ed 100644 --- a/lib/librte_vhost/vhost.c +++ b/lib/librte_vhost/vhost.c @@ -256,7 +256,7 @@ reset_device(struct virtio_net *dev) { uint32_t i; - dev->features = 0; + dev->negotiated_features = 0; dev->protocol_features = 0; dev->flags &= VIRTIO_DEV_BUILTIN_VIRTIO_NET; @@ -269,7 +269,7 @@ reset_device(struct virtio_net *dev) * there is a new virtio device being attached). */ int -vhost_new_device(void) +vhost_new_device(uint64_t features) { struct virtio_net *dev; int i; @@ -296,6 +296,7 @@ vhost_new_device(void) dev->vid = i; dev->flags = VIRTIO_DEV_BUILTIN_VIRTIO_NET; dev->slave_req_fd = -1; + dev->features = features; return i; } @@ -376,7 +377,7 @@ rte_vhost_get_mtu(int vid, uint16_t *mtu) if (!(dev->flags & VIRTIO_DEV_READY)) return -EAGAIN; - if (!(dev->features & (1ULL << VIRTIO_NET_F_MTU))) + if (!(dev->negotiated_features & (1ULL << VIRTIO_NET_F_MTU))) return -ENOTSUP; *mtu = dev->mtu; @@ -458,7 +459,7 @@ rte_vhost_get_negotiated_features(int vid, uint64_t *features) if (!dev) return -1; - *features = dev->features; + *features = dev->negotiated_features; return 0; } diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h index d947bc9e3..efbc89857 100644 --- a/lib/librte_vhost/vhost.h +++ b/lib/librte_vhost/vhost.h @@ -217,6 +217,7 @@ struct virtio_net { /* Frontend (QEMU) memory and memory region information */ struct rte_vhost_memory *mem; uint64_t features; + uint64_t negotiated_features; uint64_t protocol_features; int vid; uint32_t flags; @@ -266,8 +267,9 @@ vhost_log_write(struct virtio_net *dev, uint64_t addr, uint64_t len) { uint64_t page; - if (likely(((dev->features & (1ULL << VHOST_F_LOG_ALL)) == 0) || - !dev->log_base || !len)) + if (likely(((dev->negotiated_features & + (1ULL << VHOST_F_LOG_ALL)) == 0) || !dev->log_base || + !len)) return; if (unlikely(dev->log_size <= ((addr + len - 1) / VHOST_LOG_PAGE / 8))) @@ -347,7 +349,7 @@ gpa_to_hpa(struct virtio_net *dev, uint64_t gpa, uint64_t size) struct virtio_net *get_device(int vid); -int vhost_new_device(void); +int vhost_new_device(uint64_t features); void cleanup_device(struct virtio_net *dev, int destroy); void reset_device(struct virtio_net *dev); void vhost_destroy_device(int); diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c index 65ee33919..818fc4263 100644 --- a/lib/librte_vhost/vhost_user.c +++ b/lib/librte_vhost/vhost_user.c @@ -132,10 +132,7 @@ vhost_user_reset_owner(struct virtio_net *dev) static uint64_t vhost_user_get_features(struct virtio_net *dev) { - uint64_t features = 0; - - rte_vhost_driver_get_features(dev->ifname, &features); - return features; + return dev->features; } /* @@ -146,7 +143,7 @@ vhost_user_set_features(struct virtio_net *dev, uint64_t features) { uint64_t vhost_features = 0; - rte_vhost_driver_get_features(dev->ifname, &vhost_features); + vhost_features = vhost_user_get_features(dev); if (features & ~vhost_features) { RTE_LOG(ERR, VHOST_CONFIG, "(%d) received invalid negotiated features.\n", @@ -155,7 +152,7 @@ vhost_user_set_features(struct virtio_net *dev, uint64_t features) } if (dev->flags & VIRTIO_DEV_RUNNING) { - if (dev->features == features) + if (dev->negotiated_features == features) return 0; /* @@ -163,7 +160,8 @@ vhost_user_set_features(struct virtio_net *dev, uint64_t features) * in running state. The exception being VHOST_F_LOG_ALL, which * is enabled when the live-migration starts. */ - if ((dev->features ^ features) & ~(1ULL << VHOST_F_LOG_ALL)) { + if ((dev->negotiated_features ^ features) & + ~(1ULL << VHOST_F_LOG_ALL)) { RTE_LOG(ERR, VHOST_CONFIG, "(%d) features changed while device is running.\n", dev->vid); @@ -174,9 +172,9 @@ vhost_user_set_features(struct virtio_net *dev, uint64_t features) dev->notify_ops->features_changed(dev->vid, features); } - dev->features = features; - if (dev->features & - ((1 << VIRTIO_NET_F_MRG_RXBUF) | (1ULL << VIRTIO_F_VERSION_1))) { + dev->negotiated_features = features; + if (dev->negotiated_features & ((1 << VIRTIO_NET_F_MRG_RXBUF) | + (1ULL << VIRTIO_F_VERSION_1))) { dev->vhost_hlen = sizeof(struct virtio_net_hdr_mrg_rxbuf); } else { dev->vhost_hlen = sizeof(struct virtio_net_hdr); @@ -184,11 +182,13 @@ vhost_user_set_features(struct virtio_net *dev, uint64_t features) LOG_DEBUG(VHOST_CONFIG, "(%d) mergeable RX buffers %s, virtio 1 %s\n", dev->vid, - (dev->features & (1 << VIRTIO_NET_F_MRG_RXBUF)) ? "on" : "off", - (dev->features & (1ULL << VIRTIO_F_VERSION_1)) ? "on" : "off"); + (dev->negotiated_features & (1 << VIRTIO_NET_F_MRG_RXBUF)) ? + "on" : "off", + (dev->negotiated_features & (1ULL << VIRTIO_F_VERSION_1)) ? + "on" : "off"); if ((dev->flags & VIRTIO_DEV_BUILTIN_VIRTIO_NET) && - !(dev->features & (1ULL << VIRTIO_NET_F_MQ))) { + !(dev->negotiated_features & (1ULL << VIRTIO_NET_F_MQ))) { /* * Remove all but first queue pair if MQ hasn't been * negotiated. This is safe because the device is not @@ -395,7 +395,7 @@ static uint64_t ring_addr_to_vva(struct virtio_net *dev, struct vhost_virtqueue *vq, uint64_t ra, uint64_t size) { - if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)) { + if (dev->negotiated_features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)) { uint64_t vva; vva = vhost_user_iotlb_cache_find(vq, ra, @@ -498,7 +498,7 @@ vhost_user_set_vring_addr(struct virtio_net **pdev, VhostUserMsg *msg) vring_invalidate(dev, vq); - if (vq->enabled && (dev->features & + if (vq->enabled && (dev->negotiated_features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES))) { dev = translate_ring_addresses(dev, msg->payload.state.index); if (!dev) @@ -840,7 +840,8 @@ vhost_user_set_vring_kick(struct virtio_net **pdev, struct VhostUserMsg *pmsg) * the ring starts already enabled. Otherwise, it is enabled via * the SET_VRING_ENABLE message. */ - if (!(dev->features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES))) + if (!(dev->negotiated_features & + (1ULL << VHOST_USER_F_PROTOCOL_FEATURES))) vq->enabled = 1; if (vq->kickfd >= 0) -- 2.14.1