[Qemu-devel] [PATCH v2 0/8] qemu:virtio-net: Add MAC and VLAN filtering

[Qemu-devel] [PATCH v2 0/8] qemu:virtio-net: Add MAC and VLAN filtering

[Alex Williamson]

This series adds the ability for the guest to set the virtio-net device
MAC address, a new control virtqueue for setting configuration data from
the guest, and new interfaces making use of the control virtqueue for
setting RX mode options, MAC filter table entries, and VLAN filter bits.
The end result is that the virtio-net device has more of the features of
a real hardware NIC and can be configured to drop packets the guest
isn't interested in seeing.

This version is based on revision 6501 of the qemu tree and matches v4
of the Linux guest virtio_net changes posted to the kvm and netdev
mailing lists.  The guest patch to support setting the MAC address
(via config write) is currently in Rusty's patch queue.  Note
that this series adds some infrastructure around the virtio-net save/load
functions and increments through a couple save revisions.  It would
probably be worthwile to synchronize virtio-net before applying this
since a qemu rev 2 save image does not currently match a kvm rev 2 save
image.  Thanks,

Alex

---

Alex Williamson (8):
      qemu:virtio-net: Add VLAN filtering
      qemu:virtio-net: Add additional MACs via a filter table
      qemu:virtio-net: Enable filtering based on MAC, promisc, broadcast and allmulti
      qemu:virtio-net: Add promiscuous and all-multicast mode bits
      qemu:virtio-net: Add a virtqueue for control commands from the guest
      qemu:virtio-net: Define ETH_ALEN for use when manipulating MAC addresses
      qemu:virtio-net: Allow setting the MAC address via set_config
      qemu:virtio-net: Save status and add some save infrastructure


 hw/virtio-net.c |  275 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
 hw/virtio-net.h |   67 +++++++++++++
 2 files changed, 332 insertions(+), 10 deletions(-)

-- 
Alex Williamson

[Qemu-devel] [PATCH v2 1/8] qemu:virtio-net: Save status and add some save infrastructure

[Alex Williamson]

The status register should probably be saved since its guest visible.
Also add a little bit if infrastructure for handling various save
revisions.

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

 hw/virtio-net.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 9e40678..2eb52b8 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -16,6 +16,8 @@
 #include "qemu-timer.h"
 #include "virtio-net.h"
 
+#define VIRTIO_NET_VM_VERSION    3
+
 typedef struct VirtIONet
 {
     VirtIODevice vdev;
@@ -292,13 +294,14 @@ 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_be32(f, n->mergeable_rx_bufs);
+    qemu_put_be16(f, n->status);
 }
 
 static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
 {
     VirtIONet *n = opaque;
 
-    if (version_id != 2)
+    if (version_id < 2 || version_id > VIRTIO_NET_VM_VERSION)
         return -EINVAL;
 
     virtio_load(&n->vdev, f);
@@ -307,6 +310,9 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
     n->tx_timer_active = qemu_get_be32(f);
     n->mergeable_rx_bufs = qemu_get_be32(f);
 
+    if (version_id >= 3)
+        n->status = qemu_get_be16(f);
+
     if (n->tx_timer_active) {
         qemu_mod_timer(n->tx_timer,
                        qemu_get_clock(vm_clock) + TX_TIMER_INTERVAL);
@@ -348,6 +354,6 @@ void virtio_net_init(PCIBus *bus, NICInfo *nd, int devfn)
     n->tx_timer_active = 0;
     n->mergeable_rx_bufs = 0;
 
-    register_savevm("virtio-net", virtio_net_id++, 2,
+    register_savevm("virtio-net", virtio_net_id++, VIRTIO_NET_VM_VERSION,
                     virtio_net_save, virtio_net_load, n);
 }

[Qemu-devel] [PATCH v2 2/8] qemu:virtio-net: Allow setting the MAC address via set_config

[Alex Williamson]

Allow the guest to write to the MAC address config space and update
the network info string when it does.  Rename get_config for symmetry.

Signed-off-by: Alex Williamson <alex.williamson@hp.com>
Acked-by: Mark McLoughlin <markmc@redhat.com>
---

 hw/virtio-net.c |   18 ++++++++++++++++--
 1 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 2eb52b8..105daa9 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -40,7 +40,7 @@ static VirtIONet *to_virtio_net(VirtIODevice *vdev)
     return (VirtIONet *)vdev;
 }
 
-static void virtio_net_update_config(VirtIODevice *vdev, uint8_t *config)
+static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
 {
     VirtIONet *n = to_virtio_net(vdev);
     struct virtio_net_config netcfg;
@@ -50,6 +50,19 @@ static void virtio_net_update_config(VirtIODevice *vdev, uint8_t *config)
     memcpy(config, &netcfg, sizeof(netcfg));
 }
 
+static void virtio_net_set_config(VirtIODevice *vdev, const uint8_t *config)
+{
+    VirtIONet *n = to_virtio_net(vdev);
+    struct virtio_net_config netcfg;
+
+    memcpy(&netcfg, config, sizeof(netcfg));
+
+    if (memcmp(netcfg.mac, n->mac, 6)) {
+        memcpy(n->mac, netcfg.mac, 6);
+        qemu_format_nic_info_str(n->vc, n->mac);
+    }
+}
+
 static void virtio_net_set_link_status(VLANClientState *vc)
 {
     VirtIONet *n = vc->opaque;
@@ -337,7 +350,8 @@ void virtio_net_init(PCIBus *bus, NICInfo *nd, int devfn)
     if (!n)
         return;
 
-    n->vdev.get_config = virtio_net_update_config;
+    n->vdev.get_config = virtio_net_get_config;
+    n->vdev.set_config = virtio_net_set_config;
     n->vdev.get_features = virtio_net_get_features;
     n->vdev.set_features = virtio_net_set_features;
     n->rx_vq = virtio_add_queue(&n->vdev, 256, virtio_net_handle_rx);

[Qemu-devel] [PATCH v2 3/8] qemu:virtio-net: Define ETH_ALEN for use when manipulating MAC addresses

[Alex Williamson]

Makes it much easier to search too.

Signed-off-by: Alex Williamson <alex.williamson@hp.com>
Acked-by: Mark McLoughlin <markmc@redhat.com>
---

 hw/virtio-net.c |   14 +++++++-------
 hw/virtio-net.h |    2 ++
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 105daa9..073f23a 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -21,7 +21,7 @@
 typedef struct VirtIONet
 {
     VirtIODevice vdev;
-    uint8_t mac[6];
+    uint8_t mac[ETH_ALEN];
     uint16_t status;
     VirtQueue *rx_vq;
     VirtQueue *tx_vq;
@@ -46,7 +46,7 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
     struct virtio_net_config netcfg;
 
     netcfg.status = n->status;
-    memcpy(netcfg.mac, n->mac, 6);
+    memcpy(netcfg.mac, n->mac, ETH_ALEN);
     memcpy(config, &netcfg, sizeof(netcfg));
 }
 
@@ -57,8 +57,8 @@ static void virtio_net_set_config(VirtIODevice *vdev, const uint8_t *config)
 
     memcpy(&netcfg, config, sizeof(netcfg));
 
-    if (memcmp(netcfg.mac, n->mac, 6)) {
-        memcpy(n->mac, netcfg.mac, 6);
+    if (memcmp(netcfg.mac, n->mac, ETH_ALEN)) {
+        memcpy(n->mac, netcfg.mac, ETH_ALEN);
         qemu_format_nic_info_str(n->vc, n->mac);
     }
 }
@@ -304,7 +304,7 @@ static void virtio_net_save(QEMUFile *f, void *opaque)
 
     virtio_save(&n->vdev, f);
 
-    qemu_put_buffer(f, n->mac, 6);
+    qemu_put_buffer(f, n->mac, ETH_ALEN);
     qemu_put_be32(f, n->tx_timer_active);
     qemu_put_be32(f, n->mergeable_rx_bufs);
     qemu_put_be16(f, n->status);
@@ -319,7 +319,7 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
 
     virtio_load(&n->vdev, f);
 
-    qemu_get_buffer(f, n->mac, 6);
+    qemu_get_buffer(f, n->mac, ETH_ALEN);
     n->tx_timer_active = qemu_get_be32(f);
     n->mergeable_rx_bufs = qemu_get_be32(f);
 
@@ -356,7 +356,7 @@ void virtio_net_init(PCIBus *bus, NICInfo *nd, int devfn)
     n->vdev.set_features = virtio_net_set_features;
     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);
+    memcpy(n->mac, nd->macaddr, ETH_ALEN);
     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);
diff --git a/hw/virtio-net.h b/hw/virtio-net.h
index 148ec47..9dce663 100644
--- a/hw/virtio-net.h
+++ b/hw/virtio-net.h
@@ -18,6 +18,8 @@
 #include "net.h"
 #include "pci.h"
 
+#define ETH_ALEN    6
+
 /* from Linux's virtio_net.h */
 
 /* The ID for virtio_net */

[Qemu-devel] [PATCH v2 4/8] qemu:virtio-net: Add a virtqueue for control commands from the guest

[Alex Williamson]

This will be used for RX mode, MAC table, VLAN table control, etc...

The control transaction consists of one or more "out" sg entries and
one or more "in" sg entries.  The first out entry contains a header
defining the class and command.  Additional out entries may provide
data for the command.  A response via the ack entry is required
and the guest will typically be waiting for it.

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

 hw/virtio-net.c |   33 ++++++++++++++++++++++++++++++++-
 hw/virtio-net.h |   18 ++++++++++++++++++
 2 files changed, 50 insertions(+), 1 deletions(-)

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 073f23a..3378cc2 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -25,6 +25,7 @@ typedef struct VirtIONet
     uint16_t status;
     VirtQueue *rx_vq;
     VirtQueue *tx_vq;
+    VirtQueue *ctrl_vq;
     VLANClientState *vc;
     QEMUTimer *tx_timer;
     int tx_timer_active;
@@ -79,7 +80,9 @@ static void virtio_net_set_link_status(VLANClientState *vc)
 
 static uint32_t virtio_net_get_features(VirtIODevice *vdev)
 {
-    uint32_t features = (1 << VIRTIO_NET_F_MAC) | (1 << VIRTIO_NET_F_STATUS);
+    uint32_t features = (1 << VIRTIO_NET_F_MAC) |
+                        (1 << VIRTIO_NET_F_STATUS) |
+                        (1 << VIRTIO_NET_F_CTRL_VQ);
 
     return features;
 }
@@ -91,6 +94,33 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint32_t features)
     n->mergeable_rx_bufs = !!(features & (1 << VIRTIO_NET_F_MRG_RXBUF));
 }
 
+static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
+{
+    struct virtio_net_ctrl_hdr *ctrl;
+    virtio_net_ctrl_ack *status;
+    VirtQueueElement elem;
+
+    while (virtqueue_pop(vq, &elem)) {
+        if ((elem.in_num < 1) | (elem.out_num < 1)) {
+            fprintf(stderr, "virtio-net ctrl missing headers\n");
+            exit(1);
+        }
+
+        if (elem.out_sg[0].iov_len < sizeof(*ctrl) ||
+            elem.out_sg[elem.in_num - 1].iov_len < sizeof(*status)) {
+            fprintf(stderr, "virtio-net ctrl header not in correct element\n");
+            exit(1);
+        }
+
+        ctrl = elem.out_sg[0].iov_base;
+        status = elem.in_sg[elem.in_num - 1].iov_base;
+        *status = VIRTIO_NET_ERR;
+
+        virtqueue_push(vq, &elem, sizeof(*status));
+        virtio_notify(vdev, vq);
+    }
+}
+
 /* RX */
 
 static void virtio_net_handle_rx(VirtIODevice *vdev, VirtQueue *vq)
@@ -356,6 +386,7 @@ void virtio_net_init(PCIBus *bus, NICInfo *nd, int devfn)
     n->vdev.set_features = virtio_net_set_features;
     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);
+    n->ctrl_vq = virtio_add_queue(&n->vdev, 16, virtio_net_handle_ctrl);
     memcpy(n->mac, nd->macaddr, ETH_ALEN);
     n->status = VIRTIO_NET_S_LINK_UP;
     n->vc = qemu_new_vlan_client(nd->vlan, nd->model, nd->name,
diff --git a/hw/virtio-net.h b/hw/virtio-net.h
index 9dce663..119e38d 100644
--- a/hw/virtio-net.h
+++ b/hw/virtio-net.h
@@ -40,6 +40,7 @@
 #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_F_CTRL_VQ    17      /* Control channel available */
 
 #define VIRTIO_NET_S_LINK_UP    1       /* Link is up */
 
@@ -84,4 +85,21 @@ struct virtio_net_hdr_mrg_rxbuf
 
 void virtio_net_init(PCIBus *bus, NICInfo *nd, int devfn);
 
+/*
+ * Control virtqueue data structures
+ *
+ * The control virtqueue expects a header in the first sg entry
+ * and an ack/status response in the last entry.  Data for the
+ * command goes in between.
+ */
+struct virtio_net_ctrl_hdr {
+    uint8_t class;
+    uint8_t cmd;
+};
+
+typedef uint8_t virtio_net_ctrl_ack;
+
+#define VIRTIO_NET_OK     0
+#define VIRTIO_NET_ERR    1
+
 #endif

[Qemu-devel] [PATCH v2 5/8] qemu:virtio-net: Add promiscuous and all-multicast mode bits

[Alex Williamson]

Add a new RX_MODE control virtqueue class with commands PROMISC and
ALLMULTI and usage documented in virtio-net.h allowing the guest to
manipulate packet receiving options.  We don't export a feature for
this until we also add the MAC filter table.

Note, for compatibility with older guest drivers we need to default
to promiscuous.

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

 hw/virtio-net.c |   48 +++++++++++++++++++++++++++++++++++++++++++++++-
 hw/virtio-net.h |   11 +++++++++++
 2 files changed, 58 insertions(+), 1 deletions(-)

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 3378cc2..e8ca80b 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -16,7 +16,7 @@
 #include "qemu-timer.h"
 #include "virtio-net.h"
 
-#define VIRTIO_NET_VM_VERSION    3
+#define VIRTIO_NET_VM_VERSION    4
 
 typedef struct VirtIONet
 {
@@ -30,6 +30,8 @@ typedef struct VirtIONet
     QEMUTimer *tx_timer;
     int tx_timer_active;
     int mergeable_rx_bufs;
+    int promisc;
+    int allmulti;
 } VirtIONet;
 
 /* TODO
@@ -78,6 +80,15 @@ static void virtio_net_set_link_status(VLANClientState *vc)
         virtio_notify_config(&n->vdev);
 }
 
+static void virtio_net_reset(VirtIODevice *vdev)
+{
+    VirtIONet *n = to_virtio_net(vdev);
+
+    /* Reset back to compatibility mode */
+    n->promisc = 1;
+    n->allmulti = 0;
+}
+
 static uint32_t virtio_net_get_features(VirtIODevice *vdev)
 {
     uint32_t features = (1 << VIRTIO_NET_F_MAC) |
@@ -94,8 +105,31 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint32_t features)
     n->mergeable_rx_bufs = !!(features & (1 << VIRTIO_NET_F_MRG_RXBUF));
 }
 
