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

2013-07-22 Thread 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 
  #include 
  #include 
+#ifdef HAVE_LINUX_SOCKIOS_H
+#include  /* 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,



___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


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

2013-07-18 Thread Nahum Shalman
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 
 #include 
 #include 
+#ifdef HAVE_LINUX_SOCKIOS_H
+#include  /* 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,
-- 
1.7.7.6

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel