[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: |
Tue, 25 Mar 2014 12:08:20 +0200 |
On Tue, Mar 25, 2014 at 05:39:12PM +0800, Amos Kong wrote:
> On Wed, Mar 19, 2014 at 11:38:57PM +0100, Stefan Fritsch wrote:
> > Hi,
> >
> > Am Dienstag, 25. Februar 2014, 11:06:33 schrieb Amos Kong:
> > > > > 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.
> >
> >
> > AFAICS, no fix has been committed, yet. Is there anything I need to do
> > to get this fixed?
>
> Michael asked in IRC to continually add a RxState for vlan in
> RxFilter notification through QMP events.
>
> (sorry I didn't talk too much becaused I'm in meeting)
>
> I mentioned in [1] [2], we don't need to add this new state
> if we apply patch Stefan's patch[3]
>
> Michael said Stefan's fix is wrong, but the problem exists.
> The problem is the bug fixed by [3], not the problem that was
> mentioned in [4]
>
>
> Michael, what's wrong with Stefan's patch?
>
>
> Thanks, Amos
>
> [1] http://lists.gnu.org/archive/html/qemu-devel/2014-02/msg03835.html
> [2] https://lists.gnu.org/archive/html/qemu-devel/2014-02/msg04434.html
> [3] virtio-net: Do not filter VLANs without F_CTRL_VLAN
> [4] https://lists.gnu.org/archive/html/qemu-devel/2014-02/msg02604.html
>
Mainly what's wrong is the effect this has on the output of
query-rx-filter. the patch is incomplete: management should be able to
see that vlan filtering is disabled.
Besides, looking at guest features at reset time is also
not a good idea, it works by luck.
A better place would be virtio_net_set_features.
> > > > > Can we just update receive_filter() to filter out VLAN-tagged
> > > > > packets only when VIRTIO_NET_F_CTRL_VLAN is negotiated?
> > >
> > > If we change receive_filter(), we also need a flag to indicate
> > > management this feature isn't negotiated, management will do some
> > > additional operation to host device to get same effect.
> > >
> > >
> > > > We could. But this adds a (very small) per-packet overhead while
> > > > my patch only adds overhead during reset. Therefore I didn't
> > > > take that approach. But if changing receive_filter() makes
> > > > management much easier, that could be acceptable.
> > >
> > > Actually your solution is better, QEMU will return a long list
> > > [0,1,2,...4095] to management, host device will filter all the vlan
> > > packets and send to QEMU.
> > >
> > > So the problem raised by mst doesn't exist.
> >
> > Cheers,
> > Stefan
> >
>
> --
> Amos.