[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH 5/5] virtio-net: Add additional MACs via a filte
From: |
Alex Williamson |
Subject: |
[Qemu-devel] Re: [PATCH 5/5] virtio-net: Add additional MACs via a filter table |
Date: |
Wed, 14 Jan 2009 09:54:39 -0700 |
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.