From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56400) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WFQ74-0003MG-DI for qemu-devel@nongnu.org; Mon, 17 Feb 2014 10:28:11 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WFQ6x-0002mG-GE for qemu-devel@nongnu.org; Mon, 17 Feb 2014 10:28:06 -0500 Received: from mx1.redhat.com ([209.132.183.28]:27662) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WFQ6x-0002m1-7e for qemu-devel@nongnu.org; Mon, 17 Feb 2014 10:27:59 -0500 Message-ID: <53022A7B.2070602@redhat.com> Date: Mon, 17 Feb 2014 10:27:55 -0500 From: Vlad Yasevich MIME-Version: 1.0 References: <1392604023-20142-1-git-send-email-akong@redhat.com> In-Reply-To: <1392604023-20142-1-git-send-email-akong@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] virtio-net: only output the vlan table when VIRTIO_NET_F_CTRL_VLAN is negotiated Reply-To: vyasevic@redhat.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Amos Kong , qemu-devel@nongnu.org Cc: lcapitulino@redhat.com, sf@sfritsch.de, mst@redhat.com On 02/16/2014 09:27 PM, 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. > > We should also not send the vlan table to management, this patch makes > the vlan-talbe optional. ^^^^^^^^^^ table. One question I have from the API perspective is can we suddenly change something to be optional? If there are any users of this , wouldn't they have to change now to check the existence of this list? Thanks -vlad > [1] http://lists.nongnu.org/archive/html/qemu-devel/2014-02/msg02604.html > > Signed-off-by: Amos Kong > --- > hw/net/virtio-net.c | 38 +++++++++++++++++++++++++------------- > qapi-schema.json | 4 ++-- > qmp-commands.hx | 2 +- > 3 files changed, 28 insertions(+), 16 deletions(-) > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > index 3626608..0b32e6a 100644 > --- a/hw/net/virtio-net.c > +++ b/hw/net/virtio-net.c > @@ -222,13 +222,33 @@ static char *mac_strdup_printf(const uint8_t *mac) > mac[1], mac[2], mac[3], mac[4], mac[5]); > } > > +static intList *get_vlan_table(VirtIONet *n) > +{ > + intList *list, *entry; > + int i, j; > + > + 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)) { > + entry = g_malloc0(sizeof(*entry)); > + entry->value = (i << 5) + j; > + entry->next = list; > + list = entry; > + } > + } > + } > + > + return list; > +} > + > static RxFilterInfo *virtio_net_query_rxfilter(NetClientState *nc) > { > VirtIONet *n = qemu_get_nic_opaque(nc); > + VirtIODevice *vdev = VIRTIO_DEVICE(n); > RxFilterInfo *info; > strList *str_list, *entry; > - intList *int_list, *int_entry; > - int i, j; > + int i; > > info = g_malloc0(sizeof(*info)); > info->name = g_strdup(nc->name); > @@ -274,18 +294,10 @@ static RxFilterInfo *virtio_net_query_rxfilter(NetClientState *nc) > } > info->multicast_table = str_list; > > - 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; > - int_entry->next = int_list; > - int_list = int_entry; > - } > - } > + if ((1 << VIRTIO_NET_F_CTRL_VLAN) & vdev->guest_features) { > + info->has_vlan_table = true; > + info->vlan_table = get_vlan_table(n); > } > - 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 7cfb5e5..5d48fa9 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -4034,7 +4034,7 @@ > # > # @main-mac: the main macaddr string > # > -# @vlan-table: a list of active vlan id > +# @vlan-table: #optional a list of active vlan id > # > # @unicast-table: a list of unicast macaddr string > # > @@ -4053,7 +4053,7 @@ > 'multicast-overflow': 'bool', > 'unicast-overflow': 'bool', > 'main-mac': 'str', > - 'vlan-table': ['int'], > + '*vlan-table': ['int'], > 'unicast-table': ['str'], > 'multicast-table': ['str'] }} > > diff --git a/qmp-commands.hx b/qmp-commands.hx > index cce6b81..a1c1dfa 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -3308,7 +3308,7 @@ Each array entry contains the following: > - "multicast-overflow": multicast table is overflowed (json-bool) > - "unicast-overflow": unicast table is overflowed (json-bool) > - "main-mac": main macaddr string (json-string) > -- "vlan-table": a json-array of active vlan id > +- "vlan-table": a json-array of active vlan id (optoinal) > - "unicast-table": a json-array of unicast macaddr string > - "multicast-table": a json-array of multicast macaddr string > >