+static int virtio_net_handle_rx_mode(VirtIONet *n, uint8_t cmd,
+                                     VirtQueueElement *elem)
+{
+    uint8_t *on;
+
+    if (elem->out_num != 2 || elem->out_sg[1].iov_len != sizeof(*on)) {
+        fprintf(stderr, "virtio-net ctrl invalid rx mode command\n");
+        exit(1);
+    }
+
+    on = elem->out_sg[1].iov_base;
+
+    if (cmd == VIRTIO_NET_CTRL_RX_MODE_PROMISC)
+        n->promisc = *on;
+    else if (cmd == VIRTIO_NET_CTRL_RX_MODE_ALLMULTI)
+        n->allmulti = *on;
+    else
+        return VIRTIO_NET_ERR;
+
+    return VIRTIO_NET_OK;
+}
+
 static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
 {
+    VirtIONet *n = to_virtio_net(vdev);
     struct virtio_net_ctrl_hdr *ctrl;
     virtio_net_ctrl_ack *status;
     VirtQueueElement elem;
@@ -116,6 +150,9 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
         status = elem.in_sg[elem.in_num - 1].iov_base;
         *status = VIRTIO_NET_ERR;
 
+        if (ctrl->class == VIRTIO_NET_CTRL_RX_MODE)
+            *status = virtio_net_handle_rx_mode(n, ctrl->cmd, &elem);
+
         virtqueue_push(vq, &elem, sizeof(*status));
         virtio_notify(vdev, vq);
     }
@@ -338,6 +375,8 @@ static void virtio_net_save(QEMUFile *f, void *opaque)
     qemu_put_be32(f, n->tx_timer_active);
     qemu_put_be32(f, n->mergeable_rx_bufs);
     qemu_put_be16(f, n->status);
+    qemu_put_be32(f, n->promisc);
+    qemu_put_be32(f, n->allmulti);
 }
 
 static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
@@ -356,6 +395,11 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
     if (version_id >= 3)
         n->status = qemu_get_be16(f);
 
+    if (version_id >= 4) {
+        n->promisc = qemu_get_be32(f);
+        n->allmulti = qemu_get_be32(f);
+    }
+
     if (n->tx_timer_active) {
         qemu_mod_timer(n->tx_timer,
                        qemu_get_clock(vm_clock) + TX_TIMER_INTERVAL);
@@ -384,6 +428,7 @@ void virtio_net_init(PCIBus *bus, NICInfo *nd, int devfn)
     n->vdev.set_config = virtio_net_set_config;
     n->vdev.get_features = virtio_net_get_features;
     n->vdev.set_features = virtio_net_set_features;
+    n->vdev.reset = virtio_net_reset;
     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);
     n->ctrl_vq = virtio_add_queue(&n->vdev, 16, virtio_net_handle_ctrl);
@@ -398,6 +443,7 @@ void virtio_net_init(PCIBus *bus, NICInfo *nd, int devfn)
     n->tx_timer = qemu_new_timer(vm_clock, virtio_net_tx_timer, n);
     n->tx_timer_active = 0;
     n->mergeable_rx_bufs = 0;
+    n->promisc = 1; /* for compatibility */
 
     register_savevm("virtio-net", virtio_net_id++, VIRTIO_NET_VM_VERSION,
                     virtio_net_save, virtio_net_load, n);
diff --git a/hw/virtio-net.h b/hw/virtio-net.h
index 119e38d..ce8e4b4 100644
--- a/hw/virtio-net.h
+++ b/hw/virtio-net.h
@@ -41,6 +41,7 @@
 #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_F_CTRL_VQ    17      /* Control channel available */
+#define VIRTIO_NET_F_CTRL_RX    18      /* Control channel RX mode support */
 
 #define VIRTIO_NET_S_LINK_UP    1       /* Link is up */
 
@@ -102,4 +103,14 @@ typedef uint8_t virtio_net_ctrl_ack;
 #define VIRTIO_NET_OK     0
 #define VIRTIO_NET_ERR    1
 
+/*
+ * Control the RX mode, ie. promisucous and allmulti.  PROMISC and
+ * ALLMULTI commands require an "out" sg entry containing a 1 byte
+ * state value, zero = disable, non-zero = enable.  These commands
+ * are supported with the VIRTIO_NET_F_CTRL_RX feature.
+ */
+#define VIRTIO_NET_CTRL_RX_MODE    0
+ #define VIRTIO_NET_CTRL_RX_MODE_PROMISC      0
+ #define VIRTIO_NET_CTRL_RX_MODE_ALLMULTI     1
+
 #endif

[Qemu-devel] [PATCH v2 6/8] qemu:virtio-net: Enable filtering based on MAC, promisc, broadcast and allmulti

[Alex Williamson]

Make use of the new RX_MODE control virtqueue class by dropping
packets the guest doesn't want to see.

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

 hw/virtio-net.c |   28 ++++++++++++++++++++++++++++
 1 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index e8ca80b..d01da22 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -221,6 +221,31 @@ static int receive_header(VirtIONet *n, struct iovec *iov, int iovcnt,
     return offset;
 }
 
