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 <[email protected]> > --- > > 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. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
