Skip to contentSkip to conversation

virtio-net: Add additional MACs via a filter table

PATCH In review Alex Williamson 2 participants 3 messages Jan 13, 2009 – Jan 14, 2009 KVM

lore
Share & tools
lore
#0 lore raw

PATCH 5/5 virtio-net: Add additional MACs via a filter table

---

 qemu/hw/virtio-net.c |   74 +++++++++++++++++++++++++++++++++++++++++++++++++-
 qemu/hw/virtio-net.h |    4 +++
 2 files changed, 77 insertions(+), 1 deletions(-)
qemu/hw/virtio-net.c +73 -1
index 99e582f..69b7511 100644
@@ -21,7 +21,7 @@
 
 #define TAP_VNET_HDR
 
-#define VIRTIO_VM_VERSION       3
+#define VIRTIO_VM_VERSION       4
 
 typedef struct VirtIONet
 {
@@ -36,6 +36,11 @@ typedef struct VirtIONet
     int mergeable_rx_bufs;
     int promisc;
     int allmulti;
+    struct {
+       int entries;
+       int in_use;
+       uint8_t *macs;
+    } mac_table;
 } VirtIONet;
 
 /* TODO
@@ -69,6 +74,17 @@ static void virtio_net_set_config(VirtIODevice *vdev, const uint8_t *config)
     }
 }
 
+static void virtio_net_reset(VirtIODevice *vdev)
+{
+    VirtIONet *n = to_virtio_net(vdev);
+
+    /* Allow the MAC filter table to be re-allocated after a device reset */
+    n->mac_table.in_use = 0;
+    n->mac_table.entries = 0;
+    qemu_free(n->mac_table.macs);
+    n->mac_table.macs = NULL;
+}
+
 static uint32_t virtio_net_get_features(VirtIODevice *vdev)
 {
     uint32_t features = (1 << VIRTIO_NET_F_MAC);
@@ -157,10 +173,46 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
                 n->allmulti = *on;
             else
                 *status = VIRTIO_NET_ERR;
+
+        } else if (ctrl->class == VIRTIO_NET_CTRL_MAC_TABLE) {
+            if (ctrl->cmd == VIRTIO_NET_CTRL_MAC_TABLE_ALLOC) {
+                uint32_t *entries;
+
+                if (n->mac_table.entries || elem.out_num != 2 ||
+                    elem.out_sg[1].iov_len != sizeof(*entries)) {
+                    *status = VIRTIO_NET_ERR;
+                    goto reply;
+                }
+
+                entries = (void *)elem.out_sg[1].iov_base;
+                n->mac_table.macs = qemu_mallocz(*entries * 6);
+                if (!n->mac_table.macs) {
+                    *status = VIRTIO_NET_ERR;
+                    goto reply;
+                }
+
+                n->mac_table.entries = *entries;
+                *status = VIRTIO_NET_OK;
+            } else if (ctrl->cmd == VIRTIO_NET_CTRL_MAC_TABLE_SET) {
+                if (!n->mac_table.entries || (elem.out_num == 2 &&
+                    (elem.out_sg[1].iov_len / 6) > n->mac_table.entries)) {
+                    *status = VIRTIO_NET_ERR;
+                    goto reply;
+                }
+
+                if (elem.out_num == 2) {
+                    memcpy(n->mac_table.macs, elem.out_sg[1].iov_base,
+                           elem.out_sg[1].iov_len);
+                    n->mac_table.in_use = elem.out_sg[1].iov_len / 6;
+                } else {
+                    n->mac_table.in_use = 0;
+                }
+            }
         } else {
             *status = VIRTIO_NET_ERR;
         }
 
+reply:
         virtqueue_push(vq, &elem, sizeof(*status));
         virtio_notify(vdev, vq);
     }
@@ -276,6 +328,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};
+    int i;
 
     if (n->promisc)
         return 1;
@@ -289,6 +342,11 @@ static int receive_filter(VirtIONet *n, const uint8_t *buf, int size)
     if (!memcmp(buf, n->mac, 6))
         return 1;
 
+    for (i = 0; i < n->mac_table.in_use; i++) {
+        if (!memcmp(buf, &n->mac_table.macs[i * 6], 6))
+            return 1;
+    }
+
     return 0;
 }
 
@@ -458,6 +516,10 @@ static void virtio_net_save(QEMUFile *f, void *opaque)
 #endif
     qemu_put_be32(f, n->promisc);
     qemu_put_be32(f, n->allmulti);