+static int receive_filter(VirtIONet *n, const uint8_t *buf, int size)
+{
+    static uint8_t bcast[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
+    uint8_t *ptr = (uint8_t *)buf;
+
+    if (n->promisc)
+        return 1;
+
+#ifdef TAP_VNET_HDR
+    if (tap_has_vnet_hdr(n->vc->vlan->first_client))
+        ptr += sizeof(struct virtio_net_hdr);
+#endif
+
+    if ((ptr[0] & 1) && n->allmulti)
+        return 1;
+
+    if (!memcmp(ptr, bcast, sizeof(bcast)))
+        return 1;
+
+    if (!memcmp(ptr, n->mac, ETH_ALEN))
+        return 1;
+
+    return 0;
+}
+
 static void virtio_net_receive(void *opaque, const uint8_t *buf, int size)
 {
     VirtIONet *n = opaque;
@@ -230,6 +255,9 @@ static void virtio_net_receive(void *opaque, const uint8_t *buf, int size)
     if (!do_virtio_net_can_receive(n, size))
         return;
 
+    if (!receive_filter(n, buf, size))
+        return;
+
     /* hdr_len refers to the header we supply to the guest */
     hdr_len = n->mergeable_rx_bufs ?
         sizeof(struct virtio_net_hdr_mrg_rxbuf) : sizeof(struct virtio_net_hdr);

[Qemu-devel] [PATCH v2 7/8] qemu:virtio-net: Add additional MACs via a filter table

[Alex Williamson]

Create a filter table and allow the guest to populate it with the
MAC class control commands.  We manage the size and usage of the
filter table including enabling promiscuous and all-multi modes
as necessary.  The guest should therefore assume the table is
infinite.  Eventually this might allow us to bind directly to a
hardware NIC and manipulate a physical MAC filter.

The specifics of the TABLE_SET command are documented in
virtio-net.h.  Separate buffers in the same command are used
for unicaste and multicast addresses for priority and
sychronization.  With this we can export the VIRTIO_NET_F_CTRL_RX
feature bit.

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

 hw/virtio-net.c |   88 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 hw/virtio-net.h |   22 ++++++++++++++
 2 files changed, 108 insertions(+), 2 deletions(-)

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index d01da22..40050ff 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -16,7 +16,9 @@
 #include "qemu-timer.h"
 #include "virtio-net.h"
 
-#define VIRTIO_NET_VM_VERSION    4
+#define VIRTIO_NET_VM_VERSION    5
+
+#define MAC_TABLE_ENTRIES    32
 
 typedef struct VirtIONet
 {
@@ -32,6 +34,10 @@ typedef struct VirtIONet
     int mergeable_rx_bufs;
     int promisc;
     int allmulti;
+    struct {
+        int in_use;
+        uint8_t *macs;
+    } mac_table;
 } VirtIONet;
 
 /* TODO
@@ -87,13 +93,18 @@ static void virtio_net_reset(VirtIODevice *vdev)
     /* Reset back to compatibility mode */
     n->promisc = 1;
     n->allmulti = 0;
+
+    /* Flush any MAC filter table state */
+    n->mac_table.in_use = 0;
+    memset(n->mac_table.macs, 0, MAC_TABLE_ENTRIES * ETH_ALEN);
 }
 
 static uint32_t virtio_net_get_features(VirtIODevice *vdev)
 {
     uint32_t features = (1 << VIRTIO_NET_F_MAC) |
                         (1 << VIRTIO_NET_F_STATUS) |
-                        (1 << VIRTIO_NET_F_CTRL_VQ);
+                        (1 << VIRTIO_NET_F_CTRL_VQ) |
+                        (1 << VIRTIO_NET_F_CTRL_RX);
 
     return features;
 }
@@ -127,6 +138,52 @@ static int virtio_net_handle_rx_mode(VirtIONet *n, uint8_t cmd,
     return VIRTIO_NET_OK;
 }
 
+static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
+                                 VirtQueueElement *elem)
+{
+    struct virtio_net_ctrl_mac *mac_data;
+
+    if (cmd != VIRTIO_NET_CTRL_MAC_TABLE_SET || elem->out_num != 3 ||
+        elem->out_sg[1].iov_len < sizeof(mac_data->entries) ||
+        elem->out_sg[2].iov_len < sizeof(mac_data->entries))
+        return VIRTIO_NET_ERR;
+
+    n->mac_table.in_use = 0;
+    memset(n->mac_table.macs, 0, MAC_TABLE_ENTRIES * ETH_ALEN);
+
+    mac_data = (void *)elem->out_sg[1].iov_base;
+
+    if (sizeof(mac_data->entries) +
+        (mac_data->entries * ETH_ALEN) > elem->out_sg[1].iov_len)
+        return VIRTIO_NET_ERR;
+
+    if (mac_data->entries <= MAC_TABLE_ENTRIES) {
+        memcpy(n->mac_table.macs, &mac_data->macs,
+               mac_data->entries * ETH_ALEN);
+        n->mac_table.in_use += mac_data->entries;
+    } else {
+        n->promisc = 1;
+        return VIRTIO_NET_OK;
+    }
+
+    mac_data = (void *)elem->out_sg[2].iov_base;
+
+    if (sizeof(mac_data->entries) +
+        (mac_data->entries * ETH_ALEN) > elem->out_sg[2].iov_len)
+        return VIRTIO_NET_ERR;
+
+    if (mac_data->entries) {
+        if (n->mac_table.in_use + mac_data->entries <= MAC_TABLE_ENTRIES) {
+            memcpy(n->mac_table.macs + (n->mac_table.in_use * ETH_ALEN),
+                   &mac_data->macs, mac_data->entries * ETH_ALEN);
+            n->mac_table.in_use += mac_data->entries;
+        } else
+            n->allmulti = 1;
+    }
+
+    return VIRTIO_NET_OK;
+}
+
 static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
 {
     VirtIONet *n = to_virtio_net(vdev);
@@ -152,6 +209,8 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
 
         if (ctrl->class == VIRTIO_NET_CTRL_RX_MODE)
             *status = virtio_net_handle_rx_mode(n, ctrl->cmd, &elem);
+        else if (ctrl->class == VIRTIO_NET_CTRL_MAC)
+            *status = virtio_net_handle_mac(n, ctrl->cmd, &elem);
 
         virtqueue_push(vq, &elem, sizeof(*status));
         virtio_notify(vdev, vq);
@@ -225,6 +284,7 @@ static int receive_filter(VirtIONet *n, const uint8_t *buf, int size)
 {
     static uint8_t bcast[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
     uint8_t *ptr = (uint8_t *)buf;
+    int i;
 
     if (n->promisc)
         return 1;
@@ -243,6 +303,11 @@ static int receive_filter(VirtIONet *n, const uint8_t *buf, int size)
     if (!memcmp(ptr, n->mac, ETH_ALEN))
         return 1;
 
+    for (i = 0; i < n->mac_table.in_use; i++) {
+        if (!memcmp(ptr, &n->mac_table.macs[i * ETH_ALEN], ETH_ALEN))
+            return 1;
+    }
+
     return 0;
 }
 
@@ -405,6 +470,8 @@ static void virtio_net_save(QEMUFile *f, void *opaque)
     qemu_put_be16(f, n->status);
     qemu_put_be32(f, n->promisc);
     qemu_put_be32(f, n->allmulti);
+    qemu_put_be32(f, n->mac_table.in_use);
+    qemu_put_buffer(f, n->mac_table.macs, n->mac_table.in_use * ETH_ALEN);
 }
 
 static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
@@ -428,6 +495,19 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
         n->allmulti = qemu_get_be32(f);
     }
 
+    if (version_id >= 5) {
+        n->mac_table.in_use = qemu_get_be32(f);
+        /* MAC_TABLE_ENTRIES may be different from the saved image */
+        if (n->mac_table.in_use <= MAC_TABLE_ENTRIES) {
+            qemu_get_buffer(f, n->mac_table.macs,
+                            n->mac_table.in_use * ETH_ALEN);
+        } else if (n->mac_table.in_use) {
+            qemu_fseek(f, n->mac_table.in_use * ETH_ALEN, SEEK_CUR);
+            n->promisc = 1;
+            n->mac_table.in_use = 0;
+        }
+    }
+ 
     if (n->tx_timer_active) {
         qemu_mod_timer(n->tx_timer,
                        qemu_get_clock(vm_clock) + TX_TIMER_INTERVAL);
@@ -473,6 +553,10 @@ void virtio_net_init(PCIBus *bus, NICInfo *nd, int devfn)
     n->mergeable_rx_bufs = 0;
     n->promisc = 1; /* for compatibility */
 
+    n->mac_table.macs = qemu_mallocz(MAC_TABLE_ENTRIES * ETH_ALEN);
+    if (!n->mac_table.macs)
+        return;
+
     register_savevm("virtio-net", virtio_net_id++, VIRTIO_NET_VM_VERSION,
                     virtio_net_save, virtio_net_load, n);
 }
