qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] net/hub: remove can_receive handler


From: Fedorov Sergey
Subject: Re: [Qemu-devel] [PATCH] net/hub: remove can_receive handler
Date: Tue, 23 Apr 2013 15:58:02 +0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130329 Thunderbird/17.0.5

On 04/23/2013 03:48 PM, Stefan Hajnoczi wrote:
On Tue, Apr 23, 2013 at 01:32:11PM +0400, Fedorov Sergey wrote:
On 04/23/2013 10:58 AM, Stefan Hajnoczi wrote:
On Mon, Apr 22, 2013 at 07:27:21PM +0400, Fedorov Sergey wrote:
On 04/22/2013 06:57 PM, Stefan Hajnoczi wrote:
On Mon, Apr 22, 2013 at 04:26:16PM +0400, Fedorov Sergey wrote:
On 04/22/2013 03:47 PM, Stefan Hajnoczi wrote:
On Thu, Apr 18, 2013 at 03:31:55PM +0400, Sergey Fedorov wrote:
Network hub should always receive incoming packets. Then forward them to
the appropriate port queue and let the qemu_send_packet() do the right
things. If the destination queue cannot receive the packet it will be
appended to the queue. When the receiver call
qemu_flush_queued_packets() later the queue will be really flushed and
no packets will be stalled in the sender network queue.

Signed-off-by: Sergey Fedorov <address@hidden>
---
  net/hub.c |   20 --------------------
  1 file changed, 20 deletions(-)
What is the point of this change?  There is no semantic difference for
well-behaved net clients.

Does it fix a bug, if so, please include details?

Stefan


Yes, this fixes a bug. There were packet stalls when using user-mode
networking with USB network device. slirp_output() calls
qemu_send_packet() which eventually calls qemu_net_queue_send().
qemu_net_queue_send() calls qemu_can_send_packet(), which calls
can_receive() callback of network hub. Then
net_hub_port_can_receive() also calls qemu_can_send_packet() for
each port except packet source port.

Sometimes USB network device is not able to receive packet and
qemu_can_send_packet() returns false. In my case there is no more
ports and net_hub_port_can_receive() returns false. So
qemu_net_queue_send() call qemu_net_queue_append() instead of
qemu_net_queue_deliver(). qemu_net_queue_append() appends the packet
to the receiving port of the network hub which is not flushed when
USB netork device calls qemu_flush_queued_packets(). It is flushed
only when slirp resend the packet by timeout.

Actually there is no need in net_hub_port_can_receive() as the
network hub can always receive packets and pass it to its port
network clients with qemu_send_packet(). And if the destination port
network client cannot receive the packet it will be queued in the
*destination* port network client queue. Queued packets from that
queue will be delivered as soon as the network client call
qemu_flush_queued_packets().
Please confirm the bug is still present in qemu.git/master.  It should
have been fixed by the following commit:

   commit 199ee608f0d08510b5c6c37f31a7fbff211d63c4
   Author: Luigi Rizzo <address@hidden>
   Date:   Tue Feb 5 17:53:31 2013 +0100

       net: fix qemu_flush_queued_packets() in presence of a hub

Stefan

Yes, this commit fixes a bug but from other side. I think it's
better to just let the qemu_send_packet() do the right things.

E.g. network hub has 3 ports. Suppose when iterating through port
list in net_hub_port_can_receive() a packet is successfully
delivered to the first port, and then is queued in the source port
queue because the second port cannot receive packets. Later
net_hub_flush() will flush the packet from the source port queue and
it will be delivered in every port. But it had been already
delivered to one of them. So it will be delivered twice to some
ports. Moreover there is less chance to dequeue the packet if
several clients can't receive periodically.
Did you mean "second port" instead of "source port" in the beginning?

I don't see a scenario where the packet is delivered to the same port
twice.  If one port can receive then the packet will be sent and other
ports will queue the packet.
Whether dump network client can always receive? If so, then the
packet will always be sent to the dump client regardless of whether
the other clients can receive. Is there actually any
synchronization?
Yes, the dump net client can always receive.  Other net clients may not.

There is no explicit synchronization between send queues on the hub.

Stefan

So why don't we apply this patch?

--
Best regards,
Sergey Fedorov, Junior Software Engineer,
Samsung R&D Institute Rus.
E-mail: address@hidden




reply via email to

[Prev in Thread] Current Thread [Next in Thread]