From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:57628) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UUbrN-000244-6D for qemu-devel@nongnu.org; Tue, 23 Apr 2013 07:58:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UUbrL-0003QT-GJ for qemu-devel@nongnu.org; Tue, 23 Apr 2013 07:58:09 -0400 Received: from mailout3.w1.samsung.com ([210.118.77.13]:12680) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UUbrL-0003QJ-7B for qemu-devel@nongnu.org; Tue, 23 Apr 2013 07:58:07 -0400 Received: from eucpsbgm1.samsung.com (unknown [203.254.199.244]) by mailout3.w1.samsung.com (Oracle Communications Messaging Server 7u4-24.01(7.0.4.24.0) 64bit (built Nov 17 2011)) with ESMTP id <0MLP00DF0IIS59A0@mailout3.w1.samsung.com> for qemu-devel@nongnu.org; Tue, 23 Apr 2013 12:58:04 +0100 (BST) Message-id: <5176774A.7060205@samsung.com> Date: Tue, 23 Apr 2013 15:58:02 +0400 From: Fedorov Sergey MIME-version: 1.0 References: <1366284715-10107-1-git-send-email-s.fedorov@samsung.com> <20130422114711.GA21317@stefanha-thinkpad.redhat.com> <51752C68.6030408@samsung.com> <20130422145749.GA28049@stefanha-thinkpad.redhat.com> <517556D9.2080809@samsung.com> <20130423065841.GA6447@stefanha-thinkpad.redhat.com> <5176551B.1020606@samsung.com> <20130423114811.GB2431@stefanha-thinkpad.redhat.com> In-reply-to: <20130423114811.GB2431@stefanha-thinkpad.redhat.com> Content-type: text/plain; charset=ISO-8859-1; format=flowed Content-transfer-encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] net/hub: remove can_receive handler List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: aliguori@us.ibm.com, a.basov@samsung.com, qemu-devel@nongnu.org, Stefan Hajnoczi 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 >>>>>>>> --- >>>>>>>> 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 >>>>> 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: s.fedorov@samsung.com