diff --git a/hw/virtio-net.h b/hw/virtio-net.h
index ce8e4b4..291fa9d 100644
--- a/hw/virtio-net.h
+++ b/hw/virtio-net.h
@@ -113,4 +113,26 @@ typedef uint8_t virtio_net_ctrl_ack;
  #define VIRTIO_NET_CTRL_RX_MODE_PROMISC      0
  #define VIRTIO_NET_CTRL_RX_MODE_ALLMULTI     1
 
+/*
+ * Control the MAC filter table.
+ *
+ * The MAC filter table is managed by the hypervisor, the guest should
+ * assume the size is infinite.  Filtering should be considered
+ * non-perfect, ie. based on hypervisor resources, the guest may
+ * received packets from sources not specified in the filter list.
+ *
+ * In addition to the class/cmd header, the TABLE_SET command requires
+ * two out scatterlists.  Each contains a 4 byte count of entries followed
+ * by a concatenated byte stream of the ETH_ALEN MAC addresses.  The
+ * first sg list contains unicast addresses, the second is for multicast.
+ * This functionality is present if the VIRTIO_NET_F_CTRL_RX feature
+ * is available.
+ */
+struct virtio_net_ctrl_mac {
+    uint32_t entries;
+    uint8_t macs[][ETH_ALEN];
+};
+#define VIRTIO_NET_CTRL_MAC    1
+ #define VIRTIO_NET_CTRL_MAC_TABLE_SET        0
+
 #endif

[Qemu-devel] [PATCH v2 8/8] qemu:virtio-net: Add VLAN filtering

[Alex Williamson]

Use the control virtqueue to allow the guest to enable and manipulate
a VLAN filter table.  This allows us to drop more packets the guest
doesn't want to see.  We define a new VLAN class for the control
virtqueue with commands ADD and DEL with usage defined in virtio-net.h.

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

 hw/virtio-net.c |   52 +++++++++++++++++++++++++++++++++++++++++++++++++---
 hw/virtio-net.h |   14 ++++++++++++++
 2 files changed, 63 insertions(+), 3 deletions(-)

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 40050ff..308cec7 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -16,9 +16,10 @@
 #include "qemu-timer.h"
 #include "virtio-net.h"
 
-#define VIRTIO_NET_VM_VERSION    5
+#define VIRTIO_NET_VM_VERSION    6
 
 #define MAC_TABLE_ENTRIES    32
+#define MAX_VLAN    (1 << 12)   /* Per 802.1Q definition */
 
 typedef struct VirtIONet
 {
@@ -38,6 +39,7 @@ typedef struct VirtIONet
         int in_use;
         uint8_t *macs;
     } mac_table;
+    uint32_t *vlans;
 } VirtIONet;
 
 /* TODO
@@ -94,9 +96,10 @@ static void virtio_net_reset(VirtIODevice *vdev)
     n->promisc = 1;
     n->allmulti = 0;
 
-    /* Flush any MAC filter table state */
+    /* Flush any MAC and VLAN filter table state */
     n->mac_table.in_use = 0;
     memset(n->mac_table.macs, 0, MAC_TABLE_ENTRIES * ETH_ALEN);
+    memset(n->vlans, 0, MAX_VLAN >> 3);
 }
 
 static uint32_t virtio_net_get_features(VirtIODevice *vdev)
@@ -104,7 +107,8 @@ static uint32_t virtio_net_get_features(VirtIODevice *vdev)
     uint32_t features = (1 << VIRTIO_NET_F_MAC) |
                         (1 << VIRTIO_NET_F_STATUS) |
                         (1 << VIRTIO_NET_F_CTRL_VQ) |
-                        (1 << VIRTIO_NET_F_CTRL_RX);
+                        (1 << VIRTIO_NET_F_CTRL_RX) |
+                        (1 << VIRTIO_NET_F_CTRL_VLAN);
 
     return features;
 }
@@ -184,6 +188,31 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
     return VIRTIO_NET_OK;
 }
 
+static int virtio_net_handle_vlan_table(VirtIONet *n, uint8_t cmd,
+                                        VirtQueueElement *elem)
+{
+    uint16_t *vid;
+
+    if (elem->out_num != 2 || elem->out_sg[1].iov_len != sizeof(*vid)) {
+        fprintf(stderr, "virtio-net ctrl invalid vlan command\n");
+        return VIRTIO_NET_ERR;
+    }
+
+    vid = elem->out_sg[1].iov_base;
+
+    if (*vid >= MAX_VLAN)
+        return VIRTIO_NET_ERR;
+
+    if (cmd == VIRTIO_NET_CTRL_VLAN_ADD)
+        n->vlans[*vid >> 5] |= (1U << (*vid & 0x1f));
+    else if (cmd == VIRTIO_NET_CTRL_VLAN_DEL)
+        n->vlans[*vid >> 5] &= ~(1U << (*vid & 0x1f));
+    else
+        return VIRTIO_NET_ERR;
+
+    return VIRTIO_NET_OK;
+}
+
 static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
 {
     VirtIONet *n = to_virtio_net(vdev);
@@ -211,6 +240,8 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
             *status = virtio_net_handle_rx_mode(n, ctrl->cmd, &elem);
         else if (ctrl->class == VIRTIO_NET_CTRL_MAC)
             *status = virtio_net_handle_mac(n, ctrl->cmd, &elem);
+        else if (ctrl->class == VIRTIO_NET_CTRL_VLAN)
+            *status = virtio_net_handle_vlan_table(n, ctrl->cmd, &elem);
 
         virtqueue_push(vq, &elem, sizeof(*status));
         virtio_notify(vdev, vq);
@@ -283,6 +314,7 @@ static int receive_header(VirtIONet *n, struct iovec *iov, int iovcnt,
 static int receive_filter(VirtIONet *n, const uint8_t *buf, int size)
 {
     static uint8_t bcast[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
+    static uint8_t vlan[] = {0x81, 0x00};
     uint8_t *ptr = (uint8_t *)buf;
     int i;
 
@@ -294,6 +326,12 @@ static int receive_filter(VirtIONet *n, const uint8_t *buf, int size)
         ptr += sizeof(struct virtio_net_hdr);
 #endif
 
+    if (!memcmp(&ptr[12], vlan, sizeof(vlan))) {
+        int vid = be16_to_cpup((uint16_t *)(ptr + 14)) & 0xfff;
+        if (!(n->vlans[vid >> 5] & (1U << (vid & 0x1f))))
+            return 0;
+    }
+
     if ((ptr[0] & 1) && n->allmulti)
         return 1;
 
@@ -472,6 +510,7 @@ static void virtio_net_save(QEMUFile *f, void *opaque)
     qemu_put_be32(f, n->allmulti);
     qemu_put_be32(f, n->mac_table.in_use);
     qemu_put_buffer(f, n->mac_table.macs, n->mac_table.in_use * ETH_ALEN);
+    qemu_put_buffer(f, (uint8_t *)n->vlans, MAX_VLAN >> 3);
 }
 
 static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
@@ -508,6 +547,9 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
         }
     }
  
+    if (version_id >= 6)
+        qemu_get_buffer(f, (uint8_t *)n->vlans, MAX_VLAN >> 3);
+
     if (n->tx_timer_active) {
         qemu_mod_timer(n->tx_timer,
                        qemu_get_clock(vm_clock) + TX_TIMER_INTERVAL);
@@ -557,6 +599,10 @@ void virtio_net_init(PCIBus *bus, NICInfo *nd, int devfn)
     if (!n->mac_table.macs)
         return;
 
+    n->vlans = qemu_mallocz(MAX_VLAN >> 3);
+    if (!n->vlans)
+        return;
+
     register_savevm("virtio-net", virtio_net_id++, VIRTIO_NET_VM_VERSION,
                     virtio_net_save, virtio_net_load, n);
 }
diff --git a/hw/virtio-net.h b/hw/virtio-net.h
index 291fa9d..95587f7 100644
--- a/hw/virtio-net.h
+++ b/hw/virtio-net.h
@@ -42,6 +42,7 @@
 #define VIRTIO_NET_F_STATUS     16      /* virtio_net_config.status available */
 #define VIRTIO_NET_F_CTRL_VQ    17      /* Control channel available */
 #define VIRTIO_NET_F_CTRL_RX    18      /* Control channel RX mode support */
+#define VIRTIO_NET_F_CTRL_VLAN  19      /* Control channel VLAN filtering */
 
 #define VIRTIO_NET_S_LINK_UP    1       /* Link is up */
 
@@ -135,4 +136,17 @@ struct virtio_net_ctrl_mac {
 #define VIRTIO_NET_CTRL_MAC    1
  #define VIRTIO_NET_CTRL_MAC_TABLE_SET        0
 
+/*
+ * Control VLAN filtering
+ *
+ * The VLAN filter table is controlled via a simple ADD/DEL interface.
+ * VLAN IDs not added may be filterd by the hypervisor.  Del is the
+ * opposite of add.  Both commands expect an out entry containing a 2
+ * byte VLAN ID.  VLAN filterting is available with the
+ * VIRTIO_NET_F_CTRL_VLAN feature bit.
+ */
+#define VIRTIO_NET_CTRL_VLAN       2
+ #define VIRTIO_NET_CTRL_VLAN_ADD             0
+ #define VIRTIO_NET_CTRL_VLAN_DEL             1
+
 #endif

Re: [Qemu-devel] [PATCH v2 6/8] qemu:virtio-net: Enable filtering based on MAC, promisc, broadcast and allmulti

[Blue Swirl]

On 2/3/09, Alex Williamson <alex.williamson@hp.com> wrote:
> Make use of the new RX_MODE control virtqueue class by dropping
>  packets the guest doesn't want to see.
>
>  Signed-off-by: Alex Williamson <alex.williamson@hp.com>

>  +    static uint8_t bcast[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};

'const'?

Re: [Qemu-devel] [PATCH v2 6/8] qemu:virtio-net: Enable filtering based on MAC, promisc, broadcast and allmulti

[Anthony Liguori]

Blue Swirl wrote:
> On 2/3/09, Alex Williamson <alex.williamson@hp.com> wrote:
>   
>> Make use of the new RX_MODE control virtqueue class by dropping
>>  packets the guest doesn't want to see.
>>
>>  Signed-off-by: Alex Williamson <alex.williamson@hp.com>
>>     
>
>   
>>  +    static uint8_t bcast[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
>>     
>
> 'const'?
>   

It may be good practice but practically speaking, I don't think it will 
generate different code.  The only thing that uses bcast is memcmp() 
which should be a builtin.  Since bcast doesn't otherwise escape the 
function, it's an obvious candidate for optimization.  I think the 
compiler already has enough information that const doesn't tell it 
anything more.

Regards,

Anthony Liguori

[Qemu-devel] Re: [PATCH v2 4/8] qemu:virtio-net: Add a virtqueue for control commands from the guest

[Anthony Liguori]

Alex Williamson wrote:
> This will be used for RX mode, MAC table, VLAN table control, etc...
>
> The control transaction consists of one or more "out" sg entries and
> one or more "in" sg entries.  The first out entry contains a header
> defining the class and command.  Additional out entries may provide
> data for the command.  A response via the ack entry is required
> and the guest will typically be waiting for it.
>
> Signed-off-by: Alex Williamson <alex.williamson@hp.com>
> ---
>
>  hw/virtio-net.c |   33 ++++++++++++++++++++++++++++++++-
>  hw/virtio-net.h |   18 ++++++++++++++++++
>  2 files changed, 50 insertions(+), 1 deletions(-)
>
> diff --git a/hw/virtio-net.c b/hw/virtio-net.c
> index 073f23a..3378cc2 100644
> --- a/hw/virtio-net.c
> +++ b/hw/virtio-net.c
> @@ -25,6 +25,7 @@ typedef struct VirtIONet
>      uint16_t status;
>      VirtQueue *rx_vq;
>      VirtQueue *tx_vq;
> +    VirtQueue *ctrl_vq;
>      VLANClientState *vc;
>      QEMUTimer *tx_timer;
>      int tx_timer_active;
> @@ -79,7 +80,9 @@ static void virtio_net_set_link_status(VLANClientState *vc)
>  
>  static uint32_t virtio_net_get_features(VirtIODevice *vdev)
>  {
> -    uint32_t features = (1 << VIRTIO_NET_F_MAC) | (1 << VIRTIO_NET_F_STATUS);
> +    uint32_t features = (1 << VIRTIO_NET_F_MAC) |
> +                        (1 << VIRTIO_NET_F_STATUS) |
> +                        (1 << VIRTIO_NET_F_CTRL_VQ);
>  
>      return features;
>  }
> @@ -91,6 +94,33 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint32_t features)
>      n->mergeable_rx_bufs = !!(features & (1 << VIRTIO_NET_F_MRG_RXBUF));
>  }
>  
> +static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
> +{
> +    struct virtio_net_ctrl_hdr *ctrl;
> +    virtio_net_ctrl_ack *status;
> +    VirtQueueElement elem;
> +
> +    while (virtqueue_pop(vq, &elem)) {
> +        if ((elem.in_num < 1) | (elem.out_num < 1)) {
> +            fprintf(stderr, "virtio-net ctrl missing headers\n");
> +            exit(1);
> +        }
> +
> +        if (elem.out_sg[0].iov_len < sizeof(*ctrl) ||
> +            elem.out_sg[elem.in_num - 1].iov_len < sizeof(*status)) {
> +            fprintf(stderr, "virtio-net ctrl header not in correct element\n");
> +            exit(1);
> +        }
> +
> +        ctrl = elem.out_sg[0].iov_base;
> +        status = elem.in_sg[elem.in_num - 1].iov_base;
>   

This works only because we're dealing with uint8_ts but it's broken 
elsewhere in virtio-net.  iov_base is guest memory which means it may 
have different endianness.  We really should be using ldub_p() here.

> +        *status = VIRTIO_NET_ERR;
>   

And stb_p() here.

Regards,

Anthony Liguori

> +        virtqueue_push(vq, &elem, sizeof(*status));
> +        virtio_notify(vdev, vq);
> +    }
> +}
> +
>  /* RX */
>  
>  static void virtio_net_handle_rx(VirtIODevice *vdev, VirtQueue *vq)
> @@ -356,6 +386,7 @@ void virtio_net_init(PCIBus *bus, NICInfo *nd, int devfn)
>      n->vdev.set_features = virtio_net_set_features;
>      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);
> +    n->ctrl_vq = virtio_add_queue(&n->vdev, 16, virtio_net_handle_ctrl);
>      memcpy(n->mac, nd->macaddr, ETH_ALEN);
>      n->status = VIRTIO_NET_S_LINK_UP;
>      n->vc = qemu_new_vlan_client(nd->vlan, nd->model, nd->name,
> diff --git a/hw/virtio-net.h b/hw/virtio-net.h
> index 9dce663..119e38d 100644
> --- a/hw/virtio-net.h
> +++ b/hw/virtio-net.h
> @@ -40,6 +40,7 @@
>  #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_F_CTRL_VQ    17      /* Control channel available */
>  
>  #define VIRTIO_NET_S_LINK_UP    1       /* Link is up */
>  
> @@ -84,4 +85,21 @@ struct virtio_net_hdr_mrg_rxbuf
>  
>  void virtio_net_init(PCIBus *bus, NICInfo *nd, int devfn);
>  
> +/*
> + * Control virtqueue data structures
> + *
> + * The control virtqueue expects a header in the first sg entry
> + * and an ack/status response in the last entry.  Data for the
> + * command goes in between.
> + */
> +struct virtio_net_ctrl_hdr {
> +    uint8_t class;
> +    uint8_t cmd;
> +};
> +
> +typedef uint8_t virtio_net_ctrl_ack;
> +
> +#define VIRTIO_NET_OK     0
> +#define VIRTIO_NET_ERR    1
> +
>  #endif
>
>   

Re: [Qemu-devel] [PATCH v2 6/8] qemu:virtio-net: Enable filtering based on MAC, promisc, broadcast and allmulti

[Paul Brook]

> >>  +    static uint8_t bcast[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
> >
> > 'const'?
>
> It may be good practice but practically speaking, I don't think it will
> generate different code.  The only thing that uses bcast is memcmp()
> which should be a builtin.  Since bcast doesn't otherwise escape the
> function, it's an obvious candidate for optimization.  I think the
> compiler already has enough information that const doesn't tell it
> anything more.

const is as much for the programmer as the compiler[1]. Modifying this 
variable is just plain wrong, so it's a good idea to let the compiler enforce 
that.

Paul

[1] In many cases (especially when combined with pointers) "const" doesn't 
actually mean what may people think it does. Ask your local C language expert 
if you don't know what I'm talking about.

Re: [Qemu-devel] [PATCH v2 6/8] qemu:virtio-net: Enable filtering based on MAC, promisc, broadcast and allmulti

[malc]

On Tue, 3 Feb 2009, Anthony Liguori wrote:

> Blue Swirl wrote:
> > On 2/3/09, Alex Williamson <alex.williamson@hp.com> wrote:
> >   
> > > Make use of the new RX_MODE control virtqueue class by dropping
> > >  packets the guest doesn't want to see.
> > > 
> > >  Signed-off-by: Alex Williamson <alex.williamson@hp.com>
> > >     
> > 
> >   
> > >  +    static uint8_t bcast[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
> > >     
> > 
> > 'const'?
> >   
> 
> It may be good practice but practically speaking, I don't think it will
> generate different code.  The only thing that uses bcast is memcmp() which
> should be a builtin.  Since bcast doesn't otherwise escape the function, it's
> an obvious candidate for optimization.  I think the compiler already has
> enough information that const doesn't tell it anything more.

It does generate different code, bcast is being put into rodata in case
of const and data otherwise, whether it makes practical difference for
QEMU is another matter.

-- 
mailto:av1474@comtv.ru

[Qemu-devel] Re: [PATCH v2 4/8] qemu:virtio-net: Add a virtqueue for control commands from the guest

[Alex Williamson]

On Tue, 2009-02-03 at 14:56 -0600, Anthony Liguori wrote:
> Alex Williamson wrote:
> > +        ctrl = elem.out_sg[0].iov_base;
> > +        status = elem.in_sg[elem.in_num - 1].iov_base;
> >   
> 
> This works only because we're dealing with uint8_ts but it's broken 
> elsewhere in virtio-net.  iov_base is guest memory which means it may 
> have different endianness.  We really should be using ldub_p() here.
> 
> > +        *status = VIRTIO_NET_ERR;
> >   
> 
> And stb_p() here.

Thanks Anthony.  So before I patch bomb again, I think this turns into
something like below.  This makes the struct definitions less useful on
the backend side, but I'm not sure we can do anything about that.  I
assume for u16s and u32s used in other parts of the interface, we'll
define those as little endian, which means the guest side driver
eventually needs a follow-up patch for big endian archs.  Thanks,

Alex


qemu:virtio-net: Add a virtqueue for control commands from the guest

This will be used for RX mode, MAC table, VLAN table control, etc...

The control transaction consists of one or more "out" sg entries and
one or more "in" sg entries.  The first out entry contains a header
defining the class and command.  Additional out entries may provide
data for the command.  A response via the ack entry is required
and the guest will typically be waiting for it.

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

 hw/virtio-net.c |   34 +++++++++++++++++++++++++++++++++-
 hw/virtio-net.h |   18 ++++++++++++++++++
 2 files changed, 51 insertions(+), 1 deletions(-)


diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 073f23a..7971f95 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -25,6 +25,7 @@ typedef struct VirtIONet
     uint16_t status;
     VirtQueue *rx_vq;
     VirtQueue *tx_vq;
+    VirtQueue *ctrl_vq;
     VLANClientState *vc;
     QEMUTimer *tx_timer;
     int tx_timer_active;
@@ -79,7 +80,9 @@ static void virtio_net_set_link_status(VLANClientState *vc)
 
 static uint32_t virtio_net_get_features(VirtIODevice *vdev)
 {
-    uint32_t features = (1 << VIRTIO_NET_F_MAC) | (1 << VIRTIO_NET_F_STATUS);
+    uint32_t features = (1 << VIRTIO_NET_F_MAC) |
+                        (1 << VIRTIO_NET_F_STATUS) |
+                        (1 << VIRTIO_NET_F_CTRL_VQ);
 
     return features;
 }
@@ -91,6 +94,34 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint32_t features)
     n->mergeable_rx_bufs = !!(features & (1 << VIRTIO_NET_F_MRG_RXBUF));
 }
 
+static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
+{
+    struct virtio_net_ctrl_hdr ctrl;
+    virtio_net_ctrl_ack status = VIRTIO_NET_ERR;
+    VirtQueueElement elem;
+
+    while (virtqueue_pop(vq, &elem)) {
+        if ((elem.in_num < 1) | (elem.out_num < 1)) {
+            fprintf(stderr, "virtio-net ctrl missing headers\n");
+            exit(1);
+        }
+
+        if (elem.out_sg[0].iov_len < sizeof(ctrl) ||
+            elem.out_sg[elem.in_num - 1].iov_len < sizeof(status)) {
+            fprintf(stderr, "virtio-net ctrl header not in correct element\n");
+            exit(1);
+        }
+
+        ctrl.class = ldub_p(elem.out_sg[0].iov_base);
+        ctrl.cmd = ldub_p(elem.out_sg[0].iov_base + sizeof(ctrl.class));
+
+        stb_p(elem.in_sg[elem.in_num - 1].iov_base, status);
+
+        virtqueue_push(vq, &elem, sizeof(status));
+        virtio_notify(vdev, vq);
+    }
+}
+
 /* RX */
 
 static void virtio_net_handle_rx(VirtIODevice *vdev, VirtQueue *vq)
@@ -356,6 +387,7 @@ void virtio_net_init(PCIBus *bus, NICInfo *nd, int devfn)
     n->vdev.set_features = virtio_net_set_features;
     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);
