From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1NFuLe-0002sA-35
	for qemu-devel@nongnu.org; Wed, 02 Dec 2009 13:54:46 -0500
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43)
	id 1NFuLZ-0002iq-Dd
	for qemu-devel@nongnu.org; Wed, 02 Dec 2009 13:54:45 -0500
Received: from [199.232.76.173] (port=34980 helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1NFuLY-0002iV-Ux
	for qemu-devel@nongnu.org; Wed, 02 Dec 2009 13:54:40 -0500
Received: from mx1.redhat.com ([209.132.183.28]:38582)
	by monty-python.gnu.org with esmtp (Exim 4.60)
	(envelope-from <quintela@redhat.com>) id 1NFuLY-00082N-Bw
	for qemu-devel@nongnu.org; Wed, 02 Dec 2009 13:54:40 -0500
From: Juan Quintela <quintela@redhat.com>
In-Reply-To: <20091202182939.GD3956@redhat.com> (Michael S. Tsirkin's message
	of "Wed, 2 Dec 2009 20:29:39 +0200")
References: <cover.1259754426.git.quintela@redhat.com>
	<0212bb69ec7e94e572f4f0a7d8718a6131e9ceaa.1259754427.git.quintela@redhat.com>
	<20091202144915.GD18519@redhat.com> <m3y6llcjvt.fsf@neno.neno>
	<20091202182939.GD3956@redhat.com>
Date: Wed, 02 Dec 2009 19:53:58 +0100
Message-ID: <m37ht5cim1.fsf@neno.neno>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Subject: [Qemu-devel] Re: [PATCH 27/41] virtio-net: abstract vlans operations
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/pipermail/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: qemu-devel@nongnu.org

"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Wed, Dec 02, 2009 at 07:26:30PM +0100, Juan Quintela wrote:
>> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>> > On Wed, Dec 02, 2009 at 01:04:25PM +0100, Juan Quintela wrote:
>> >> 
>> >> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> >> ---
>> >>  hw/virtio-net.c |   21 ++++++++++++++++++---
>> >>  1 files changed, 18 insertions(+), 3 deletions(-)
>> >> 
>> >> diff --git a/hw/virtio-net.c b/hw/virtio-net.c
>> >> index 97db0d0..cf13e94 100644
>> >> --- a/hw/virtio-net.c
>> >> +++ b/hw/virtio-net.c
>> >> @@ -63,6 +63,21 @@ typedef struct VirtIONet
>> >>   * - we could suppress RX interrupt if we were so inclined.
>> >>   */
>> >> 
>> >> +static void vlan_add(VirtIONet *n, int vid)
>> >> +{
>> >> +    n->vlans[vid >> 5] |= (1U << (vid & 0x1f));
>> >> +}
>> >> +
>> >> +static void vlan_del(VirtIONet *n, int vid)
>> >> +{
>> >> +    n->vlans[vid >> 5] &= ~(1U << (vid & 0x1f));
>> >> +}
>> >> +
>> >> +static bool vlan_is_set(VirtIONet *n, int vid)
>> >> +{
>> >> +    return n->vlans[vid >> 5] & ~(1U << (vid & 0x1f));
>> >
>> > This one looks wrong. Did you check this does not break vlans?
>> 
>> I don't know how to use vlans (more than one).
>> 
>> Current code is wrong (it is not big<->little endian safe.
>> And it is not trivial to make it correct and work from big/little endian
>> old states.
>
> So migration is broken and you want to fix it, fine,
> but please do not break the feature itself.
> You can't check whether bit "vid"
> is set by doing  & ~(1 << vid), can you?

/me stares at it.  stares something more.
I am enlighted now!!!

/me put brown paper bag on head.

Fixing in next version, thanks.
>
>> I commented in the cover patch that this needed testing/investigation.
>> How is that supposde to be tested?
>> 
>> Later, Juan.
>
> Pass in a packet, see if it makes it in?

I use default networking, and networking works.  but I don't have more
than one vlan.

Later, Juan.