ctdb-daemon: Store node addresses as ctdb_sock_addr rather than strings
authorMartin Schwenke <martin@meltin.net>
Fri, 20 Feb 2015 00:47:23 +0000 (11:47 +1100)
committerAmitay Isaacs <amitay@samba.org>
Mon, 23 Mar 2015 11:23:12 +0000 (12:23 +0100)
Every time a nodemap is contructed the node IP addresses all need to
be parsed.  This isn't very productive use of CPU.

Instead, parse each string once when the nodes file is loaded.  This
results in much simpler code.

This code also removes the use of ctdb_address.  Duplicating the port
is pointless without an abstraction layer around ctdb_address.  If
CTDB gets an incompatible transport in the future then add an
abstraction layer.

Note that the infiniband code is not updated.  Compilation of the
infiniband code is already broken.  Fixing it will be a separate,
properly tested effort.

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>
Pair-programmed-with: Amitay Isaacs <amitay@gmail.com>

ctdb/common/ctdb_util.c
ctdb/common/system_util.c
ctdb/include/ctdb_client.h
ctdb/include/ctdb_private.h
ctdb/server/ctdb_daemon.c
ctdb/server/ctdb_recover.c
ctdb/server/ctdb_server.c
ctdb/tcp/tcp_connect.c
ctdb/tcp/tcp_init.c
ctdb/tests/src/ctdb_test_stubs.c

index b3b30b45822f636dbeaf21e8fe5810a7d9abc4dd..59a24af967bcba81837b2f56667f8af104285fc7 100644 (file)
@@ -156,33 +156,25 @@ void ctdb_external_trace(void)
   parse a IP:port pair
 */
 int ctdb_parse_address(TALLOC_CTX *mem_ctx, const char *str,
-                      struct ctdb_address *address)
+                      ctdb_sock_addr *address)
 {
        struct servent *se;
-       ctdb_sock_addr addr;
+       int port;
 
        setservent(0);
        se = getservbyname("ctdb", "tcp");
        endservent();
 
-       /* Parse IP address and re-convert to string.  This ensure correct
-        * string form for IPv6 addresses.
-        */
-       if (! parse_ip(str, NULL, 0, &addr)) {
-               return -1;
+       if (se == NULL) {
+               port = CTDB_PORT;
+       } else {
+               port = ntohs(se->s_port);
        }
 
-       address->address = talloc_strdup(mem_ctx, ctdb_addr_to_str(&addr));
-       if (address->address == NULL) {
-               DEBUG(DEBUG_ERR, (__location__ " Out of memory\n"));
+       if (! parse_ip(str, NULL, port, address)) {
                return -1;
        }
 
-       if (se == NULL) {
-               address->port = CTDB_PORT;
-       } else {
-               address->port = ntohs(se->s_port);
-       }
        return 0;
 }
 
@@ -190,9 +182,10 @@ int ctdb_parse_address(TALLOC_CTX *mem_ctx, const char *str,
 /*
   check if two addresses are the same
 */
-bool ctdb_same_address(struct ctdb_address *a1, struct ctdb_address *a2)
+bool ctdb_same_address(ctdb_sock_addr *a1, ctdb_sock_addr *a2)
 {
-       return strcmp(a1->address, a2->address) == 0 && a1->port == a2->port;
+       return ctdb_same_ip(a1, a2) &&
+               ctdb_addr_to_port(a1) == ctdb_addr_to_port(a2);
 }
 
 
index 6ff82f7147be36bcb6e07b9fe1cfac8a5bf16504..1ae0bae80396ea8d28e38ca8ee61c6138ebab867 100644 (file)
@@ -156,6 +156,9 @@ bool parse_ipv4(const char *s, unsigned port, struct sockaddr_in *sin)
                return false;
        }
 
+#ifdef HAVE_SOCK_SIN_LEN
+       sin->ip.sin_len = sizeof(*sin);
+#endif
        return true;
 }
 
@@ -181,6 +184,9 @@ static bool parse_ipv6(const char *s, const char *ifaces, unsigned port, ctdb_so
                saddr->ip6.sin6_scope_id = if_nametoindex(ifaces);
        }
 
+#ifdef HAVE_SOCK_SIN_LEN
+       saddr->ip6.sin6_len = sizeof(*saddr);
+#endif
        return true;
 }
 
