qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] net: set a default value for sndbuf=
@ 2009-06-30  9:02 Mark McLoughlin
  2009-06-30  9:11 ` [Qemu-devel] " Herbert Xu
  0 siblings, 1 reply; 2+ messages in thread
From: Mark McLoughlin @ 2009-06-30  9:02 UTC (permalink / raw)
  To: qemu-devel; +Cc: Herbert Xu

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

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* [Qemu-devel] Re: [PATCH] net: set a default value for sndbuf=
  2009-06-30  9:02 [Qemu-devel] [PATCH] net: set a default value for sndbuf= Mark McLoughlin
@ 2009-06-30  9:11 ` Herbert Xu
  0 siblings, 0 replies; 2+ messages in thread
From: Herbert Xu @ 2009-06-30  9:11 UTC (permalink / raw)
  To: Mark McLoughlin; +Cc: qemu-devel

On Tue, Jun 30, 2009 at 10:02:57AM +0100, Mark McLoughlin wrote:
> 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.

You mean 1MB :)

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

Looks good to me!

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2009-06-30  9:12 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-06-30  9:02 [Qemu-devel] [PATCH] net: set a default value for sndbuf= Mark McLoughlin
2009-06-30  9:11 ` [Qemu-devel] " Herbert Xu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).