+    n->ctrl_vq = virtio_add_queue(&n->vdev, 16, virtio_net_handle_ctrl);
     memcpy(n->mac, nd->macaddr, ETH_ALEN);
     n->status = VIRTIO_NET_S_LINK_UP;
     n->vc = qemu_new_vlan_client(nd->vlan, nd->model, nd->name,
diff --git a/hw/virtio-net.h b/hw/virtio-net.h
index 9dce663..119e38d 100644
--- a/hw/virtio-net.h
+++ b/hw/virtio-net.h
@@ -40,6 +40,7 @@
 #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_F_CTRL_VQ    17      /* Control channel available */
 
 #define VIRTIO_NET_S_LINK_UP    1       /* Link is up */
 
@@ -84,4 +85,21 @@ struct virtio_net_hdr_mrg_rxbuf
 
 void virtio_net_init(PCIBus *bus, NICInfo *nd, int devfn);
 
+/*
+ * Control virtqueue data structures
+ *
+ * The control virtqueue expects a header in the first sg entry
+ * and an ack/status response in the last entry.  Data for the
+ * command goes in between.
+ */
+struct virtio_net_ctrl_hdr {
+    uint8_t class;
+    uint8_t cmd;
+};
+
+typedef uint8_t virtio_net_ctrl_ack;
+
+#define VIRTIO_NET_OK     0
+#define VIRTIO_NET_ERR    1
+
 #endif

Re: [Qemu-devel] Re: [PATCH v2 4/8] qemu:virtio-net: Add a virtqueue for control commands from the guest

[malc]

On Wed, 4 Feb 2009, Alex Williamson wrote:

[..snip..]

>  
> +static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
> +{
> +    struct virtio_net_ctrl_hdr ctrl;
> +    virtio_net_ctrl_ack status = VIRTIO_NET_ERR;
> +    VirtQueueElement elem;
> +
> +    while (virtqueue_pop(vq, &elem)) {
> +        if ((elem.in_num < 1) | (elem.out_num < 1)) {
                                 ^ intended ?

[..snip..]

-- 
mailto:av1474@comtv.ru

[Qemu-devel] Re: [PATCH v2 4/8] qemu:virtio-net: Add a virtqueue for control commands from the guest

[Anthony Liguori]

Alex Williamson wrote:
> On Tue, 2009-02-03 at 14:56 -0600, Anthony Liguori wrote:
>   
>> Alex Williamson wrote:
>>     
>>> +        ctrl = elem.out_sg[0].iov_base;
>>> +        status = elem.in_sg[elem.in_num - 1].iov_base;
>>>   
>>>       
>> This works only because we're dealing with uint8_ts but it's broken 
>> elsewhere in virtio-net.  iov_base is guest memory which means it may 
>> have different endianness.  We really should be using ldub_p() here.
>>
>>     
>>> +        *status = VIRTIO_NET_ERR;
>>>   
>>>       
>> And stb_p() here.
>>     
>
> Thanks Anthony.  So before I patch bomb again, I think this turns into
> something like below.  This makes the struct definitions less useful on
> the backend side, but I'm not sure we can do anything about that.  I
> assume for u16s and u32s used in other parts of the interface, we'll
> define those as little endian, which means the guest side driver
> eventually needs a follow-up patch for big endian archs.  Thanks,
>
> Alex
>
>
> qemu:virtio-net: Add a virtqueue for control commands from the guest
>
> This will be used for RX mode, MAC table, VLAN table control, etc...
>
> The control transaction consists of one or more "out" sg entries and
> one or more "in" sg entries.  The first out entry contains a header
> defining the class and command.  Additional out entries may provide
> data for the command.  A response via the ack entry is required
> and the guest will typically be waiting for it.
>
> Signed-off-by: Alex Williamson <alex.williamson@hp.com>
> ---
>
>  hw/virtio-net.c |   34 +++++++++++++++++++++++++++++++++-
>  hw/virtio-net.h |   18 ++++++++++++++++++
>  2 files changed, 51 insertions(+), 1 deletions(-)
>
>
> diff --git a/hw/virtio-net.c b/hw/virtio-net.c
> index 073f23a..7971f95 100644
> --- a/hw/virtio-net.c
> +++ b/hw/virtio-net.c
> @@ -25,6 +25,7 @@ typedef struct VirtIONet
>      uint16_t status;
>      VirtQueue *rx_vq;
>      VirtQueue *tx_vq;
> +    VirtQueue *ctrl_vq;
>      VLANClientState *vc;
>      QEMUTimer *tx_timer;
>      int tx_timer_active;
> @@ -79,7 +80,9 @@ static void virtio_net_set_link_status(VLANClientState *vc)
>  
>  static uint32_t virtio_net_get_features(VirtIODevice *vdev)
>  {
> -    uint32_t features = (1 << VIRTIO_NET_F_MAC) | (1 << VIRTIO_NET_F_STATUS);
> +    uint32_t features = (1 << VIRTIO_NET_F_MAC) |
> +                        (1 << VIRTIO_NET_F_STATUS) |
> +                        (1 << VIRTIO_NET_F_CTRL_VQ);
>  
>      return features;
>  }
> @@ -91,6 +94,34 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint32_t features)
>      n->mergeable_rx_bufs = !!(features & (1 << VIRTIO_NET_F_MRG_RXBUF));
>  }
>  
> +static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
> +{
> +    struct virtio_net_ctrl_hdr ctrl;
> +    virtio_net_ctrl_ack status = VIRTIO_NET_ERR;
> +    VirtQueueElement elem;
> +
> +    while (virtqueue_pop(vq, &elem)) {
> +        if ((elem.in_num < 1) | (elem.out_num < 1)) {
> +            fprintf(stderr, "virtio-net ctrl missing headers\n");
> +            exit(1);
> +        }
> +
> +        if (elem.out_sg[0].iov_len < sizeof(ctrl) ||
> +            elem.out_sg[elem.in_num - 1].iov_len < sizeof(status)) {
> +            fprintf(stderr, "virtio-net ctrl header not in correct element\n");
> +            exit(1);
> +        }
> +
> +        ctrl.class = ldub_p(elem.out_sg[0].iov_base);
> +        ctrl.cmd = ldub_p(elem.out_sg[0].iov_base + sizeof(ctrl.class));
> +
> +        stb_p(elem.in_sg[elem.in_num - 1].iov_base, status);
> +
> +        virtqueue_push(vq, &elem, sizeof(status));
> +        virtio_notify(vdev, vq);
> +    }
> +}
>
>   

Yup, this looks right to me.

Regards,

Anthony Liguori

Re: [Qemu-devel] Re: [PATCH v2 4/8] qemu:virtio-net: Add a virtqueue for control commands from the guest

[Alex Williamson]

On Thu, 2009-02-05 at 02:09 +0300, malc wrote:
> On Wed, 4 Feb 2009, Alex Williamson wrote:
> 
> [..snip..]
> 
> >  
> > +static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
> > +{
> > +    struct virtio_net_ctrl_hdr ctrl;
> > +    virtio_net_ctrl_ack status = VIRTIO_NET_ERR;
> > +    VirtQueueElement elem;
> > +
> > +    while (virtqueue_pop(vq, &elem)) {
> > +        if ((elem.in_num < 1) | (elem.out_num < 1)) {
>                                  ^ intended ?

No, good catch.  Thank you.

-- 
Alex Williamson                             HP Open Source & Linux Org.