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

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1675 bytes --]

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 <akong@redhat.com>
> ---
> 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 <eblake@redhat.com>


> +
> +    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?

> -    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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]