[Spice-devel] [(spice) PATCHv2 RFC 2/2] TIOCOUTQ -> SIOCOUTQ and portability ifdefs

[Yonit Halperin]

Ack.

The ping is sent over the display channel to check an upper limit for 
the roundtrip time. If the tcp stack is busy, we postpone the ping.
So, without the ioctl, when the tcp stack is busy, the roundtrip 
estimation will  probably be much bigger than the real one. However, we 
currently take the minimum out of all the pings.
Another option is to disable the pings (set monitor_latency to FALSE 
when creating the display channel inside red_worker). In this case, the 
latency estimation will be fetched from the main channel (derived from 
one ping msg that is transmitted upon client connection).

Cheers,
Yonit.

On 07/18/2013 02:07 PM, Nahum Shalman wrote:
> The ioctl on sockets is actually named SIOCOUTQ though its value
> is identical to TIOCOUTQ which is for terminals.
> SIOCOUTQ is linux specific so we add a header check and ifdef based
> on the presence of the header
> This prevents bogus ioctls on non-Linux platforms
> ---
>   configure.ac         |    1 +
>   server/red_channel.c |   13 +++++++++++--
>   2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index a549ed9..fa1ba31 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -51,6 +51,7 @@ PKG_PROG_PKG_CONFIG
>
>   AC_CHECK_HEADERS([sys/time.h])
>   AC_CHECK_HEADERS([execinfo.h])
> +AC_CHECK_HEADERS([linux/sockios.h])
>   AC_FUNC_ALLOCA
>
>   AC_DEFINE([__STDC_FORMAT_MACROS],[],[Force definition of format macros for C++])
> diff --git a/server/red_channel.c b/server/red_channel.c
> index 85d7ebc..31c991b 100644
> --- a/server/red_channel.c
> +++ b/server/red_channel.c
> @@ -30,6 +30,9 @@
>   #include <unistd.h>
>   #include <errno.h>
>   #include <sys/ioctl.h>
> +#ifdef HAVE_LINUX_SOCKIOS_H
> +#include <linux/sockios.h> /* SIOCOUTQ */
> +#endif
>
>   #include "common/generated_server_marshallers.h"
>   #include "common/ring.h"
> @@ -722,9 +725,11 @@ static void red_channel_client_ping_timer(void *opaque)
>
>       spice_assert(rcc->latency_monitor.state == PING_STATE_TIMER);
>       red_channel_client_cancel_ping_timer(rcc);
> +
> +#ifdef HAVE_LINUX_SOCKIOS_H /* SIOCOUTQ is a Linux only ioctl on sockets. */
>       /* retrieving the occupied size of the socket's tcp snd buffer (unacked + unsent) */
> -    if (ioctl(rcc->stream->socket, TIOCOUTQ, &so_unsent_size) == -1) {
> -        spice_printerr("ioctl(TIOCOUTQ) failed, %s", strerror(errno));
> +    if (ioctl(rcc->stream->socket, SIOCOUTQ, &so_unsent_size) == -1) {
> +        spice_printerr("ioctl(SIOCOUTQ) failed, %s", strerror(errno));
>       }
>       if (so_unsent_size > 0) {
>           /* tcp snd buffer is still occupied. rescheduling ping */
> @@ -732,6 +737,10 @@ static void red_channel_client_ping_timer(void *opaque)
>       } else {
>           red_channel_client_push_ping(rcc);
>       }
> +#else /* ifdef HAVE_LINUX_SOCKIOS_H */
> +    /* More portable alternative code path (less accurate but avoids bogus ioctls)*/
> +    red_channel_client_push_ping(rcc);
> +#endif /* ifdef HAVE_LINUX_SOCKIOS_H */
>   }
>
>   RedChannelClient *red_channel_client_create(int size, RedChannel *channel, RedClient  *client,
>