Patchwork fix qemu_flush_queued_packets() in presence of a hub

login
register
mail settings
Submitter Luigi Rizzo
Date Jan. 22, 2013, 6:54 a.m.
Message ID <20130122065420.GA37378@onelab2.iet.unipi.it>
Download mbox | patch
Permalink /patch/214349/
State New
Headers show

Comments

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

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