Re: [Qemu-devel] [PATCH v4 for 2.0] virtio-net: add vlan receive state to RxFilterInfo

[Amos Kong]

On Wed, Mar 26, 2014 at 08:46:35AM +0200, Michael S. Tsirkin wrote:
> On Wed, Mar 26, 2014 at 08:19:43AM +0800, Amos Kong wrote:
> > Stefan Fritsch just fixed a virtio-net driver bug [1], virtio-net won't
> > filter out VLAN-tagged packets if VIRTIO_NET_F_CTRL_VLAN isn't negotiated.
> 
> Yes but that fix is unfortunately wrong as it tests guest_features
> on reset.
> How about preparing a correct one?
 
I just sent a v2:
[PATCH v2 for 2.0] virtio-net: Do not filter VLANs without F_CTRL_VLAN
 
> > This patch added a new field to @RxFilterInfo to indicate vlan receive
> > state ('normal', 'none', 'all'). If VIRTIO_NET_F_CTRL_VLAN isn't
> > negotiated, vlan receive state will be 'all', then all VLAN-tagged packets
> > will be received by guest.
> > 
> > This patch also fixed a boundary issue in visiting vlan table.
> > 
> > [1] http://lists.nongnu.org/archive/html/qemu-devel/2014-02/msg02604.html
> > 
> > Signed-off-by: Amos Kong <address@hidden>
> > ---
> > V2: don't make vlan-table optional, add a flag to indicate
> >     if vlan table is used by management
> > V3: change the new filed to RxState (mst)
> > V4: fix boundary of vlan mapping (eric)

...

> > +    if (!((1 << VIRTIO_NET_F_CTRL_VLAN) & vdev->guest_features)) {
> > +        info->vlan = RX_STATE_ALL;
> > +    } else if (!info->vlan_table) {
> > +        info->vlan = RX_STATE_NONE;
> > +    } else {
> > +        info->vlan = RX_STATE_NORMAL;
> >      }
> 
> Generally I'm not sure why do we have NONE - for mac
> and unicast too - we could send an empty list instead.
> 
> I'm fine with keeping it as is for now though.

It's only be used at the beginning of init stage, guest driver hasn't
change vlan table. state will be normal if default '0' is added to
vlan table.

> > -    info->vlan_table = int_list;
> >  
> >      /* enable event notification after query */
> >      nc->rxfilter_notify_enabled = 1;
> > diff --git a/qapi-schema.json b/qapi-schema.json
> > index b68cd44..391356f 100644

Thanks, Amos