[PATCH 0/3] Implement link status for virtio_net

[PATCH 0/3] Implement link status for virtio_net

[Mark McLoughlin]

Hi,
        Here are the KVM specific changes for use to implement
virtio_net link status.

        These patches depend firstly on the 'set_link' patches
being accepted into qemu and secondly on the virtio_net link
status ABI being accepted.

Cheers,
Mark.

[PATCH 1/3] kvm: qemu: fix qemu_send_packet

[Mark McLoughlin]

KVM's qemu_send_packet() has a return value, qemu's doesn't.

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 qemu/net.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/qemu/net.c b/qemu/net.c
index 3154140..16a0990 100644
--- a/qemu/net.c
+++ b/qemu/net.c
@@ -362,7 +362,7 @@ int qemu_send_packet(VLANClientState *vc1, const uint8_t *buf, int size)
     int ret = -EAGAIN;
 
     if (vc1->link_down)
-        return;
+        return 0;
 
 #ifdef DEBUG_NET
     printf("vlan %d send:\n", vlan->id);
-- 
1.5.4.3

[PATCH 2/3] kvm: qemu: handle link status in qemu_sendv_packet()

[Mark McLoughlin]

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 qemu/net.c |   18 +++++++++++++++++-
 1 files changed, 17 insertions(+), 1 deletions(-)

diff --git a/qemu/net.c b/qemu/net.c
index 16a0990..f23a17f 100644
--- a/qemu/net.c
+++ b/qemu/net.c
@@ -400,6 +400,17 @@ static ssize_t vc_sendv_compat(VLANClientState *vc, const struct iovec *iov,
     return offset;
 }
 
