* [Qemu-devel] [PATCH v4 for 2.0] virtio-net: add vlan receive state to RxFilterInfo
@ 2014-03-26 0:19 Amos Kong
2014-03-26 1:51 ` Eric Blake
2014-03-26 6:46 ` Michael S. Tsirkin
0 siblings, 2 replies; 4+ messages in thread
From: Amos Kong @ 2014-03-26 0:19 UTC (permalink / raw)
To: qemu-devel; +Cc: vyasevic, lcapitulino, sf, mst
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.
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 <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 (mst)
V4: fix boundary of vlan mapping (eric)
---
hw/net/virtio-net.c | 42 +++++++++++++++++++++++++++++-------------
qapi-schema.json | 3 +++
qmp-commands.hx | 2 ++
3 files changed, 34 insertions(+), 13 deletions(-)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index fd23c46..43b4eda 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);
@@ -273,19 +293,15 @@ static RxFilterInfo *virtio_net_query_rxfilter(NetClientState *nc)
str_list = entry;
}
info->multicast_table = str_list;
+ info->vlan_table = get_vlan_table(n);
- 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->vlan = RX_STATE_ALL;
+ } else if (!info->vlan_table) {
+ info->vlan = RX_STATE_NONE;
+ } else {
+ info->vlan = RX_STATE_NORMAL;
}
- 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
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4184,6 +4184,8 @@
#
# @unicast: unicast receive state
#
+# @vlan: vlan receive state (Since 2.0)
+#
# @broadcast-allowed: whether to receive broadcast
#
# @multicast-overflow: multicast table is overflowed or not
@@ -4207,6 +4209,7 @@
'promiscuous': 'bool',
'multicast': 'RxState',
'unicast': 'RxState',
+ 'vlan': 'RxState',
'broadcast-allowed': 'bool',
'multicast-overflow': 'bool',
'unicast-overflow': 'bool',
diff --git a/qmp-commands.hx b/qmp-commands.hx
index a22621f..ed3ab92 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3407,6 +3407,7 @@ Each array entry contains the following:
- "promiscuous": promiscuous mode is enabled (json-bool)
- "multicast": multicast receive state (one of 'normal', 'none', 'all')
- "unicast": unicast receive state (one of 'normal', 'none', 'all')
+- "vlan": vlan receive state (one of 'normal', 'none', 'all') (Since 2.0)
- "broadcast-allowed": allow to receive broadcast (json-bool)
- "multicast-overflow": multicast table is overflowed (json-bool)
- "unicast-overflow": unicast table is overflowed (json-bool)
@@ -3424,6 +3425,7 @@ Example:
"name": "vnet0",
"main-mac": "52:54:00:12:34:56",
"unicast": "normal",
+ "vlan": "normal",
"vlan-table": [
4,
0
--
1.8.5.3
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH v4 for 2.0] virtio-net: add vlan receive state to RxFilterInfo
2014-03-26 0:19 [Qemu-devel] [PATCH v4 for 2.0] virtio-net: add vlan receive state to RxFilterInfo Amos Kong
@ 2014-03-26 1:51 ` Eric Blake
2014-03-26 6:46 ` Michael S. Tsirkin
1 sibling, 0 replies; 4+ messages in thread
From: Eric Blake @ 2014-03-26 1:51 UTC (permalink / raw)
To: Amos Kong, qemu-devel; +Cc: vyasevic, mst, sf, lcapitulino
[-- Attachment #1: Type: text/plain, Size: 987 bytes --]
On 03/25/2014 06:19 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.
>
> 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.
The use of "also" means this might have been better as two patches (bug
fix separate from qapi addition). But as we are getting close to 2.0,
I'm okay if you add:
>
> [1] http://lists.nongnu.org/archive/html/qemu-devel/2014-02/msg02604.html
>
> Signed-off-by: Amos Kong <akong@redhat.com>
> ---
Reviewed-by: Eric Blake <eblake@redhat.com>
--
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 --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH v4 for 2.0] virtio-net: add vlan receive state to RxFilterInfo
2014-03-26 0:19 [Qemu-devel] [PATCH v4 for 2.0] virtio-net: add vlan receive state to RxFilterInfo Amos Kong
2014-03-26 1:51 ` Eric Blake
@ 2014-03-26 6:46 ` Michael S. Tsirkin
2014-03-26 10:36 ` Amos Kong
1 sibling, 1 reply; 4+ messages in thread
From: Michael S. Tsirkin @ 2014-03-26 6:46 UTC (permalink / raw)
To: Amos Kong; +Cc: vyasevic, sf, qemu-devel, lcapitulino
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?
> 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 <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 (mst)
> V4: fix boundary of vlan mapping (eric)
> ---
> hw/net/virtio-net.c | 42 +++++++++++++++++++++++++++++-------------
> qapi-schema.json | 3 +++
> qmp-commands.hx | 2 ++
> 3 files changed, 34 insertions(+), 13 deletions(-)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index fd23c46..43b4eda 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);
> @@ -273,19 +293,15 @@ static RxFilterInfo *virtio_net_query_rxfilter(NetClientState *nc)
> str_list = entry;
> }
> info->multicast_table = str_list;
> + info->vlan_table = get_vlan_table(n);
>
> - 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->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.
> - 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
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -4184,6 +4184,8 @@
> #
> # @unicast: unicast receive state
> #
> +# @vlan: vlan receive state (Since 2.0)
> +#
> # @broadcast-allowed: whether to receive broadcast
> #
> # @multicast-overflow: multicast table is overflowed or not
> @@ -4207,6 +4209,7 @@
> 'promiscuous': 'bool',
> 'multicast': 'RxState',
> 'unicast': 'RxState',
> + 'vlan': 'RxState',
> 'broadcast-allowed': 'bool',
> 'multicast-overflow': 'bool',
> 'unicast-overflow': 'bool',
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index a22621f..ed3ab92 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -3407,6 +3407,7 @@ Each array entry contains the following:
> - "promiscuous": promiscuous mode is enabled (json-bool)
> - "multicast": multicast receive state (one of 'normal', 'none', 'all')
> - "unicast": unicast receive state (one of 'normal', 'none', 'all')
> +- "vlan": vlan receive state (one of 'normal', 'none', 'all') (Since 2.0)
> - "broadcast-allowed": allow to receive broadcast (json-bool)
> - "multicast-overflow": multicast table is overflowed (json-bool)
> - "unicast-overflow": unicast table is overflowed (json-bool)
> @@ -3424,6 +3425,7 @@ Example:
> "name": "vnet0",
> "main-mac": "52:54:00:12:34:56",
> "unicast": "normal",
> + "vlan": "normal",
> "vlan-table": [
> 4,
> 0
> --
> 1.8.5.3
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH v4 for 2.0] virtio-net: add vlan receive state to RxFilterInfo
2014-03-26 6:46 ` Michael S. Tsirkin
@ 2014-03-26 10:36 ` Amos Kong
0 siblings, 0 replies; 4+ messages in thread
From: Amos Kong @ 2014-03-26 10:36 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: vyasevic, sf, qemu-devel, lcapitulino
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 <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 (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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-03-26 10:36 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-26 0:19 [Qemu-devel] [PATCH v4 for 2.0] virtio-net: add vlan receive state to RxFilterInfo Amos Kong
2014-03-26 1:51 ` Eric Blake
2014-03-26 6:46 ` Michael S. Tsirkin
2014-03-26 10:36 ` Amos Kong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).