[Qemu-devel] Re: [PATCH][RFC] qemu:virtio-net: Use TUNSETTXFILTER for MAC filteringlogin register about

[Dragos Tatulea]

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 <address@hidden>
Signed-off-by: Dragos Tatulea <address@hidden>
---
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 <net/if.h>
+
 #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 <linux/if_tun.h>
+
 /* 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)
 {
@@ -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;