stream-tcp: Call setsockopt TCP_NODELAY after TCP is connected.

On Windows platform, TCP_NODELAY can only be set when TCP is established.
(This is an observed behavior and not written in any MSDN documentation.)
The current code does not create any problems while running unit tests
(because connections get established immediately) but is reportedly
observed while connecting to a different machine.

commit 8b76839(Move setsockopt TCP_NODELAY to when TCP is connected.)
made changes to call setsockopt with TCP_NODELAY after TCP is connected
only in lib/stream-ssl.c. We need the same change for stream-tcp too and
this commit does that.

Currently, a failure of setting TCP_NODELAY results in reporting
the error and then closing the socket. This commit changes that
behavior such that an error is reported if setting TCP_NODELAY
fails, but the connection itself is not torn down.

Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>

diff --git a/lib/socket-util.c b/lib/socket-util.c
index c0438d9..755ce4e 100644 (file)
--- a/lib/socket-util.c
+++ b/lib/socket-util.c
@@ -21,6 +21,7 @@
 #include <fcntl.h>
 #include <net/if.h>
 #include <netdb.h>
+#include <netinet/tcp.h>
 #include <poll.h>
 #include <stddef.h>
 #include <stdio.h>
@@ -87,6 +88,19 @@ xset_nonblocking(int fd)
     }
 }
 
