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

[Jan Kiszka]

Mark McLoughlin wrote:
> 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.

That's actually a good idea - in case something else once comes around
that gets confused by packet ordering. I'll check if we can do this more
or less transparently in the network layer (queue tx requests while
delivering). Shouldn't be hard (at least not as hard as changing slirp ;) ).

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux