Re: [Qemu-devel] [PATCH] fix qemu_flush_queued_packets() in presence of a hub

[Stefan Hajnoczi]

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

[Qemu-devel] [PATCH] fix qemu_flush_queued_packets() in presence of a hub

[Luigi Rizzo]

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 ri...@iet.unipi.it
 net/hub.c |   13 +
 net/hub.h |1 +
 net/net.c |6 ++

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