+    qemu_put_be32(f, n->mac_table.entries);
+    qemu_put_be32(f, n->mac_table.in_use);
+    if (n->mac_table.entries)
+        qemu_put_buffer(f, n->mac_table.macs, n->mac_table.entries * 6);
 }
 
 static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
@@ -485,6 +547,15 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
         n->promisc = 1;
     }
 
+    if (version_id >= 4) {
+        n->mac_table.entries = qemu_get_be32(f);
+        n->mac_table.in_use = qemu_get_be32(f);
+	if (n->mac_table.entries) {
+            n->mac_table.macs = qemu_mallocz(n->mac_table.entries * 6);
+            qemu_get_buffer(f, n->mac_table.macs, n->mac_table.entries * 6);
+        }
+    }
+        
     if (n->tx_timer_active) {
         qemu_mod_timer(n->tx_timer,
                        qemu_get_clock(vm_clock) + TX_TIMER_INTERVAL);
@@ -509,6 +580,7 @@ PCIDevice *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);
qemu/hw/virtio-net.h +4 -0
index beae4a4..4111036 100644
@@ -84,4 +84,8 @@ PCIDevice *virtio_net_init(PCIBus *bus, NICInfo *nd, int devfn);
  #define VIRTIO_NET_CTRL_RX_MODE_PROMISC      0
  #define VIRTIO_NET_CTRL_RX_MODE_ALLMULTI     1
 
+#define VIRTIO_NET_CTRL_MAC_TABLE  1
+ #define VIRTIO_NET_CTRL_MAC_TABLE_ALLOC      0
+ #define VIRTIO_NET_CTRL_MAC_TABLE_SET        1
+
 #endif
#1 lore raw

Hi Alex,

Whole series looks good to me, my suggestions are mostly trivial.

Quoted 12 lines · Alex Williamson

---

 qemu/hw/virtio-net.c |   74 +++++++++++++++++++++++++++++++++++++++++++++++++-
 qemu/hw/virtio-net.h |    4 +++
 2 files changed, 77 insertions(+), 1 deletions(-)
qemu/hw/virtio-net.c +0 -0
index 99e582f..69b7511 100644

qemu/hw/virtio-net.c line 24

Quoted 5 lines
qemu/hw/virtio-net.c +1 -1
@@ -21,7 +21,7 @@
 
 #define TAP_VNET_HDR
 
-#define VIRTIO_VM_VERSION       3
+#define VIRTIO_VM_VERSION       4

We could probably do with an ETH_ALEN macro at this stage. There's a lot of '6's around the place :-)

...

qemu/hw/virtio-net.c static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) · line 177

Quoted 6 lines
qemu/hw/virtio-net.c +2 -0
@@ -157,10 +173,46 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
                 n->allmulti = *on;
             else
                 *status = VIRTIO_NET_ERR;
+
+        } else if (ctrl->class == VIRTIO_NET_CTRL_MAC_TABLE) {

Hmm, it'd be nice to factor each of the commands out in their own function.

qemu/hw/virtio-net.c static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) · line 187

Quoted 10 lines
qemu/hw/virtio-net.c +10 -0
+            if (ctrl->cmd == VIRTIO_NET_CTRL_MAC_TABLE_ALLOC) {
+                uint32_t *entries;
+
+                if (n->mac_table.entries || elem.out_num != 2 ||
+                    elem.out_sg[1].iov_len != sizeof(*entries)) {
+                    *status = VIRTIO_NET_ERR;
+                    goto reply;
+                }
+
+                entries = (void *)elem.out_sg[1].iov_base;

No need for the cast.

qemu/hw/virtio-net.c static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) · line 188

Quoted 1 line
qemu/hw/virtio-net.c +1 -0
+                n->mac_table.macs = qemu_mallocz(*entries * 6);

We should put a limit on the size of the table.

qemu/hw/virtio-net.c static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) · line 197

