[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.