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<address@hidden>
> ---
>
> 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"
>