Re: [Qemu-devel] [PATCH v1 01/12] net: Add a hub net client

[Zhi Yong Wu]

On Fri, Mar 9, 2012 at 6:33 PM, Paolo Bonzini <address@hidden> wrote:
> Il 09/03/2012 10:00, address@hidden ha scritto:
>> +        /* TODO use qemu_send_packet() or need to call *_deliver_* 
>> directly? */
>> +        /* TODO ignore return value? */
>> +        qemu_send_packet(&port->nc, buf, len);
>
> Did you see my message at
>
> http://permalink.gmane.org/gmane.comp.emulators.qemu/134998
Yeah, i ever saw this, but read it just now. In your comments,
there're some places where i have not completely understand.

> 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.

> 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.

> 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.
sent-packet callback is sent_cb here? i noticed that sent_cb is currently NULL.
> When the count is non-zero, can_receive returns false (and vice versa).
>Can you elaborate this? since can_receive is called finally by 
>qemu_send_packet, how can can_receive return false or true based on the count?
 > The sent_cb also needs to call qemu_flush_queued_packets when the
> count drop to zero.
When sent_cb is called, it means that the packet has been sent out, right?

> 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 ports might linearize the same packet multiple times in
> qemu_net_queue_append_iov.  The limit however is packet size * number of
I think that when one packet need to be enqueued, we should at first
determine if it has existed in send queue.
> 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.

>
> ?
>
> Paolo



-- 
Regards,

Zhi Yong Wu