Quoted 9 lines
qemu/hw/virtio-net.c +9 -0
+                if (!n->mac_table.macs) {
+                    *status = VIRTIO_NET_ERR;
+                    goto reply;
+                }
+
+                n->mac_table.entries = *entries;
+                *status = VIRTIO_NET_OK;
+            } else if (ctrl->cmd == VIRTIO_NET_CTRL_MAC_TABLE_SET) {
+                if (!n->mac_table.entries || (elem.out_num == 2 &&

Think I'd just check that out_num is 1 or 2 here ...

then e.g.

   n_entries = 0;
   if (elem.out_num == 2)
       n_entries = elem.out_sg[1].iov_len / 6;

   if (n_entries > n->mac_table.entries) {
       *status = VIRTIO_NET_HDR
   ...

   n->mac_table.in_use = 0;
   if (n_entries) {
   ...

qemu/hw/virtio-net.c static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) · line 210

Quoted 13 lines
qemu/hw/virtio-net.c +13 -0
+                    (elem.out_sg[1].iov_len / 6) > n->mac_table.entries)) {
+                    *status = VIRTIO_NET_ERR;
+                    goto reply;
+                }
+
+                if (elem.out_num == 2) {
+                    memcpy(n->mac_table.macs, elem.out_sg[1].iov_base,
+                           elem.out_sg[1].iov_len);
+                    n->mac_table.in_use = elem.out_sg[1].iov_len / 6;
+                } else {
+                    n->mac_table.in_use = 0;
+                }
+            }

...

Cheers,
Mark.

#2 lore raw
Quoted 5 lines · Mark McLoughlin

qemu/hw/virtio-net.c line 24

Quoted 2 lines
qemu/hw/virtio-net.c +1 -1
-#define VIRTIO_VM_VERSION       3
+#define VIRTIO_VM_VERSION       4

We could probably do with an ETH_ALEN macro at this stage. There's a lot of '6's around the place :-)

Yep, that'd be easier to search for too. I'll at least make a local define for that.

Quoted 10 lines

...

qemu/hw/virtio-net.c static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) · line 177

Quoted 6 lines
qemu/hw/virtio-net.c +2 -0
@@ -157,10 +173,46 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
                 n->allmulti = *on;
             else
                 *status = VIRTIO_NET_ERR;
+
+        } else if (ctrl->class == VIRTIO_NET_CTRL_MAC_TABLE) {

Hmm, it'd be nice to factor each of the commands out in their own function.

It's setup to do that, but each case is still pretty short and simple, so I hadn't made the cut yet.

Quoted 13 lines

qemu/hw/virtio-net.c static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) · line 187

Quoted 10 lines
qemu/hw/virtio-net.c +10 -0
+            if (ctrl->cmd == VIRTIO_NET_CTRL_MAC_TABLE_ALLOC) {
+                uint32_t *entries;
+
+                if (n->mac_table.entries || elem.out_num != 2 ||
+                    elem.out_sg[1].iov_len != sizeof(*entries)) {
+                    *status = VIRTIO_NET_ERR;
+                    goto reply;
+                }
+
+                entries = (void *)elem.out_sg[1].iov_base;

No need for the cast.

Quoted 3 lines

qemu/hw/virtio-net.c static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) · line 188

Quoted 1 line
qemu/hw/virtio-net.c +1 -0
+                n->mac_table.macs = qemu_mallocz(*entries * 6);

We should put a limit on the size of the table.

Agree, I put one on the guest driver and thought I had one here, but must have missed it. Need to protect from malicious guests.

Quoted 11 lines

qemu/hw/virtio-net.c static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) · line 197

Quoted 9 lines
qemu/hw/virtio-net.c +9 -0
+                if (!n->mac_table.macs) {
+                    *status = VIRTIO_NET_ERR;
+                    goto reply;
+                }
+
+                n->mac_table.entries = *entries;
+                *status = VIRTIO_NET_OK;
+            } else if (ctrl->cmd == VIRTIO_NET_CTRL_MAC_TABLE_SET) {
+                if (!n->mac_table.entries || (elem.out_num == 2 &&

Think I'd just check that out_num is 1 or 2 here ...

Good idea, I'll play with this and see what comes out. Thanks for the comments.

Alex

Quoted 28 lines

then e.g.

   n_entries = 0;
   if (elem.out_num == 2)
       n_entries = elem.out_sg[1].iov_len / 6;

   if (n_entries > n->mac_table.entries) {
       *status = VIRTIO_NET_HDR
   ...

   n->mac_table.in_use = 0;
   if (n_entries) {
   ...
Quoted 13 lines
qemu/hw/virtio-net.c +13 -0
+                    (elem.out_sg[1].iov_len / 6) > n->mac_table.entries)) {
+                    *status = VIRTIO_NET_ERR;
+                    goto reply;
+                }
+
+                if (elem.out_num == 2) {
+                    memcpy(n->mac_table.macs, elem.out_sg[1].iov_base,
+                           elem.out_sg[1].iov_len);
+                    n->mac_table.in_use = elem.out_sg[1].iov_len / 6;
+                } else {
+                    n->mac_table.in_use = 0;
+                }
+            }

...

Signature · 1 line
Alex Williamson                             HP Open Source & Linux Org.