[Qemu-devel] [PATCH] vhost: disable on tap link down

[Qemu-devel] [PATCH] vhost: disable on tap link down

[Michael S. Tsirkin]

qemu makes it possible to disable link at tap
which is not communicated to the guest but
causes all packets to be dropped.

Handle this with vhost simply by moving to the userspace emulation.

Note: it might be a good idea to make peer link status match
tap in this case, so the guest gets an event
and updates the carrier state. For now
stay bug for bug compatible with what we used to have.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reported-by: pradeep <psuriset@linux.vnet.ibm.com>
---

Untested.
Pradeep, mind trying this patch out and reporting?
Thanks!

 hw/virtio-net.c |    4 ++++
 net.c           |    7 +++++++
 2 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 671d952..fc2d6f5 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -112,6 +112,10 @@ static void virtio_net_vhost_status(VirtIONet *n, uint8_t status)
         return;
     }
 
+    if (n->nic->nc.peer->link_down) {
+        return;
+    }
+
     if (!tap_get_vhost_net(n->nic->nc.peer)) {
         return;
     }
diff --git a/net.c b/net.c
index 9ba5be2..57ee997 100644
--- a/net.c
+++ b/net.c
@@ -1324,6 +1324,13 @@ done:
     if (vc->info->link_status_changed) {
         vc->info->link_status_changed(vc);
     }
+
+    /* Notify peer. Don't update peer link status: this makes it possible to
+     * disconnect from host network without notifying the guest.
+     * FIXME: is this useful? Could just be an artifact of vlan support. */
+    if (vc->peer && vc->peer->info->link_status_changed) {
+        vc->peer->info->link_status_changed(vc->peer);
+    }
     return 0;
 }
 
-- 
1.7.3.2.91.g446ac

[Qemu-devel] Re: [PATCH] vhost: disable on tap link down

[pradeep]

On Mon, 7 Feb 2011 15:50:01 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> qemu makes it possible to disable link at tap
> which is not communicated to the guest but
> causes all packets to be dropped.
> 
> Handle this with vhost simply by moving to the userspace emulation.
> 
> Note: it might be a good idea to make peer link status match
> tap in this case, so the guest gets an event
> and updates the carrier state. For now
> stay bug for bug compatible with what we used to have.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Reported-by: pradeep <psuriset@linux.vnet.ibm.com>
> ---
> 
> Untested.
> Pradeep, mind trying this patch out and reporting?

Hi mst

This patch works. Thanks

--Pradeep


> Thanks!
> 
>  hw/virtio-net.c |    4 ++++
>  net.c           |    7 +++++++
>  2 files changed, 11 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/virtio-net.c b/hw/virtio-net.c
> index 671d952..fc2d6f5 100644
> --- a/hw/virtio-net.c
> +++ b/hw/virtio-net.c
> @@ -112,6 +112,10 @@ static void virtio_net_vhost_status(VirtIONet
> *n, uint8_t status) return;
>      }
> 
> +    if (n->nic->nc.peer->link_down) {
> +        return;
> +    }
> +
>      if (!tap_get_vhost_net(n->nic->nc.peer)) {
>          return;
>      }
> diff --git a/net.c b/net.c
> index 9ba5be2..57ee997 100644
> --- a/net.c
> +++ b/net.c
> @@ -1324,6 +1324,13 @@ done:
>      if (vc->info->link_status_changed) {
>          vc->info->link_status_changed(vc);
>      }
> +
> +    /* Notify peer. Don't update peer link status: this makes it
> possible to
> +     * disconnect from host network without notifying the guest.
> +     * FIXME: is this useful? Could just be an artifact of vlan
> support. */
> +    if (vc->peer && vc->peer->info->link_status_changed) {
> +        vc->peer->info->link_status_changed(vc->peer);
> +    }
>      return 0;
>  }
> 

[Qemu-devel] Re: [PATCH] vhost: disable on tap link down

[Michael S. Tsirkin]

On Tue, Feb 08, 2011 at 05:40:58PM +0530, pradeep wrote:
> On Mon, 7 Feb 2011 15:50:01 +0200
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > qemu makes it possible to disable link at tap
> > which is not communicated to the guest but
> > causes all packets to be dropped.
> > 
> > Handle this with vhost simply by moving to the userspace emulation.
> > 
> > Note: it might be a good idea to make peer link status match
> > tap in this case, so the guest gets an event
> > and updates the carrier state. For now
> > stay bug for bug compatible with what we used to have.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > Reported-by: pradeep <psuriset@linux.vnet.ibm.com>
> > ---
> > 
> > Untested.
> > Pradeep, mind trying this patch out and reporting?
> 
> Hi mst
> 
> This patch works. Thanks

Strange actually. Did you put the link down before guest booted?  I went
to test it with set link after guest is up, and it didn't work, I needed
this on top - can you ack this in your setup as well pls?

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index fc2d6f5..3e3d73a 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -112,14 +112,11 @@ static void virtio_net_vhost_status(VirtIONet *n, uint8_t status)
         return;
     }
 
-    if (n->nic->nc.peer->link_down) {
-        return;
-    }
-
     if (!tap_get_vhost_net(n->nic->nc.peer)) {
         return;
     }
-    if (!!n->vhost_started == virtio_net_started(n, status)) {
+    if (!!n->vhost_started == virtio_net_started(n, status) &&
+        !n->nic->nc.peer->link_down) {
         return;
     }
     if (!n->vhost_started) {

[Qemu-devel] Re: [PATCH] vhost: disable on tap link down

[pradeep]

On Tue, 8 Feb 2011 17:41:12 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Tue, Feb 08, 2011 at 05:40:58PM +0530, pradeep wrote:
> > On Mon, 7 Feb 2011 15:50:01 +0200
> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > 
> > > qemu makes it possible to disable link at tap
> > > which is not communicated to the guest but
> > > causes all packets to be dropped.
> > > 
> > > Handle this with vhost simply by moving to the userspace
> > > emulation.
> > > 
> > > Note: it might be a good idea to make peer link status match
> > > tap in this case, so the guest gets an event
> > > and updates the carrier state. For now
> > > stay bug for bug compatible with what we used to have.
> > > 
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > Reported-by: pradeep <psuriset@linux.vnet.ibm.com>
> > > ---
> > > 
> > > Untested.
> > > Pradeep, mind trying this patch out and reporting?
> > 
> > Hi mst
> > 
> > This patch works. Thanks
> 
> Strange actually. Did you put the link down before guest booted?  I
> went to test it with set link after guest is up, and it didn't work,
> I needed this on top - can you ack this in your setup as well pls?

ahhhh. Old one dint.  New patch works for me. 
Thanks mst

--Pradeep


> 
> diff --git a/hw/virtio-net.c b/hw/virtio-net.c
> index fc2d6f5..3e3d73a 100644
> --- a/hw/virtio-net.c
> +++ b/hw/virtio-net.c
> @@ -112,14 +112,11 @@ static void virtio_net_vhost_status(VirtIONet
> *n, uint8_t status) return;
>      }
> 
> -    if (n->nic->nc.peer->link_down) {
> -        return;
> -    }
> -
>      if (!tap_get_vhost_net(n->nic->nc.peer)) {
>          return;
>      }
> -    if (!!n->vhost_started == virtio_net_started(n, status)) {
> +    if (!!n->vhost_started == virtio_net_started(n, status) &&
> +        !n->nic->nc.peer->link_down) {
>          return;
>      }
>      if (!n->vhost_started) {