Re: [PATCH][RFC] qemu:virtio-net: Use TUNSETTXFILTER for MAC filtering

[Anthony Liguori <anthony@codemonkey.ws>]

Alex Williamson wrote:
> Now that virtio-net knows what packets the guest wants to see, we
> can start moving the filtering down the stack.  This patch adds
> an interface to set the software filter in the tap device.  It's
> fairly limited, but we can back it up with our own filtering if it
> overflows.
>
> Here are a couple issues I'm still pondering:
>  - Is the fd_rx_filter() interface sufficiently generic
>  - Should vlan_set_hw_rx_filter() live in net.c or elsewhere
>  - Is it ok to call fd_rx_filter() against all the vlan clients.  I
>    exit on the first one, which covers the simple config.
>
> Insterested in feedback.  Thanks,
>
> Alex
>
> Signed-off-by: Alex Williamson <alex.williamson@hp.com>
> ---
>
>  hw/virtio-net.c |   57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  net.c           |   28 +++++++++++++++++++++++++++
>  net.h           |    3 +++
>  3 files changed, 88 insertions(+), 0 deletions(-)
>
> diff --git a/hw/virtio-net.c b/hw/virtio-net.c
> index 62153e9..2556f42 100644
> --- a/hw/virtio-net.c
> +++ b/hw/virtio-net.c
> @@ -15,6 +15,7 @@
>  #include "net.h"
>  #include "qemu-timer.h"
>  #include "virtio-net.h"
> +#include <net/if.h>
>  
>  #define VIRTIO_NET_VM_VERSION    6
>  
> @@ -35,6 +36,7 @@ typedef struct VirtIONet
>      int mergeable_rx_bufs;
>      int promisc;
>      int allmulti;
> +    int hw_mac_filter;
>      struct {
>          int in_use;
>          uint8_t *macs;
> @@ -88,6 +90,51 @@ static void virtio_net_set_link_status(VLANClientState *vc)
>          virtio_notify_config(&n->vdev);
>  }
>  
> +static int vlan_set_hw_rx_filter(VLANState *vlan, int flags,
> +                                 int count, uint8_t *buf)
> +{
> +    VLANClientState *vc;
> +
> +    for (vc = vlan->first_client; vc != NULL; vc = vc->next) {
> +        int ret;
> +
> +        if (!vc->fd_rx_filter)
> +            continue;
> +
> +        ret = vc->fd_rx_filter(vc->opaque, flags, count, buf);
> +        return (ret == count);
> +    } 
> +    return 0;
> +}
>   

This should go in net.c.

> +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;
> +
> +    if (n->promisc)
> +        flags |= IFF_PROMISC;
> +    if (n->allmulti)
> +        flags |= IFF_ALLMULTI;
> +
> +    buf = qemu_mallocz((n->mac_table.in_use + 2) * ETH_ALEN);
> +    if (!buf) {
>   
Don't need to handle these failures anymore.
> +static int tap_rx_filter(void *opaque, unsigned int flags, int count,
> +                         uint8_t *list)
>   

Instead of having each network device do it's own filtering if the VLAN 
doesn't support it, I think we should move the software filtering to the 
VLAN layer so that we aren't duplicating code in each network device.

Also, instead of using IFF_xxx, I think we should introduce our own 
flags.  net/if.h doesn't exist on Windows most likely.

Regards,

Anthony Liguori