[Qemu-devel] [PATCH] net: set a default value for sndbuf=

[Mark McLoughlin <markmc@redhat.com>]

On reflection, perhaps it does make sense to set a default value for
the sndbuf= tap parameter.

For best effect, sndbuf= should be set to just below the capacity of
the physical NIC.

Setting it higher will cause packets to be dropped before the limit
is hit. Setting it much lower will not cause any problems unless
you set it low enough such that the guest cannot queue up new packets
before the NIC has emptied its queue.

In Linux, txqueuelen=1000 by default for ethernet NICs. Given a 1500
byte MTU, 1Mb is a good choice for sndbuf.

If it turns out that txqueuelen is actually much lower than this, then
sndbuf is essentially disabled. In the event that txqueuelen is much
higher, it's unlikely that the NIC will be able to empty a 1Mb queue.

Thanks to Herbert Xu for this logic.

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
Cc: Herbert Xu <herbert.xu@redhat.com>
---
 net.c           |   34 +++++++++++++++++++++++++++++-----
 qemu-options.hx |    3 ++-
 2 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/net.c b/net.c
index 6d82d59..21842d0 100644
--- a/net.c
+++ b/net.c
@@ -1396,17 +1396,39 @@ static void tap_send(void *opaque)
     } while (size > 0);
 }
 
-static void tap_set_sndbuf(TAPState *s, int sndbuf, Monitor *mon)
-{
 #ifdef TUNSETSNDBUF
+/* sndbuf should be set to a value lower than the tx queue
+ * capacity of any destination network interface.
+ * Ethernet NICs generally have txqueuelen=1000, so 1Mb is
+ * a good default, given a 1500 byte MTU.
+ */
+#define TAP_DEFAULT_SNDBUF 1024*1024
+
+static void tap_set_sndbuf(TAPState *s, const char *sndbuf_str, Monitor *mon)
+{
+    int sndbuf = TAP_DEFAULT_SNDBUF;
+
+    if (sndbuf_str) {
+        sndbuf = atoi(sndbuf_str);
+    }
+
+    if (!sndbuf) {
+        sndbuf = INT_MAX;
+    }
+
     if (ioctl(s->fd, TUNSETSNDBUF, &sndbuf) == -1) {
         config_error(mon, "TUNSETSNDBUF ioctl failed: %s\n",
                      strerror(errno));
     }
+}
 #else
-    config_error(mon, "No '-net tap,sndbuf=<nbytes>' support available\n");
-#endif
+static void tap_set_sndbuf(TAPState *s, const char *sndbuf_str, Monitor *mon)
+{
+    if (sndbuf_str) {
+        config_error(mon, "No '-net tap,sndbuf=<nbytes>' support available\n");
+    }
 }
+#endif /* TUNSETSNDBUF */
 
 static void tap_cleanup(VLANClientState *vc)
 {
@@ -2653,9 +2675,11 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
             s = net_tap_init(vlan, device, name, ifname, setup_script, down_script);
         }
         if (s != NULL) {
+            const char *sndbuf_str = NULL;
             if (get_param_value(buf, sizeof(buf), "sndbuf", p)) {
-                tap_set_sndbuf(s, atoi(buf), mon);
+                sndbuf_str = buf;
             }
+            tap_set_sndbuf(s, sndbuf_str, mon);
             ret = 0;
         } else {
             ret = -1;
diff --git a/qemu-options.hx b/qemu-options.hx
index a94f9d3..8947d05 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -778,7 +778,8 @@ DEF("net", HAS_ARG, QEMU_OPTION_net,
     "                use '[down]script=no' to disable script execution;\n"
     "                use 'fd=h' to connect to an already opened TAP interface\n"
 #ifdef TUNSETSNDBUF
-    "                use 'sndbuf=nbytes' to limit the size of the send buffer\n"
+    "                use 'sndbuf=nbytes' to limit the size of the send buffer; the\n"
+    "                default of 'sndbuf=1048576' can be disabled using 'sndbuf=0'\n"
 #endif
 #endif
     "-net socket[,vlan=n][,name=str][,fd=h][,listen=[host]:port][,connect=host:port]\n"
-- 
1.6.2.2