rte_vhost: mantain separate virtio features field in virtio_net

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.

Fixes #214

Change-Id: Ic4b2dd8cae6050813fc9a420b2ed30bc5ae60393
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/386294
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Pawel Wodkowski <pawelx.wodkowski@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>

lib/vhost/rte_vhost/socket.c

@@ -220,7 +220,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;
 	}

lib/vhost/rte_vhost/vhost.c

@@ -168,7 +168,7 @@ reset_device(struct virtio_net *dev)
 {
 	uint32_t i;
 
-	dev->features = 0;
+	dev->negotiated_features = 0;
 	dev->protocol_features = 0;
 	dev->flags = 0;
 
@@ -181,7 +181,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;
@@ -206,6 +206,7 @@ vhost_new_device(void)
 
 	vhost_devices[i] = dev;
 	dev->vid = i;
+	dev->features = features;
 
 	return i;
 }
@@ -272,7 +273,7 @@ rte_vhost_get_mtu(int vid, uint16_t *mtu)
 	if (!(dev->flags & VIRTIO_DEV_READY))
 		return -EAGAIN;
 
-	if (!(dev->features & VIRTIO_NET_F_MTU))
+	if (!(dev->negotiated_features & VIRTIO_NET_F_MTU))
 		return -ENOTSUP;
 
 	*mtu = dev->mtu;
@@ -353,7 +354,7 @@ rte_vhost_get_negotiated_features(int vid, uint64_t *features)
 	if (!dev)
 		return -1;
 
-	*features = dev->features;
+	*features = dev->negotiated_features;
 	return 0;
 }
 

lib/vhost/rte_vhost/vhost.h

@@ -175,6 +175,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;
@@ -216,7 +217,7 @@ 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) ||
+	if (likely(((dev->negotiated_features & (1ULL << VHOST_F_LOG_ALL)) == 0) ||
 		   !dev->log_base || !len))
 		return;
 
@@ -297,7 +298,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);

lib/vhost/rte_vhost/vhost_user.c

@@ -161,10 +161,7 @@ vhost_user_reset_owner(struct virtio_net *dev)
 static uint64_t
 vhost_user_get_features(struct virtio_net *dev)
 {
-	uint64_t features = 0;
+	return dev->features;
-
-	rte_vhost_driver_get_features(dev->ifname, &features);
-	return features;
 }
 
 /*
@@ -175,7 +172,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",
@@ -183,13 +180,13 @@ vhost_user_set_features(struct virtio_net *dev, uint64_t features)
 		return -1;
 	}
 
-	if ((dev->flags & VIRTIO_DEV_RUNNING) && dev->features != features) {
+	if ((dev->flags & VIRTIO_DEV_RUNNING) && dev->negotiated_features != features) {
 		if (dev->notify_ops->features_changed)
 			dev->notify_ops->features_changed(dev->vid, features);
 	}
 
-	dev->features = features;
+	dev->negotiated_features = features;
-	if (dev->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 {
@@ -198,8 +195,8 @@ vhost_user_set_features(struct virtio_net *dev, uint64_t features)
 	VHOST_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->negotiated_features & (1 << VIRTIO_NET_F_MRG_RXBUF)) ? "on" : "off",
-		(dev->features & (1ULL << VIRTIO_F_VERSION_1)) ? "on" : "off");
+		(dev->negotiated_features & (1ULL << VIRTIO_F_VERSION_1)) ? "on" : "off");
 
 	return 0;
 }