From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1LN90U-0001Ud-10 for qemu-devel@nongnu.org; Wed, 14 Jan 2009 11:54:18 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1LN90Q-0001SI-FZ for qemu-devel@nongnu.org; Wed, 14 Jan 2009 11:54:15 -0500 Received: from [199.232.76.173] (port=34347 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1LN90Q-0001S3-9F for qemu-devel@nongnu.org; Wed, 14 Jan 2009 11:54:14 -0500 Received: from g4t0014.houston.hp.com ([15.201.24.17]:40592) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1LN90P-0004KQ-PB for qemu-devel@nongnu.org; Wed, 14 Jan 2009 11:54:13 -0500 From: Alex Williamson In-Reply-To: <1231928117.4944.294.camel@localhost.localdomain> References: <1231881842.9095.196.camel@bling> <1231928117.4944.294.camel@localhost.localdomain> Content-Type: text/plain Date: Wed, 14 Jan 2009 09:54:39 -0700 Message-Id: <1231952079.7109.340.camel@lappy> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH 5/5] virtio-net: Add additional MACs via a filter table Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , 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.