fix qemu_flush_queued_packets() in presence of a hub

Submitted by Luigi Rizzo on Jan. 22, 2013, 6:54 a.m.

Details

Message ID 20130122065420.GA37378@onelab2.iet.unipi.it
State New
Headers show

Commit Message

Luigi Rizzo Jan. 22, 2013, 6:54 a.m.
when frontend and backend are connected through a hub as below
(showing only one direction), and the frontend (or in general, all
output ports of the hub) cannot accept more traffic, the backend
queues packets in queue-A.

When the frontend (or in general, one output port) becomes ready again,
quemu tries to flush packets from queue-B, which is unfortunately empty.

   e1000.0 <--[queue B]-- hub0port0(hub)hub0port1 <--[queue A]-- tap.0

To fix this i propose to introduce a new function net_hub_flush()
which is called when trying to flush a queue connected to a hub.

	cheers
	luigi

Signed-off-by: Luigi Rizzo <rizzo@iet.unipi.it>
 net/hub.c |   13 +++++++++++++
 net/hub.h |    1 +
 net/net.c |    6 ++++++

Comments

Stefan Hajnoczi Jan. 29, 2013, 1:08 p.m.
On Tue, Jan 22, 2013 at 07:54:20AM +0100, Luigi Rizzo wrote:
> when frontend and backend are connected through a hub as below
> (showing only one direction), and the frontend (or in general, all
> output ports of the hub) cannot accept more traffic, the backend
> queues packets in queue-A.
> 
> When the frontend (or in general, one output port) becomes ready again,
> quemu tries to flush packets from queue-B, which is unfortunately empty.
> 
>    e1000.0 <--[queue B]-- hub0port0(hub)hub0port1 <--[queue A]-- tap.0
> 
> To fix this i propose to introduce a new function net_hub_flush()
> which is called when trying to flush a queue connected to a hub.
> 
> 	cheers
> 	luigi
> 
> Signed-off-by: Luigi Rizzo <rizzo@iet.unipi.it>
>  net/hub.c |   13 +++++++++++++
>  net/hub.h |    1 +
>  net/net.c |    6 ++++++

Please run scripts/checkpatch.pl:

 * 4 space indentation instead of tabs.
 * Always use {} even for a 1-statement if body.

It's cleaner to add a bool (*flush)(NetClientState *nc) function to
NetClientInfo but given that net.c already knows about hub.c
specifically we don't gain much by trying to decouple this callback.  If
you want to leave it hardcoded that's fine by me.

Stefan

Patch hide | download patch | download mbox

diff --git a/net/hub.c b/net/hub.c
index a24c9d1..08f95d0 100644
--- a/net/hub.c
+++ b/net/hub.c
@@ -338,3 +338,16 @@  void net_hub_check_clients(void)
         }
     }
 }
+
+bool net_hub_flush(NetClientState *nc)
+{
+    NetHubPort *port;
+    NetHubPort *source_port = DO_UPCAST(NetHubPort, nc, nc);
+    int ret = 0;
+
+    QLIST_FOREACH(port, &source_port->hub->ports, next) {
+	if (port != source_port)
+	    ret += qemu_net_queue_flush(port->nc.send_queue);
+    }
+    return ret ? true : false;
+}
diff --git a/net/hub.h b/net/hub.h
index 583ada8..a625eff 100644
--- a/net/hub.h
+++ b/net/hub.h
@@ -21,5 +21,6 @@  NetClientState *net_hub_add_port(int hub_id, const char *name);
 NetClientState *net_hub_find_client_by_name(int hub_id, const char *name);
 void net_hub_info(Monitor *mon);
 void net_hub_check_clients(void);
+bool net_hub_flush(NetClientState *nc);
 
 #endif /* NET_HUB_H */
diff --git a/net/net.c b/net/net.c
index 816c987..8caa368 100644
--- a/net/net.c
+++ b/net/net.c
@@ -355,6 +355,12 @@  void qemu_flush_queued_packets(NetClientState *nc)
 {
     nc->receive_disabled = 0;
 
+    if (nc->peer && nc->peer->info->type == NET_CLIENT_OPTIONS_KIND_HUBPORT) {
+	if (net_hub_flush(nc->peer)) {
+	    qemu_notify_event();
+	}
+	return;
+    }
     if (qemu_net_queue_flush(nc->send_queue)) {
         /* We emptied the queue successfully, signal to the IO thread to repoll
          * the file descriptor (for tap, for example).