From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1Lu8ft-0004BN-9a
	for qemu-devel@nongnu.org; Wed, 15 Apr 2009 13:13:25 -0400
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43)
	id 1Lu8fo-00048T-Or
	for qemu-devel@nongnu.org; Wed, 15 Apr 2009 13:13:24 -0400
Received: from [199.232.76.173] (port=36390 helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1Lu8fo-00048K-EU
	for qemu-devel@nongnu.org; Wed, 15 Apr 2009 13:13:20 -0400
Received: from mx2.redhat.com ([66.187.237.31]:35282)
	by monty-python.gnu.org with esmtp (Exim 4.60)
	(envelope-from <markmc@redhat.com>) id 1Lu8fo-0005Ul-2D
	for qemu-devel@nongnu.org; Wed, 15 Apr 2009 13:13:20 -0400
From: Mark McLoughlin <markmc@redhat.com>
In-Reply-To: <49E5EA9F.2090206@siemens.com>
References: <20090414172954.15035.73053.stgit@mchn012c.ww002.siemens.net>
	<20090414172955.15035.35614.stgit@mchn012c.ww002.siemens.net>
	<1239801006.4431.132.camel@blaa>  <49E5EA9F.2090206@siemens.com>
Content-Type: text/plain
Date: Wed, 15 Apr 2009 18:13:16 +0100
Message-Id: <1239815596.4431.157.camel@blaa>
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: [Qemu-devel] Re: [PATCH 6/7] net: Add support for capturing VLANs
Reply-To: Mark McLoughlin <markmc@redhat.com>, qemu-devel@nongnu.org
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: Jan Kiszka <jan.kiszka@siemens.com>
Cc: qemu-devel@nongnu.org

On Wed, 2009-04-15 at 16:09 +0200, Jan Kiszka wrote:
> >> Besides rebasing and some minor cleanups, the major differences to the
> >> original version are:
> >>  - support for enabling/disabling via the monitor (host_net_add/remove)
> >>  - always register dump client at the head of a VLAN queue
> >>    (instead of special handling for slirp)
> > 
> > Could you explain why you need this? 
> > 
> > I'd prefer if we didn't have to add qemu_new_vlan_head_client()
> 
> Packet ordering: If you are sniffing from behind a vlan client in the
> queue, you may see its immediate reply to a certain packet before the
> triggering packet. Tristan solved this by pushing slirp (the only source
> for reordering so far) at the end of the queue, but I think it's rather
> the sniffer which has special requirements, so I pushed that one to the top.

Uggh, nasty ... slirp strikes again :-)

If we need to make the sniffer a special case, I'd suggest really making
it a special case by doing e.g.

static void dump_packet(Dumper *d, const uint8_t *buf, int size)
{
    /* write packet to pcap file */
}

void qemu_send_packet(...)
{
    if (vlan->dumper)
        dump_packet(d, buf, size);

    /* dispatch packets to vlan clients */
}

I think making it explicit is better than munging the vlan abstraction
any further.

Alternatively, we could fix slirp to push packets to a queue and flush
that queue in a bottom half.

Cheers,
Mark.