[Xen-changelog] [xen-unstable] [NET] back: Disable packet queuing when a client has no receive buffers.

[Xen patchbot-unstable <patchbot-unstable@xxxxxxxxxxxxxxxxxxx>]

# HG changeset patch
# User kfraser@xxxxxxxxxxxxxxxxxxxxx
# Node ID 224b80785d02e99de5f4a8718250e6256c2f9c91
# Parent  6a8ae4c34abef937cadb321157f7b295e3cd24c2
[NET] back: Disable packet queuing when a client has no receive buffers.

This turns out to be dangerous as packets can be queued indefinitely,
introducing resource dependencies between guests (if the queued
packets originate from another virtual machine). For example, this
can prevent interfaces from being torn down and hence defunct
virtual machines from being destroyed.

The queuing functionality is retained as a module option, but a
warning is printed if it is enabled. It is not intended for general use!

Signed-off-by: Keir Fraser <keir@xxxxxxxxxxxxx>
---
 linux-2.6-xen-sparse/drivers/xen/netback/interface.c |   27 +++++++++++++++----
 linux-2.6-xen-sparse/drivers/xen/netback/xenbus.c    |   22 +++++++++------
 2 files changed, 36 insertions(+), 13 deletions(-)

diff -r 6a8ae4c34abe -r 224b80785d02 
linux-2.6-xen-sparse/drivers/xen/netback/interface.c
--- a/linux-2.6-xen-sparse/drivers/xen/netback/interface.c      Wed Oct 04 
09:43:45 2006 +0100
+++ b/linux-2.6-xen-sparse/drivers/xen/netback/interface.c      Wed Oct 04 
16:12:35 2006 +0100
@@ -34,6 +34,24 @@
 #include <linux/ethtool.h>
 #include <linux/rtnetlink.h>
 
+/*
+ * Module parameter 'queue_length':
+ * 
+ * Enables queuing in the network stack when a client has run out of receive
+ * descriptors. Although this feature can improve receive bandwidth by avoiding
+ * packet loss, it can also result in packets sitting in the 'tx_queue' for
+ * unbounded time. This is bad if those packets hold onto foreign resources.
+ * For example, consider a packet that holds onto resources belonging to the
+ * guest for which it is queued (e.g., packet received on vif1.0, destined for
+ * vif1.1 which is not activated in the guest): in this situation the guest
+ * will never be destroyed, unless vif1.1 is taken down (which flushes the
+ * 'tx_queue').
+ * 
+ * Only set this parameter to non-zero value if you know what you are doing!
+ */
+static unsigned long netbk_queue_length = 0;
+module_param_named(queue_length, netbk_queue_length, ulong, 0);
+
 static void __netif_up(netif_t *netif)
 {
        enable_irq(netif->irq);
@@ -144,11 +162,10 @@ netif_t *netif_alloc(domid_t domid, unsi
 
        SET_ETHTOOL_OPS(dev, &network_ethtool_ops);
 
-       /*
-        * Reduce default TX queuelen so that each guest interface only
-        * allows it to eat around 6.4MB of host memory.
-        */
-       dev->tx_queue_len = 100;
+       dev->tx_queue_len = netbk_queue_length;
+       if (dev->tx_queue_len != 0)
+               printk(KERN_WARNING "netbk: WARNING: device '%s' has non-zero "
+                      "queue length (%lu)!\n", dev->name, dev->tx_queue_len);
 
        for (i = 0; i < ETH_ALEN; i++)
                if (be_mac[i] != 0)
diff -r 6a8ae4c34abe -r 224b80785d02 
linux-2.6-xen-sparse/drivers/xen/netback/xenbus.c
--- a/linux-2.6-xen-sparse/drivers/xen/netback/xenbus.c Wed Oct 04 09:43:45 
2006 +0100
+++ b/linux-2.6-xen-sparse/drivers/xen/netback/xenbus.c Wed Oct 04 16:12:35 
2006 +0100
@@ -366,6 +366,10 @@ static void connect(struct backend_info 
        be->netif->remaining_credit = be->netif->credit_bytes;
 
        xenbus_switch_state(dev, XenbusStateConnected);
+
+       /* May not get a kick from the frontend, so start the tx_queue now. */
+       if (!netbk_can_queue(be->netif->dev))
+               netif_start_queue(be->netif->dev);
 }
 
 
@@ -403,14 +407,16 @@ static int connect_rings(struct backend_
        }
        be->netif->copying_receiver = !!rx_copy;
 
-       if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-rx-notify", "%d",
-                        &val) < 0)
-               val = 0;
-       if (val)
-               be->netif->can_queue = 1;
-       else
-               /* Must be non-zero for pfifo_fast to work. */
-               be->netif->dev->tx_queue_len = 1;
+       if (be->netif->dev->tx_queue_len != 0) {
+               if (xenbus_scanf(XBT_NIL, dev->otherend,
+                                "feature-rx-notify", "%d", &val) < 0)
+                       val = 0;
+               if (val)
+                       be->netif->can_queue = 1;
+               else
+                       /* Must be non-zero for pfifo_fast to work. */
+                       be->netif->dev->tx_queue_len = 1;
+       }
 
        if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-sg", "%d", &val) < 0)
                val = 0;

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog