Patchwork [v2] fix qemu_flush_queued_packets() in presence of a hub

login
register
mail settings
Submitter Luigi Rizzo
Date Feb. 5, 2013, 2:46 p.m.
Message ID <20130205144619.GA29583@onelab2.iet.unipi.it>
Download mbox | patch
Permalink /patch/218266/
State New
Headers show

Comments

Luigi Rizzo - Feb. 5, 2013, 2:46 p.m.
On Tue, Jan 29, 2013 at 02:08:27PM +0100, Stefan Hajnoczi wrote:

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

I'd leave it like this for now. Here is the version with fixed style bugs.

----------------------------------------

DESCRIPTION:

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>
Stefan Hajnoczi - Feb. 5, 2013, 3:55 p.m.
On Tue, Feb 05, 2013 at 03:46:19PM +0100, Luigi Rizzo wrote:
> On Tue, Jan 29, 2013 at 02:08:27PM +0100, Stefan Hajnoczi wrote:
> 
> > 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.
> 
> I'd leave it like this for now. Here is the version with fixed style bugs.

Great, please send the patch as a new email thread.  That way patch
tracking scripts will detect it and it's easier for maintainers to spot
the patch as well.

Stefan

Patch

diff --git a/net/hub.c b/net/hub.c
index a24c9d1..df32074 100644
--- a/net/hub.c
+++ b/net/hub.c
@@ -338,3 +338,17 @@  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 9806862..9489702 100644
--- a/net/net.c
+++ b/net/net.c
@@ -439,6 +439,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).