Attachment 15599 Details for Bug 14175 – Patch for 4.11, 4.10 and possibly 4.9

[patch] Patch for 4.11, 4.10 and possibly 4.9

BZ14175.patch (text/plain), 6.28 KB, created by Martin Schwenke on 2019-11-06 05:55:45 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Schwenke

Created: 2019-11-06 05:55:45 UTC

Size: 6.28 KB

patch

obsolete

>From 65e06d6419b93b794c984631357b5614766d9667 Mon Sep 17 00:00:00 2001
>From: Martin Schwenke <martin@meltin.net>
>Date: Tue, 29 Oct 2019 15:25:26 +1100
>Subject: [PATCH 1/3] ctdb-tcp: Check incoming queue to see if incoming
> connection is up
>
>This makes it consistent with the reverse case.  Also, in_fd will soon
>be removed.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14175
>
>Signed-off-by: Martin Schwenke <martin@meltin.net>
>Reviewed-by: Amitay Isaacs <amitay@gmail.com>
>(cherry picked from commit e62b3a05a874db13a848573d2e2fb1c157393b9c)
>---
> ctdb/tcp/tcp_connect.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/ctdb/tcp/tcp_connect.c b/ctdb/tcp/tcp_connect.c
>index 6123380ca9f..5bcb5216bd2 100644
>--- a/ctdb/tcp/tcp_connect.c
>+++ b/ctdb/tcp/tcp_connect.c
>@@ -153,7 +153,7 @@ static void ctdb_node_connect_write(struct tevent_context *ev,
> 	 * as connected, but only if the corresponding listening
> 	 * socket is also connected
> 	 */
>-	if (tnode->in_fd != -1) {
>+	if (tnode->in_queue != NULL) {
> 		node->ctdb->upcalls->node_connected(node);
> 	}
> }
>-- 
>2.24.0.rc1
>
>
>From 9d4aa99a34e3b1d4d3011128841212040737445b Mon Sep 17 00:00:00 2001
>From: Martin Schwenke <martin@meltin.net>
>Date: Tue, 29 Oct 2019 15:29:11 +1100
>Subject: [PATCH 2/3] ctdb-tcp: Avoid orphaning the TCP incoming queue
>
>CTDB's incoming queue handling does not check whether an existing
>queue exists, so can overwrite the pointer to the queue.  This used to
>be harmless until commit c68b6f96f26664459187ab2fbd56767fb31767e0
>changed the read callback to use a parent structure as the callback
>data.  Instead of cleaning up an orphaned queue on disconnect, as
>before, this will now free the new queue.
>
>At first glance it doesn't seem possible that 2 incoming connections
>from the same node could be processed before the intervening
>disconnect.  However, the incoming connections and disconnect occur on
>different file descriptors.  The queue can become orphaned on node A
>when the following sequence occurs:
>
>1. Node A comes up
>2. Node A accepts an incoming connection from node B
>3. Node B processes a timeout before noticing that outgoing the queue is writable
>4. Node B tears down the outgoing connection to node A
>5. Node B initiates a new connection to node A
>6. Node A accepts an incoming connection from node B
>
>Node A processes then the disconnect of the old incoming connection
>from (2) but tears down the new incoming connection from (6).  This
>then occurs until the originally affected node is restarted.
>
>However, due to the number of outgoing connection attempts and
>associated teardowns, this induces the same behaviour on the
>corresponding incoming queue on all nodes that node A attempts to
>connect to.  Therefore, other nodes become affected and need to be
>restarted too.
>
>As a result, the whole cluster probably needs to be restarted to
>recover from this situation.
>
>The problem can occur any time CTDB is started on a node.
>
>The fix is to avoid accepting new incoming connections when a queue
>for incoming connections is already present.  The connecting node will
>simply retry establishing its outgoing connection.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14175
>
>Signed-off-by: Martin Schwenke <martin@meltin.net>
>Reviewed-by: Amitay Isaacs <amitay@gmail.com>
>(cherry picked from commit d0baad257e511280ff3e5c7372c38c43df841070)
>---
> ctdb/tcp/tcp_connect.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
>diff --git a/ctdb/tcp/tcp_connect.c b/ctdb/tcp/tcp_connect.c
>index 5bcb5216bd2..df115b783e0 100644
>--- a/ctdb/tcp/tcp_connect.c
>+++ b/ctdb/tcp/tcp_connect.c
>@@ -312,6 +312,13 @@ static void ctdb_listen_event(struct tevent_context *ev, struct tevent_fd *fde,
> 		return;
> 	}
> 
>+	if (tnode->in_queue != NULL) {
>+		DBG_ERR("Incoming queue active, rejecting connection from %s\n",
>+			ctdb_addr_to_str(&addr));
>+		close(fd);
>+		return;
>+	}
>+
> 	ret = set_blocking(fd, false);
> 	if (ret != 0) {
> 		DBG_ERR("Failed to set socket non-blocking (%s)\n",
>-- 
>2.24.0.rc1
>
>
>From 6d6bceb60b9c936068a14b50e22d19da7f154175 Mon Sep 17 00:00:00 2001
>From: Martin Schwenke <martin@meltin.net>
>Date: Tue, 29 Oct 2019 17:28:22 +1100
>Subject: [PATCH 3/3] ctdb-tcp: Drop tracking of file descriptor for incoming
> connections
>
>This file descriptor is owned by the incoming queue.  It will be
>closed when the queue is torn down.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14175
>RN: Avoid communication breakdown on node reconnect
>
>Signed-off-by: Martin Schwenke <martin@meltin.net>
>Reviewed-by: Amitay Isaacs <amitay@gmail.com>
>(cherry picked from commit bf47bc18bb8a94231870ef821c0352b7a15c2e28)
>---
> ctdb/tcp/ctdb_tcp.h    | 1 -
> ctdb/tcp/tcp_connect.c | 2 --
> ctdb/tcp/tcp_init.c    | 6 ------
> ctdb/tcp/tcp_io.c      | 2 --
> 4 files changed, 11 deletions(-)
>
>diff --git a/ctdb/tcp/ctdb_tcp.h b/ctdb/tcp/ctdb_tcp.h
>index 9a615fc6393..daabad74297 100644
>--- a/ctdb/tcp/ctdb_tcp.h
>+++ b/ctdb/tcp/ctdb_tcp.h
>@@ -37,7 +37,6 @@ struct ctdb_tcp_node {
> 	struct tevent_timer *connect_te;
> 
> 	struct ctdb_context *ctdb;
>-	int in_fd;
> 	struct ctdb_queue *in_queue;
> };
> 
>diff --git a/ctdb/tcp/tcp_connect.c b/ctdb/tcp/tcp_connect.c
>index df115b783e0..a75f35a809e 100644
>--- a/ctdb/tcp/tcp_connect.c
>+++ b/ctdb/tcp/tcp_connect.c
>@@ -355,8 +355,6 @@ static void ctdb_listen_event(struct tevent_context *ev, struct tevent_fd *fde,
> 		return;
> 	}
> 
>-	tnode->in_fd = fd;
>-
>        /*
> 	* Mark the connecting node as connected, but only if the
> 	* corresponding outbound connected is also up
>diff --git a/ctdb/tcp/tcp_init.c b/ctdb/tcp/tcp_init.c
>index a9cb9b36a01..3be16bd7d5e 100644
>--- a/ctdb/tcp/tcp_init.c
>+++ b/ctdb/tcp/tcp_init.c
>@@ -43,11 +43,6 @@ static int tnode_destructor(struct ctdb_tcp_node *tnode)
> 		tnode->out_fd = -1;
> 	}
> 
>-	if (tnode->in_fd != -1) {
>-		close(tnode->in_fd);
>-		tnode->in_fd = -1;
>-	}
>-
> 	return 0;
> }
> 
>@@ -61,7 +56,6 @@ static int ctdb_tcp_add_node(struct ctdb_node *node)
> 	CTDB_NO_MEMORY(node->ctdb, tnode);
> 
> 	tnode->out_fd = -1;
>-	tnode->in_fd = -1;
> 	tnode->ctdb = node->ctdb;
> 
> 	node->private_data = tnode;
>diff --git a/ctdb/tcp/tcp_io.c b/ctdb/tcp/tcp_io.c
>index e8ebff887e1..2d8ec0f7062 100644
>--- a/ctdb/tcp/tcp_io.c
>+++ b/ctdb/tcp/tcp_io.c
>@@ -76,8 +76,6 @@ void ctdb_tcp_read_cb(uint8_t *data, size_t cnt, void *args)
> 
> failed:
> 	TALLOC_FREE(tnode->in_queue);
>-	close(tnode->in_fd);
>-	tnode->in_fd = -1;
> 	node->ctdb->upcalls->node_dead(node);
> 
> 	TALLOC_FREE(data);
>-- 
>2.24.0.rc1
>

Actions: View

Attachments on bug 14175: 15599 | 15620