[PATCH 0/2] kvm tools: fix vhost-net broken since fa7226f88012713a73d0cba4955444ea109e9458

[PATCH 0/2] kvm tools: fix vhost-net broken since fa7226f88012713a73d0cba4955444ea109e9458

[Ying-Shiuan Pan]

These patch series fix 2 vhost-net problems. There were 2 problems after
commit fa7226f88012713a73d0cba4955444ea109e9458. 
(1) vhost-net requires tap_fd for VHOST_SET_BACKEND, but opening tap device
was postponed to VIRTIO_CONFIG_S_DRIVER_OK. The wrong order of initialization
caused vhost-net used a invalid tap_fd for ioctl.
(2) virtio-net of kvm tool started to support mergeable rx buffers since
commit 8c2684de6fb1afc95321e7e0f30550acdcc08186, but kvm tool did not tell
vhost-net that he is using mergeable rx buffers.

test script:
$ sudo ./lkvm run --kernel ../../arch/x86/boot/bzImage --no-dhcp --net mode=tap,vhost=1

Ying-Shiuan Pan (2):
  kvm tools: virtio-net has to open tap device before vhost-net init.
  kvm tools: vhost-net: setup mergeable rx buffers feature

 tools/kvm/virtio/net.c | 54 +++++++++++++++++++++++++++-----------------------
 1 file changed, 29 insertions(+), 25 deletions(-)

-- 
1.8.1.2

[PATCH 1/2] kvm tools: virtio-net has to open tap device before vhost-net init.

[Ying-Shiuan Pan]

Enabling vhost-net encounted an error:
  Fatal: VHOST_NET_SET_BACKEND failed 88

The reason is that vhost-net requires tap_fd for VHOST_NET_SET_BACKEND,
however tap_fd is opened after VIRTIO_CONFIG_S_DRIVER_OK. Because the
initialization needs to know the guest features, I suppose the initialization
could be moved to set_guest_features(). Therefore, initialization can be
finished before status VIRTIO_CONFIG_S_DRIVER_OK, and tap_fd can be set
before vhost-net sets backend.

Signed-off-by: Ying-Shiuan Pan <yspan@itri.org.tw>
---
 tools/kvm/virtio/net.c | 31 ++++++++++---------------------
 1 file changed, 10 insertions(+), 21 deletions(-)

diff --git a/tools/kvm/virtio/net.c b/tools/kvm/virtio/net.c
index 3715aaf..dfc4aad 100644
--- a/tools/kvm/virtio/net.c
+++ b/tools/kvm/virtio/net.c
@@ -389,6 +389,16 @@ static void set_guest_features(struct kvm *kvm, void *dev, u32 features)
 	struct net_dev *ndev = dev;
 
 	ndev->features = features;
+
+	if (ndev->mode == NET_MODE_TAP) {
+		if (!virtio_net__tap_init(ndev))
+			die_perror("You have requested a TAP device, but creation of one has failed because");
+	} else {
+		ndev->info.vnet_hdr_len = has_virtio_feature(ndev, VIRTIO_NET_F_MRG_RXBUF) ?
+						sizeof(struct virtio_net_hdr_mrg_rxbuf) :
+						sizeof(struct virtio_net_hdr);
+		uip_init(&ndev->info);
+	}
 }
 
 static bool is_ctrl_vq(struct net_dev *ndev, u32 vq)
@@ -530,8 +540,6 @@ static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size)
 	return size;
 }
 
-static void notify_status(struct kvm *kvm, void *dev, u8 status);
-
 static struct virtio_ops net_dev_virtio_ops = (struct virtio_ops) {
 	.get_config		= get_config,
 	.get_host_features	= get_host_features,
@@ -543,7 +551,6 @@ static struct virtio_ops net_dev_virtio_ops = (struct virtio_ops) {
 	.notify_vq		= notify_vq,
 	.notify_vq_gsi		= notify_vq_gsi,
 	.notify_vq_eventfd	= notify_vq_eventfd,
-	.notify_status		= notify_status,
 };
 
 static void virtio_net__vhost_init(struct kvm *kvm, struct net_dev *ndev)
@@ -731,24 +738,6 @@ static int virtio_net__init_one(struct virtio_net_params *params)
 	return 0;
 }
 
