[Qemu-devel] Re: [PATCH 6/7] net: Add support for capturing VLANs

[Mark McLoughlin <markmc@redhat.com>]

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.