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

[Amos Kong]

On Tue, Mar 25, 2014 at 10:24:33AM -0600, Eric Blake wrote:
> On 03/25/2014 09:40 AM, 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.
> > 
> > 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.
> > 
> > [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
> > ---
> >  hw/net/virtio-net.c | 42 +++++++++++++++++++++++++++++-------------
> >  qapi-schema.json    |  3 +++
> >  qmp-commands.hx     |  2 ++
> >  3 files changed, 34 insertions(+), 13 deletions(-)
> 
> Reviewed-by: Eric Blake <address@hidden>
> 
> 
> > +
> > +    list = NULL;
> > +    for (i = 0; i < MAX_VLAN >> 5; i++) {
> > +        for (j = 0; n->vlans[i] && j < 0x1f; j++) {
> 
> Pre-existing, but isn't this an off-by-one?  That is, don't you also
> want the iteration for j==0x1f?  Or are there really only 31 instead of
> 32 vlan maps?

Good catch!  it should be  'j <= 0x1f'
 
> > -    int_list = NULL;
> > -    for (i = 0; i < MAX_VLAN >> 5; i++) {
> > -        for (j = 0; n->vlans[i] && j < 0x1f; j++) {
> > -            if (n->vlans[i] & (1U << j)) {
> > -                int_entry = g_malloc0(sizeof(*int_entry));
> > -                int_entry->value = (i << 5) + j;
> 
> 
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
> 



-- 
                        Amos.

pgp4yZYxjUD3m.pgp
Description: PGP signature