+void
+setsockopt_tcp_nodelay(int fd)
+{
+    int on = 1;
+    int retval;
+
+    retval = setsockopt(fd, IPPROTO_TCP, TCP_NODELAY, &on, sizeof on);
+    if (retval) {
+        retval = sock_errno();
+        VLOG_ERR("setsockopt(TCP_NODELAY): %s", sock_strerror(retval));
+    }
+}
+
 int
 set_dscp(int fd, uint8_t dscp)
 {

diff --git a/lib/socket-util.h b/lib/socket-util.h
index 3db9a89..5b94f20 100644 (file)
--- a/lib/socket-util.h
+++ b/lib/socket-util.h
@@ -29,6 +29,7 @@
 
 int set_nonblocking(int fd);
 void xset_nonblocking(int fd);
+void setsockopt_tcp_nodelay(int fd);
 int set_dscp(int fd, uint8_t dscp);
 
 int lookup_ip(const char *host_name, struct in_addr *address);

diff --git a/lib/stream-fd.c b/lib/stream-fd.c
index ead8c3e..1b80701 100644 (file)
--- a/lib/stream-fd.c
+++ b/lib/stream-fd.c
@@ -39,6 +39,7 @@ struct stream_fd
 {
     struct stream stream;
     int fd;
+    int fd_type;
 };
 
 static const struct stream_class stream_fd_class;
@@ -49,12 +50,13 @@ static void maybe_unlink_and_free(char *path);
 
 /* Creates a new stream named 'name' that will send and receive data on 'fd'
  * and stores a pointer to the stream in '*streamp'.  Initial connection status
- * 'connect_status' is interpreted as described for stream_init().
+ * 'connect_status' is interpreted as described for stream_init(). 'fd_type'
+ * tells whether the socket is TCP or Unix domain socket.
  *
  * Returns 0 if successful, otherwise a positive errno value.  (The current
  * implementation never fails.) */
 int
-new_fd_stream(const char *name, int fd, int connect_status,
+new_fd_stream(const char *name, int fd, int connect_status, int fd_type,
               struct stream **streamp)
 {
     struct stream_fd *s;
@@ -62,6 +64,7 @@ new_fd_stream(const char *name, int fd, int connect_status,
     s = xmalloc(sizeof *s);
     stream_init(&s->stream, &stream_fd_class, connect_status, name);
     s->fd = fd;
+    s->fd_type = fd_type;
     *streamp = &s->stream;
     return 0;
 }
@@ -85,7 +88,11 @@ static int
 fd_connect(struct stream *stream)
 {
     struct stream_fd *s = stream_fd_cast(stream);
-    return check_connection_completion(s->fd);
+    int retval = check_connection_completion(s->fd);
+    if (retval == 0 && s->fd_type == AF_INET) {
+        setsockopt_tcp_nodelay(s->fd);
+    }
+    return retval;
 }
 
 static ssize_t

diff --git a/lib/stream-fd.h b/lib/stream-fd.h
index 9f138a7..d2937c1 100644 (file)
--- a/lib/stream-fd.h
+++ b/lib/stream-fd.h
@@ -29,7 +29,7 @@ struct pstream;
 struct sockaddr_storage;
 
 int new_fd_stream(const char *name, int fd, int connect_status,
-                  struct stream **streamp);
+                  int fd_type, struct stream **streamp);
 int new_fd_pstream(const char *name, int fd,
                    int (*accept_cb)(int fd, const struct sockaddr_storage *ss,
                                     size_t ss_len, struct stream **),

diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
index d1fa88a..353f171 100644 (file)
--- a/lib/stream-ssl.c
+++ b/lib/stream-ssl.c
@@ -218,20 +218,6 @@ want_to_poll_events(int want)
     }
 }
 
-static int
-setsockopt_tcp_nodelay(int fd)
-{
-    int on = 1;
-    int retval;
-
-    retval = setsockopt(fd, IPPROTO_TCP, TCP_NODELAY, &on, sizeof on);
-    if (retval) {
-        retval = sock_errno();
-        VLOG_ERR("setsockopt(TCP_NODELAY): %s", sock_strerror(retval));
-    }
-    return retval;
-}
-
 static int
 new_ssl_stream(const char *name, int fd, enum session_type type,
                enum ssl_state state, struct stream **streamp)
@@ -267,10 +253,7 @@ new_ssl_stream(const char *name, int fd, enum session_type type,
      * On windows platforms, this can only be called upon TCP connected.
      */
     if (state == STATE_SSL_CONNECTING) {
-        retval = setsockopt_tcp_nodelay(fd);
-        if (retval) {
-            goto error;
-        }
+        setsockopt_tcp_nodelay(fd);
     }
 
     /* Create and configure OpenSSL stream. */
@@ -454,10 +437,7 @@ ssl_connect(struct stream *stream)
             return retval;
         }
         sslv->state = STATE_SSL_CONNECTING;
-        retval = setsockopt_tcp_nodelay(sslv->fd);
-        if (retval) {
-            return retval;
-        }
+        setsockopt_tcp_nodelay(sslv->fd);
         /* Fall through. */
 
     case STATE_SSL_CONNECTING:

diff --git a/lib/stream-tcp.c b/lib/stream-tcp.c
index 043a30b..e58c5ad 100644 (file)
--- a/lib/stream-tcp.c
+++ b/lib/stream-tcp.c
@@ -20,7 +20,6 @@
 #include <inttypes.h>
 #include <sys/types.h>
 #include <netinet/in.h>
-#include <netinet/tcp.h>
 #include <netdb.h>
 #include <stdlib.h>
 #include <string.h>
@@ -42,19 +41,11 @@ static int
 new_tcp_stream(const char *name, int fd, int connect_status,
                struct stream **streamp)
 {
-    int on = 1;
-    int retval;
-
-    retval = setsockopt(fd, IPPROTO_TCP, TCP_NODELAY, &on, sizeof on);
-    if (retval) {
-        int error = sock_errno();
-        VLOG_ERR("%s: setsockopt(TCP_NODELAY): %s",
-                 name, sock_strerror(error));
-        closesocket(fd);
-        return error;
+    if (connect_status == 0) {
+        setsockopt_tcp_nodelay(fd);
     }
 
-    return new_fd_stream(name, fd, connect_status, streamp);
+    return new_fd_stream(name, fd, connect_status, AF_INET, streamp);
 }
 
 static int

diff --git a/lib/stream-unix.c b/lib/stream-unix.c
index ab5252b..29077c6 100644 (file)
--- a/lib/stream-unix.c
+++ b/lib/stream-unix.c
@@ -57,7 +57,8 @@ unix_open(const char *name, char *suffix, struct stream **streamp,
     }
 
     free(connect_path);
-    return new_fd_stream(name, fd, check_connection_completion(fd), streamp);
+    return new_fd_stream(name, fd, check_connection_completion(fd),
+                         AF_UNIX, streamp);
 }
 
 const struct stream_class unix_stream_class = {
@@ -117,7 +118,7 @@ punix_accept(int fd, const struct sockaddr_storage *ss, size_t ss_len,
     } else {
         strcpy(name, "unix");
     }
-    return new_fd_stream(name, fd, 0, streamp);
+    return new_fd_stream(name, fd, 0, AF_UNIX, streamp);
 }
 
 const struct pstream_class punix_pstream_class = {