From 746639a7945a47e932910332000f1a572e3f70f5 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 16 Nov 2023 11:56:59 +0100 Subject: [PATCH 01/16] ctdb: remove unused ctdb->client_ip_list and print debug on ctdb_tcp_list instead BUG: https://bugzilla.samba.org/show_bug.cgi?id=15523 Signed-off-by: Stefan Metzmacher Reviewed-by: Martin Schwenke (cherry picked from commit 92badd3bdd82d1fa79727efcf81b6f479016811f) --- ctdb/include/ctdb_private.h | 1 - ctdb/server/ctdb_takeover.c | 74 ++++++++++++++++++------------------- 2 files changed, 37 insertions(+), 38 deletions(-) diff --git a/ctdb/include/ctdb_private.h b/ctdb/include/ctdb_private.h index 3395f077ab93..37842a151d68 100644 --- a/ctdb/include/ctdb_private.h +++ b/ctdb/include/ctdb_private.h @@ -296,7 +296,6 @@ struct ctdb_context { struct ctdb_statistics statistics_history[MAX_STAT_HISTORY]; struct ctdb_vnn_map *vnn_map; uint32_t num_clients; - struct ctdb_client_ip *client_ip_list; bool do_checkpublicip; bool do_setsched; const char *event_script_dir; diff --git a/ctdb/server/ctdb_takeover.c b/ctdb/server/ctdb_takeover.c index 4d2d04167528..36abbdfabfb1 100644 --- a/ctdb/server/ctdb_takeover.c +++ b/ctdb/server/ctdb_takeover.c @@ -348,20 +348,10 @@ struct ctdb_takeover_arp { */ struct ctdb_tcp_list { struct ctdb_tcp_list *prev, *next; + struct ctdb_client *client; struct ctdb_connection connection; }; -/* - list of clients to kill on IP release - */ -struct ctdb_client_ip { - struct ctdb_client_ip *prev, *next; - struct ctdb_context *ctdb; - ctdb_sock_addr addr; - uint32_t client_id; -}; - - /* send a gratuitous arp */ @@ -1233,16 +1223,37 @@ int ctdb_set_public_addresses(struct ctdb_context *ctdb, bool check_addresses) } /* - destroy a ctdb_client_ip structure + destroy a ctdb_tcp_list structure */ -static int ctdb_client_ip_destructor(struct ctdb_client_ip *ip) +static int ctdb_tcp_list_destructor(struct ctdb_tcp_list *tcp) { - DEBUG(DEBUG_DEBUG,("destroying client tcp for %s:%u (client_id %u)\n", - ctdb_addr_to_str(&ip->addr), - ntohs(ip->addr.ip.sin_port), - ip->client_id)); + struct ctdb_client *client = tcp->client; + struct ctdb_connection *conn = &tcp->connection; + char conn_str[132] = { 0, }; + int ret; + + ret = ctdb_connection_to_buf(conn_str, + sizeof(conn_str), + conn, + false, + " -> "); + if (ret != 0) { + strlcpy(conn_str, "UNKNOWN", sizeof(conn_str)); + } + + D_DEBUG("removing client TCP connection %s " + "(client_id %u pid %d)\n", + conn_str, client->client_id, client->pid); + + DLIST_REMOVE(client->tcp_list, tcp); + + /* + * We don't call ctdb_remove_connection(vnn, conn) here + * as we want the caller to decide if it's called + * directly (local only) or indirectly via a + * CTDB_CONTROL_TCP_REMOVE broadcast + */ - DLIST_REMOVE(ip->ctdb->client_ip_list, ip); return 0; } @@ -1259,7 +1270,6 @@ int32_t ctdb_control_tcp_client(struct ctdb_context *ctdb, uint32_t client_id, struct ctdb_connection t; int ret; TDB_DATA data; - struct ctdb_client_ip *ip; struct ctdb_vnn *vnn; ctdb_sock_addr src_addr; ctdb_sock_addr dst_addr; @@ -1324,22 +1334,15 @@ int32_t ctdb_control_tcp_client(struct ctdb_context *ctdb, uint32_t client_id, return -1; } - ip = talloc(client, struct ctdb_client_ip); - CTDB_NO_MEMORY(ctdb, ip); - - ip->ctdb = ctdb; - ip->addr = dst_addr; - ip->client_id = client_id; - talloc_set_destructor(ip, ctdb_client_ip_destructor); - DLIST_ADD(ctdb->client_ip_list, ip); - tcp = talloc(client, struct ctdb_tcp_list); CTDB_NO_MEMORY(ctdb, tcp); + tcp->client = client; tcp->connection.src = tcp_sock->src; tcp->connection.dst = tcp_sock->dst; DLIST_ADD(client->tcp_list, tcp); + talloc_set_destructor(tcp, ctdb_tcp_list_destructor); t.src = tcp_sock->src; t.dst = tcp_sock->dst; @@ -1599,20 +1602,12 @@ void ctdb_takeover_client_destructor_hook(struct ctdb_client *client) struct ctdb_tcp_list *tcp = client->tcp_list; struct ctdb_connection *conn = &tcp->connection; - DLIST_REMOVE(client->tcp_list, tcp); - vnn = find_public_ip_vnn(client->ctdb, &conn->dst); - if (vnn == NULL) { - DEBUG(DEBUG_ERR, - (__location__ " unable to find public address %s\n", - ctdb_addr_to_str(&conn->dst))); - continue; - } /* If the IP address is hosted on this node then * remove the connection. */ - if (vnn->pnn == client->ctdb->pnn) { + if (vnn != NULL && vnn->pnn == client->ctdb->pnn) { ctdb_remove_connection(vnn, conn); } @@ -1621,6 +1616,11 @@ void ctdb_takeover_client_destructor_hook(struct ctdb_client *client) * and the client has exited. This means that we * should not delete the connection information. The * takeover node processes connections too. */ + + /* + * The destructor removes from the list + */ + TALLOC_FREE(tcp); } } -- 2.34.1 From a00f22589dba5cf2cb513f77c104f7ecb4e56fcd Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 12 Dec 2023 13:26:46 +0100 Subject: [PATCH 02/16] ctdb: add ctdb_canonicalize_ip_inplace() helper Signed-off-by: Stefan Metzmacher Reviewed-by: Martin Schwenke (cherry picked from commit f2d9c012fc803b48564c3203ed640c02f99bcbaa) --- ctdb/common/common.h | 1 + ctdb/common/ctdb_util.c | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/ctdb/common/common.h b/ctdb/common/common.h index c50b52a5eb50..9a73bec1ac6c 100644 --- a/ctdb/common/common.h +++ b/ctdb/common/common.h @@ -132,6 +132,7 @@ struct ctdb_rec_data_old *ctdb_marshall_loop_next( TDB_DATA *key, TDB_DATA *data); void ctdb_canonicalize_ip(const ctdb_sock_addr *ip, ctdb_sock_addr *cip); +void ctdb_canonicalize_ip_inplace(ctdb_sock_addr *ip); bool ctdb_same_ip(const ctdb_sock_addr *tip1, const ctdb_sock_addr *tip2); diff --git a/ctdb/common/ctdb_util.c b/ctdb/common/ctdb_util.c index 3f8fff925f0d..5c7731c5d2a9 100644 --- a/ctdb/common/ctdb_util.c +++ b/ctdb/common/ctdb_util.c @@ -388,6 +388,13 @@ void ctdb_canonicalize_ip(const ctdb_sock_addr *ip, ctdb_sock_addr *cip) } } +void ctdb_canonicalize_ip_inplace(ctdb_sock_addr *ip) +{ + ctdb_sock_addr tmp; + ctdb_canonicalize_ip(ip, &tmp); + memcpy(ip, &tmp, sizeof(tmp)); +} + bool ctdb_same_ip(const ctdb_sock_addr *tip1, const ctdb_sock_addr *tip2) { ctdb_sock_addr ip1, ip2; -- 2.34.1 From f4eadb194b8273be3cacc77deb516c028d667617 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 12 Dec 2023 13:27:17 +0100 Subject: [PATCH 03/16] ctdb: make use of ctdb_canonicalize_ip_inplace() in ctdb_control_tcp_client() We could also remove the src_addr and dest_addr helper variables completely, but that would be too much for this commit. Signed-off-by: Stefan Metzmacher Reviewed-by: Martin Schwenke (cherry picked from commit 5f52d140f7b676ed68b5ce49d4445357bcbcb1a6) --- ctdb/server/ctdb_takeover.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/ctdb/server/ctdb_takeover.c b/ctdb/server/ctdb_takeover.c index 36abbdfabfb1..abafffd03fc7 100644 --- a/ctdb/server/ctdb_takeover.c +++ b/ctdb/server/ctdb_takeover.c @@ -1281,15 +1281,11 @@ int32_t ctdb_control_tcp_client(struct ctdb_context *ctdb, uint32_t client_id, tcp_sock = (struct ctdb_connection *)indata.dptr; + ctdb_canonicalize_ip_inplace(&tcp_sock->src); src_addr = tcp_sock->src; - ctdb_canonicalize_ip(&src_addr, &tcp_sock->src); - ZERO_STRUCT(src_addr); - memcpy(&src_addr, &tcp_sock->src, sizeof(src_addr)); + ctdb_canonicalize_ip_inplace(&tcp_sock->dst); dst_addr = tcp_sock->dst; - ctdb_canonicalize_ip(&dst_addr, &tcp_sock->dst); - ZERO_STRUCT(dst_addr); - memcpy(&dst_addr, &tcp_sock->dst, sizeof(dst_addr)); vnn = find_public_ip_vnn(ctdb, &dst_addr); if (vnn == NULL) { -- 2.34.1 From 04082e2cc3d7a766fc0401b2457724728aeedb6c Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 12 Dec 2023 13:39:21 +0100 Subject: [PATCH 04/16] ctdb: add ctdb_connection_same() helper Signed-off-by: Stefan Metzmacher Reviewed-by: Martin Schwenke (cherry picked from commit 8395fd369d3c9d216817e922423727748581f133) --- ctdb/protocol/protocol_util.c | 18 ++++++++++++++++++ ctdb/protocol/protocol_util.h | 3 +++ 2 files changed, 21 insertions(+) diff --git a/ctdb/protocol/protocol_util.c b/ctdb/protocol/protocol_util.c index fe757658f482..87ecc87ac361 100644 --- a/ctdb/protocol/protocol_util.c +++ b/ctdb/protocol/protocol_util.c @@ -497,6 +497,24 @@ bool ctdb_sock_addr_same(const ctdb_sock_addr *addr1, return (ctdb_sock_addr_cmp(addr1, addr2) == 0); } +bool ctdb_connection_same(const struct ctdb_connection *conn1, + const struct ctdb_connection *conn2) +{ + bool same; + + same = ctdb_sock_addr_same(&conn1->src, &conn2->src); + if (!same) { + return false; + } + + same = ctdb_sock_addr_same(&conn1->dst, &conn2->dst); + if (!same) { + return false; + } + + return true; +} + int ctdb_connection_to_buf(char *buf, size_t buflen, struct ctdb_connection *conn, diff --git a/ctdb/protocol/protocol_util.h b/ctdb/protocol/protocol_util.h index 2bdbb0c2ad01..70f35d122a89 100644 --- a/ctdb/protocol/protocol_util.h +++ b/ctdb/protocol/protocol_util.h @@ -55,6 +55,9 @@ bool ctdb_sock_addr_same_ip(const ctdb_sock_addr *addr1, bool ctdb_sock_addr_same(const ctdb_sock_addr *addr1, const ctdb_sock_addr *addr2); +bool ctdb_connection_same(const struct ctdb_connection *conn1, + const struct ctdb_connection *conn2); + int ctdb_connection_to_buf(char *buf, size_t buflen, struct ctdb_connection * conn, -- 2.34.1 From 7225b2d3647553cb4f379bddeaf8c834ad7d7a3f Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 15 Nov 2023 16:31:53 +0100 Subject: [PATCH 05/16] ctdb: add/implement CTDB_CONTROL_TCP_CLIENT_DISCONNECTED With multichannel a ctdb connection from smbd may hold multiple tcp connections, which can be disconnected before the smbd process terminates the whole ctdb connection, so we a way to remove undo 'CTDB_CONTROL_TCP_CLIENT' again. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15523 Signed-off-by: Stefan Metzmacher Reviewed-by: Martin Schwenke (cherry picked from commit c6602b686b4e50d93272667ef86d3904181fb1ab) --- ctdb/include/ctdb_private.h | 3 ++ ctdb/protocol/protocol.h | 1 + ctdb/protocol/protocol_control.c | 15 ++++++ ctdb/protocol/protocol_debug.c | 1 + ctdb/server/ctdb_control.c | 4 ++ ctdb/server/ctdb_takeover.c | 86 ++++++++++++++++++++++++++++++++ 6 files changed, 110 insertions(+) diff --git a/ctdb/include/ctdb_private.h b/ctdb/include/ctdb_private.h index 37842a151d68..ca350df2cf47 100644 --- a/ctdb/include/ctdb_private.h +++ b/ctdb/include/ctdb_private.h @@ -892,6 +892,9 @@ int ctdb_set_public_addresses(struct ctdb_context *ctdb, bool check_addresses); int32_t ctdb_control_tcp_client(struct ctdb_context *ctdb, uint32_t client_id, TDB_DATA indata); +int32_t ctdb_control_tcp_client_disconnected(struct ctdb_context *ctdb, + uint32_t client_id, + TDB_DATA indata); int32_t ctdb_control_tcp_add(struct ctdb_context *ctdb, TDB_DATA indata, bool tcp_update_needed); int32_t ctdb_control_tcp_remove(struct ctdb_context *ctdb, TDB_DATA indata); diff --git a/ctdb/protocol/protocol.h b/ctdb/protocol/protocol.h index fb6e39f33b55..42b992ae6db0 100644 --- a/ctdb/protocol/protocol.h +++ b/ctdb/protocol/protocol.h @@ -381,6 +381,7 @@ enum ctdb_controls {CTDB_CONTROL_PROCESS_EXISTS = 0, CTDB_CONTROL_ECHO_DATA = 156, CTDB_CONTROL_DISABLE_NODE = 157, CTDB_CONTROL_ENABLE_NODE = 158, + CTDB_CONTROL_TCP_CLIENT_DISCONNECTED = 159, }; #define MAX_COUNT_BUCKETS 16 diff --git a/ctdb/protocol/protocol_control.c b/ctdb/protocol/protocol_control.c index a7c797f5dbca..5e2cf13c579b 100644 --- a/ctdb/protocol/protocol_control.c +++ b/ctdb/protocol/protocol_control.c @@ -410,6 +410,10 @@ static size_t ctdb_req_control_data_len(struct ctdb_req_control_data *cd) case CTDB_CONTROL_ENABLE_NODE: break; + + case CTDB_CONTROL_TCP_CLIENT_DISCONNECTED: + len = ctdb_connection_len(cd->data.conn); + break; } return len; @@ -1016,6 +1020,14 @@ static int ctdb_req_control_data_pull(uint8_t *buf, size_t buflen, &cd->data.echo_data, &np); break; + + case CTDB_CONTROL_TCP_CLIENT_DISCONNECTED: + ret = ctdb_connection_pull(buf, + buflen, + mem_ctx, + &cd->data.conn, + &np); + break; } if (ret != 0) { @@ -1376,6 +1388,9 @@ static size_t ctdb_reply_control_data_len(struct ctdb_reply_control_data *cd) case CTDB_CONTROL_ENABLE_NODE: break; + + case CTDB_CONTROL_TCP_CLIENT_DISCONNECTED: + break; } return len; diff --git a/ctdb/protocol/protocol_debug.c b/ctdb/protocol/protocol_debug.c index d94cb548d68b..2dc4a702eae1 100644 --- a/ctdb/protocol/protocol_debug.c +++ b/ctdb/protocol/protocol_debug.c @@ -245,6 +245,7 @@ static void ctdb_opcode_print(uint32_t opcode, FILE *fp) { CTDB_CONTROL_ECHO_DATA, "ECHO_DATA" }, { CTDB_CONTROL_DISABLE_NODE, "DISABLE_NODE" }, { CTDB_CONTROL_ENABLE_NODE, "ENABLE_NODE" }, + { CTDB_CONTROL_TCP_CLIENT_DISCONNECTED, "TCP_CLIENT_DISCONNECTED" }, { MAP_END, "" }, }; diff --git a/ctdb/server/ctdb_control.c b/ctdb/server/ctdb_control.c index 08268512bfa0..3ea93f52cfe4 100644 --- a/ctdb/server/ctdb_control.c +++ b/ctdb/server/ctdb_control.c @@ -868,6 +868,10 @@ static int32_t ctdb_control_dispatch(struct ctdb_context *ctdb, CHECK_CONTROL_DATA_SIZE(0); return ctdb_control_enable_node(ctdb); + case CTDB_CONTROL_TCP_CLIENT_DISCONNECTED: + CHECK_CONTROL_DATA_SIZE(sizeof(struct ctdb_connection)); + return ctdb_control_tcp_client_disconnected(ctdb, client_id, indata); + default: DEBUG(DEBUG_CRIT,(__location__ " Unknown CTDB control opcode %u\n", opcode)); return -1; diff --git a/ctdb/server/ctdb_takeover.c b/ctdb/server/ctdb_takeover.c index abafffd03fc7..cedb2b921c4b 100644 --- a/ctdb/server/ctdb_takeover.c +++ b/ctdb/server/ctdb_takeover.c @@ -1377,6 +1377,92 @@ int32_t ctdb_control_tcp_client(struct ctdb_context *ctdb, uint32_t client_id, return 0; } +static bool ctdb_client_remove_tcp(struct ctdb_client *client, + const struct ctdb_connection *conn) +{ + struct ctdb_tcp_list *tcp = NULL; + struct ctdb_tcp_list *tcp_next = NULL; + bool found = false; + + for (tcp = client->tcp_list; tcp != NULL; tcp = tcp_next) { + bool same; + + tcp_next = tcp->next; + + same = ctdb_connection_same(conn, &tcp->connection); + if (!same) { + continue; + } + + TALLOC_FREE(tcp); + found = true; + } + + return found; +} + +/* + called by a client to inform us of a TCP connection that was disconnected + */ +int32_t ctdb_control_tcp_client_disconnected(struct ctdb_context *ctdb, + uint32_t client_id, + TDB_DATA indata) +{ + struct ctdb_client *client = reqid_find(ctdb->idr, client_id, struct ctdb_client); + struct ctdb_connection *tcp_sock = NULL; + int ret; + TDB_DATA data; + char conn_str[132] = { 0, }; + bool found = false; + + tcp_sock = (struct ctdb_connection *)indata.dptr; + + ctdb_canonicalize_ip_inplace(&tcp_sock->src); + ctdb_canonicalize_ip_inplace(&tcp_sock->dst); + + ret = ctdb_connection_to_buf(conn_str, + sizeof(conn_str), + tcp_sock, + false, + " -> "); + if (ret != 0) { + strlcpy(conn_str, "UNKNOWN", sizeof(conn_str)); + } + + found = ctdb_client_remove_tcp(client, tcp_sock); + if (!found) { + DBG_DEBUG("TCP connection %s not found " + "(client_id %u pid %u).\n", + conn_str, client_id, client->pid); + return 0; + } + + D_INFO("deregistered TCP connection %s " + "(client_id %u pid %u)\n", + conn_str, client_id, client->pid); + + data.dptr = (uint8_t *)tcp_sock; + data.dsize = sizeof(*tcp_sock); + + /* tell all nodes about this tcp connection is gone */ + ret = ctdb_daemon_send_control(ctdb, + CTDB_BROADCAST_CONNECTED, + 0, + CTDB_CONTROL_TCP_REMOVE, + 0, + CTDB_CTRL_FLAG_NOREPLY, + data, + NULL, + NULL); + if (ret != 0) { + DBG_ERR("Failed to send CTDB_CONTROL_TCP_REMOVE: %s\n", + conn_str); + return -1; + } + + return 0; +} + /* find a tcp address on a list */ -- 2.34.1 From 81c1d5928248c9ed0629981dfd59754810bd25f6 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 17 Nov 2023 15:59:57 +0100 Subject: [PATCH 06/16] ctdb: add/implement CTDB_CONTROL_TCP_CLIENT_PASSED With multichannel a tcp connection is registered first with a temporary smbd process, that calls CTDB_CONTROL_TCP_CLIENT first and then passes the tcp connection to the longterm smbd that already handles all connections belonging to the specific client_guid. That smbd process calls CTDB_CONTROL_TCP_CLIENT again, but the 'tickle' information is already there. When the temporary smbd process exists/disconnects from ctdb or calls CTDB_CONTROL_TCP_CLIENT_DISCONNECTED, the 'tickle' information is removed, while the longterm smbd process still serves the tcp connection. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15523 Signed-off-by: Stefan Metzmacher Reviewed-by: Martin Schwenke (cherry picked from commit 037e8e449deb136ad5ed5e4de05439411b545b6d) --- ctdb/include/ctdb_private.h | 3 ++ ctdb/protocol/protocol.h | 1 + ctdb/protocol/protocol_control.c | 15 ++++++++++ ctdb/protocol/protocol_debug.c | 1 + ctdb/server/ctdb_control.c | 4 +++ ctdb/server/ctdb_takeover.c | 49 ++++++++++++++++++++++++++++++++ 6 files changed, 73 insertions(+) diff --git a/ctdb/include/ctdb_private.h b/ctdb/include/ctdb_private.h index ca350df2cf47..802781237781 100644 --- a/ctdb/include/ctdb_private.h +++ b/ctdb/include/ctdb_private.h @@ -895,6 +895,9 @@ int32_t ctdb_control_tcp_client(struct ctdb_context *ctdb, uint32_t client_id, int32_t ctdb_control_tcp_client_disconnected(struct ctdb_context *ctdb, uint32_t client_id, TDB_DATA indata); +int32_t ctdb_control_tcp_client_passed(struct ctdb_context *ctdb, + uint32_t client_id, + TDB_DATA indata); int32_t ctdb_control_tcp_add(struct ctdb_context *ctdb, TDB_DATA indata, bool tcp_update_needed); int32_t ctdb_control_tcp_remove(struct ctdb_context *ctdb, TDB_DATA indata); diff --git a/ctdb/protocol/protocol.h b/ctdb/protocol/protocol.h index 42b992ae6db0..3b66c403ab84 100644 --- a/ctdb/protocol/protocol.h +++ b/ctdb/protocol/protocol.h @@ -382,6 +382,7 @@ enum ctdb_controls {CTDB_CONTROL_PROCESS_EXISTS = 0, CTDB_CONTROL_DISABLE_NODE = 157, CTDB_CONTROL_ENABLE_NODE = 158, CTDB_CONTROL_TCP_CLIENT_DISCONNECTED = 159, + CTDB_CONTROL_TCP_CLIENT_PASSED = 160, }; #define MAX_COUNT_BUCKETS 16 diff --git a/ctdb/protocol/protocol_control.c b/ctdb/protocol/protocol_control.c index 5e2cf13c579b..83ed6cb4ee18 100644 --- a/ctdb/protocol/protocol_control.c +++ b/ctdb/protocol/protocol_control.c @@ -414,6 +414,10 @@ static size_t ctdb_req_control_data_len(struct ctdb_req_control_data *cd) case CTDB_CONTROL_TCP_CLIENT_DISCONNECTED: len = ctdb_connection_len(cd->data.conn); break; + + case CTDB_CONTROL_TCP_CLIENT_PASSED: + len = ctdb_connection_len(cd->data.conn); + break; } return len; @@ -1028,6 +1032,14 @@ static int ctdb_req_control_data_pull(uint8_t *buf, size_t buflen, &cd->data.conn, &np); break; + + case CTDB_CONTROL_TCP_CLIENT_PASSED: + ret = ctdb_connection_pull(buf, + buflen, + mem_ctx, + &cd->data.conn, + &np); + break; } if (ret != 0) { @@ -1391,6 +1403,9 @@ static size_t ctdb_reply_control_data_len(struct ctdb_reply_control_data *cd) case CTDB_CONTROL_TCP_CLIENT_DISCONNECTED: break; + + case CTDB_CONTROL_TCP_CLIENT_PASSED: + break; } return len; diff --git a/ctdb/protocol/protocol_debug.c b/ctdb/protocol/protocol_debug.c index 2dc4a702eae1..ae091b04d321 100644 --- a/ctdb/protocol/protocol_debug.c +++ b/ctdb/protocol/protocol_debug.c @@ -246,6 +246,7 @@ static void ctdb_opcode_print(uint32_t opcode, FILE *fp) { CTDB_CONTROL_DISABLE_NODE, "DISABLE_NODE" }, { CTDB_CONTROL_ENABLE_NODE, "ENABLE_NODE" }, { CTDB_CONTROL_TCP_CLIENT_DISCONNECTED, "TCP_CLIENT_DISCONNECTED" }, + { CTDB_CONTROL_TCP_CLIENT_PASSED, "TCP_CLIENT_PASSED" }, { MAP_END, "" }, }; diff --git a/ctdb/server/ctdb_control.c b/ctdb/server/ctdb_control.c index 3ea93f52cfe4..422c4cf1e58e 100644 --- a/ctdb/server/ctdb_control.c +++ b/ctdb/server/ctdb_control.c @@ -872,6 +872,10 @@ static int32_t ctdb_control_dispatch(struct ctdb_context *ctdb, CHECK_CONTROL_DATA_SIZE(sizeof(struct ctdb_connection)); return ctdb_control_tcp_client_disconnected(ctdb, client_id, indata); + case CTDB_CONTROL_TCP_CLIENT_PASSED: + CHECK_CONTROL_DATA_SIZE(sizeof(struct ctdb_connection)); + return ctdb_control_tcp_client_passed(ctdb, client_id, indata); + default: DEBUG(DEBUG_CRIT,(__location__ " Unknown CTDB control opcode %u\n", opcode)); return -1; diff --git a/ctdb/server/ctdb_takeover.c b/ctdb/server/ctdb_takeover.c index cedb2b921c4b..bbef628e1bc8 100644 --- a/ctdb/server/ctdb_takeover.c +++ b/ctdb/server/ctdb_takeover.c @@ -1463,6 +1463,55 @@ int32_t ctdb_control_tcp_client_disconnected(struct ctdb_context *ctdb, return 0; } +/* + called by a client to inform us of a TCP connection was passed to a different + "client" (typically with multichannel to another smbd process). + */ +int32_t ctdb_control_tcp_client_passed(struct ctdb_context *ctdb, + uint32_t client_id, + TDB_DATA indata) +{ + struct ctdb_client *client = reqid_find(ctdb->idr, client_id, struct ctdb_client); + struct ctdb_connection *tcp_sock = NULL; + int ret; + char conn_str[132] = { 0, }; + bool found = false; + + tcp_sock = (struct ctdb_connection *)indata.dptr; + + ctdb_canonicalize_ip_inplace(&tcp_sock->src); + ctdb_canonicalize_ip_inplace(&tcp_sock->dst); + + ret = ctdb_connection_to_buf(conn_str, + sizeof(conn_str), + tcp_sock, + false, + " -> "); + if (ret != 0) { + strlcpy(conn_str, "UNKNOWN", sizeof(conn_str)); + } + + found = ctdb_client_remove_tcp(client, tcp_sock); + if (!found) { + DBG_DEBUG("TCP connection from %s not found " + "(client_id %u pid %u).\n", + conn_str, client_id, client->pid); + return 0; + } + + D_INFO("TCP connection from %s " + "(client_id %u pid %u) passed to another client\n", + conn_str, client_id, client->pid); + + /* + * We don't call CTDB_CONTROL_TCP_REMOVE + * nor ctdb_remove_connection() as the connection + * is still alive, but handled by another client + */ + + return 0; +} + /* find a tcp address on a list */ -- 2.34.1 From 915e04943651e57579d077c08d2754f85f9dabeb Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 16 Nov 2023 13:29:18 +0100 Subject: [PATCH 07/16] ctdbd_conn: don't use uninitialized memory in ctdbd_register_ips() We dump the structure into the socket, so we need to zero the content including possible padding. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15523 Signed-off-by: Stefan Metzmacher Reviewed-by: Martin Schwenke (cherry picked from commit 240139370aa19f53dd3de0ff468afd994d3bd973) --- source3/lib/ctdbd_conn.c | 1 + 1 file changed, 1 insertion(+) diff --git a/source3/lib/ctdbd_conn.c b/source3/lib/ctdbd_conn.c index dd9206b00fd9..4382bae54fcf 100644 --- a/source3/lib/ctdbd_conn.c +++ b/source3/lib/ctdbd_conn.c @@ -1144,6 +1144,7 @@ int ctdbd_register_ips(struct ctdbd_connection *conn, smbd_ctdb_canonicalize_ip(_client, &client); smbd_ctdb_canonicalize_ip(_server, &server); + ZERO_STRUCT(p); switch (client.ss_family) { case AF_INET: memcpy(&p.dst.ip, &server, sizeof(p.dst.ip)); -- 2.34.1 From f36b53d2b39565d8a2009ec33381f67891e1f090 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 16 Nov 2023 13:04:12 +0100 Subject: [PATCH 08/16] ctdbd_conn: let register_with_ctdbd() call CTDB_CONTROL_REGISTER_SRVID just once We do the dispatching to multiple handlers in ctdbd_msg_call_back() and we don't need more than one message from ctdb. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15523 Signed-off-by: Stefan Metzmacher Reviewed-by: Martin Schwenke (cherry picked from commit 77a559432ffde2d435e29bed126d20a09d33f48e) --- source3/lib/ctdbd_conn.c | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/source3/lib/ctdbd_conn.c b/source3/lib/ctdbd_conn.c index 4382bae54fcf..a11c31d7f633 100644 --- a/source3/lib/ctdbd_conn.c +++ b/source3/lib/ctdbd_conn.c @@ -131,19 +131,32 @@ int register_with_ctdbd(struct ctdbd_connection *conn, uint64_t srvid, void *private_data), void *private_data) { - - int ret; - int32_t cstatus; - size_t num_callbacks; + size_t num_callbacks = talloc_array_length(conn->callbacks); struct ctdbd_srvid_cb *tmp; + bool need_register = true; + size_t i; - ret = ctdbd_control_local(conn, CTDB_CONTROL_REGISTER_SRVID, srvid, 0, - tdb_null, NULL, NULL, &cstatus); - if (ret != 0) { - return ret; + for (i = 0; i < num_callbacks; i++) { + struct ctdbd_srvid_cb *c = &conn->callbacks[i]; + + if (c->srvid == srvid) { + need_register = false; + break; + } + } + + if (need_register) { + int ret; + int32_t cstatus; + + ret = ctdbd_control_local(conn, CTDB_CONTROL_REGISTER_SRVID, + srvid, 0, tdb_null, NULL, NULL, + &cstatus); + if (ret != 0) { + return ret; + } } - num_callbacks = talloc_array_length(conn->callbacks); tmp = talloc_realloc(conn, conn->callbacks, struct ctdbd_srvid_cb, num_callbacks + 1); -- 2.34.1 From c43e69f7af5cd5c6ebd65fd2cd30fa5750ddc152 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Thu, 12 Oct 2023 17:11:42 +0200 Subject: [PATCH 09/16] ctdbd_conn: Add deregister_from_ctdbd() This is to remove a callback during rundown of smbds. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15523 Pair-Programmed-With: Stefan Metzmacher Signed-off-by: Volker Lendecke Signed-off-by: Stefan Metzmacher Reviewed-by: Martin Schwenke (cherry picked from commit 75aa6693940201a928b46f6880b43820c0e1c555) --- source3/include/ctdbd_conn.h | 10 ++++++ source3/lib/ctdb_dummy.c | 13 +++++++ source3/lib/ctdbd_conn.c | 67 ++++++++++++++++++++++++++++++++++++ 3 files changed, 90 insertions(+) diff --git a/source3/include/ctdbd_conn.h b/source3/include/ctdbd_conn.h index 74db96e89e7d..3fa948710298 100644 --- a/source3/include/ctdbd_conn.h +++ b/source3/include/ctdbd_conn.h @@ -116,6 +116,16 @@ int register_with_ctdbd(struct ctdbd_connection *conn, uint64_t srvid, const uint8_t *msg, size_t msglen, void *private_data), void *private_data); +void deregister_from_ctdbd(struct ctdbd_connection *conn, + uint64_t srvid, + int (*cb)(struct tevent_context *ev, + uint32_t src_vnn, + uint32_t dst_vnn, + uint64_t dst_srvid, + const uint8_t *msg, + size_t msglen, + void *private_data), + void *private_data); int ctdbd_probe(const char *sockname, int timeout); struct ctdb_req_header; diff --git a/source3/lib/ctdb_dummy.c b/source3/lib/ctdb_dummy.c index 294d178966b2..f21037e0c309 100644 --- a/source3/lib/ctdb_dummy.c +++ b/source3/lib/ctdb_dummy.c @@ -49,6 +49,19 @@ int register_with_ctdbd(struct ctdbd_connection *conn, uint64_t srvid, return ENOSYS; } +void deregister_from_ctdbd(struct ctdbd_connection *conn, + uint64_t srvid, + int (*cb)(struct tevent_context *ev, + uint32_t src_vnn, + uint32_t dst_vnn, + uint64_t dst_srvid, + const uint8_t *msg, + size_t msglen, + void *private_data), + void *private_data) +{ +} + int ctdbd_register_ips(struct ctdbd_connection *conn, const struct sockaddr_storage *_server, const struct sockaddr_storage *_client, diff --git a/source3/lib/ctdbd_conn.c b/source3/lib/ctdbd_conn.c index a11c31d7f633..41239f702b9d 100644 --- a/source3/lib/ctdbd_conn.c +++ b/source3/lib/ctdbd_conn.c @@ -172,6 +172,73 @@ int register_with_ctdbd(struct ctdbd_connection *conn, uint64_t srvid, return 0; } +void deregister_from_ctdbd(struct ctdbd_connection *conn, + uint64_t srvid, + int (*cb)(struct tevent_context *ev, + uint32_t src_vnn, + uint32_t dst_vnn, + uint64_t dst_srvid, + const uint8_t *msg, + size_t msglen, + void *private_data), + void *private_data) +{ + struct ctdbd_srvid_cb *cbs = conn->callbacks; + size_t i, num_callbacks = talloc_array_length(cbs); + bool need_deregister = false; + bool keep_registration = false; + + if (num_callbacks == 0) { + return; + } + + for (i = 0; i < num_callbacks;) { + struct ctdbd_srvid_cb *c = &cbs[i]; + + if (c->srvid != srvid) { + i++; + continue; + } + + if ((c->cb == cb) && (c->private_data == private_data)) { + need_deregister = true; + ARRAY_DEL_ELEMENT(cbs, i, num_callbacks); + num_callbacks--; + continue; + } + + keep_registration = true; + i++; + } + + conn->callbacks = talloc_realloc(conn, + cbs, + struct ctdbd_srvid_cb, + num_callbacks); + + if (keep_registration) { + need_deregister = false; + } + + if (need_deregister) { + int ret; + int32_t cstatus; + + ret = ctdbd_control_local(conn, CTDB_CONTROL_DEREGISTER_SRVID, + srvid, 0, tdb_null, NULL, NULL, + &cstatus); + if (ret != 0) { + /* + * If CTDB_CONTROL_DEREGISTER_SRVID fails we may still + * get messages later, but we don't have a callback + * anymore, we just ignore these. + */ + } + } + + return; +} + static int ctdbd_msg_call_back(struct tevent_context *ev, struct ctdbd_connection *conn, struct ctdb_req_message_old *msg) -- 2.34.1 From 13087b594dc7df963191bd7017ab620c9a6f2ead Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 16 Nov 2023 13:18:03 +0100 Subject: [PATCH 10/16] ctdbd_conn: add ctdbd_unregister_ips() This reverts the effect of ctdbd_register_ips(). We'll use this in order to disconnect individual multichannel connections. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15523 Signed-off-by: Stefan Metzmacher Reviewed-by: Martin Schwenke (cherry picked from commit f3a03f3f774f0795fc1a163f12cccb9cedeebec1) --- source3/include/ctdbd_conn.h | 11 +++++++ source3/lib/ctdb_dummy.c | 15 +++++++++ source3/lib/ctdbd_conn.c | 64 ++++++++++++++++++++++++++++++++++++ 3 files changed, 90 insertions(+) diff --git a/source3/include/ctdbd_conn.h b/source3/include/ctdbd_conn.h index 3fa948710298..a08012007946 100644 --- a/source3/include/ctdbd_conn.h +++ b/source3/include/ctdbd_conn.h @@ -84,6 +84,17 @@ int ctdbd_register_ips(struct ctdbd_connection *conn, const uint8_t *msg, size_t msglen, void *private_data), void *private_data); +void ctdbd_unregister_ips(struct ctdbd_connection *conn, + const struct sockaddr_storage *_server, + const struct sockaddr_storage *_client, + int (*cb)(struct tevent_context *ev, + uint32_t src_vnn, + uint32_t dst_vnn, + uint64_t dst_srvid, + const uint8_t *msg, + size_t msglen, + void *private_data), + void *private_data); /* * call @cb for each public IP. If @cb returns non-zero, then break the loop diff --git a/source3/lib/ctdb_dummy.c b/source3/lib/ctdb_dummy.c index f21037e0c309..99f27d843d28 100644 --- a/source3/lib/ctdb_dummy.c +++ b/source3/lib/ctdb_dummy.c @@ -75,6 +75,21 @@ int ctdbd_register_ips(struct ctdbd_connection *conn, return ENOSYS; } +void ctdbd_unregister_ips(struct ctdbd_connection *conn, + const struct sockaddr_storage *_server, + const struct sockaddr_storage *_client, + int (*cb)(struct tevent_context *ev, + uint32_t src_vnn, + uint32_t dst_vnn, + uint64_t dst_srvid, + const uint8_t *msg, + size_t msglen, + void *private_data), + void *private_data) +{ + return; +} + int ctdbd_public_ip_foreach(struct ctdbd_connection *conn, int (*cb)(uint32_t total_ip_count, const struct sockaddr_storage *ip, diff --git a/source3/lib/ctdbd_conn.c b/source3/lib/ctdbd_conn.c index 41239f702b9d..408dd80e951f 100644 --- a/source3/lib/ctdbd_conn.c +++ b/source3/lib/ctdbd_conn.c @@ -1263,6 +1263,70 @@ int ctdbd_register_ips(struct ctdbd_connection *conn, return 0; } +void ctdbd_unregister_ips(struct ctdbd_connection *conn, + const struct sockaddr_storage *_server, + const struct sockaddr_storage *_client, + int (*cb)(struct tevent_context *ev, + uint32_t src_vnn, + uint32_t dst_vnn, + uint64_t dst_srvid, + const uint8_t *msg, + size_t msglen, + void *private_data), + void *private_data) +{ + struct ctdb_connection p; + TDB_DATA data = { .dptr = (uint8_t *)&p, .dsize = sizeof(p) }; + int ret; + struct sockaddr_storage client; + struct sockaddr_storage server; + + /* + * Only one connection so far + */ + + smbd_ctdb_canonicalize_ip(_client, &client); + smbd_ctdb_canonicalize_ip(_server, &server); + + ZERO_STRUCT(p); + switch (client.ss_family) { + case AF_INET: + memcpy(&p.dst.ip, &server, sizeof(p.dst.ip)); + memcpy(&p.src.ip, &client, sizeof(p.src.ip)); + break; + case AF_INET6: + memcpy(&p.dst.ip6, &server, sizeof(p.dst.ip6)); + memcpy(&p.src.ip6, &client, sizeof(p.src.ip6)); + break; + default: + return; + } + + /* + * We no longer want to be told about IP releases + * for the given callback/private_data combination + */ + deregister_from_ctdbd(conn, CTDB_SRVID_RELEASE_IP, + cb, private_data); + + /* + * inform ctdb of our tcp connection is no longer active + */ + ret = ctdbd_control_local(conn, + CTDB_CONTROL_TCP_CLIENT_DISCONNECTED, 0, + CTDB_CTRL_FLAG_NOREPLY, data, NULL, NULL, + NULL); + if (ret != 0) { + /* + * We ignore errors here, as we'll just + * no longer have a callback handler + * registered and messages may just be ignored + */ + } + + return; +} + static int ctdbd_control_get_public_ips(struct ctdbd_connection *conn, uint32_t flags, TALLOC_CTX *mem_ctx, -- 2.34.1 From 28d011bc5884185a07db62e826c55b5bd5dac723 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 20 Nov 2023 14:57:46 +0100 Subject: [PATCH 11/16] ctdbd_conn: add ctdbd_passed_ips() This is similar to ctdbd_unregister_ips(), but with the difference that ctdb keeps the 'tickle' information for the tcp connection alive, because another smbd process took care of that tcp connection in a multichannel scenario. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15523 Signed-off-by: Stefan Metzmacher Reviewed-by: Martin Schwenke (cherry picked from commit 2e784789d78d09dfbc599085e5eb1c70c5b866b8) --- source3/include/ctdbd_conn.h | 11 ++++++ source3/lib/ctdb_dummy.c | 14 ++++++++ source3/lib/ctdbd_conn.c | 65 ++++++++++++++++++++++++++++++++++++ 3 files changed, 90 insertions(+) diff --git a/source3/include/ctdbd_conn.h b/source3/include/ctdbd_conn.h index a08012007946..312f0eea810e 100644 --- a/source3/include/ctdbd_conn.h +++ b/source3/include/ctdbd_conn.h @@ -95,6 +95,17 @@ void ctdbd_unregister_ips(struct ctdbd_connection *conn, size_t msglen, void *private_data), void *private_data); +void ctdbd_passed_ips(struct ctdbd_connection *conn, + const struct sockaddr_storage *_server, + const struct sockaddr_storage *_client, + int (*cb)(struct tevent_context *ev, + uint32_t src_vnn, + uint32_t dst_vnn, + uint64_t dst_srvid, + const uint8_t *msg, + size_t msglen, + void *private_data), + void *private_data); /* * call @cb for each public IP. If @cb returns non-zero, then break the loop diff --git a/source3/lib/ctdb_dummy.c b/source3/lib/ctdb_dummy.c index 99f27d843d28..3954fac22c82 100644 --- a/source3/lib/ctdb_dummy.c +++ b/source3/lib/ctdb_dummy.c @@ -89,6 +89,20 @@ void ctdbd_unregister_ips(struct ctdbd_connection *conn, { return; } +void ctdbd_passed_ips(struct ctdbd_connection *conn, + const struct sockaddr_storage *_server, + const struct sockaddr_storage *_client, + int (*cb)(struct tevent_context *ev, + uint32_t src_vnn, + uint32_t dst_vnn, + uint64_t dst_srvid, + const uint8_t *msg, + size_t msglen, + void *private_data), + void *private_data) +{ + return; +} int ctdbd_public_ip_foreach(struct ctdbd_connection *conn, int (*cb)(uint32_t total_ip_count, diff --git a/source3/lib/ctdbd_conn.c b/source3/lib/ctdbd_conn.c index 408dd80e951f..a739c97f3fd2 100644 --- a/source3/lib/ctdbd_conn.c +++ b/source3/lib/ctdbd_conn.c @@ -1327,6 +1327,71 @@ void ctdbd_unregister_ips(struct ctdbd_connection *conn, return; } +void ctdbd_passed_ips(struct ctdbd_connection *conn, + const struct sockaddr_storage *_server, + const struct sockaddr_storage *_client, + int (*cb)(struct tevent_context *ev, + uint32_t src_vnn, + uint32_t dst_vnn, + uint64_t dst_srvid, + const uint8_t *msg, + size_t msglen, + void *private_data), + void *private_data) +{ + struct ctdb_connection p; + TDB_DATA data = { .dptr = (uint8_t *)&p, .dsize = sizeof(p) }; + int ret; + struct sockaddr_storage client; + struct sockaddr_storage server; + + /* + * Only one connection so far + */ + + smbd_ctdb_canonicalize_ip(_client, &client); + smbd_ctdb_canonicalize_ip(_server, &server); + + ZERO_STRUCT(p); + switch (client.ss_family) { + case AF_INET: + memcpy(&p.dst.ip, &server, sizeof(p.dst.ip)); + memcpy(&p.src.ip, &client, sizeof(p.src.ip)); + break; + case AF_INET6: + memcpy(&p.dst.ip6, &server, sizeof(p.dst.ip6)); + memcpy(&p.src.ip6, &client, sizeof(p.src.ip6)); + break; + default: + return; + } + + /* + * We no longer want to be told about IP releases + * for the given callback/private_data combination + */ + deregister_from_ctdbd(conn, CTDB_SRVID_RELEASE_IP, + cb, private_data); + + /* + * inform ctdb of our tcp connection is now passed to + * another process. + */ + ret = ctdbd_control_local(conn, + CTDB_CONTROL_TCP_CLIENT_PASSED, 0, + CTDB_CTRL_FLAG_NOREPLY, data, NULL, NULL, + NULL); + if (ret != 0) { + /* + * We ignore errors here, as we'll just + * no longer have a callback handler + * registered and messages may just be ignored + */ + } + + return; +} + static int ctdbd_control_get_public_ips(struct ctdbd_connection *conn, uint32_t flags, TALLOC_CTX *mem_ctx, -- 2.34.1 From de9970830007988dd6e31784ffe570a949e2850f Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 17 Nov 2023 11:45:30 +0100 Subject: [PATCH 12/16] selftest: export/use CTDB related envvars in order to run the ctdb command This makes it easier to test things... BUG: https://bugzilla.samba.org/show_bug.cgi?id=15523 Signed-off-by: Stefan Metzmacher Reviewed-by: Martin Schwenke (cherry picked from commit 38b74d4ca9a59e7f12850c20c410f9df26cbad0a) --- selftest/target/Samba.pm | 19 +++++++++++++++++++ selftest/target/Samba3.pm | 22 ++++++++++++++++++++-- source3/selftest/tests.py | 2 +- 3 files changed, 40 insertions(+), 3 deletions(-) diff --git a/selftest/target/Samba.pm b/selftest/target/Samba.pm index 4f0f29df5cf9..a2018a040587 100644 --- a/selftest/target/Samba.pm +++ b/selftest/target/Samba.pm @@ -971,6 +971,25 @@ my @exported_envvars = ( # resolv_wrapper "RESOLV_WRAPPER_CONF", "RESOLV_WRAPPER_HOSTS", + + # ctdb stuff + "NUM_NODES", + "CTDB_BASE", + "CTDB_SOCKET", + "CTDB_SERVER_NAME", + "CTDB_IFACE_IP", + "CTDB_BASE_NODE0", + "CTDB_SOCKET_NODE0", + "CTDB_SERVER_NAME_NODE0", + "CTDB_IFACE_IP_NODE0", + "CTDB_BASE_NODE1", + "CTDB_SOCKET_NODE1", + "CTDB_SERVER_NAME_NODE1", + "CTDB_IFACE_IP_NODE1", + "CTDB_BASE_NODE2", + "CTDB_SOCKET_NODE2", + "CTDB_SERVER_NAME_NODE2", + "CTDB_IFACE_IP_NODE2", ); sub exported_envvars_str diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm index fb5814518017..60775433de26 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -527,8 +527,8 @@ sub setup_clusteredmember my $pub_iface = $node->{SOCKET_WRAPPER_DEFAULT_IFACE}; my $node_prefix = $node->{NODE_PREFIX}; - print "NODE_PREFIX=${node_prefix}\n"; - print "SOCKET=${socket}\n"; + print "CTDB_BASE=${node_prefix}\n"; + print "CTDB_SOCKET=${socket}\n"; my $require_mutexes = "dbwrap_tdb_require_mutexes:* = yes"; if ($ENV{SELFTEST_DONT_REQUIRE_TDB_MUTEX_SUPPORT} // '' eq "1") { @@ -4120,6 +4120,24 @@ sub provision_ctdb($$$$) $ret{CTDB_NODES} = \@nodes; $ret{CTDB_NODES_FILE} = $nodes_file; + for (my $i = 0; $i < $num_nodes; $i++) { + my $node = $nodes[$i]; + my $socket = $node->{SOCKET_FILE}; + my $server_name = $node->{SERVER_NAME}; + my $node_prefix = $node->{NODE_PREFIX}; + my $ip = $node->{IP}; + + $ret{"CTDB_BASE_NODE${i}"} = $node_prefix; + $ret{"CTDB_SOCKET_NODE${i}"} = $socket; + $ret{"CTDB_SERVER_NAME_NODE${i}"} = $server_name; + $ret{"CTDB_IFACE_IP_NODE${i}"} = $ip; + } + + $ret{CTDB_BASE} = $ret{CTDB_BASE_NODE0}; + $ret{CTDB_SOCKET} = $ret{CTDB_SOCKET_NODE0}; + $ret{CTDB_SERVER_NAME} = $ret{CTDB_SERVER_NAME_NODE0}; + $ret{CTDB_IFACE_IP} = $ret{CTDB_IFACE_IP_NODE0}; + return \%ret; } diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index 2c8336d35e87..adea6096c030 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -1631,7 +1631,7 @@ plantestsuite("samba3.blackbox.smbXsrv_client_cross_node", "clusteredmember:loca [os.path.join(samba3srcdir, "script/tests/test_smbXsrv_client_cross_node.sh"), configuration, - 'ctdb0', 'ctdb1', + '$CTDB_SERVER_NAME_NODE0', '$CTDB_SERVER_NAME_NODE1', "tmp"]) plantestsuite("samba3.blackbox.registry_share", "clusteredmember", [os.path.join(samba3srcdir, -- 2.34.1 From 6cf9579b81ab161ec3bee216ad6b9f226abf6b00 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 17 Nov 2023 11:46:27 +0100 Subject: [PATCH 13/16] s3:selftest: add samba3.blackbox.smbXsrv_client_ctdb_registered_ips This demonstrates the crash that happens if a client connects to a non-public address first followed by a connect to public address with the same client_guid and a connection to the non-public address gets disconnected first, we hit by a use-after-free talloc_get_type_abort() called from release_ip() as "xconn" is already gone, taking smbd_release_ip_state with it. Note that we also need to mark some subtests as flapping as there's a 2nd problem that happens in the interaction between smbd processes and ctdb when passing a multichannel connection to an existing process, it means we sometimes loose the 'tickle' information within ctdb to that tcp connection. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15523 Signed-off-by: Stefan Metzmacher Reviewed-by: Martin Schwenke (cherry picked from commit 082c7df4d04c2a94c5413c1d6b7eae7be610f950) --- .../smbXsrv_client_ctdb_registered_ips | 4 + .../smbXsrv_client_ctdb_registered_ips | 1 + ...test_smbXsrv_client_ctdb_registered_ips.sh | 159 ++++++++++++++++++ source3/selftest/tests.py | 6 + 4 files changed, 170 insertions(+) create mode 100644 selftest/flapping.d/smbXsrv_client_ctdb_registered_ips create mode 100644 selftest/knownfail.d/smbXsrv_client_ctdb_registered_ips create mode 100755 source3/script/tests/test_smbXsrv_client_ctdb_registered_ips.sh diff --git a/selftest/flapping.d/smbXsrv_client_ctdb_registered_ips b/selftest/flapping.d/smbXsrv_client_ctdb_registered_ips new file mode 100644 index 000000000000..740bb87251b2 --- /dev/null +++ b/selftest/flapping.d/smbXsrv_client_ctdb_registered_ips @@ -0,0 +1,4 @@ +^samba3.blackbox.smbXsrv_client_ctdb_registered_ips.step5:.ctdb_gettickles.NUM +^samba3.blackbox.smbXsrv_client_ctdb_registered_ips.step5:.ctdb_gettickles.DST +^samba3.blackbox.smbXsrv_client_ctdb_registered_ips.step6:.ctdb_gettickles.NUM +^samba3.blackbox.smbXsrv_client_ctdb_registered_ips.step6:.ctdb_gettickles.DST diff --git a/selftest/knownfail.d/smbXsrv_client_ctdb_registered_ips b/selftest/knownfail.d/smbXsrv_client_ctdb_registered_ips new file mode 100644 index 000000000000..8bce2bbb5dbf --- /dev/null +++ b/selftest/knownfail.d/smbXsrv_client_ctdb_registered_ips @@ -0,0 +1 @@ +^samba3.blackbox.smbXsrv_client_ctdb_registered_ips.step7:.smbstatus.0.sessions diff --git a/source3/script/tests/test_smbXsrv_client_ctdb_registered_ips.sh b/source3/script/tests/test_smbXsrv_client_ctdb_registered_ips.sh new file mode 100755 index 000000000000..025a0fa837c4 --- /dev/null +++ b/source3/script/tests/test_smbXsrv_client_ctdb_registered_ips.sh @@ -0,0 +1,159 @@ +#!/usr/bin/env bash +# +# Test smbd let cleanup registered ip addresses in a multichannel +# scenario +# + +if [ $# -lt 3 ]; then + echo Usage: test_smbXsrv_client_ctdb_registered_ips.sh SERVERCONFFILE CTDB_IFACE_IP SHARENAME + exit 1 +fi + +CONF=$1 +CTDB_IFACE_IP=$2 +SHARE=$3 + +SMBCLIENT="$BINDIR/smbclient" +SMBSTATUS="$BINDIR/smbstatus" +CTDB="$BINDIR/ctdb" +TIMELIMIT="$BINDIR/timelimit" + +incdir=$(dirname "$0")/../../../testprogs/blackbox +. "$incdir"/subunit.sh + +failed=0 + +test_smbclient() +{ + name="$1" + server="$2" + share="$3" + cmd="$4" + shift + shift + subunit_start_test "$name" + output=$($VALGRIND $SMBCLIENT //$server/$share -c "$cmd" "$@" 2>&1) + status=$? + if [ x$status = x0 ]; then + subunit_pass_test "$name" + else + echo "$output" | subunit_fail_test "$name" + fi + return $status +} + +cd "$SELFTEST_TMPDIR" || exit 1 + +# Create the smbclient communication pipes. +rm -f smbclient1-stdin smbclient1-stdout smbclient1-stderr +mkfifo smbclient1-stdin smbclient1-stdout smbclient1-stderr +rm -f smbclient2-stdin smbclient2-stdout smbclient2-stderr +mkfifo smbclient2-stdin smbclient2-stdout smbclient2-stderr + +smbstatus_num_sessions() +{ + # We don't check for died processes + UID_WRAPPER_INITIAL_RUID=0 UID_WRAPPER_INITIAL_EUID=0 "$SMBSTATUS" "$CONF" --fast --json | jq -M '.sessions | length' +} + +ctdb_add_public_ip() +{ + UID_WRAPPER_INITIAL_RUID=0 UID_WRAPPER_INITIAL_EUID=0 "$CTDB" addip ${CTDB_IFACE_IP}/24 lo + UID_WRAPPER_INITIAL_RUID=0 UID_WRAPPER_INITIAL_EUID=0 "$CTDB" ipreallocate +} + +ctdb_ip() +{ + UID_WRAPPER_INITIAL_RUID=0 UID_WRAPPER_INITIAL_EUID=0 "$CTDB" ip +} + +ctdb_gettickles() +{ + UID_WRAPPER_INITIAL_RUID=0 UID_WRAPPER_INITIAL_EUID=0 "$CTDB" gettickles ${CTDB_IFACE_IP} +} + +ctdb_reload_public_ips() +{ + UID_WRAPPER_INITIAL_RUID=0 UID_WRAPPER_INITIAL_EUID=0 "$CTDB" reloadips 0 + UID_WRAPPER_INITIAL_RUID=0 UID_WRAPPER_INITIAL_EUID=0 "$CTDB" ipreallocate +} + +testit_grep_count "step1: smbstatus 0 sessions" '^0$' 1 smbstatus_num_sessions || failed=$(expr $failed + 1) + +test_smbclient "step2: smbclient against node0[${CTDB_IFACE_IP}]" "${CTDB_IFACE_IP}" "${SHARE}" "ls" -U"${DC_USERNAME}"%"${DC_PASSWORD}" \ + --option="libsmb:client_guid=6112f7d3-9528-4a2a-8861-0ca129aae6c4" \ + || failed=$(expr $failed + 1) + +testit_grep_count "step2: smbstatus 0 sessions" '^0$' 1 smbstatus_num_sessions || failed=$(expr $failed + 1) + +CLI_FORCE_INTERACTIVE=1 +export CLI_FORCE_INTERACTIVE + +testit "step3: start backgroup smbclient against node0[${CTDB_IFACE_IP}]" true || failed=$(expr $failed + 1) + +# Connect a first time +${SMBCLIENT} //"${CTDB_IFACE_IP}"/"${SHARE}" -U"${DC_USERNAME}"%"${DC_PASSWORD}" \ + --option="libsmb:client_guid=6112f7d3-9528-4a2a-8861-0ca129aae6c4" \ + smbclient1-stdout 2>smbclient1-stderr & +CLIENT1_PID=$! + +exec 100>smbclient1-stdin 101smbclient2-stdout 2>smbclient2-stderr & +CLIENT2_PID=$! + +exec 200>smbclient2-stdin 201 Date: Thu, 12 Oct 2023 17:19:45 +0200 Subject: [PATCH 14/16] smbd: Remove callback for release_ip when "state" is free'ed If a client connects to a non-public address first followed by a connect to public address with the same client_guid and a connection to the non-public address gets disconnected first, we hit by a use-after-free talloc_get_type_abort() called from release_ip() as "xconn" is already gone, taking smbd_release_ip_state with it. We need to decide between calling ctdbd_unregister_ips() by default, as it means the tcp connection is really gone and ctdb needs to remove the 'tickle' information. But when a connection was passed to a different smbd process, we need to use ctdbd_passed_ips() as the tcp connection is still alive and the 'tickle' information should not be removed within ctdb. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15523 Pair-Programmed-With: Stefan Metzmacher Signed-off-by: Volker Lendecke Signed-off-by: Stefan Metzmacher Reviewed-by: Martin Schwenke (cherry picked from commit ddf47e7fe314e0f5bf71ff53e35350e0ba530d08) --- .../smbXsrv_client_ctdb_registered_ips | 4 --- .../smbXsrv_client_ctdb_registered_ips | 1 - source3/smbd/smb2_negprot.c | 7 ++++ source3/smbd/smb2_process.c | 33 +++++++++++++++++++ 4 files changed, 40 insertions(+), 5 deletions(-) delete mode 100644 selftest/flapping.d/smbXsrv_client_ctdb_registered_ips delete mode 100644 selftest/knownfail.d/smbXsrv_client_ctdb_registered_ips diff --git a/selftest/flapping.d/smbXsrv_client_ctdb_registered_ips b/selftest/flapping.d/smbXsrv_client_ctdb_registered_ips deleted file mode 100644 index 740bb87251b2..000000000000 --- a/selftest/flapping.d/smbXsrv_client_ctdb_registered_ips +++ /dev/null @@ -1,4 +0,0 @@ -^samba3.blackbox.smbXsrv_client_ctdb_registered_ips.step5:.ctdb_gettickles.NUM -^samba3.blackbox.smbXsrv_client_ctdb_registered_ips.step5:.ctdb_gettickles.DST -^samba3.blackbox.smbXsrv_client_ctdb_registered_ips.step6:.ctdb_gettickles.NUM -^samba3.blackbox.smbXsrv_client_ctdb_registered_ips.step6:.ctdb_gettickles.DST diff --git a/selftest/knownfail.d/smbXsrv_client_ctdb_registered_ips b/selftest/knownfail.d/smbXsrv_client_ctdb_registered_ips deleted file mode 100644 index 8bce2bbb5dbf..000000000000 --- a/selftest/knownfail.d/smbXsrv_client_ctdb_registered_ips +++ /dev/null @@ -1 +0,0 @@ -^samba3.blackbox.smbXsrv_client_ctdb_registered_ips.step7:.smbstatus.0.sessions diff --git a/source3/smbd/smb2_negprot.c b/source3/smbd/smb2_negprot.c index c0e558b2c4d0..d89e6d49d2fe 100644 --- a/source3/smbd/smb2_negprot.c +++ b/source3/smbd/smb2_negprot.c @@ -877,7 +877,14 @@ static void smbd_smb2_request_process_negprot_mc_done(struct tevent_req *subreq) if (NT_STATUS_EQUAL(status, NT_STATUS_MESSAGE_RETRIEVED)) { /* * The connection was passed to another process + * + * We mark the error as NT_STATUS_CONNECTION_IN_USE, + * in order to indicate to low level code if + * ctdbd_unregister_ips() or ctdbd_passed_ips() + * is more useful. */ + smbXsrv_connection_disconnect_transport(xconn, + NT_STATUS_CONNECTION_IN_USE); smbd_server_connection_terminate(xconn, "passed connection"); /* diff --git a/source3/smbd/smb2_process.c b/source3/smbd/smb2_process.c index 11f556c88ace..7afc84529390 100644 --- a/source3/smbd/smb2_process.c +++ b/source3/smbd/smb2_process.c @@ -1017,9 +1017,37 @@ static void smbd_server_connection_handler(struct tevent_context *ev, struct smbd_release_ip_state { struct smbXsrv_connection *xconn; struct tevent_immediate *im; + struct sockaddr_storage srv; + struct sockaddr_storage clnt; char addr[INET6_ADDRSTRLEN]; }; +static int release_ip(struct tevent_context *ev, + uint32_t src_vnn, + uint32_t dst_vnn, + uint64_t dst_srvid, + const uint8_t *msg, + size_t msglen, + void *private_data); + +static int smbd_release_ip_state_destructor(struct smbd_release_ip_state *s) +{ + struct ctdbd_connection *cconn = messaging_ctdb_connection(); + struct smbXsrv_connection *xconn = s->xconn; + + if (cconn == NULL) { + return 0; + } + + if (NT_STATUS_EQUAL(xconn->transport.status, NT_STATUS_CONNECTION_IN_USE)) { + ctdbd_passed_ips(cconn, &s->srv, &s->clnt, release_ip, s); + } else { + ctdbd_unregister_ips(cconn, &s->srv, &s->clnt, release_ip, s); + } + + return 0; +} + static void smbd_release_ip_immediate(struct tevent_context *ctx, struct tevent_immediate *im, void *private_data) @@ -1162,6 +1190,8 @@ static NTSTATUS smbd_register_ips(struct smbXsrv_connection *xconn, if (state->im == NULL) { return NT_STATUS_NO_MEMORY; } + state->srv = *srv; + state->clnt = *clnt; if (print_sockaddr(state->addr, sizeof(state->addr), srv) == NULL) { return NT_STATUS_NO_MEMORY; } @@ -1185,6 +1215,9 @@ static NTSTATUS smbd_register_ips(struct smbXsrv_connection *xconn, if (ret != 0) { return map_nt_error_from_unix(ret); } + + talloc_set_destructor(state, smbd_release_ip_state_destructor); + return NT_STATUS_OK; } -- 2.34.1 From 9480092d10714c073421a2f942839bf558c3dec9 Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Wed, 13 Dec 2023 10:22:04 +1100 Subject: [PATCH 15/16] ctdb-daemon: Use ctdb_connection_to_buf() to simplify The one case that is no longer handled specially is when the destination address is IPv4 loopback. This may previously have been used to avoid flooding the logs when testing. However, that seems unnecessary - if testing with 127.0.0.1 then make it a public address. Modernise debug while touching the code. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15523 Signed-off-by: Martin Schwenke Reviewed-by: Stefan Metzmacher (cherry picked from commit 8fc3872557f715dc38f9898754a785fd073ace96) --- ctdb/server/ctdb_takeover.c | 66 +++++++++---------------------------- 1 file changed, 15 insertions(+), 51 deletions(-) diff --git a/ctdb/server/ctdb_takeover.c b/ctdb/server/ctdb_takeover.c index bbef628e1bc8..0148ea24de52 100644 --- a/ctdb/server/ctdb_takeover.c +++ b/ctdb/server/ctdb_takeover.c @@ -1271,8 +1271,8 @@ int32_t ctdb_control_tcp_client(struct ctdb_context *ctdb, uint32_t client_id, int ret; TDB_DATA data; struct ctdb_vnn *vnn; - ctdb_sock_addr src_addr; ctdb_sock_addr dst_addr; + char conn_str[132] = { 0, }; /* If we don't have public IPs, tickles are useless */ if (ctdb->vnn == NULL) { @@ -1282,43 +1282,23 @@ int32_t ctdb_control_tcp_client(struct ctdb_context *ctdb, uint32_t client_id, tcp_sock = (struct ctdb_connection *)indata.dptr; ctdb_canonicalize_ip_inplace(&tcp_sock->src); - src_addr = tcp_sock->src; - ctdb_canonicalize_ip_inplace(&tcp_sock->dst); dst_addr = tcp_sock->dst; + ret = ctdb_connection_to_buf(conn_str, + sizeof(conn_str), + tcp_sock, + false, + " -> "); + if (ret != 0) { + strlcpy(conn_str, "UNKNOWN", sizeof(conn_str)); + } + vnn = find_public_ip_vnn(ctdb, &dst_addr); if (vnn == NULL) { - char *src_addr_str = NULL; - char *dst_addr_str = NULL; - - switch (dst_addr.sa.sa_family) { - case AF_INET: - if (ntohl(dst_addr.ip.sin_addr.s_addr) == INADDR_LOOPBACK) { - /* ignore ... */ - return 0; - } - break; - case AF_INET6: - break; - default: - DEBUG(DEBUG_ERR,(__location__ " Unknown family type %d\n", - dst_addr.sa.sa_family)); - return 0; - } - - src_addr_str = ctdb_sock_addr_to_string(client, &src_addr, false); - dst_addr_str = ctdb_sock_addr_to_string(client, &dst_addr, false); - DEBUG(DEBUG_ERR,( - "Could not register TCP connection from " - "%s to %s (not a public address) (port %u) " - "(client_id %u pid %u).\n", - src_addr_str, - dst_addr_str, - ctdb_sock_addr_port(&dst_addr), - client_id, client->pid)); - TALLOC_FREE(src_addr_str); - TALLOC_FREE(dst_addr_str); + D_ERR("Could not register TCP connection %s - " + "not a public address (client_id %u pid %u)\n", + conn_str, client_id, client->pid); return 0; } @@ -1346,24 +1326,8 @@ int32_t ctdb_control_tcp_client(struct ctdb_context *ctdb, uint32_t client_id, data.dptr = (uint8_t *)&t; data.dsize = sizeof(t); - switch (dst_addr.sa.sa_family) { - case AF_INET: - DEBUG(DEBUG_INFO,("registered tcp client for %u->%s:%u (client_id %u pid %u)\n", - (unsigned)ntohs(tcp_sock->dst.ip.sin_port), - ctdb_addr_to_str(&tcp_sock->src), - (unsigned)ntohs(tcp_sock->src.ip.sin_port), client_id, client->pid)); - break; - case AF_INET6: - DEBUG(DEBUG_INFO,("registered tcp client for %u->%s:%u (client_id %u pid %u)\n", - (unsigned)ntohs(tcp_sock->dst.ip6.sin6_port), - ctdb_addr_to_str(&tcp_sock->src), - (unsigned)ntohs(tcp_sock->src.ip6.sin6_port), client_id, client->pid)); - break; - default: - DEBUG(DEBUG_ERR,(__location__ " Unknown family %d\n", - dst_addr.sa.sa_family)); - } - + D_INFO("Registered TCP connection %s (client_id %u pid %u)\n", + conn_str, client_id, client->pid); /* tell all nodes about this tcp connection */ ret = ctdb_daemon_send_control(ctdb, CTDB_BROADCAST_CONNECTED, 0, -- 2.34.1 From b6049f36c9352c90e615c34c194590029f71392f Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Wed, 13 Dec 2023 10:29:05 +1100 Subject: [PATCH 16/16] ctdb-server: Drop unnecessary copy of destination address Modernise debug while touching the code. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15523 Signed-off-by: Martin Schwenke Reviewed-by: Stefan Metzmacher Autobuild-User(master): Stefan Metzmacher Autobuild-Date(master): Fri Dec 15 12:09:21 UTC 2023 on atb-devel-224 (cherry picked from commit 4b7329f15820f1b4d9a7b7f0947719c4217b312a) --- ctdb/server/ctdb_takeover.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/ctdb/server/ctdb_takeover.c b/ctdb/server/ctdb_takeover.c index 0148ea24de52..3ecb7382e2a5 100644 --- a/ctdb/server/ctdb_takeover.c +++ b/ctdb/server/ctdb_takeover.c @@ -1271,7 +1271,6 @@ int32_t ctdb_control_tcp_client(struct ctdb_context *ctdb, uint32_t client_id, int ret; TDB_DATA data; struct ctdb_vnn *vnn; - ctdb_sock_addr dst_addr; char conn_str[132] = { 0, }; /* If we don't have public IPs, tickles are useless */ @@ -1283,7 +1282,6 @@ int32_t ctdb_control_tcp_client(struct ctdb_context *ctdb, uint32_t client_id, ctdb_canonicalize_ip_inplace(&tcp_sock->src); ctdb_canonicalize_ip_inplace(&tcp_sock->dst); - dst_addr = tcp_sock->dst; ret = ctdb_connection_to_buf(conn_str, sizeof(conn_str), @@ -1294,7 +1292,7 @@ int32_t ctdb_control_tcp_client(struct ctdb_context *ctdb, uint32_t client_id, strlcpy(conn_str, "UNKNOWN", sizeof(conn_str)); } - vnn = find_public_ip_vnn(ctdb, &dst_addr); + vnn = find_public_ip_vnn(ctdb, &tcp_sock->dst); if (vnn == NULL) { D_ERR("Could not register TCP connection %s - " "not a public address (client_id %u pid %u)\n", @@ -1303,9 +1301,10 @@ int32_t ctdb_control_tcp_client(struct ctdb_context *ctdb, uint32_t client_id, } if (vnn->pnn != ctdb->pnn) { - DEBUG(DEBUG_ERR,("Attempt to register tcp client for IP %s we don't hold - failing (client_id %u pid %u)\n", - ctdb_addr_to_str(&dst_addr), - client_id, client->pid)); + D_ERR("Attempt to register tcp client for IP %s we don't hold - " + "failing (client_id %u pid %u)\n", + ctdb_addr_to_str(&tcp_sock->dst), + client_id, client->pid); /* failing this call will tell smbd to die */ return -1; } -- 2.34.1