index 18b74b255faeb6c3c1354e164cfe6ad4da190c16..f8850be79f67e7ce6a65a742568d16f663198e7a 100644 (file)
@@ -81,7 +81,7 @@ const char *ctdb_get_socketname(struct ctdb_context *ctdb);
   Check that a specific ip address exists in the node list and returns
   the id for the node or -1
 */
-int ctdb_ip_to_nodeid(struct ctdb_context *ctdb, const char *nodeip);
+int ctdb_ip_to_nodeid(struct ctdb_context *ctdb, const ctdb_sock_addr *nodeip);
 
 /*
   start the ctdb protocol
index 5664dd04a8bc42eab9ae633398734fa4e45ca9fc..8a90ae98e428128fc41f7865319086206318b003 100644 (file)
@@ -139,15 +139,6 @@ struct ctdb_registered_call {
        ctdb_fn_t fn;
 };
 
-/*
-  this address structure might need to be generalised later for some
-  transports
-*/
-struct ctdb_address {
-       const char *address;
-       int port;
-};
-
 /*
   check that a pnn is valid
  */
@@ -221,7 +212,7 @@ struct ctdb_vnn {
 */
 struct ctdb_node {
        struct ctdb_context *ctdb;
-       struct ctdb_address address;
+       ctdb_sock_addr address;
        const char *name; /* for debug messages */
        void *private_data; /* private to transport */
        uint32_t pnn;
@@ -466,7 +457,7 @@ struct ctdb_context {
        struct ctdb_freeze_handle *freeze_handles[NUM_DB_PRIORITIES+1];
        bool freeze_transaction_started;
        uint32_t freeze_transaction_id;
-       struct ctdb_address address;
+       ctdb_sock_addr *address;
        const char *name;
        const char *db_directory;
        const char *db_directory_persistent;
@@ -725,9 +716,9 @@ void ctdb_die(struct ctdb_context *ctdb, const char *msg);
 bool ctdb_set_helper(const char *type, char *helper, size_t size,
                     const char *envvar, const char *dir, const char *file);
 void ctdb_external_trace(void);
-bool ctdb_same_address(struct ctdb_address *a1, struct ctdb_address *a2);
+bool ctdb_same_address(ctdb_sock_addr *a1, ctdb_sock_addr *a2);
 int ctdb_parse_address(TALLOC_CTX *mem_ctx, const char *str,
-                      struct ctdb_address *address);
+                      ctdb_sock_addr *address);
 bool ctdb_same_ip(const ctdb_sock_addr *ip1, const ctdb_sock_addr *ip2);
 bool ctdb_same_sockaddr(const ctdb_sock_addr *ip1, const ctdb_sock_addr *ip2);
 uint32_t ctdb_hash(const TDB_DATA *key);
index b04cf58e05215144d0c3b72ba3a0b8f64f4602b6..bfaa6ca3f3c2a50d4cefaa26a77c51bbe98a131b 100644 (file)
@@ -1172,12 +1172,12 @@ static void ctdb_set_my_pnn(struct ctdb_context *ctdb)
 {
        int nodeid;
 
-       if (ctdb->address.address == NULL) {
+       if (ctdb->address == NULL) {
                ctdb_fatal(ctdb,
                           "Can not determine PNN - node address is not set\n");
        }
 
-       nodeid = ctdb_ip_to_nodeid(ctdb, ctdb->address.address);
+       nodeid = ctdb_ip_to_nodeid(ctdb, ctdb->address);
        if (nodeid == -1) {
                ctdb_fatal(ctdb,
                           "Can not determine PNN - node address not found in node list\n");
index db88f060d1c10d8bb567f0a1273bd880634e35c5..eb3f46da52ce2658b4c637838efa16da84d15352 100644 (file)
@@ -138,14 +138,7 @@ ctdb_control_getnodemap(struct ctdb_context *ctdb, uint32_t opcode, TDB_DATA ind
        node_map = (struct ctdb_node_map *)outdata->dptr;
        node_map->num = num_nodes;
        for (i=0; i<num_nodes; i++) {
-               if (parse_ip(ctdb->nodes[i]->address.address,
-                            NULL, /* TODO: pass in the correct interface here*/
-                            0,
-                            &node_map->nodes[i].addr) == 0)
-               {
-                       DEBUG(DEBUG_ERR, (__location__ " Failed to parse %s into a sockaddr\n", ctdb->nodes[i]->address.address));
-               }
-
+               node_map->nodes[i].addr = ctdb->nodes[i]->address;
                node_map->nodes[i].pnn   = ctdb->nodes[i]->pnn;
                node_map->nodes[i].flags = ctdb->nodes[i]->flags;
        }
@@ -176,11 +169,7 @@ ctdb_control_getnodemapv4(struct ctdb_context *ctdb, uint32_t opcode, TDB_DATA i
        node_map = (struct ctdb_node_mapv4 *)outdata->dptr;
        node_map->num = num_nodes;
        for (i=0; i<num_nodes; i++) {
-               if (parse_ipv4(ctdb->nodes[i]->address.address, 0, &node_map->nodes[i].sin) == 0) {
-                       DEBUG(DEBUG_ERR, (__location__ " Failed to parse %s into a sockaddr\n", ctdb->nodes[i]->address.address));
-                       return -1;
-               }
-
+               node_map->nodes[i].sin   = ctdb->nodes[i]->address.ip;
                node_map->nodes[i].pnn   = ctdb->nodes[i]->pnn;
                node_map->nodes[i].flags = ctdb->nodes[i]->flags;
        }
index 14e7d3292af715b294224e060bdf55d2e1c330b6..697812cf02191a33597d021d3076fa36a996c689 100644 (file)
@@ -39,7 +39,7 @@ int ctdb_set_transport(struct ctdb_context *ctdb, const char *transport)
   Check whether an ip is a valid node ip
   Returns the node id for this ip address or -1
 */
-int ctdb_ip_to_nodeid(struct ctdb_context *ctdb, const char *nodeip)
+int ctdb_ip_to_nodeid(struct ctdb_context *ctdb, const ctdb_sock_addr *nodeip)
 {
        int nodeid;
 
@@ -47,7 +47,7 @@ int ctdb_ip_to_nodeid(struct ctdb_context *ctdb, const char *nodeip)
                if (ctdb->nodes[nodeid]->flags & NODE_FLAGS_DELETED) {
                        continue;
                }
-               if (!strcmp(ctdb->nodes[nodeid]->address.address, nodeip)) {
+               if (ctdb_same_ip(&ctdb->nodes[nodeid]->address, nodeip)) {
                        return nodeid;
                }
        }
@@ -96,9 +96,9 @@ static int ctdb_add_node(struct ctdb_context *ctdb, const char *nstr, uint32_t f
                return -1;
        }
        node->ctdb = ctdb;
-       node->name = talloc_asprintf(node, "%s:%u", 
-                                    node->address.address, 
-                                    node->address.port);
+       node->name = talloc_asprintf(node, "%s:%u",
+                                    ctdb_addr_to_str(&node->address),
+                                    ctdb_addr_to_port(&node->address));
        /* this assumes that the nodes are kept in sorted order, and no gaps */
        node->pnn = ctdb->num_nodes;
 
@@ -181,13 +181,16 @@ void ctdb_load_nodes_file(struct ctdb_context *ctdb)
 */
 int ctdb_set_address(struct ctdb_context *ctdb, const char *address)
 {
-       if (ctdb_parse_address(ctdb, address, &ctdb->address) != 0) {
+       ctdb->address = talloc(ctdb, ctdb_sock_addr);
+       CTDB_NO_MEMORY(ctdb, ctdb->address);
+
+       if (ctdb_parse_address(ctdb, address, ctdb->address) != 0) {
                return -1;
        }
-       
-       ctdb->name = talloc_asprintf(ctdb, "%s:%u", 
-                                    ctdb->address.address, 
-                                    ctdb->address.port);
+
+       ctdb->name = talloc_asprintf(ctdb, "%s:%u",
+                                    ctdb_addr_to_str(ctdb->address),
+                                    ctdb_addr_to_port(ctdb->address));
        return 0;
 }
 
index 51aa21fc944b2208614f8de9df8d5990cb90fda5..6950ac85522b29d1ce6c3e439423c59f3a1d84ae 100644 (file)
@@ -111,16 +111,6 @@ static void ctdb_node_connect_write(struct event_context *ev, struct fd_event *f
 }
 
 
-static int ctdb_tcp_get_address(struct ctdb_context *ctdb,
-                               const char *address, ctdb_sock_addr *addr)
-{
-       if (parse_ip(address, NULL, 0, addr) == 0) {
-               DEBUG(DEBUG_CRIT, (__location__ " Unparsable address : %s.\n", address));
-               return -1;
-       }
-       return 0;
-}
-
 /*
   called when we should try and establish a tcp connection to a node
 */
@@ -139,25 +129,7 @@ void ctdb_tcp_node_connect(struct event_context *ev, struct timed_event *te,
 
        ctdb_tcp_stop_connection(node);
 
-       ZERO_STRUCT(sock_out);
-#ifdef HAVE_SOCK_SIN_LEN
-       sock_out.ip.sin_len = sizeof(sock_out);
-#endif
-       if (ctdb_tcp_get_address(ctdb, node->address.address, &sock_out) != 0) {
-               return;
-       }
-       switch (sock_out.sa.sa_family) {
-       case AF_INET:
-               sock_out.ip.sin_port = htons(node->address.port);
-               break;
-       case AF_INET6:
-               sock_out.ip6.sin6_port = htons(node->address.port);
-               break;
-       default:
-               DEBUG(DEBUG_ERR, (__location__ " unknown family %u\n",
-                       sock_out.sa.sa_family));
-               return;
-       }
+       sock_out = node->address;
 
        tnode->fd = socket(sock_out.sa.sa_family, SOCK_STREAM, IPPROTO_TCP);
        if (tnode->fd == -1) {
@@ -175,12 +147,7 @@ void ctdb_tcp_node_connect(struct event_context *ev, struct timed_event *te,
         * the remote side is actually routable in case CTDB traffic will run on
         * a dedicated non-routeable network.
         */
-       ZERO_STRUCT(sock_in);
-       if (ctdb_tcp_get_address(ctdb, ctdb->address.address, &sock_in) != 0) {
-               DEBUG(DEBUG_ERR, (__location__ " Failed to find our address. Failing bind.\n"));
-               close(tnode->fd);
-               return;
-       }
+       sock_in = *ctdb->address;
 
        /* AIX libs check to see if the socket address and length
           arguments are consistent with each other on calls like
@@ -189,10 +156,12 @@ void ctdb_tcp_node_connect(struct event_context *ev, struct timed_event *te,
        */
        switch (sock_in.sa.sa_family) {
        case AF_INET:
+               sock_in.ip.sin_port = 0 /* Any port */;
                sockin_size = sizeof(sock_in.ip);
                sockout_size = sizeof(sock_out.ip);
                break;
        case AF_INET6:
+               sock_in.ip6.sin6_port = 0 /* Any port */;
                sockin_size = sizeof(sock_in.ip6);
                sockout_size = sizeof(sock_out.ip6);
                break;
@@ -202,10 +171,7 @@ void ctdb_tcp_node_connect(struct event_context *ev, struct timed_event *te,
                close(tnode->fd);
                return;
        }
-#ifdef HAVE_SOCK_SIN_LEN
-       sock_in.ip.sin_len = sockin_size;
-       sock_out.ip.sin_len = sockout_size;
-#endif
+
        if (bind(tnode->fd, (struct sockaddr *)&sock_in, sockin_size) == -1) {
                DEBUG(DEBUG_ERR, (__location__ "Failed to bind socket %s(%d)\n",
                                  strerror(errno), errno));
@@ -249,18 +215,16 @@ static void ctdb_listen_event(struct event_context *ev, struct fd_event *fde,
        int fd, nodeid;
        struct ctdb_incoming *in;
        int one = 1;
-       const char *incoming_node;
 
        memset(&addr, 0, sizeof(addr));
        len = sizeof(addr);
        fd = accept(ctcp->listen_fd, (struct sockaddr *)&addr, &len);
        if (fd == -1) return;
 
-       incoming_node = ctdb_addr_to_str(&addr);
-       nodeid = ctdb_ip_to_nodeid(ctdb, incoming_node);
+       nodeid = ctdb_ip_to_nodeid(ctdb, &addr);
 
        if (nodeid == -1) {
-               DEBUG(DEBUG_ERR, ("Refused connection from unknown node %s\n", incoming_node));
+               DEBUG(DEBUG_ERR, ("Refused connection from unknown node %s\n", ctdb_addr_to_str(&addr)));
                close(fd);
                return;
        }
@@ -279,8 +243,8 @@ static void ctdb_listen_event(struct event_context *ev, struct fd_event *fde,
                                      strerror(errno)));
        }
 
-       in->queue = ctdb_queue_setup(ctdb, in, in->fd, CTDB_TCP_ALIGNMENT, 
-                                    ctdb_tcp_read_cb, in, "ctdbd-%s", incoming_node);
+       in->queue = ctdb_queue_setup(ctdb, in, in->fd, CTDB_TCP_ALIGNMENT,
+                                    ctdb_tcp_read_cb, in, "ctdbd-%s", ctdb_addr_to_str(&addr));
 }
 
 
@@ -334,20 +298,13 @@ static int ctdb_tcp_listen_automatic(struct ctdb_context *ctdb)
                if (ctdb->nodes[i]->flags & NODE_FLAGS_DELETED) {
                        continue;
                }
-               ZERO_STRUCT(sock);
-               if (ctdb_tcp_get_address(ctdb,
-                               ctdb->nodes[i]->address.address, 
-                               &sock) != 0) {
-                       continue;
-               }
-       
+               sock = ctdb->nodes[i]->address;
+
                switch (sock.sa.sa_family) {
                case AF_INET:
-                       sock.ip.sin_port = htons(ctdb->nodes[i]->address.port);
                        sock_size = sizeof(sock.ip);
                        break;
                case AF_INET6:
-                       sock.ip6.sin6_port = htons(ctdb->nodes[i]->address.port);
                        sock_size = sizeof(sock.ip6);
                        break;
                default:
@@ -355,9 +312,6 @@ static int ctdb_tcp_listen_automatic(struct ctdb_context *ctdb)
                                sock.sa.sa_family));
                        continue;
                }
-#ifdef HAVE_SOCK_SIN_LEN
-               sock.ip.sin_len = sock_size;
-#endif
 
                ctcp->listen_fd = socket(sock.sa.sa_family, SOCK_STREAM, IPPROTO_TCP);
                if (ctcp->listen_fd == -1) {
@@ -390,14 +344,14 @@ static int ctdb_tcp_listen_automatic(struct ctdb_context *ctdb)
                DEBUG(DEBUG_CRIT,("Unable to bind to any of the node addresses - giving up\n"));
                goto failed;
        }
-       ctdb->address.address = talloc_strdup(ctdb, ctdb->nodes[i]->address.address);
-       ctdb->address.port    = ctdb->nodes[i]->address.port;
+       ctdb->address = talloc_memdup(ctdb,
+                                     &ctdb->nodes[i]->address,
+                                     sizeof(ctdb_sock_addr));
+       CTDB_NO_MEMORY(ctdb, ctdb->address);
        ctdb->name = talloc_asprintf(ctdb, "%s:%u",
-                                    ctdb->address.address,
-                                    ctdb->address.port);
-       DEBUG(DEBUG_INFO,("ctdb chose network address %s:%u\n",
-                ctdb->address.address,
-                ctdb->address.port));
+                                    ctdb_addr_to_str(ctdb->address),
+                                    ctdb_addr_to_port(ctdb->address));
+       DEBUG(DEBUG_INFO,("ctdb chose network address %s\n", ctdb->name));
 
        if (listen(ctcp->listen_fd, 10) == -1) {
                goto failed;
@@ -410,7 +364,7 @@ static int ctdb_tcp_listen_automatic(struct ctdb_context *ctdb)
        close(lock_fd);
 
        return 0;
-       
+
 failed:
        close(lock_fd);
        if (ctcp->listen_fd != -1) {
@@ -435,23 +389,17 @@ int ctdb_tcp_listen(struct ctdb_context *ctdb)
 
        /* we can either auto-bind to the first available address, or we can
           use a specified address */
-       if (!ctdb->address.address) {
+       if (!ctdb->address) {
                return ctdb_tcp_listen_automatic(ctdb);
        }
 
-       ZERO_STRUCT(sock);
-       if (ctdb_tcp_get_address(ctdb, ctdb->address.address, 
-                                &sock) != 0) {
-               goto failed;
-       }
-       
+       sock = *ctdb->address;
+
        switch (sock.sa.sa_family) {
        case AF_INET:
-               sock.ip.sin_port = htons(ctdb->address.port);
                sock_size = sizeof(sock.ip);
                break;
        case AF_INET6:
-               sock.ip6.sin6_port = htons(ctdb->address.port);
                sock_size = sizeof(sock.ip6);
                break;
        default:
@@ -459,9 +407,6 @@ int ctdb_tcp_listen(struct ctdb_context *ctdb)
                        sock.sa.sa_family));
                goto failed;
        }
-#ifdef HAVE_SOCK_SIN_LEN
-       sock.ip.sin_len = sock_size;
-#endif
 
        ctcp->listen_fd = socket(sock.sa.sa_family, SOCK_STREAM, IPPROTO_TCP);
        if (ctcp->listen_fd == -1) {
index a65e7320f5e64aad48130b9ac2b442de4505beee..dba3be3394794f1124aef3e24249d7b6b3c2fc5f 100644 (file)
@@ -92,7 +92,7 @@ static int ctdb_tcp_connect_node(struct ctdb_node *node)
 
        /* startup connection to the other server - will happen on
           next event loop */
-       if (!ctdb_same_address(&ctdb->address, &node->address)) {
+       if (!ctdb_same_address(ctdb->address, &node->address)) {
                tnode->connect_te = event_add_timed(ctdb->ev, tnode, 
                                                    timeval_zero(), 
                                                    ctdb_tcp_node_connect, node);
index eb36d9b8fe87efd7e93890e8e38bb5b63376d0a9..526a6257bf36ba8a12a68e685c9c0908407e46fd 100644 (file)
@@ -128,8 +128,7 @@ static void ctdb_test_stubs_read_nodemap(struct ctdb_context *ctdb)
                ctdb->nodes[ctdb->num_nodes]->ctdb = ctdb;
                ctdb->nodes[ctdb->num_nodes]->name = "fakectdb";
                ctdb->nodes[ctdb->num_nodes]->pnn = pnn;
-               ctdb->nodes[ctdb->num_nodes]->address.address = ip;
-               ctdb->nodes[ctdb->num_nodes]->address.port = 0;
+               parse_ip(ip, NULL, 0, &ctdb->nodes[ctdb->num_nodes]->address);
                ctdb->nodes[ctdb->num_nodes]->flags = flags;
                ctdb->nodes[ctdb->num_nodes]->capabilities = capabilities;
                ctdb->num_nodes++;
@@ -443,14 +442,7 @@ ctdb_control_getnodemap(struct ctdb_context *ctdb, uint32_t opcode, TDB_DATA ind
        node_map = (struct ctdb_node_map *)outdata->dptr;
        node_map->num = num_nodes;
        for (i=0; i<num_nodes; i++) {
-               if (parse_ip(ctdb->nodes[i]->address.address,
-                            NULL, /* TODO: pass in the correct interface here*/
-                            0,
-                            &node_map->nodes[i].addr) == 0)
-               {
-                       DEBUG(DEBUG_ERR, (__location__ " Failed to parse %s into a sockaddr\n", ctdb->nodes[i]->address.address));
-               }
-
+               node_map->nodes[i].addr = ctdb->nodes[i]->address;
                node_map->nodes[i].pnn   = ctdb->nodes[i]->pnn;
                node_map->nodes[i].flags = ctdb->nodes[i]->flags;
        }
@@ -794,14 +786,8 @@ bool ctdb_sys_have_ip_stub(ctdb_sock_addr *addr)
        assert_nodes_set(ctdb);
 
        for (i = 0; i < ctdb->num_nodes; i++) {
-               ctdb_sock_addr node_addr;
-
                if (ctdb->pnn == ctdb->nodes[i]->pnn) {
-                       if (!parse_ip(ctdb->nodes[i]->address.address, NULL, 0,
-                                     &node_addr)) {
-                               continue;
-                       }
-                       if (ctdb_same_ip(addr, &node_addr)) {
+                       if (ctdb_same_ip(addr, &ctdb->nodes[i]->address)) {
                                return true;
                        }
                }