qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Stefan Hajnoczi <stefanha@redhat.com>
To: Fedorov Sergey <s.fedorov@samsung.com>
Cc: Stefan Hajnoczi <stefanha@gmail.com>,
	aliguori@us.ibm.com, a.basov@samsung.com, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH] net/hub: remove can_receive handler
Date: Mon, 22 Apr 2013 16:57:49 +0200	[thread overview]
Message-ID: <20130422145749.GA28049@stefanha-thinkpad.redhat.com> (raw)
In-Reply-To: <51752C68.6030408@samsung.com>

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 <s.fedorov@samsung.com>
> >>---
> >>  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 <rizzo@iet.unipi.it>
  Date:   Tue Feb 5 17:53:31 2013 +0100

      net: fix qemu_flush_queued_packets() in presence of a hub

Stefan

  reply	other threads:[~2013-04-22 14:58 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-18 11:31 [Qemu-devel] [PATCH] net/hub: remove can_receive handler Sergey Fedorov
2013-04-22 11:47 ` Stefan Hajnoczi
2013-04-22 12:26   ` Fedorov Sergey
2013-04-22 14:57     ` Stefan Hajnoczi [this message]
2013-04-22 15:27       ` Fedorov Sergey
2013-04-22 16:09         ` Paolo Bonzini
2013-04-23  7:27           ` Fedorov Sergey
2013-04-23  6:58         ` Stefan Hajnoczi
2013-04-23  7:41           ` Fedorov Sergey
2013-04-23 12:00             ` Stefan Hajnoczi
2013-04-23 13:14               ` Fedorov Sergey
2013-10-21 11:44               ` Fedorov Sergey
2013-10-21 11:52                 ` Fedorov Sergey
2013-10-28  7:26                   ` Fedorov Sergey
2013-10-29 14:55                 ` Stefan Hajnoczi
2013-10-30 10:29                   ` Fedorov Sergey
2013-10-30 12:55                     ` Stefan Hajnoczi
2013-04-23  9:32           ` Fedorov Sergey
2013-04-23 11:48             ` Stefan Hajnoczi
2013-04-23 11:58               ` Fedorov Sergey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130422145749.GA28049@stefanha-thinkpad.redhat.com \
    --to=stefanha@redhat.com \
    --cc=a.basov@samsung.com \
    --cc=aliguori@us.ibm.com \
    --cc=qemu-devel@nongnu.org \
    --cc=s.fedorov@samsung.com \
    --cc=stefanha@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).