vconn: Fix detection of vconn local IP address, to fix in-band control.

The in-band control code needs to know the IP and port of both ends of the
control connection.  However, the vconn code was only reporting the local
address after the connection had already succeeded, which created a
chicken-and-egg problem.  In practice we would fail to connect until the
switch went into fail-open, at which point the connection would go through.

Fortunately, we can get the local IP address right after we try to connect,
not just after the connection completes, so this commit changes the code
to do that.

This commit also breaks setting the remote IP and port into functions
separate from vconn_init(), which makes the code more readable.

diff --git a/lib/vconn-provider.h b/lib/vconn-provider.h
index 0b25dee..ae025f7 100644 (file)
--- a/lib/vconn-provider.h
+++ b/lib/vconn-provider.h
@@ -43,8 +43,9 @@ struct vconn {
 };
 
 void vconn_init(struct vconn *, struct vconn_class *, int connect_status,
-                uint32_t remote_ip, uint16_t remote_port,
                 const char *name, bool reconnectable);
+void vconn_set_remote_ip(struct vconn *, uint32_t remote_ip);
+void vconn_set_remote_port(struct vconn *, uint16_t remote_port);
 void vconn_set_local_ip(struct vconn *, uint32_t local_ip);
 void vconn_set_local_port(struct vconn *, uint16_t local_port);
 static inline void vconn_assert_class(const struct vconn *vconn,

diff --git a/lib/vconn-ssl.c b/lib/vconn-ssl.c
index d4dbc9f..ef1b564 100644 (file)
--- a/lib/vconn-ssl.c
+++ b/lib/vconn-ssl.c
@@ -181,9 +181,11 @@ want_to_poll_events(int want)
 
 static int
 new_ssl_vconn(const char *name, int fd, enum session_type type,
-              enum ssl_state state, const struct sockaddr_in *sin,
+              enum ssl_state state, const struct sockaddr_in *remote,
               struct vconn **vconnp)
 {
+    struct sockaddr_in local;
+    socklen_t local_len = sizeof local;
     struct ssl_vconn *sslv;
     SSL *ssl = NULL;
     int on = 1;
@@ -212,6 +214,12 @@ new_ssl_vconn(const char *name, int fd, enum session_type type,
         goto error;
     }
 
+    /* Get the local IP and port information */
+    retval = getsockname(fd, (struct sockaddr *) &local, &local_len);
+    if (retval) {
+        memset(&local, 0, sizeof local);
+    }
+
     /* Disable Nagle. */
     retval = setsockopt(fd, IPPROTO_TCP, TCP_NODELAY, &on, sizeof on);
     if (retval) {
@@ -238,8 +246,11 @@ new_ssl_vconn(const char *name, int fd, enum session_type type,
 
     /* Create and return the ssl_vconn. */
     sslv = xmalloc(sizeof *sslv);
-    vconn_init(&sslv->vconn, &ssl_vconn_class, EAGAIN, 
-               sin->sin_addr.s_addr, sin->sin_port, name, true);
+    vconn_init(&sslv->vconn, &ssl_vconn_class, EAGAIN, name, true);
+    vconn_set_remote_ip(&sslv->vconn, remote->sin_addr.s_addr);
+    vconn_set_remote_port(&sslv->vconn, remote->sin_port);
+    vconn_set_local_ip(&sslv->vconn, local.sin_addr.s_addr);
+    vconn_set_local_port(&sslv->vconn, local.sin_port);
     sslv->state = state;
     sslv->type = type;
     sslv->fd = fd;
@@ -426,19 +437,7 @@ ssl_connect(struct vconn *vconn)
         sslv->state = STATE_SSL_CONNECTING;
         /* Fall through. */
 
-    case STATE_SSL_CONNECTING: {
-        struct sockaddr_in local_addr;
-        socklen_t addrlen = sizeof(local_addr);
-
-        /* Get the local IP and port information */
-        retval = getsockname(sslv->fd, (struct sockaddr *)&local_addr, 
-                             &addrlen);
-        if (retval) {
-            memset(&local_addr, 0, sizeof local_addr);
-        }
-        vconn_set_local_ip(vconn, local_addr.sin_addr.s_addr);
-        vconn_set_local_port(vconn, local_addr.sin_port);
-
+    case STATE_SSL_CONNECTING:
         retval = (sslv->type == CLIENT
                    ? SSL_connect(sslv->ssl) : SSL_accept(sslv->ssl));
         if (retval != 1) {
@@ -470,8 +469,6 @@ ssl_connect(struct vconn *vconn)
         } else {
             return 0;
         }
-
-        }
     }
 
     NOT_REACHED();

diff --git a/lib/vconn-stream.c b/lib/vconn-stream.c
index 46279e5..6d38255 100644 (file)
--- a/lib/vconn-stream.c
+++ b/lib/vconn-stream.c
@@ -41,7 +41,6 @@ struct stream_vconn
 {
     struct vconn vconn;
     int fd;
-    void (*connect_success_cb)(struct vconn *, int);
     struct ofpbuf *rxbuf;
     struct ofpbuf *txbuf;
     struct poll_waiter *tx_waiter;
@@ -55,21 +54,17 @@ static void stream_clear_txbuf(struct stream_vconn *);
 
 int
 new_stream_vconn(const char *name, int fd, int connect_status,
-                 uint32_t remote_ip, uint16_t remote_port, 
-                 bool reconnectable, 
-                 connect_success_cb_func *connect_success_cb,
-                 struct vconn **vconnp)
+                 bool reconnectable, struct vconn **vconnp)
 {
     struct stream_vconn *s;
 
     s = xmalloc(sizeof *s);
-    vconn_init(&s->vconn, &stream_vconn_class, connect_status, remote_ip, 
-               remote_port, name, reconnectable);
+    vconn_init(&s->vconn, &stream_vconn_class, connect_status,
+               name, reconnectable);
     s->fd = fd;
     s->txbuf = NULL;
     s->tx_waiter = NULL;
     s->rxbuf = NULL;
-    s->connect_success_cb = connect_success_cb;
     *vconnp = &s->vconn;
     return 0;
 }
@@ -96,14 +91,7 @@ static int
 stream_connect(struct vconn *vconn)
 {
     struct stream_vconn *s = stream_vconn_cast(vconn);
-    int retval = check_connection_completion(s->fd);
-    if (retval) {
-        return retval;
-    }
-    if (s->connect_success_cb) {
-        s->connect_success_cb(vconn, s->fd);
-    }
-    return 0;
+    return check_connection_completion(s->fd);
 }
 
 static int

diff --git a/lib/vconn-stream.h b/lib/vconn-stream.h
index efebdd8..0adac9a 100644 (file)
--- a/lib/vconn-stream.h
+++ b/lib/vconn-stream.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008 Nicira Networks.
+ * Copyright (c) 2008, 2009 Nicira Networks.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -25,12 +25,8 @@ struct vconn;
 struct pvconn;
 struct sockaddr;
 
-typedef void connect_success_cb_func(struct vconn *, int);
-
 int new_stream_vconn(const char *name, int fd, int connect_status,
-                     uint32_t remote_ip, uint16_t remote_port,
-                     bool reconnectable, connect_success_cb_func *,
-                     struct vconn **vconnp);
+                     bool reconnectable, struct vconn **vconnp);
 int new_pstream_pvconn(const char *name, int fd,
                       int (*accept_cb)(int fd, const struct sockaddr *,
                                        size_t sa_len, struct vconn **),

diff --git a/lib/vconn-tcp.c b/lib/vconn-tcp.c
index 5036780..a91b7d3 100644 (file)
--- a/lib/vconn-tcp.c
+++ b/lib/vconn-tcp.c
@@ -36,15 +36,21 @@
 
 /* Active TCP. */
 
-void tcp_connect_success_cb(struct vconn *vconn, int fd);
-
 static int
 new_tcp_vconn(const char *name, int fd, int connect_status,
-              const struct sockaddr_in *sin, struct vconn **vconnp)
+              const struct sockaddr_in *remote, struct vconn **vconnp)
 {
+    struct sockaddr_in local;
+    socklen_t local_len = sizeof local;
     int on = 1;
     int retval;
 
+    /* Get the local IP and port information */
+    retval = getsockname(fd, (struct sockaddr *)&local, &local_len);
+    if (retval) {
+        memset(&local, 0, sizeof local);
+    }
+
     retval = setsockopt(fd, IPPROTO_TCP, TCP_NODELAY, &on, sizeof on);
     if (retval) {
         VLOG_ERR("%s: setsockopt(TCP_NODELAY): %s", name, strerror(errno));
@@ -52,9 +58,15 @@ new_tcp_vconn(const char *name, int fd, int connect_status,
         return errno;
     }
 
-    return new_stream_vconn(name, fd, connect_status, 
-                            sin->sin_addr.s_addr, sin->sin_port, 
-                            true, tcp_connect_success_cb, vconnp);
+    retval = new_stream_vconn(name, fd, connect_status, true, vconnp);
+    if (!retval) {
+        struct vconn *vconn = *vconnp;
+        vconn_set_remote_ip(vconn, remote->sin_addr.s_addr);
+        vconn_set_remote_port(vconn, remote->sin_port);
+        vconn_set_local_ip(vconn, local.sin_addr.s_addr);
+        vconn_set_local_port(vconn, local.sin_port);
+    }
+    return retval;
 }
 
 static int
@@ -108,22 +120,6 @@ tcp_open(const char *name, char *suffix, struct vconn **vconnp)
     }
 }
 
-void
-tcp_connect_success_cb(struct vconn *vconn, int fd)
-{
-    int retval;
-    struct sockaddr_in local_addr;
-    socklen_t addrlen = sizeof(local_addr);
-
-    /* Get the local IP and port information */
-    retval = getsockname(fd, (struct sockaddr *)&local_addr, &addrlen);
-    if (retval) {
-        memset(&local_addr, 0, sizeof local_addr);
-    }
-    vconn_set_local_ip(vconn, local_addr.sin_addr.s_addr);
-    vconn_set_local_port(vconn, local_addr.sin_port);
-}
-
 struct vconn_class tcp_vconn_class = {
     "tcp",                      /* name */
     tcp_open,                   /* open */

diff --git a/lib/vconn-unix.c b/lib/vconn-unix.c
index fddf6e8..e39eaea 100644 (file)
--- a/lib/vconn-unix.c
+++ b/lib/vconn-unix.c
@@ -60,7 +60,7 @@ unix_open(const char *name, char *suffix, struct vconn **vconnp)
     }
 
     return new_stream_vconn(name, fd, check_connection_completion(fd),
-                            0, 0, true, NULL, vconnp);
+                            true, vconnp);
 }
 
 struct vconn_class unix_vconn_class = {
@@ -105,7 +105,7 @@ punix_accept(int fd, const struct sockaddr *sa, size_t sa_len,
     } else {
         strcpy(name, "unix");
     }
-    return new_stream_vconn(name, fd, 0, 0, 0, true, NULL, vconnp);
+    return new_stream_vconn(name, fd, 0, true, vconnp);
 }
 
 struct pvconn_class punix_pvconn_class = {

diff --git a/lib/vconn.c b/lib/vconn.c
index 2940c11..f493f83 100644 (file)
--- a/lib/vconn.c
+++ b/lib/vconn.c
@@ -1407,8 +1407,7 @@ normalize_match(struct ofp_match *m)
 
 void
 vconn_init(struct vconn *vconn, struct vconn_class *class, int connect_status,
-           uint32_t remote_ip, uint16_t remote_port, const char *name, 
-           bool reconnectable)
+           const char *name, bool reconnectable)
 {
     vconn->class = class;
     vconn->state = (connect_status == EAGAIN ? VCS_CONNECTING
@@ -1417,14 +1416,26 @@ vconn_init(struct vconn *vconn, struct vconn_class *class, int connect_status,
     vconn->error = connect_status;
     vconn->version = -1;
     vconn->min_version = -1;
-    vconn->remote_ip = remote_ip;
-    vconn->remote_port = remote_port;
+    vconn->remote_ip = 0;
+    vconn->remote_port = 0;
     vconn->local_ip = 0;
     vconn->local_port = 0;
     vconn->name = xstrdup(name);
     vconn->reconnectable = reconnectable;
 }
 
+void
+vconn_set_remote_ip(struct vconn *vconn, uint32_t ip)
+{
+    vconn->remote_ip = ip;
+}
+
+void
+vconn_set_remote_port(struct vconn *vconn, uint16_t port)
+{
+    vconn->remote_port = port;
+}
+
 void 
 vconn_set_local_ip(struct vconn *vconn, uint32_t ip)
 {