Re: [Qemu-devel] [PATCH] virtio-net: Do not filter VLANs without F_CTRL_VLAN

[Amos Kong]

On Sun, Feb 23, 2014 at 09:27:33AM +0100, Stefan Fritsch wrote:
> On Fri, 21 Feb 2014, Amos Kong wrote:
> 
> > On Wed, Feb 12, 2014 at 10:46:28PM +0100, Stefan Fritsch wrote:
> > > If VIRTIO_NET_F_CTRL_VLAN is not negotiated, do not filter out all
> > > VLAN-tagged packets but send them to the guest.
> > 
> > Can we just update receive_filter() to filter out VLAN-tagged packets
> > only when VIRTIO_NET_F_CTRL_VLAN is negotiated?

If we change receive_filter(), we also need a flag to indicate
management this feature isn't negotiated, management will do some
additional operation to host device to get same effect.
 
> We could. But this adds a (very small) per-packet overhead while my patch 
> only adds overhead during reset. Therefore I didn't take that approach. 
> But if changing receive_filter() makes management much easier, that could 
> be acceptable.

Actually your solution is better, QEMU will return a long list
[0,1,2,...4095] to management, host device will filter all the vlan
packets and send to QEMU.

So the problem raised by mst doesn't exist.

Thanks, Amos

> > @@ -913,7 +940,8 @@ static int receive_filter(VirtIONet
> > *n, const uint8_t *buf, int size)
> >  
> >      if (!memcmp(&ptr[12], vlan, sizeof(vlan))) {
> >          int vid = be16_to_cpup((uint16_t *)(ptr + 14)) & 0xfff;
> > -        if (!(n->vlans[vid >> 5] & (1U << (vid & 0x1f))))
> > +        if ((vdev->guest_features & (1 << VIRTIO_NET_F_CTRL_VLAN)) &&
> > +            !(n->vlans[vid >> 5] & (1U << (vid & 0x1f))))
> >              return 0;
> >      }
> >  
> > > Signed-off-by: Stefan Fritsch <address@hidden>
> > > ---
> > > 
> > > This time CCing the maintainers.
> > > 
> > > This fixes VLANs with OpenBSD guests (and probably NetBSD, too, because
> > > the OpenBSD driver started as a port from NetBSD).
> > > 
> > > 
> > >  hw/net/virtio-net.c |   12 +++++++++++-
> > >  1 file changed, 11 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > > index 3626608..0ae9a91 100644
> > > --- a/hw/net/virtio-net.c
> > > +++ b/hw/net/virtio-net.c
> > > @@ -315,7 +315,11 @@ static void virtio_net_reset(VirtIODevice *vdev)
> > >      memset(n->mac_table.macs, 0, MAC_TABLE_ENTRIES * ETH_ALEN);
> > >      memcpy(&n->mac[0], &n->nic->conf->macaddr, sizeof(n->mac));
> > >      qemu_format_nic_info_str(qemu_get_queue(n->nic), n->mac);
> > > -    memset(n->vlans, 0, MAX_VLAN >> 3);
> > > +    if (vdev->guest_features & (1 << VIRTIO_NET_F_CTRL_VLAN)) {
> > > +        memset(n->vlans, 0, MAX_VLAN >> 3);
> > > +    } else {
> > > +        memset(n->vlans, 0xff, MAX_VLAN >> 3);
> > > +    }
> > >  }
> > >  
> > >  static void peer_test_vnet_hdr(VirtIONet *n)
> > > @@ -515,6 +519,12 @@ static void virtio_net_set_features(VirtIODevice 
> > > *vdev, uint32_t features)
> > >          }
> > >          vhost_net_ack_features(tap_get_vhost_net(nc->peer), features);
> > >      }
> > > +
> > > +    if (vdev->guest_features & (1 << VIRTIO_NET_F_CTRL_VLAN)) {
> > > +        memset(n->vlans, 0, MAX_VLAN >> 3);
> > > +    } else {
> > > +        memset(n->vlans, 0xff, MAX_VLAN >> 3);
> > > +    }
> > >  }
> > >  
> > >  static int virtio_net_handle_rx_mode(VirtIONet *n, uint8_t cmd,
> > > -- 
> > > 1.7.10.4
> > 
> > -- 
> >                     Amos.
> > 

-- 
                        Amos.