diff mbox

[3/5,RFC] virtio-net: Name the status bits, adding promisc and allmulti

Message ID 1231349859.7109.82.camel@lappy (mailing list archive)
State Accepted, archived
Headers show

Commit Message

Alex Williamson Jan. 7, 2009, 5:37 p.m. UTC 
virtio-net: Name the status bits, adding promisc and allmulti

Signed-off-by: Alex Williamson <alex.williamson@hp.com>
---

 hw/virtio-net.c |   36 ++++++++++++++++++++++++------------
 hw/virtio-net.h |   11 ++++++++++-
 2 files changed, 34 insertions(+), 13 deletions(-)



--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Anthony Liguori Jan. 7, 2009, 6:09 p.m. UTC | #1 
Alex Williamson wrote:
> virtio-net: Name the status bits, adding promisc and allmulti
>
> Signed-off-by: Alex Williamson <alex.williamson@hp.com>
> ---
>
>  hw/virtio-net.c |   36 ++++++++++++++++++++++++------------
>  hw/virtio-net.h |   11 ++++++++++-
>  2 files changed, 34 insertions(+), 13 deletions(-)
>
> diff --git a/hw/virtio-net.c b/hw/virtio-net.c
> index 77e3077..653cad4 100644
> --- a/hw/virtio-net.c
> +++ b/hw/virtio-net.c
> @@ -22,7 +22,14 @@ typedef struct VirtIONet
>  {
>      VirtIODevice vdev;
>      uint8_t mac[6];
> -    uint16_t status;
> +    union {
> +        uint16_t raw;
> +        struct {
> +            uint16_t link:1;
> +            uint16_t promisc:1;
> +            uint16_t allmulti:1;
> +        } bits;
> +    } status;
>   

I'd prefer the use of #define's like we have today.  bit fields have 
really weird packing and ordering properties across architectures.

Regards,

Anthony Liguori

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alex Williamson Jan. 7, 2009, 6:14 p.m. UTC | #2 
On Wed, 2009-01-07 at 12:09 -0600, Anthony Liguori wrote:
> Alex Williamson wrote:
> > virtio-net: Name the status bits, adding promisc and allmulti
> >   
> 
> I'd prefer the use of #define's like we have today.  bit fields have 
> really weird packing and ordering properties across architectures.

Ok, it made a few things easier, but I'll work on using a mask
interface.  Thanks,

Alex
diff mbox

Patch

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 77e3077..653cad4 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -22,7 +22,14 @@  typedef struct VirtIONet
 {
     VirtIODevice vdev;
     uint8_t mac[6];
-    uint16_t status;
+    union {
+        uint16_t raw;
+        struct {
+            uint16_t link:1;
+            uint16_t promisc:1;
+            uint16_t allmulti:1;
+        } bits;
+    } status;
     VirtQueue *rx_vq;
     VirtQueue *tx_vq;
     VLANClientState *vc;
@@ -45,7 +52,7 @@  static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
     VirtIONet *n = to_virtio_net(vdev);
     struct virtio_net_config netcfg;
 
-    netcfg.status = n->status;
+    netcfg.status.raw = n->status.raw;
     memcpy(netcfg.mac, n->mac, 6);
     memcpy(config, &netcfg, sizeof(netcfg));
 }
@@ -64,20 +71,23 @@  static void virtio_net_set_config(VirtIODevice *vdev, const uint8_t *config)
                  n->mac[0], n->mac[1], n->mac[2],
                  n->mac[3], n->mac[4], n->mac[5]);
     }
+
+    if (netcfg.status.raw != n->status.raw) {
+	if (netcfg.status.bits.promisc != n->status.bits.promisc)
+            n->status.bits.promisc = netcfg.status.bits.promisc;
+	if (netcfg.status.bits.allmulti != n->status.bits.allmulti)
+            n->status.bits.allmulti = netcfg.status.bits.allmulti;
+    }
 }
 
 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)
+    if (n->status.bits.link != !(vc->link_down)) {
+	n->status.bits.link = !(vc->link_down);
         virtio_notify_config(&n->vdev);
+    }
 }
 
 static uint32_t virtio_net_get_features(VirtIODevice *vdev)
@@ -309,7 +319,7 @@  static void virtio_net_save(QEMUFile *f, void *opaque)
 
     qemu_put_buffer(f, n->mac, 6);
     qemu_put_be32(f, n->tx_timer_active);
-    qemu_put_be16(f, n->status);
+    qemu_put_be16(f, n->status.raw);
 }
 
 static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
@@ -325,7 +335,9 @@  static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
     n->tx_timer_active = qemu_get_be32(f);
 
     if (version_id >= 2)
-        n->status = qemu_get_be16(f);
+        n->status.raw = qemu_get_be16(f);
+    else
+        n->status.raw |= (VIRTIO_NET_S_PROMISC | VIRTIO_NET_S_ALLMULTI);
 
     if (n->tx_timer_active) {
         qemu_mod_timer(n->tx_timer,
@@ -355,7 +367,7 @@  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->status.raw = 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;
diff --git a/hw/virtio-net.h b/hw/virtio-net.h
index 9ac9e34..74f1595 100644
--- a/hw/virtio-net.h
+++ b/hw/virtio-net.h
@@ -40,6 +40,8 @@ 
 #define VIRTIO_NET_F_STATUS     16      /* virtio_net_config.status available */
 
 #define VIRTIO_NET_S_LINK_UP    1       /* Link is up */
+#define VIRTIO_NET_S_PROMISC    2       /* Promiscuous mode */
+#define VIRTIO_NET_S_ALLMULTI   4       /* All-multicast mode */
 
 #define TX_TIMER_INTERVAL 150000 /* 150 us */
 
@@ -51,7 +53,14 @@  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;
+    union {
+        uint16_t raw;
+        struct {
+            uint16_t link:1;
+            uint16_t promisc:1;
+            uint16_t allmulti:1;
+        } bits;
+    } status;
 } __attribute__((packed));
 
 /* This is the first element of the scatter-gather list.  If you don't