Re: [Qemu-devel] [QEMU] net: adapt dump to support the new syntax

[Zhi Yong Wu]

On Mon, Feb 6, 2012 at 6:18 AM, Paolo Bonzini <address@hidden> wrote:
> On 01/24/2012 11:42 AM, Stefan Hajnoczi wrote:
>>
>> I refactored the network subsystem to drop the "VLAN" concept a while
>> back but never got around to submitting the patches:
>>
>> http://repo.or.cz/w/qemu/stefanha.git/shortlog/refs/heads/vlan-hub
>>
>> This branch completely removes the "VLAN" feature.  Instead it
>> introduces hubs, which are net clients that have multiple ports and
>> broadcast packets between them.  This allows you to achieve the same
>> behavior as "VLANs" except we remove all the hardcoded special cases
>> in the net subsystem and instead push that feature out into the hub
>> net client.
>
>
> That looks cool!
>
> I never worked with the network subsystem, but it seems to me that the only
> difference between hubs and VLANs in your branch is related to queues and
> flow control.  VLAN flow control is actually a bit mysterious to me, because
> all devices on a VLAN share a queue and I don't quite understand the
> consequences...  With a N-port hub instead you have N+1 queues.
No,  if N ports are used, it should have 2N queues.(each of emulated
NIC, 2 hub ports and net backend has 1 queue).
>
> Regarding your TODO here:
>
>> +        /* TODO use qemu_send_packet() or need to call *_deliver_*
>> directly? */
>
>
> I think uses qemu_send_packet() is right but I have some other doubts.
> Initially I spotted this code, where hubs and VLANs had separate handling.
>
> int qemu_can_send_packet(VLANClientState *sender)
> {
>    VLANState *vlan = sender->vlan;
>    VLANClientState *vc;
>
>    if (sender->peer) {
>        ...
>    }
>    ...
>    QTAILQ_FOREACH(vc, &vlan->clients, next) {
>        if (vc == sender) {
>            continue;
>        }
>
>        /* no can_receive() handler, they can always receive */
>        if (vc->info->can_receive && !vc->info->can_receive(vc)) {
>            return 0;
>        }
>    }
>    return 1;
> }
>
> This means VLANs will wait for all receivers to be ready and then do N-1
> synchronous sends.  Your code will queue N-1 asynchronous sends.
Yes.
>
> However, then I noticed that qemu_can_send_packet is not called very much,
> and I do not understand why qemu_net_queue_send and qemu_net_queue_send_iov
> do not call qemu_can_send_packet before calling deliver/deliver_iov.
Network backends such as tap, slirp have called qemu_can_send_packet
function before it call qemu_net_queue_send or
qemu_net_queue_send_iov.
eg. tap_send() when packets are sent to guest emulated NIC from network backend.
Moreover, only emulated NICs provide can_recieve function, but network
backends such as tap, slirp, etc. have not; So it means that
qemu_can_send_packet is currently used to control packets flow from
network backends to emulated NIC.

For those packets from emulated NIC to network backends, we can do
this type of flow contol but need network backends or hub port provide
their can_recieve function. otherwise it will not get expected result.

>
> If they did, hubs could then do their own flow control via can_receive.
>  When qemu_send_packet returns zero you increment a count of in-flight
> packets, and a sent-packet callback would decrement the same count. When the
> count is non-zero, can_receive returns false (and vice versa).  The sent_cb
> also needs to call qemu_flush_queued_packets when the count drop to zero.
great idea.
>
> With this in place, I think the other TODO about the return value is easily
> solved; receive/receive_iov callbacks can simply return immediate success,
> and later block further sends.
>
> Due to the separate per-port send queues, when many sends are blocking many
One guest should usually only have several ports, not many in regular system.
> ports might linearize the same packet multiple times in
> qemu_net_queue_append_iov.  The limit however is packet size * number of
I am not sure which case you are talking about? when the packets are
sent from emulated NIC to hub port, or from one hub port to another
hub ports?
For every scenario, i don't think that your case can take place.
> ports, which is not more than a few KB; it's more of a performance problem
> and VLANs/hubs should only be used when performance doesn't matter (as with
> the dump client).
>
> BTW, an additional cleanup that you can do on top is to remove the
> deliver/deliver_iov function pointers in net/queue.c and qemu_new_net_queue,
> because they will always be qemu_deliver_packet and qemu_deliver_packet_iov.
Done.
>
> Paolo
>



-- 
Regards,

Zhi Yong Wu