[Qemu-devel] [PATCH 0.14] tap: safe sndbuf default

[Qemu-devel] [PATCH 0.14] tap: safe sndbuf default

[Michael S. Tsirkin]

With current sndbuf default value, a blocked
target guest can prevent another guest from
transmitting any packets. While current
sndbuf value (1M) is reported to help some
UDP based workloads, the default should
be safe (0).

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

I think this should go into 0.14.
Comments?

 net/tap-linux.c |   13 +++++++++----
 qemu-options.hx |    2 +-
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/net/tap-linux.c b/net/tap-linux.c
index f7aa904..ff8cad0 100644
--- a/net/tap-linux.c
+++ b/net/tap-linux.c
@@ -82,12 +82,17 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr, int vnet_hdr_required
     return fd;
 }
 
-/* sndbuf should be set to a value lower than the tx queue
- * capacity of any destination network interface.
+/* sndbuf implements a kind of flow control for tap.
+ * Unfortunately when it's enabled, and packets are sent
+ * to other guests on the same host, the receiver
+ * can lock up the transmitter indefinitely.
+ *
+ * To avoid packet loss, 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.
+ * a good value, given a 1500 byte MTU.
  */
-#define TAP_DEFAULT_SNDBUF 1024*1024
+#define TAP_DEFAULT_SNDBUF 0
 
 int tap_set_sndbuf(int fd, QemuOpts *opts)
 {
diff --git a/qemu-options.hx b/qemu-options.hx
index 11c93a2..ca4d5c9 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1057,7 +1057,7 @@ 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"
     "                use 'sndbuf=nbytes' to limit the size of the send buffer (the\n"
-    "                default of 'sndbuf=1048576' can be disabled using 'sndbuf=0')\n"
+    "                default is disabled 'sndbuf=0' to enable flow control set 'sndbuf=1048576')\n"
     "                use vnet_hdr=off to avoid enabling the IFF_VNET_HDR tap flag\n"
     "                use vnet_hdr=on to make the lack of IFF_VNET_HDR support an error condition\n"
     "                use vhost=on to enable experimental in kernel accelerator\n"
-- 
1.7.3.2.91.g446ac

Re: [Qemu-devel] [PATCH 0.14] tap: safe sndbuf default

[Anthony Liguori]

On 02/01/2011 06:25 AM, Michael S. Tsirkin wrote:
> With current sndbuf default value, a blocked
> target guest can prevent another guest from
> transmitting any packets. While current
> sndbuf value (1M) is reported to help some
> UDP based workloads, the default should
> be safe (0).
>    

Can you be more specific about the workload this helps and by how much?

Regards,

Anthony Liguori

> Signed-off-by: Michael S. Tsirkin<mst@redhat.com>
> ---
>
> I think this should go into 0.14.
> Comments?
>
>   net/tap-linux.c |   13 +++++++++----
>   qemu-options.hx |    2 +-
>   2 files changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/net/tap-linux.c b/net/tap-linux.c
> index f7aa904..ff8cad0 100644
> --- a/net/tap-linux.c
> +++ b/net/tap-linux.c
> @@ -82,12 +82,17 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr, int vnet_hdr_required
>       return fd;
>   }
>
> -/* sndbuf should be set to a value lower than the tx queue
> - * capacity of any destination network interface.
> +/* sndbuf implements a kind of flow control for tap.
> + * Unfortunately when it's enabled, and packets are sent
> + * to other guests on the same host, the receiver
> + * can lock up the transmitter indefinitely.
> + *
> + * To avoid packet loss, 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.
> + * a good value, given a 1500 byte MTU.
>    */
> -#define TAP_DEFAULT_SNDBUF 1024*1024
> +#define TAP_DEFAULT_SNDBUF 0
>
>   int tap_set_sndbuf(int fd, QemuOpts *opts)
>   {
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 11c93a2..ca4d5c9 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1057,7 +1057,7 @@ 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"
>       "                use 'sndbuf=nbytes' to limit the size of the send buffer (the\n"
> -    "                default of 'sndbuf=1048576' can be disabled using 'sndbuf=0')\n"
> +    "                default is disabled 'sndbuf=0' to enable flow control set 'sndbuf=1048576')\n"
>       "                use vnet_hdr=off to avoid enabling the IFF_VNET_HDR tap flag\n"
>       "                use vnet_hdr=on to make the lack of IFF_VNET_HDR support an error condition\n"
>       "                use vhost=on to enable experimental in kernel accelerator\n"
>