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 <dariuszx.stojac...@intel.com>
Signed-off-by: Tomasz Kulasek <tomaszx.kula...@intel.com>
---
 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

Reply via email to