+static ssize_t calc_iov_length(const struct iovec *iov, int iovcnt)
+{
+    size_t offset = 0;
+    int i;
+
+    for (i = 0; i < iovcnt; i++)
+	offset += iov[i].iov_len;
+
+    return offset;
+}
+
 ssize_t qemu_sendv_packet(VLANClientState *vc1, const struct iovec *iov,
 			  int iovcnt)
 {
@@ -407,13 +418,18 @@ ssize_t qemu_sendv_packet(VLANClientState *vc1, const struct iovec *iov,
     VLANClientState *vc;
     ssize_t max_len = 0;
 
+    if (vc1->link_down)
+	return calc_iov_length(iov, iovcnt);
+
     for (vc = vlan->first_client; vc != NULL; vc = vc->next) {
 	ssize_t len = 0;
 
 	if (vc == vc1)
 	    continue;
 
-	if (vc->fd_readv)
+	if (vc->link_down)
+	    len = calc_iov_length(iov, iovcnt);
+	else if (vc->fd_readv)
 	    len = vc->fd_readv(vc->opaque, iov, iovcnt);
 	else if (vc->fd_read)
 	    len = vc_sendv_compat(vc, iov, iovcnt);
-- 
1.5.4.3

[PATCH 3/3] kvm: qemu: virtio-net: implement link status

[Mark McLoughlin]

Implement the VIRTIO_NET_F_STATUS feature by exposing the link status
through virtio_net_config::status.

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 qemu/hw/virtio-net.c |   23 +++++++++++++++++++++--
 qemu/hw/virtio-net.h |    7 ++++++-
 2 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/qemu/hw/virtio-net.c b/qemu/hw/virtio-net.c
index 18da625..4d47422 100644
--- a/qemu/hw/virtio-net.c
+++ b/qemu/hw/virtio-net.c
@@ -25,6 +25,7 @@ typedef struct VirtIONet
 {
     VirtIODevice vdev;
     uint8_t mac[6];
+    uint16_t status;
     VirtQueue *rx_vq;
     VirtQueue *tx_vq;
     VLANClientState *vc;
@@ -47,13 +48,28 @@ static void virtio_net_update_config(VirtIODevice *vdev, uint8_t *config)
     VirtIONet *n = to_virtio_net(vdev);
     struct virtio_net_config netcfg;
 
+    netcfg.status = n->status;
     memcpy(netcfg.mac, n->mac, 6);
     memcpy(config, &netcfg, sizeof(netcfg));
 }
 
+static void virtio_net_set_link_status(VLANClientState *vc)
+{
+    VirtIONet *n = vc->opaque;
+    uint16_t old_status = n->status;
+
+    if (vc->link_down)
+        n->status &= ~VIRTIO_NET_S_LINK_UP;
+    else
+        n->status |= VIRTIO_NET_S_LINK_UP;
+
+    if (n->status != old_status)
+        virtio_notify_config(&n->vdev);
+}
+
 static uint32_t virtio_net_get_features(VirtIODevice *vdev)
 {
-    uint32_t features = (1 << VIRTIO_NET_F_MAC);
+    uint32_t features = (1 << VIRTIO_NET_F_MAC) | (1 << VIRTIO_NET_F_STATUS);
 #ifdef TAP_VNET_HDR
     VirtIONet *n = to_virtio_net(vdev);
     VLANClientState *host = n->vc->vlan->first_client;
@@ -390,7 +406,8 @@ PCIDevice *virtio_net_init(PCIBus *bus, NICInfo *nd, int devfn)
     n = (VirtIONet *)virtio_init_pci(bus, "virtio-net", 6900, 0x1000,
                                      0, VIRTIO_ID_NET,
                                      0x02, 0x00, 0x00,
-                                     6, sizeof(VirtIONet));
+                                     sizeof(struct virtio_net_config),
+                                     sizeof(VirtIONet));
     if (!n)
         return NULL;
 
@@ -400,8 +417,10 @@ PCIDevice *virtio_net_init(PCIBus *bus, NICInfo *nd, int devfn)
     n->rx_vq = virtio_add_queue(&n->vdev, 256, virtio_net_handle_rx);
     n->tx_vq = virtio_add_queue(&n->vdev, 256, virtio_net_handle_tx);
     memcpy(n->mac, nd->macaddr, 6);
+    n->status = VIRTIO_NET_S_LINK_UP;
     n->vc = qemu_new_vlan_client(nd->vlan, virtio_net_receive,
                                  virtio_net_can_receive, n);
+    n->vc->link_status_changed = virtio_net_set_link_status;
 
     snprintf(n->vc->info_str, sizeof(n->vc->info_str),
              "virtio macaddr=%02x:%02x:%02x:%02x:%02x:%02x",
diff --git a/qemu/hw/virtio-net.h b/qemu/hw/virtio-net.h
index 0d9f71b..9ac9e34 100644
--- a/qemu/hw/virtio-net.h
+++ b/qemu/hw/virtio-net.h
@@ -37,16 +37,21 @@
 #define VIRTIO_NET_F_HOST_ECN   13      /* Host can handle TSO[6] w/ ECN in. */
 #define VIRTIO_NET_F_HOST_UFO   14      /* Host can handle UFO in. */
 #define VIRTIO_NET_F_MRG_RXBUF  15      /* Host can merge receive buffers. */
+#define VIRTIO_NET_F_STATUS     16      /* virtio_net_config.status available */
+
+#define VIRTIO_NET_S_LINK_UP    1       /* Link is up */
 
 #define TX_TIMER_INTERVAL 150000 /* 150 us */
 
 /* Maximum packet size we can receive from tap device: header + 64k */
 #define VIRTIO_NET_MAX_BUFSIZE (sizeof(struct virtio_net_hdr) + (64 << 10))
 
-/* The config defining mac address (6 bytes) */
 struct virtio_net_config
 {
+    /* The config defining mac address (6 bytes) */
     uint8_t mac[6];
+    /* See VIRTIO_NET_F_STATUS and VIRTIO_NET_S_* above */
+    uint16_t status;
 } __attribute__((packed));
 
 /* This is the first element of the scatter-gather list.  If you don't
-- 
1.5.4.3

Re: [PATCH 2/3] kvm: qemu: handle link status in qemu_sendv_packet()

[Anthony Liguori]

Mark McLoughlin wrote:
> Signed-off-by: Mark McLoughlin <markmc@redhat.com>
> ---
>  qemu/net.c |   18 +++++++++++++++++-
>  1 files changed, 17 insertions(+), 1 deletions(-)
>
> diff --git a/qemu/net.c b/qemu/net.c
> index 16a0990..f23a17f 100644
> --- a/qemu/net.c
> +++ b/qemu/net.c
> @@ -400,6 +400,17 @@ static ssize_t vc_sendv_compat(VLANClientState *vc, const struct iovec *iov,
>      return offset;
>  }
>  
> +static ssize_t calc_iov_length(const struct iovec *iov, int iovcnt)
> +{
> +    size_t offset = 0;
> +    int i;
> +
> +    for (i = 0; i < iovcnt; i++)
> +	offset += iov[i].iov_len;
> +
> +    return offset;
> +}
> +
>  ssize_t qemu_sendv_packet(VLANClientState *vc1, const struct iovec *iov,
>  			  int iovcnt)
>  {
> @@ -407,13 +418,18 @@ ssize_t qemu_sendv_packet(VLANClientState *vc1, const struct iovec *iov,
>      VLANClientState *vc;
>      ssize_t max_len = 0;
>  
> +    if (vc1->link_down)
> +	return calc_iov_length(iov, iovcnt);
> +
>      for (vc = vlan->first_client; vc != NULL; vc = vc->next) {
>  	ssize_t len = 0;
>  
>  	if (vc == vc1)
>  	    continue;
>  
> -	if (vc->fd_readv)
> +	if (vc->link_down)
> +	    len = calc_iov_length(iov, iovcnt);
>   

Instead of returning a success and silently dropping the packet, maybe 
it would be better to return an error and let the card deal with 
dropping the packet.

In real hardware, the link down would mean the TX queue would fill up 
because packets aren't able to be sent.

Regards,

Anthony Liguori

> +	else if (vc->fd_readv)
>  	    len = vc->fd_readv(vc->opaque, iov, iovcnt);
>  	else if (vc->fd_read)
>  	    len = vc_sendv_compat(vc, iov, iovcnt);
>   

Re: [PATCH 2/3] kvm: qemu: handle link status in qemu_sendv_packet()

[Mark McLoughlin]

On Tue, 2008-12-09 at 08:57 -0600, Anthony Liguori wrote:
> Mark McLoughlin wrote:
> > Signed-off-by: Mark McLoughlin <markmc@redhat.com>
> > ---
> >  qemu/net.c |   18 +++++++++++++++++-
> >  1 files changed, 17 insertions(+), 1 deletions(-)
> >
> > diff --git a/qemu/net.c b/qemu/net.c
> > index 16a0990..f23a17f 100644
> > --- a/qemu/net.c
> > +++ b/qemu/net.c
> > @@ -400,6 +400,17 @@ static ssize_t vc_sendv_compat(VLANClientState *vc, const struct iovec *iov,
> >      return offset;
> >  }
> >  
> > +static ssize_t calc_iov_length(const struct iovec *iov, int iovcnt)
> > +{
> > +    size_t offset = 0;
> > +    int i;
> > +
> > +    for (i = 0; i < iovcnt; i++)
> > +	offset += iov[i].iov_len;
> > +
> > +    return offset;
> > +}
> > +
> >  ssize_t qemu_sendv_packet(VLANClientState *vc1, const struct iovec *iov,
> >  			  int iovcnt)
> >  {
> > @@ -407,13 +418,18 @@ ssize_t qemu_sendv_packet(VLANClientState *vc1, const struct iovec *iov,
> >      VLANClientState *vc;
> >      ssize_t max_len = 0;
> >  
> > +    if (vc1->link_down)
> > +	return calc_iov_length(iov, iovcnt);
> > +
> >      for (vc = vlan->first_client; vc != NULL; vc = vc->next) {
> >  	ssize_t len = 0;
> >  
> >  	if (vc == vc1)
> >  	    continue;
> >  
> > -	if (vc->fd_readv)
> > +	if (vc->link_down)
> > +	    len = calc_iov_length(iov, iovcnt);
> >   
> 
> Instead of returning a success and silently dropping the packet, maybe 
> it would be better to return an error and let the card deal with 
> dropping the packet.
> 
> In real hardware, the link down would mean the TX queue would fill up 
> because packets aren't able to be sent.

I failed to figure out for real what typical hardware would do, e.g. by
reading the e1000 spec, but I figure the current behaviour is good
enough for now?

Re-based patches follow.

Cheers,
Mark.

[Qemu-devel] [PATCH 1/4] Add 'set_link' monitor command

[Mark McLoughlin]

Add a monitor command to setting a given network device's link status
to 'up' or 'down'.

Allows simulation of network cable disconnect.

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 monitor.c |    1 +
 net.c     |   31 ++++++++++++++++++++++++++++++-
 net.h     |    2 ++
 3 files changed, 33 insertions(+), 1 deletions(-)

diff --git a/monitor.c b/monitor.c
index 745c335..7ff9890 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1516,6 +1516,7 @@ static const term_cmd_t term_cmds[] = {
       "value", "set maximum speed (in bytes) for migrations" },
     { "balloon", "i", do_balloon,
       "target", "request VM to change it's memory allocation (in MB)" },
+    { "set_link", "ss", do_set_link, "name [up|down]" },
     { NULL, NULL, },
 };
 
diff --git a/net.c b/net.c
index 4609be2..eef0564 100644
--- a/net.c
+++ b/net.c
@@ -386,12 +386,15 @@ void qemu_send_packet(VLANClientState *vc1, const uint8_t *buf, int size)
     VLANState *vlan = vc1->vlan;
     VLANClientState *vc;
 
+    if (vc1->link_down)
+        return;
+
 #ifdef DEBUG_NET
     printf("vlan %d send:\n", vlan->id);
     hex_dump(stdout, buf, size);
 #endif
     for(vc = vlan->first_client; vc != NULL; vc = vc->next) {
-        if (vc != vc1) {
+        if (vc != vc1 && !vc->link_down) {
             vc->fd_read(vc->opaque, buf, size);
         }
     }
@@ -1717,6 +1720,32 @@ void do_info_network(void)
     }
 }
 
+int do_set_link(const char *name, const char *up_or_down)
+{
+    VLANState *vlan;
+    VLANClientState *vc = NULL;
+
+    for (vlan = first_vlan; vlan != NULL; vlan = vlan->next)
+        for (vc = vlan->first_client; vc != NULL; vc = vc->next)
+            if (strcmp(vc->name, name) == 0)
+                break;
+
+    if (!vc) {
+        term_printf("could not find network device '%s'", name);
+        return 0;
+    }
+
+    if (strcmp(up_or_down, "up") == 0)
+        vc->link_down = 0;
+    else if (strcmp(up_or_down, "down") == 0)
+        vc->link_down = 1;
+    else
+        term_printf("invalid link status '%s'; only 'up' or 'down' valid\n",
+                    up_or_down);
+
+    return 1;
+}
+
 void net_cleanup(void)
 {
     VLANState *vlan;
diff --git a/net.h b/net.h
index 19ab649..3c0dab9 100644
--- a/net.h
+++ b/net.h
@@ -15,6 +15,7 @@ struct VLANClientState {
     /* Packets may still be sent if this returns zero.  It's used to
        rate-limit the slirp code.  */
     IOCanRWHandler *fd_can_read;
+    int link_down;
     void *opaque;
     struct VLANClientState *next;
     struct VLANState *vlan;
@@ -49,6 +50,7 @@ void qemu_check_nic_model_list(NICInfo *nd, const char * const *models,
 void qemu_handler_true(void *opaque);
 
 void do_info_network(void);
+int do_set_link(const char *name, const char *up_or_down);
 
 /* NIC info */
 
-- 
1.6.0.6

[Qemu-devel] [PATCH 2/4] Allow devices be notified of link status change

[Mark McLoughlin]

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 net.c |    3 +++
 net.h |    3 +++
 2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/net.c b/net.c
index eef0564..0a182b2 100644
--- a/net.c
+++ b/net.c
@@ -1743,6 +1743,9 @@ int do_set_link(const char *name, const char *up_or_down)
         term_printf("invalid link status '%s'; only 'up' or 'down' valid\n",
                     up_or_down);
 
+    if (vc->link_status_changed)
+        vc->link_status_changed(vc);
+
     return 1;
 }
 
diff --git a/net.h b/net.h
index 3c0dab9..291807a 100644
--- a/net.h
+++ b/net.h
@@ -9,12 +9,15 @@ typedef ssize_t (IOReadvHandler)(void *, const struct iovec *, int);
 
 typedef struct VLANClientState VLANClientState;
 
+typedef void (LinkStatusChanged)(VLANClientState *);
+
 struct VLANClientState {
     IOReadHandler *fd_read;
     IOReadvHandler *fd_readv;
     /* Packets may still be sent if this returns zero.  It's used to
        rate-limit the slirp code.  */
     IOCanRWHandler *fd_can_read;
+    LinkStatusChanged *link_status_changed;
     int link_down;
     void *opaque;
     struct VLANClientState *next;
-- 
1.6.0.6

[Qemu-devel] [PATCH 3/4] Implement e1000 link status

[Mark McLoughlin]

On link up or down we set the E1000_STATUS_LU ("link up") bit
in the status register and set the E1000_ICR_LSC ("link
status changed") bit in the interrupt cause register before
interrupting the guest.

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 hw/e1000.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/hw/e1000.c b/hw/e1000.c
index 09cb96a..ccf9bc0 100644
--- a/hw/e1000.c
+++ b/hw/e1000.c
@@ -570,6 +570,21 @@ receive_filter(E1000State *s, const uint8_t *buf, int size)
     return 0;
 }
 
+static void
+e1000_set_link_status(VLANClientState *vc)
+{
+    E1000State *s = vc->opaque;
+    uint32_t old_status = s->mac_reg[STATUS];
+
+    if (vc->link_down)
+        s->mac_reg[STATUS] &= ~E1000_STATUS_LU;
+    else
+        s->mac_reg[STATUS] |= E1000_STATUS_LU;
+
+    if (s->mac_reg[STATUS] != old_status)
+        set_ics(s, 0, E1000_ICR_LSC);
+}
+
 static int
 e1000_can_receive(void *opaque)
 {
@@ -1073,6 +1088,7 @@ pci_e1000_init(PCIBus *bus, NICInfo *nd, int devfn)
 
     d->vc = qemu_new_vlan_client(nd->vlan, nd->model, nd->name,
                                  e1000_receive, e1000_can_receive, d);
+    d->vc->link_status_changed = e1000_set_link_status;
 
     qemu_format_nic_info_str(d->vc, d->nd->macaddr);
 
-- 
1.6.0.6

[Qemu-devel] [PATCH 4/4] Implement virtio_net link status

[Mark McLoughlin]

Implement the VIRTIO_NET_F_STATUS feature by exposing the link status
through virtio_net_config::status.

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 hw/virtio-net.c |   23 +++++++++++++++++++++--
 hw/virtio-net.h |    7 ++++++-
 2 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 43fde20..54c0030 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -20,6 +20,7 @@ typedef struct VirtIONet
 {
     VirtIODevice vdev;
     uint8_t mac[6];
+    uint16_t status;
     VirtQueue *rx_vq;
     VirtQueue *tx_vq;
     VLANClientState *vc;
@@ -42,13 +43,28 @@ static void virtio_net_update_config(VirtIODevice *vdev, uint8_t *config)
     VirtIONet *n = to_virtio_net(vdev);
     struct virtio_net_config netcfg;
 
+    netcfg.status = n->status;
     memcpy(netcfg.mac, n->mac, 6);
     memcpy(config, &netcfg, sizeof(netcfg));
 }
 
+static void virtio_net_set_link_status(VLANClientState *vc)
+{
+    VirtIONet *n = vc->opaque;
+    uint16_t old_status = n->status;
+
+    if (vc->link_down)
+        n->status &= ~VIRTIO_NET_S_LINK_UP;
+    else
+        n->status |= VIRTIO_NET_S_LINK_UP;
+
+    if (n->status != old_status)
+        virtio_notify_config(&n->vdev);
+}
+
 static uint32_t virtio_net_get_features(VirtIODevice *vdev)
 {
-    uint32_t features = (1 << VIRTIO_NET_F_MAC);
+    uint32_t features = (1 << VIRTIO_NET_F_MAC) | (1 << VIRTIO_NET_F_STATUS);
 
     return features;
 }
@@ -307,7 +323,8 @@ void virtio_net_init(PCIBus *bus, NICInfo *nd, int devfn)
     n = (VirtIONet *)virtio_init_pci(bus, "virtio-net", 6900, 0x1000,
                                      0, VIRTIO_ID_NET,
                                      0x02, 0x00, 0x00,
-                                     6, sizeof(VirtIONet));
+                                     sizeof(struct virtio_net_config),
+                                     sizeof(VirtIONet));
     if (!n)
         return;
 
@@ -317,8 +334,10 @@ void virtio_net_init(PCIBus *bus, NICInfo *nd, int devfn)
     n->rx_vq = virtio_add_queue(&n->vdev, 256, virtio_net_handle_rx);
     n->tx_vq = virtio_add_queue(&n->vdev, 256, virtio_net_handle_tx);
     memcpy(n->mac, nd->macaddr, 6);
+    n->status = VIRTIO_NET_S_LINK_UP;
     n->vc = qemu_new_vlan_client(nd->vlan, nd->model, nd->name,
                                  virtio_net_receive, virtio_net_can_receive, n);
+    n->vc->link_status_changed = virtio_net_set_link_status;
 
     qemu_format_nic_info_str(n->vc, n->mac);
 
diff --git a/hw/virtio-net.h b/hw/virtio-net.h
index 7446b11..148ec47 100644
--- a/hw/virtio-net.h
+++ b/hw/virtio-net.h
@@ -37,16 +37,21 @@
 #define VIRTIO_NET_F_HOST_ECN   13      /* Host can handle TSO[6] w/ ECN in. */
 #define VIRTIO_NET_F_HOST_UFO   14      /* Host can handle UFO in. */
 #define VIRTIO_NET_F_MRG_RXBUF  15      /* Host can merge receive buffers. */
+#define VIRTIO_NET_F_STATUS     16      /* virtio_net_config.status available */
+
+#define VIRTIO_NET_S_LINK_UP    1       /* Link is up */
 
 #define TX_TIMER_INTERVAL 150000 /* 150 us */
 
 /* Maximum packet size we can receive from tap device: header + 64k */
 #define VIRTIO_NET_MAX_BUFSIZE (sizeof(struct virtio_net_hdr) + (64 << 10))
 
-/* The config defining mac address (6 bytes) */
 struct virtio_net_config
 {
+    /* The config defining mac address (6 bytes) */
     uint8_t mac[6];
+    /* See VIRTIO_NET_F_STATUS and VIRTIO_NET_S_* above */
+    uint16_t status;
 } __attribute__((packed));
 
 /* This is the first element of the scatter-gather list.  If you don't
-- 
1.6.0.6

[Qemu-devel] Re: [PATCH 1/4] Add 'set_link' monitor command

[Anthony Liguori]

Mark McLoughlin wrote:
> Add a monitor command to setting a given network device's link status
> to 'up' or 'down'.
>
> Allows simulation of network cable disconnect.
>
> Signed-off-by: Mark McLoughlin <markmc@redhat.com>
> ---
>
>   
Applied all four.  Thanks.

Regards,

Anthony Liguori