Attachment 18760 Details for Bug 15935 – Patch for v4-23-test

[patch] Patch for v4-23-test

BZ15935-v4-23-test.patch (text/plain), 7.63 KB, created by Martin Schwenke on 2025-10-17 08:25:14 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Schwenke

Created: 2025-10-17 08:25:14 UTC

Size: 7.63 KB

patch

obsolete

>From e1fde0bd4a7da20a1568a9e06032fc53e92fd969 Mon Sep 17 00:00:00 2001
>From: Martin Schwenke <mschwenke@ddn.com>
>Date: Thu, 16 Oct 2025 08:17:44 +1100
>Subject: [PATCH 1/4] ctdb-daemon: Fix a crash due to a failed updateip
>
>This should really be a takeip.  However, CTDB's weak check of the IP
>address state (using bind(2)) incorrectly indicates that the IP
>address is assigned to an interface so it is converted to an updateip.
>
>After commit 0536d7a98b832fc00d26b09c26bf14fb63dbf5fb (which improves
>IP address state checking), this will almost certainly not occur on
>platforms with getifaddrs(3) (e.g. Linux).  This means it is only
>likely to occur in 4.21 when net.ipv4.ip_nonlocal_bind=1.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15935
>
>Reported-by: Bailey Allison <ballison@45drives.com>
>Signed-off-by: Martin Schwenke <mschwenke@ddn.com>
>Reviewed-by: Anoop C S <anoopcs@samba.org>
>(cherry picked from commit d08f9ebd2755671d30c73a4e979029d353848828)
>---
> ctdb/server/ctdb_takeover.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
>diff --git a/ctdb/server/ctdb_takeover.c b/ctdb/server/ctdb_takeover.c
>index 60f60e29ffc..cbf8d0a5b10 100644
>--- a/ctdb/server/ctdb_takeover.c
>+++ b/ctdb/server/ctdb_takeover.c
>@@ -617,7 +617,15 @@ static void ctdb_do_updateip_callback(struct ctdb_context *ctdb, int status,
> 		 */
> 		ctdb_vnn_unassign_iface(ctdb, state->vnn);
> 		state->vnn->iface = state->old;
>-		state->vnn->iface->references++;
>+		/*
>+		 * state->old (above) can be NULL if the IP wasn't
>+		 * recorded as held by this node but the system thinks
>+		 * the IP was assigned.  In that case, a move could
>+		 * still be desirable..
>+		 */
>+		if (state->vnn->iface != NULL) {
>+			state->vnn->iface->references++;
>+		}
> 
> 		ctdb_request_control_reply(ctdb, state->c, NULL, status, NULL);
> 		talloc_free(state);
>-- 
>2.47.3
>
>
>From f97046d335bdf03163af7bf174cfa687f6a212da Mon Sep 17 00:00:00 2001
>From: Martin Schwenke <mschwenke@ddn.com>
>Date: Thu, 16 Oct 2025 10:42:22 +1100
>Subject: [PATCH 2/4] ctdb-tests: Add an event script unit test for updateip
>
>This illustrates the current failure where an unassigned public IP
>address causes updateip to fail.
>
>After commit 0536d7a98b832fc00d26b09c26bf14fb63dbf5fb (which improves
>IP address state checking), this will almost certainly not occur on
>platforms with getifaddrs(3) (e.g. Linux).  This means it is only
>likely to occur in 4.21 when net.ipv4.ip_nonlocal_bind=1.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15935
>
>Reported-by: Bailey Allison <ballison@45drives.com>
>Signed-off-by: Martin Schwenke <mschwenke@ddn.com>
>Reviewed-by: Anoop C S <anoopcs@samba.org>
>(cherry picked from commit a98ffb96efc4a9ea2110c654860a4ba3896ab3d5)
>---
> .../eventscripts/10.interface.updateip.001.sh    | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
> create mode 100755 ctdb/tests/UNIT/eventscripts/10.interface.updateip.001.sh
>
>diff --git a/ctdb/tests/UNIT/eventscripts/10.interface.updateip.001.sh b/ctdb/tests/UNIT/eventscripts/10.interface.updateip.001.sh
>new file mode 100755
>index 00000000000..af87bc14326
>--- /dev/null
>+++ b/ctdb/tests/UNIT/eventscripts/10.interface.updateip.001.sh
>@@ -0,0 +1,16 @@
>+#!/bin/sh
>+
>+. "${TEST_SCRIPTS_DIR}/unit.sh"
>+
>+define_test "error - update a non-existent ip"
>+
>+setup
>+
>+public_address=$(ctdb_get_1_public_address)
>+ip="${public_address% *}"
>+ip="${ip#* }"
>+
>+required_result 1 "ERROR: Unable to determine interface for IP ${ip}"
>+# Want separate words from public_address: interface IP maskbits
>+# shellcheck disable=SC2086
>+simple_test "__none__" $public_address
>-- 
>2.47.3
>
>
>From 298d36c1ba9f8c810fde43e262d04d4ee4c5314f Mon Sep 17 00:00:00 2001
>From: Martin Schwenke <mschwenke@ddn.com>
>Date: Thu, 16 Oct 2025 13:51:27 +1100
>Subject: [PATCH 3/4] ctdb-scripts: Avoid printing a message if no connections
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15935
>
>Signed-off-by: Martin Schwenke <mschwenke@ddn.com>
>Reviewed-by: Anoop C S <anoopcs@samba.org>
>(cherry picked from commit 01d3d25c0139a3dd49a2322a9416698d08733377)
>---
> ctdb/config/functions | 4 ++++
> 1 file changed, 4 insertions(+)
>
>diff --git a/ctdb/config/functions b/ctdb/config/functions
>index 1d80c61e5f2..f5194237843 100755
>--- a/ctdb/config/functions
>+++ b/ctdb/config/functions
>@@ -630,6 +630,10 @@ tickle_tcp_connections()
> 	_conns=$(get_tcp_connections_for_ip "$_ip" |
> 		awk '{ print $1, $2 ; print $2, $1 }')
> 
>+	if [ -z "$_conns" ]; then
>+		return
>+	fi
>+
> 	echo "$_conns" | awk '{ print "Tickle TCP connection", $1, $2 }'
> 	echo "$_conns" | ctdb tickle
> }
>-- 
>2.47.3
>
>
>From f381b66e249079da8da2c88ffc24aead691d7e72 Mon Sep 17 00:00:00 2001
>From: Martin Schwenke <mschwenke@ddn.com>
>Date: Thu, 16 Oct 2025 13:54:22 +1100
>Subject: [PATCH 4/4] ctdb-scripts: Avoid failing updateip when IP is not
> assigned
>
>There is no use failing this when it could behave more like takeip.
>
>Use old interface of "__none__" as a hint that ctdbd doesn't think the
>IP is assigned either.  In this case print a warning instead of an
>error.  Take some care to avoid spurious errors in updateip.
>
>After commit 0536d7a98b832fc00d26b09c26bf14fb63dbf5fb (which improves
>IP address state checking), this will almost certainly not occur on
>platforms with getifaddrs(3) (e.g. Linux).  This means it is only
>likely to occur in 4.21 when net.ipv4.ip_nonlocal_bind=1.
>
>Update test to match.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15935
>
>Reported-by: Bailey Allison <ballison@45drives.com>
>Signed-off-by: Martin Schwenke <mschwenke@ddn.com>
>Reviewed-by: Anoop C S <anoopcs@samba.org>
>
>Autobuild-User(master): Anoop C S <anoopcs@samba.org>
>Autobuild-Date(master): Fri Oct 17 06:28:30 UTC 2025 on atb-devel-224
>
>(cherry picked from commit 0e73781bf84a1e8e596d8be3f55eeb5f8f927990)
>---
> ctdb/config/events/legacy/10.interface.script   | 17 +++++++++++++----
> .../eventscripts/10.interface.updateip.001.sh   |  2 +-
> 2 files changed, 14 insertions(+), 5 deletions(-)
>
>diff --git a/ctdb/config/events/legacy/10.interface.script b/ctdb/config/events/legacy/10.interface.script
>index cb7c958ec95..5eaefd7fa86 100755
>--- a/ctdb/config/events/legacy/10.interface.script
>+++ b/ctdb/config/events/legacy/10.interface.script
>@@ -78,6 +78,11 @@ get_iface_ip_maskbits()
> 				"$ip" "$maskbits" "$_maskbits_in"
> 		fi
> 	else
>+		if [ "$_iface_in" = "__none__" ]; then
>+			echo "WARNING: Unable to determine interface for IP ${ip}"
>+			iface="$_iface_in"
>+			return
>+		fi
> 		die "ERROR: Unable to determine interface for IP ${ip}"
> 	fi
> }
>@@ -214,10 +219,14 @@ updateip)
> 		exit 0
> 	fi
> 
>-	ip_block "$ip" "$oiface"
>-
>-	delete_ip_from_iface "$oiface" "$ip" "$maskbits" 2>/dev/null
>-	delete_ip_from_iface "$niface" "$ip" "$maskbits" 2>/dev/null
>+	# Behave more like takeip when the IP is not assigned.  No
>+	# need for a similar condition around ip_unblock()s because
>+	# they will silently fail.
>+	if [ "$oiface" != "__none__" ]; then
>+		ip_block "$ip" "$oiface"
>+		delete_ip_from_iface "$oiface" "$ip" "$maskbits" >/dev/null 2>&1
>+	fi
>+	delete_ip_from_iface "$niface" "$ip" "$maskbits" >/dev/null 2>&1
> 
> 	add_ip_to_iface "$niface" "$ip" "$maskbits" || {
> 		ip_unblock "$ip" "$oiface"
>diff --git a/ctdb/tests/UNIT/eventscripts/10.interface.updateip.001.sh b/ctdb/tests/UNIT/eventscripts/10.interface.updateip.001.sh
>index af87bc14326..e9567a8d114 100755
>--- a/ctdb/tests/UNIT/eventscripts/10.interface.updateip.001.sh
>+++ b/ctdb/tests/UNIT/eventscripts/10.interface.updateip.001.sh
>@@ -10,7 +10,7 @@ public_address=$(ctdb_get_1_public_address)
> ip="${public_address% *}"
> ip="${ip#* }"
> 
>-required_result 1 "ERROR: Unable to determine interface for IP ${ip}"
>+ok "WARNING: Unable to determine interface for IP ${ip}"
> # Want separate words from public_address: interface IP maskbits
> # shellcheck disable=SC2086
> simple_test "__none__" $public_address
>-- 
>2.47.3
>

Flags:

anoopcs: review+

Actions: View

Attachments on bug 15935: 18760 | 18761 | 18762