[Spice-devel] [spice-server v4 2/6] reds-stream: Introduce reds_stream_set_no_delay() helper

Christophe Fergeau cfergeau at redhat.com
Fri Mar 31 09:21:32 UTC 2017


The code to enable/disable on a TCP socket is duplicated in multiple
places in the code base, this commit replaces this duplicated code with
a helper in RedsStream.

Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>
---
 server/Makefile.am               |  2 ++
 server/common-graphics-channel.c | 13 ++--------
 server/net-utils.c               | 54 ++++++++++++++++++++++++++++++++++++++++
 server/net-utils.h               | 25 +++++++++++++++++++
 server/red-channel-client.c      | 17 ++-----------
 server/reds-stream.c             | 14 +++++++++++
 server/reds-stream.h             |  1 +
 server/reds.c                    |  8 +++---
 server/sound.c                   |  8 +-----
 9 files changed, 104 insertions(+), 38 deletions(-)
 create mode 100644 server/net-utils.c
 create mode 100644 server/net-utils.h

diff --git a/server/Makefile.am b/server/Makefile.am
index 74a1302..ef8d31f 100644
--- a/server/Makefile.am
+++ b/server/Makefile.am
@@ -125,6 +125,8 @@ libserver_la_SOURCES =				\
 	memslot.h				\
 	migration-protocol.h			\
 	mjpeg-encoder.c				\
+	net-utils.c				\
+	net-utils.h				\
 	pixmap-cache.c				\
 	pixmap-cache.h				\
 	red-channel.c				\
diff --git a/server/common-graphics-channel.c b/server/common-graphics-channel.c
index 91374fc..e8c18a5 100644
--- a/server/common-graphics-channel.c
+++ b/server/common-graphics-channel.c
@@ -19,9 +19,6 @@
 #endif
 
 #include <fcntl.h>
-#include <sys/socket.h>
-#include <netinet/in.h>
-#include <netinet/tcp.h>
 
 #include "common-graphics-channel.h"
 #include "dcc.h"
@@ -125,24 +122,18 @@ bool common_channel_client_config_socket(RedChannelClient *rcc)
     RedClient *client = red_channel_client_get_client(rcc);
     MainChannelClient *mcc = red_client_get_main(client);
     RedsStream *stream = red_channel_client_get_stream(rcc);
-    int delay_val;
     gboolean is_low_bandwidth;
 
     // TODO - this should be dynamic, not one time at channel creation
     is_low_bandwidth = main_channel_client_is_low_bandwidth(mcc);
-    delay_val = is_low_bandwidth ? 0 : 1;
     /* FIXME: Using Nagle's Algorithm can lead to apparent delays, depending
      * on the delayed ack timeout on the other side.
      * Instead of using Nagle's, we need to implement message buffering on
      * the application level.
      * see: http://www.stuartcheshire.org/papers/NagleDelayedAck/
      */
-    if (setsockopt(stream->socket, IPPROTO_TCP, TCP_NODELAY, &delay_val,
-                   sizeof(delay_val)) == -1) {
-        if (errno != ENOTSUP) {
-            spice_warning("setsockopt failed, %s", strerror(errno));
-        }
-    }
+    reds_stream_set_no_delay(stream, !is_low_bandwidth);
+
     // TODO: move wide/narrow ack setting to red_channel.
     red_channel_client_ack_set_client_window(rcc,
         is_low_bandwidth ?
diff --git a/server/net-utils.c b/server/net-utils.c
new file mode 100644
index 0000000..10f447b
--- /dev/null
+++ b/server/net-utils.c
@@ -0,0 +1,54 @@
+/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
+/*
+   Copyright (C) 2009, 2017 Red Hat, Inc.
+
+   This library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   This library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with this library; if not, see <http://www.gnu.org/licenses/>.
+*/
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <errno.h>
+#include <fcntl.h>
+#include <stdbool.h>
+#include <string.h>
+#include <netinet/ip.h>
+#include <netinet/tcp.h>
+#include <sys/socket.h>
+
+#include <common/log.h>
+
+#include "net-utils.h"
+
+/**
+ * red_socket_set_no_delay:
+ * @fd: a socket file descriptor
+ * @no_delay: whether to enable TCP_NODELAY on @fd
+ *
+ * Returns: #true if the operation succeeded, #false otherwise.
+ */
+bool red_socket_set_no_delay(int fd, bool no_delay)
+{
+    int optval = no_delay;
+
+    if (setsockopt(fd, IPPROTO_TCP, TCP_NODELAY,
+                   &optval, sizeof(optval)) != 0) {
+        if (errno != ENOTSUP && errno != ENOPROTOOPT) {
+            spice_warning("setsockopt failed, %s", strerror(errno));
+            return false;
+        }
+    }
+
+    return true;
+}
diff --git a/server/net-utils.h b/server/net-utils.h
new file mode 100644
index 0000000..9f4932e
--- /dev/null
+++ b/server/net-utils.h
@@ -0,0 +1,25 @@
+/*
+   Copyright (C) 2009-2017 Red Hat, Inc.
+
+   This library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   This library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with this library; if not, see <http://www.gnu.org/licenses/>.
+*/
+
+#ifndef RED_NET_UTILS_H_
+#define RED_NET_UTILS_H_
+
+#include <stdbool.h>
+
+bool red_socket_set_no_delay(int fd, bool no_delay);
+
+#endif
diff --git a/server/red-channel-client.c b/server/red-channel-client.c
index a86833d..9fca8c1 100644
--- a/server/red-channel-client.c
+++ b/server/red-channel-client.c
@@ -571,13 +571,7 @@ static void red_channel_client_send_ping(RedChannelClient *rcc)
         }  else {
             rcc->priv->latency_monitor.tcp_nodelay = delay_val;
             if (!delay_val) {
-                delay_val = 1;
-                if (setsockopt(rcc->priv->stream->socket, IPPROTO_TCP, TCP_NODELAY, &delay_val,
-                               sizeof(delay_val)) == -1) {
-                   if (errno != ENOTSUP) {
-                        spice_warning("setsockopt failed, %s", strerror(errno));
-                    }
-                }
+                reds_stream_set_no_delay(rcc->priv->stream, TRUE);
             }
         }
     }
