[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] virtio-net: Do not filter VLANs without F_CTRL_
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH] virtio-net: Do not filter VLANs without F_CTRL_VLAN |
Date: |
Sun, 16 Feb 2014 12:33:05 +0200 |
On Wed, Feb 12, 2014 at 10:46:28PM +0100, Stefan Fritsch wrote:
> If VIRTIO_NET_F_CTRL_VLAN is not negotiated, do not filter out all
> VLAN-tagged packets but send them to the guest.
>
> Signed-off-by: Stefan Fritsch <address@hidden>
Thanks for the patch.
I think there are still some issues after this
patch: we need to notify management when
this bit state changes.
And I think libvirt still does not look at the filter info
so it's probably not too late, and cleaner to simply tell it:
"all-vlans".
that is, add
'*vlan': 'RxState',
to the schema.
(is it true that it needs to be * because old qemu does not produce it?
maybe not ...)
Taking all this into account - this calls for checking
this bit in receive_filter like we do for e.g.
unicast addresses.
Amos, you wrote
commit b1be42803b31a913bab65bab563a8760ad2e7f7f
Author: Amos Kong <address@hidden>
Date: Fri Jun 14 15:45:52 2013 +0800
net: add support of mac-programming over macvtap in QEMU side
which conflicts here - could you take a look please?
Also Cc schema maintainers.
> ---
>
> This time CCing the maintainers.
>
> This fixes VLANs with OpenBSD guests (and probably NetBSD, too, because
> the OpenBSD driver started as a port from NetBSD).
>
>
> hw/net/virtio-net.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 3626608..0ae9a91 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -315,7 +315,11 @@ static void virtio_net_reset(VirtIODevice *vdev)
> memset(n->mac_table.macs, 0, MAC_TABLE_ENTRIES * ETH_ALEN);
> memcpy(&n->mac[0], &n->nic->conf->macaddr, sizeof(n->mac));
> qemu_format_nic_info_str(qemu_get_queue(n->nic), n->mac);
> - memset(n->vlans, 0, MAX_VLAN >> 3);
> + if (vdev->guest_features & (1 << VIRTIO_NET_F_CTRL_VLAN)) {
> + memset(n->vlans, 0, MAX_VLAN >> 3);
> + } else {
> + memset(n->vlans, 0xff, MAX_VLAN >> 3);
> + }
> }
>
> static void peer_test_vnet_hdr(VirtIONet *n)
This chunk doesn't make sense to me.
features are never set at reset, are they?
> @@ -515,6 +519,12 @@ static void virtio_net_set_features(VirtIODevice *vdev,
> uint32_t features)
> }
> vhost_net_ack_features(tap_get_vhost_net(nc->peer), features);
> }
> +
> + if (vdev->guest_features & (1 << VIRTIO_NET_F_CTRL_VLAN)) {
> + memset(n->vlans, 0, MAX_VLAN >> 3);
> + } else {
> + memset(n->vlans, 0xff, MAX_VLAN >> 3);
> + }
> }
>
> static int virtio_net_handle_rx_mode(VirtIONet *n, uint8_t cmd,
> --
> 1.7.10.4