* [Qemu-devel] [PATCH 5/5] virtio-net: Add additional MACs via a filter table
@ 2009-01-13 21:24 Alex Williamson
2009-01-14 10:15 ` [Qemu-devel] " Mark McLoughlin
0 siblings, 1 reply; 3+ messages in thread
From: Alex Williamson @ 2009-01-13 21:24 UTC (permalink / raw)
To: kvm; +Cc: Mark McLoughlin, qemu-devel
Signed-off-by: Alex Williamson <alex.williamson@hp.com>
---
qemu/hw/virtio-net.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++-
qemu/hw/virtio-net.h | 4 +++
2 files changed, 77 insertions(+), 1 deletions(-)
diff --git a/qemu/hw/virtio-net.c b/qemu/hw/virtio-net.c
index 99e582f..69b7511 100644
--- a/qemu/hw/virtio-net.c
+++ b/qemu/hw/virtio-net.c
@@ -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);
diff --git a/qemu/hw/virtio-net.h b/qemu/hw/virtio-net.h
index beae4a4..4111036 100644
--- a/qemu/hw/virtio-net.h
+++ b/qemu/hw/virtio-net.h
@@ -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
^ permalink raw reply related [flat|nested] 3+ messages in thread* [Qemu-devel] Re: [PATCH 5/5] virtio-net: Add additional MACs via a filter table
2009-01-13 21:24 [Qemu-devel] [PATCH 5/5] virtio-net: Add additional MACs via a filter table Alex Williamson
@ 2009-01-14 10:15 ` Mark McLoughlin
2009-01-14 16:54 ` Alex Williamson
0 siblings, 1 reply; 3+ messages in thread
From: Mark McLoughlin @ 2009-01-14 10:15 UTC (permalink / raw)
To: Alex Williamson; +Cc: qemu-devel, kvm
Hi Alex,
Whole series looks good to me, my suggestions are mostly trivial.
On Tue, 2009-01-13 at 14:24 -0700, Alex Williamson wrote:
> Signed-off-by: Alex Williamson <alex.williamson@hp.com>
> ---
>
> qemu/hw/virtio-net.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++-
> qemu/hw/virtio-net.h | 4 +++
> 2 files changed, 77 insertions(+), 1 deletions(-)
>
> diff --git a/qemu/hw/virtio-net.c b/qemu/hw/virtio-net.c
> index 99e582f..69b7511 100644
> --- a/qemu/hw/virtio-net.c
> +++ b/qemu/hw/virtio-net.c
> @@ -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 :-)
...
> @@ -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.
> + 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.
> + n->mac_table.macs = qemu_mallocz(*entries * 6);
We should put a limit on the size of the table.
> + 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) {
...
> + (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.
^ permalink raw reply [flat|nested] 3+ messages in thread* [Qemu-devel] Re: [PATCH 5/5] virtio-net: Add additional MACs via a filter table
2009-01-14 10:15 ` [Qemu-devel] " Mark McLoughlin
@ 2009-01-14 16:54 ` Alex Williamson
0 siblings, 0 replies; 3+ messages in thread
From: Alex Williamson @ 2009-01-14 16:54 UTC (permalink / raw)
To: Mark McLoughlin; +Cc: qemu-devel, kvm
On Wed, 2009-01-14 at 10:15 +0000, Mark McLoughlin wrote:
> > -#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.
> ...
> > @@ -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.
> > + 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.
>
> > + 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.
> > + 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
> 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) {
> ...
>
> > + (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;
> > + }
> > + }
> ...
--
Alex Williamson HP Open Source & Linux Org.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2009-01-14 16:54 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-01-13 21:24 [Qemu-devel] [PATCH 5/5] virtio-net: Add additional MACs via a filter table Alex Williamson
2009-01-14 10:15 ` [Qemu-devel] " Mark McLoughlin
2009-01-14 16:54 ` Alex Williamson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).