@@ -1385,14 +1379,7 @@ static void red_channel_client_handle_pong(RedChannelClient *rcc, SpiceMsgPing *
 
     /* set TCP_NODELAY=0, in case we reverted it for the test*/
     if (!rcc->priv->latency_monitor.tcp_nodelay) {
-        int delay_val = 0;
-
-        if (setsockopt(rcc->priv->stream->socket, IPPROTO_TCP, TCP_NODELAY, &delay_val,
-                       sizeof(delay_val)) == -1) {
-            if (errno != ENOTSUP) {
-                spice_warning("setsockopt failed, %s", strerror(errno));
-            }
-        }
+        reds_stream_set_no_delay(rcc->priv->stream, FALSE);
     }
 
     /*
diff --git a/server/reds-stream.c b/server/reds-stream.c
index 77f9424..dbe6962 100644
--- a/server/reds-stream.c
+++ b/server/reds-stream.c
@@ -21,6 +21,7 @@
 
 #include <errno.h>
 #include <netdb.h>
+#include <netinet/tcp.h>
 #include <unistd.h>
 #include <sys/socket.h>
 #include <fcntl.h>
@@ -32,6 +33,7 @@
 #include <common/log.h>
 
 #include "main-dispatcher.h"
+#include "net-utils.h"
 #include "red-common.h"
 #include "reds-stream.h"
 #include "reds.h"
@@ -259,6 +261,18 @@ bool reds_stream_is_plain_unix(const RedsStream *s)
 
 }
 
+/**
+ * reds_stream_set_no_delay:
+ * @stream: a #RedsStream
+ * @no_delay: whether to enable TCP_NODELAY on @@stream
+ *
+ * Returns: #true if the operation succeeded, #false otherwise.
+ */
+bool reds_stream_set_no_delay(RedsStream *stream, bool no_delay)
+{
+    return red_socket_set_no_delay(stream->socket, no_delay);
+}
+
 int reds_stream_send_msgfd(RedsStream *stream, int fd)
 {
     struct msghdr msgh = { 0, };
diff --git a/server/reds-stream.h b/server/reds-stream.h
index d5bd682..37ba87c 100644
--- a/server/reds-stream.h
+++ b/server/reds-stream.h
@@ -72,6 +72,7 @@ RedsStreamSslStatus reds_stream_ssl_accept(RedsStream *stream);
 int reds_stream_enable_ssl(RedsStream *stream, SSL_CTX *ctx);
 int reds_stream_get_family(const RedsStream *stream);
 bool reds_stream_is_plain_unix(const RedsStream *stream);
+bool reds_stream_set_no_delay(RedsStream *stream, bool no_delay);
 int reds_stream_send_msgfd(RedsStream *stream, int fd);
 
 typedef enum {
diff --git a/server/reds.c b/server/reds.c
index c730daa..653a045 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -75,6 +75,7 @@
 #include "main-channel-client.h"
 #include "red-client.h"
 #include "glib-compat.h"
+#include "net-utils.h"
 
 #define REDS_MAX_STAT_NODES 100
 
@@ -2405,7 +2406,6 @@ static bool reds_init_keepalive(int socket)
 static RedLinkInfo *reds_init_client_connection(RedsState *reds, int socket)
 {
     RedLinkInfo *link;
-    int delay_val = 1;
     int flags;
 
     if ((flags = fcntl(socket, F_GETFL)) == -1) {
@@ -2418,10 +2418,8 @@ static RedLinkInfo *reds_init_client_connection(RedsState *reds, int socket)
         goto error;
     }
 
-    if (setsockopt(socket, IPPROTO_TCP, TCP_NODELAY, &delay_val, sizeof(delay_val)) == -1) {
-        if (errno != ENOTSUP) {
-            spice_warning("setsockopt failed, %s", strerror(errno));
-        }
+    if (!red_socket_set_no_delay(socket, TRUE)) {
+       goto error;
     }
 
     reds_init_keepalive(socket);
diff --git a/server/sound.c b/server/sound.c
index 9a80bb7..75bd0e7 100644
--- a/server/sound.c
+++ b/server/sound.c
@@ -734,7 +734,6 @@ static void record_channel_send_item(RedChannelClient *rcc, G_GNUC_UNUSED RedPip
 
 static bool snd_channel_client_config_socket(RedChannelClient *rcc)
 {
-    int delay_val;
 #ifdef SO_PRIORITY
     int priority;
 #endif
@@ -760,12 +759,7 @@ static bool snd_channel_client_config_socket(RedChannelClient *rcc)
         }
     }
 
-    delay_val = main_channel_client_is_low_bandwidth(mcc) ? 0 : 1;
-    if (setsockopt(stream->socket, IPPROTO_TCP, TCP_NODELAY, &delay_val, sizeof(delay_val)) == -1) {
-        if (errno != ENOTSUP) {
-            spice_printerr("setsockopt failed, %s", strerror(errno));
-        }
-    }
+    reds_stream_set_no_delay(stream, !main_channel_client_is_low_bandwidth(mcc));
 
     return TRUE;
 }
-- 
2.9.3



More information about the Spice-devel mailing list