From patchwork Wed Jan 12 17:26:55 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dragos Tatulea X-Patchwork-Id: 78595 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [199.232.76.165]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 1BF6DB70A3 for ; Thu, 13 Jan 2011 04:35:22 +1100 (EST) Received: from localhost ([127.0.0.1]:37167 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Pd4aR-00086G-3F for incoming@patchwork.ozlabs.org; Wed, 12 Jan 2011 12:34:19 -0500 Received: from [140.186.70.92] (port=33818 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Pd4X3-0006mH-0a for qemu-devel@nongnu.org; Wed, 12 Jan 2011 12:30:52 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Pd4Te-00069L-5F for qemu-devel@nongnu.org; Wed, 12 Jan 2011 12:27:19 -0500 Received: from mail-iy0-f173.google.com ([209.85.210.173]:41432) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Pd4Te-00068y-0E for qemu-devel@nongnu.org; Wed, 12 Jan 2011 12:27:18 -0500 Received: by iye19 with SMTP id 19so821781iye.4 for ; Wed, 12 Jan 2011 09:27:16 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-type; bh=Wlsyx4ipfeFw7/Uw0FUKM9+pIxg95O6RHgmIYjtwPRk=; b=Gz3rJIMkkrZ0NtKVj7jcAEQAlhd+/qdjKBaAlh1IEIeHApLVOs4FbbL/Rkwk0s04Lx ETnH7SSH9n8Lba/Xwc+G78LLxx/q5D0tV/aLDSVTZ3J5O1cl958vRJ0YlmZJimHRdnGo QMXu14R4syf0jGBLXCmb1r1Yo/geiHl36BKvc= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-type; b=w6ajYZZzO7ebvMQCKnqRNwhHNpVRSnm9wquD8EqkAs89sazTrytNOfFzlnEuKOq3l/ 8mvjCS5qK5W0MZdx/9lW4T0fcVV7K4XA4jTZOinfkZVinwhTXCA6yiCFH9w+0tSWldhd VhuuJTr9OlkxKX6MF4D/Z/EsX1NY3352vvfLo= Received: by 10.231.37.74 with SMTP id w10mr1331081ibd.4.1294853236493; Wed, 12 Jan 2011 09:27:16 -0800 (PST) MIME-Version: 1.0 Received: by 10.231.173.135 with HTTP; Wed, 12 Jan 2011 09:26:55 -0800 (PST) In-Reply-To: References: From: Dragos Tatulea Date: Wed, 12 Jan 2011 18:26:55 +0100 Message-ID: To: alex.williamson@redhat.com, paul@codesourcery.com, anthony@codemonkey.ws, "Michael S. Tsirkin" X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 2) Cc: qemu-devel@nongnu.org, kvm@vger.kernel.org Subject: [Qemu-devel] Re: [PATCH][RFC] qemu:virtio-net: Use TUNSETTXFILTER for MAC filteringlogin register about X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Hi, Trying to stir up a year old conversation [1] about mac filtering. The patch below is Alex Williamson's work updated for the current qemu taking into account some comments. An extra check for multiple nics on the same vlan has been added as well. Now, I know it's not ideal but I'm looking for feedback here. The original thread contained some ideas that weren't discussed extensively: * Alex Williamson's patch that makes the nic call the associated tap vlan client to set the mac filters. This is possible only for 1 nix x 1 tap setup. It's a bit hackish (even more visible in the current version) but it's straightforward. * Paul Brook considered that the nic shouldn't know anything about what else is connected to the vlan. He proposed a model that is more generic and would allow us to filter stuff from device to vlan and the other way around. * Anthony Liguori proposed moving all the mac filtering to the vlan side. This would be nice, but it would imply removing & rewriting all the emulated nics. Don't know how acceptable this is since, I suppose, the emulated behavior will change. The bottom line here is that neither of these 3 approaches is ok for what we want to achieve: basically tap & macvtap filtering for virtio. What do we want? The simple change for virtio tun tx fiiltering or filtering MACs in both directions taking into account how nics are connected and if hw filtering is possible? The latter being quite a change. [1] - http://thread.gmane.org/gmane.comp.emulators.qemu/37714/focus=37719 -- Dragos Tatulea Signed-off-by: Alex Williamson Signed-off-by: Dragos Tatulea --- { @@ -333,6 +360,7 @@ static TAPState *net_tap_fd_init(VLANState *vlan, nc = qemu_new_net_client(&net_tap_info, vlan, NULL, model, name); + nc->info->set_receive_filter = tap_set_receive_filter; s = DO_UPCAST(TAPState, nc, nc); s->fd = fd; diff --git a/hw/virtio-net.c b/hw/virtio-net.c index ec1bf8d..0276f3a 100644 --- a/hw/virtio-net.c +++ b/hw/virtio-net.c @@ -21,6 +21,8 @@ #include "virtio-net.h" #include "vhost_net.h" +#include + #define VIRTIO_NET_VM_VERSION 11 #define MAC_TABLE_ENTRIES 64 @@ -49,6 +51,7 @@ typedef struct VirtIONet int mergeable_rx_bufs; uint8_t promisc; uint8_t allmulti; + uint8_t hw_mac_filter; uint8_t alluni; uint8_t nomulti; uint8_t nouni; @@ -176,6 +179,30 @@ static void virtio_net_set_link_status(VLANClientState *nc) virtio_net_set_status(&n->vdev, n->vdev.status); } +static void virtio_net_set_hw_rx_filter(VirtIONet *n) +{ + static const uint8_t bcast[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff}; + uint8_t *buf; + int flags = 0; + VLANState *vlan = n->nic->nc.vlan; + + if (n->promisc || vlan_count_nics(vlan) > 1) + /* can't have several nics within the same tap */ + flags |= IFF_PROMISC; + if (n->allmulti) + flags |= IFF_ALLMULTI; + + buf = qemu_mallocz((n->mac_table.in_use + 2) * ETH_ALEN); + + memcpy(&buf[ETH_ALEN*0], n->mac, ETH_ALEN); + memcpy(&buf[ETH_ALEN*1], bcast, ETH_ALEN); + memcpy(&buf[ETH_ALEN*2], n->mac_table.macs, n->mac_table.in_use * ETH_ALEN); + + n->hw_mac_filter = vlan_set_hw_receive_filter(vlan, flags, + n->mac_table.in_use + 2, buf); + qemu_free(buf); +} + static void virtio_net_reset(VirtIODevice *vdev) { VirtIONet *n = to_virtio_net(vdev); @@ -194,6 +221,7 @@ static void virtio_net_reset(VirtIODevice *vdev) n->mac_table.multi_overflow = 0; n->mac_table.uni_overflow = 0; memset(n->mac_table.macs, 0, MAC_TABLE_ENTRIES * ETH_ALEN); + virtio_net_set_hw_rx_filter(n); memset(n->vlans, 0, MAX_VLAN >> 3); } @@ -423,11 +451,13 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) ctrl.class = ldub_p(elem.out_sg[0].iov_base); ctrl.cmd = ldub_p(elem.out_sg[0].iov_base + sizeof(ctrl.class)); - if (ctrl.class == VIRTIO_NET_CTRL_RX_MODE) + if (ctrl.class == VIRTIO_NET_CTRL_RX_MODE) { status = virtio_net_handle_rx_mode(n, ctrl.cmd, &elem); - else if (ctrl.class == VIRTIO_NET_CTRL_MAC) + virtio_net_set_hw_rx_filter(n); + } else if (ctrl.class == VIRTIO_NET_CTRL_MAC) { status = virtio_net_handle_mac(n, ctrl.cmd, &elem); - else if (ctrl.class == VIRTIO_NET_CTRL_VLAN) + virtio_net_set_hw_rx_filter(n); + } else if (ctrl.class == VIRTIO_NET_CTRL_VLAN) status = virtio_net_handle_vlan_table(n, ctrl.cmd, &elem); stb_p(elem.in_sg[elem.in_num - 1].iov_base, status); @@ -547,6 +577,9 @@ static int receive_filter(VirtIONet *n, const uint8_t *buf, int size) if (n->promisc) return 1; + if (n->hw_mac_filter) + return 1; + if (n->has_vnet_hdr) { ptr += sizeof(struct virtio_net_hdr); } @@ -969,6 +1002,9 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id) } } n->mac_table.first_multi = i; + + virtio_net_set_hw_rx_filter(n); + return 0; } diff --git a/net.c b/net.c index 9ba5be2..8814e8b 100644 --- a/net.c +++ b/net.c @@ -492,6 +492,37 @@ static ssize_t qemu_vlan_deliver_packet(VLANClientState *sender, return ret; } +int vlan_count_nics(VLANState *vlan) +{ + VLANClientState *vc; + int count = 0; + + QTAILQ_FOREACH(vc, &vlan->clients, next) { + if (vc->info->type == NET_CLIENT_TYPE_NIC) + count++; + } + + return count; +} + +int vlan_set_hw_receive_filter(VLANState *vlan, int flags, + int count, uint8_t *buf) +{ + VLANClientState *vc; + + QTAILQ_FOREACH(vc, &vlan->clients, next) { + int ret; + + if (!vc->info->set_receive_filter) + continue; + + ret = vc->info->set_receive_filter(vc, flags, count, buf); + return (ret == count); + } + + return 0; +} + void qemu_purge_queued_packets(VLANClientState *vc) { NetQueue *queue; diff --git a/net.h b/net.h index 6ceca50..fe12c27 100644 --- a/net.h +++ b/net.h @@ -42,6 +42,7 @@ typedef void (NetPoll)(VLANClientState *, bool enable); typedef int (NetCanReceive)(VLANClientState *); typedef ssize_t (NetReceive)(VLANClientState *, const uint8_t *, size_t); typedef ssize_t (NetReceiveIOV)(VLANClientState *, const struct iovec *, int); +typedef int (NetSetReceiveFilter)(VLANClientState *, unsigned int, int, uint8_t *); typedef void (NetCleanup) (VLANClientState *); typedef void (LinkStatusChanged)(VLANClientState *); @@ -52,6 +53,7 @@ typedef struct NetClientInfo { NetReceive *receive_raw; NetReceiveIOV *receive_iov; NetCanReceive *can_receive; + NetSetReceiveFilter *set_receive_filter; NetCleanup *cleanup; LinkStatusChanged *link_status_changed; NetPoll *poll; @@ -99,6 +101,11 @@ NICState *qemu_new_nic(NetClientInfo *info, void qemu_del_vlan_client(VLANClientState *vc); VLANClientState *qemu_find_vlan_client_by_name(Monitor *mon, int vlan_id, const char *client_str); + +int vlan_count_nics(VLANState *vlan); +int vlan_set_hw_receive_filter(VLANState *vlan, int flags, + int count, uint8_t *buf); + typedef void (*qemu_nic_foreach)(NICState *nic, void *opaque); void qemu_foreach_nic(qemu_nic_foreach func, void *opaque); int qemu_can_send_packet(VLANClientState *vc); diff --git a/net/tap.c b/net/tap.c index eada34a..f3b9b1b 100644 --- a/net/tap.c +++ b/net/tap.c @@ -44,6 +44,8 @@ #include "hw/vhost_net.h" +#include + /* Maximum GSO packet size (64k) plus plenty of room for * the ethernet and virtio_net headers */ @@ -115,6 +117,31 @@ static ssize_t tap_write_packet(TAPState *s, const struct iovec *iov, int iovcnt return len; } +static int tap_set_receive_filter(VLANClientState *nc, unsigned int flags, + int count, uint8_t *list) +{ + TAPState *s = DO_UPCAST(TAPState, nc, nc); + struct tun_filter *filter; + int ret; + + if (flags & IFF_PROMISC) + count = 0; + + filter = qemu_mallocz(sizeof(*filter) + (count * ETH_ALEN)); + + memcpy(filter->addr, list, count * ETH_ALEN); + filter->count += count; + + if (flags & IFF_ALLMULTI) + filter->flags |= TUN_FLT_ALLMULTI; + + ret = ioctl(s->fd, TUNSETTXFILTER, filter); + + qemu_free(filter); + + return ret; +} + static ssize_t tap_receive_iov(VLANClientState *nc, const struct iovec *iov, int iovcnt)