-static void notify_status(struct kvm *kvm, void *dev, u8 status)
-{
-	struct net_dev *ndev = dev;
-
-	if (!(status & VIRTIO_CONFIG_S_DRIVER_OK))
-		return;
-
-	if (ndev->mode == NET_MODE_TAP) {
-		if (!virtio_net__tap_init(ndev))
-			die_perror("You have requested a TAP device, but creation of one has failed because");
-	} else {
-		ndev->info.vnet_hdr_len = has_virtio_feature(ndev, VIRTIO_NET_F_MRG_RXBUF) ?
-						sizeof(struct virtio_net_hdr_mrg_rxbuf) :
-						sizeof(struct virtio_net_hdr);
-		uip_init(&ndev->info);
-	}
-}
-
 int virtio_net__init(struct kvm *kvm)
 {
 	int i;
-- 
1.8.1.2

[PATCH 2/2] kvm tools: vhost-net: setup mergeable rx buffers feature

[Ying-Shiuan Pan]

After features negotiation, kvmtool should tell vhost-net that he's
using mergeable rx buffers.

Signed-off-by: Ying-Shiuan Pan <yspan@itri.org.tw>
---
 tools/kvm/virtio/net.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/tools/kvm/virtio/net.c b/tools/kvm/virtio/net.c
index dfc4aad..2f61718 100644
--- a/tools/kvm/virtio/net.c
+++ b/tools/kvm/virtio/net.c
@@ -384,6 +384,22 @@ static u32 get_host_features(struct kvm *kvm, void *dev)
 		| 1UL << (ndev->queue_pairs > 1 ? VIRTIO_NET_F_MQ : 0);
 }
 
+static int virtio_net__vhost_set_features(struct net_dev *ndev)
+{
+	u64 features = 1UL << VIRTIO_RING_F_EVENT_IDX;
+	u64 vhost_features;
+
+	if (ioctl(ndev->vhost_fd, VHOST_GET_FEATURES, &vhost_features) != 0)
+		die_perror("VHOST_GET_FEATURES failed");
+
+	/* make sure both side support mergable rx buffers */
+	if (vhost_features & 1UL << VIRTIO_NET_F_MRG_RXBUF &&
+			has_virtio_feature(ndev, VIRTIO_NET_F_MRG_RXBUF))
+		features |= 1UL << VIRTIO_NET_F_MRG_RXBUF;
+
+	return ioctl(ndev->vhost_fd, VHOST_SET_FEATURES, &features);
+}
+
 static void set_guest_features(struct kvm *kvm, void *dev, u32 features)
 {
 	struct net_dev *ndev = dev;
@@ -393,6 +409,9 @@ static void set_guest_features(struct kvm *kvm, void *dev, u32 features)
 	if (ndev->mode == NET_MODE_TAP) {
 		if (!virtio_net__tap_init(ndev))
 			die_perror("You have requested a TAP device, but creation of one has failed because");
+		if (ndev->vhost_fd &&
+				virtio_net__vhost_set_features(ndev) != 0)
+			die_perror("VHOST_SET_FEATURES failed");
 	} else {
 		ndev->info.vnet_hdr_len = has_virtio_feature(ndev, VIRTIO_NET_F_MRG_RXBUF) ?
 						sizeof(struct virtio_net_hdr_mrg_rxbuf) :
@@ -555,7 +574,6 @@ static struct virtio_ops net_dev_virtio_ops = (struct virtio_ops) {
 
 static void virtio_net__vhost_init(struct kvm *kvm, struct net_dev *ndev)
 {
-	u64 features = 1UL << VIRTIO_RING_F_EVENT_IDX;
 	struct vhost_memory *mem;
 	int r;
 
@@ -578,9 +596,6 @@ static void virtio_net__vhost_init(struct kvm *kvm, struct net_dev *ndev)
 	if (r != 0)
 		die_perror("VHOST_SET_OWNER failed");
 
-	r = ioctl(ndev->vhost_fd, VHOST_SET_FEATURES, &features);
-	if (r != 0)
-		die_perror("VHOST_SET_FEATURES failed");
 	r = ioctl(ndev->vhost_fd, VHOST_SET_MEM_TABLE, mem);
 	if (r != 0)
 		die_perror("VHOST_SET_MEM_TABLE failed");
-- 
1.8.1.2

Re: [PATCH 0/2] kvm tools: fix vhost-net broken since fa7226f88012713a73d0cba4955444ea109e9458

[Pekka Enberg]

On 11/01/2013 04:50 AM, Ying-Shiuan Pan wrote:
> These patch series fix 2 vhost-net problems. There were 2 problems after
> commit fa7226f88012713a73d0cba4955444ea109e9458.
> (1) vhost-net requires tap_fd for VHOST_SET_BACKEND, but opening tap device
> was postponed to VIRTIO_CONFIG_S_DRIVER_OK. The wrong order of initialization
> caused vhost-net used a invalid tap_fd for ioctl.
> (2) virtio-net of kvm tool started to support mergeable rx buffers since
> commit 8c2684de6fb1afc95321e7e0f30550acdcc08186, but kvm tool did not tell
> vhost-net that he is using mergeable rx buffers.
>
> test script:
> $ sudo ./lkvm run --kernel ../../arch/x86/boot/bzImage --no-dhcp --net mode=tap,vhost=1
>
> Ying-Shiuan Pan (2):
>    kvm tools: virtio-net has to open tap device before vhost-net init.
>    kvm tools: vhost-net: setup mergeable rx buffers feature
>
>   tools/kvm/virtio/net.c | 54 +++++++++++++++++++++++++++-----------------------
>   1 file changed, 29 insertions(+), 25 deletions(-)
>

Asias, Sasha?