vconn: Fix detection of vconn local IP address, to fix in-band control.
authorBen Pfaff <blp@nicira.com>
Mon, 13 Jul 2009 22:05:51 +0000 (15:05 -0700)
committerBen Pfaff <blp@nicira.com>
Mon, 13 Jul 2009 22:06:59 +0000 (15:06 -0700)
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.

lib/vconn-provider.h
lib/vconn-ssl.c
lib/vconn-stream.c
lib/vconn-stream.h
lib/vconn-tcp.c
lib/vconn-unix.c
lib/vconn.c

index 0b25dee8d58c00d39fc07939559b30bc19c0b615..ae025f7cbf23d532ecdfbc06aad127089ee751a1 100644 (file)
@@ -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,
index d4dbc9f7979f627b8de17126a91cbf47a701122b..ef1b56439376e1886dad8bb7f1adc07cb912fef2 100644 (file)
@@ -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();
index 46279e57ea03ab0a5bf79075e2e1f8235f9cf02f..6d3825518b76690720bfb84a6654029b7c0e15df 100644 (file)
@@ -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
index efebdd848fb81b3c6f2df3aac8e9bbf17a1e7b91..0adac9ada2f274ca72a750a9dc4a3bbf8b7064c5 100644 (file)
@@ -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 **),
index 50367806f3d420d5b5cd68825f2e26273d1a3163..a91b7d38188887235da2d61f4d5d62bdeede3490 100644 (file)
 
 /* 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 */
index fddf6e85185c6e6a7394bcd6455cf8d55c651d3c..e39eaea2a137889990722c0847e1eea9ae943669 100644 (file)
@@ -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 = {
index 2940c114d65e1390940de5462dd539a576e86fc9..f493f8337769971ff81f8686193f2a6cd8f3a420 100644 (file)
@@ -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)
 {