The Samba-Bugzilla – Attachment 18762 Details for
Bug 15935
Crash in ctdbd on failed updateip
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch for v4-21-test
BZ15935-v4-21-test.patch (text/plain), 12.63 KB, created by
Martin Schwenke
on 2025-10-17 08:28:12 UTC
(
hide
)
Description:
Patch for v4-21-test
Filename:
MIME Type:
Creator:
Martin Schwenke
Created:
2025-10-17 08:28:12 UTC
Size:
12.63 KB
patch
obsolete
>From 7f920206854a294d65fc1fefe8810a1b5ea8b901 Mon Sep 17 00:00:00 2001 >From: Martin Schwenke <mschwenke@ddn.com> >Date: Fri, 13 Sep 2024 16:21:24 +1000 >Subject: [PATCH 1/5] ctdb-scripts: Reformat with "shfmt -w -p -i 0 -fn" > >Massage a couple of lines manually so they're formatted sanely given >the new indentation. Re-run shfmt to ensure no further changes. > >Best reviewed with "git show -w". > >Signed-off-by: Martin Schwenke <mschwenke@ddn.com> >Reviewed-by: Volker Lendecke <vl@samba.org> >Reviewed-by: Jerry Heyman <jheyman@ddn.com> >(cherry picked from commit 3410eddd932b430acc687c81a5dc6e62a0a420a6) >--- > ctdb/config/events/legacy/10.interface.script | 80 +++++++++---------- > 1 file changed, 39 insertions(+), 41 deletions(-) > >diff --git a/ctdb/config/events/legacy/10.interface.script b/ctdb/config/events/legacy/10.interface.script >index dfd796563fd..137d558b3b7 100755 >--- a/ctdb/config/events/legacy/10.interface.script >+++ b/ctdb/config/events/legacy/10.interface.script >@@ -5,7 +5,7 @@ > # this adds/removes IPs from your > # public interface > >-[ -n "$CTDB_BASE" ] || \ >+[ -n "$CTDB_BASE" ] || > CTDB_BASE=$(d=$(dirname "$0") && cd -P "$d" && dirname "$PWD") > > . "${CTDB_BASE}/functions" >@@ -13,7 +13,7 @@ > load_script_options > > if ! have_public_addresses; then >- if [ "$1" = "init" ] ; then >+ if [ "$1" = "init" ]; then > echo "No public addresses file found" > fi > exit 0 >@@ -32,8 +32,8 @@ monitor_interfaces() > # > # public_ifaces set by get_public_ifaces() above > # shellcheck disable=SC2154 >- for _iface in $public_ifaces ; do >- if interface_monitor "$_iface" ; then >+ for _iface in $public_ifaces; do >+ if interface_monitor "$_iface"; then > up_interfaces_found=true > $CTDB setifacelink "$_iface" up >/dev/null 2>&1 > else >@@ -42,11 +42,11 @@ monitor_interfaces() > fi > done > >- if ! $down_interfaces_found ; then >+ if ! $down_interfaces_found; then > return 0 > fi > >- if ! $up_interfaces_found ; then >+ if ! $up_interfaces_found; then > return 1 > fi > >@@ -58,63 +58,61 @@ monitor_interfaces() > } > > # Sets: iface, ip, maskbits >-get_iface_ip_maskbits () >+get_iface_ip_maskbits() > { >- _iface_in="$1" >- ip="$2" >- _maskbits_in="$3" >- >- # Intentional word splitting here >- # shellcheck disable=SC2046 >- set -- $(ip_maskbits_iface "$ip") >- if [ -n "$1" ] ; then >- maskbits="$1" >- iface="$2" >- >- if [ "$iface" != "$_iface_in" ] ; then >- printf \ >- 'WARNING: Public IP %s hosted on interface %s but VNN says %s\n' \ >- "$ip" "$iface" "$_iface_in" >- fi >- if [ "$maskbits" != "$_maskbits_in" ] ; then >- printf \ >- 'WARNING: Public IP %s has %s bit netmask but VNN says %s\n' \ >- "$ip" "$maskbits" "$_maskbits_in" >+ _iface_in="$1" >+ ip="$2" >+ _maskbits_in="$3" >+ >+ # Intentional word splitting here >+ # shellcheck disable=SC2046 >+ set -- $(ip_maskbits_iface "$ip") >+ if [ -n "$1" ]; then >+ maskbits="$1" >+ iface="$2" >+ >+ if [ "$iface" != "$_iface_in" ]; then >+ printf 'WARNING: Public IP %s hosted on interface %s but VNN says %s\n' \ >+ "$ip" "$iface" "$_iface_in" >+ fi >+ if [ "$maskbits" != "$_maskbits_in" ]; then >+ printf 'WARNING: Public IP %s has %s bit netmask but VNN says %s\n' \ >+ "$ip" "$maskbits" "$_maskbits_in" >+ fi >+ else >+ die "ERROR: Unable to determine interface for IP ${ip}" > fi >- else >- die "ERROR: Unable to determine interface for IP ${ip}" >- fi > } > >-ip_block () >+ip_block() > { > _ip="$1" > _iface="$2" > > case "$_ip" in > *:*) _family="inet6" ;; >- *) _family="inet" ;; >+ *) _family="inet" ;; > esac > > # Extra delete copes with previously killed script > iptables_wrapper "$_family" \ >- -D INPUT -i "$_iface" -d "$_ip" -j DROP 2>/dev/null >+ -D INPUT -i "$_iface" -d "$_ip" -j DROP 2>/dev/null > iptables_wrapper "$_family" \ >- -I INPUT -i "$_iface" -d "$_ip" -j DROP >+ -I INPUT -i "$_iface" -d "$_ip" -j DROP > } > >-ip_unblock () >+ip_unblock() > { > _ip="$1" > _iface="$2" > > case "$_ip" in > *:*) _family="inet6" ;; >- *) _family="inet" ;; >+ *) _family="inet" ;; > esac > > iptables_wrapper "$_family" \ >- -D INPUT -i "$_iface" -d "$_ip" -j DROP 2>/dev/null >+ -D INPUT -i "$_iface" -d "$_ip" -j DROP 2>/dev/null > } > > ctdb_check_args "$@" >@@ -128,8 +126,8 @@ init) > } > > _promote="sys/net/ipv4/conf/all/promote_secondaries" >- get_proc "$_promote" >/dev/null 2>&1 || \ >- die "Public IPs only supported if promote_secondaries is available" >+ get_proc "$_promote" >/dev/null 2>&1 || >+ die "Public IPs only supported if promote_secondaries is available" > > # make sure we drop any ips that might still be held if > # previous instance of ctdb got killed with -9 or similar >@@ -152,7 +150,7 @@ takeip) > update_my_public_ip_addresses "takeip" "$ip" > > add_ip_to_iface "$iface" "$ip" "$maskbits" || { >- exit 1; >+ exit 1 > } > > # In case a previous "releaseip" for this IP was killed... >@@ -213,7 +211,7 @@ updateip) > > # Could check maskbits too. However, that should never change > # so we want to notice if it does. >- if [ "$oiface" = "$niface" ] ; then >+ if [ "$oiface" = "$niface" ]; then > echo "Redundant \"updateip\" - ${ip} already on ${niface}" > exit 0 > fi >-- >2.47.3 > > >From 457bc85c0c0b478a61920061984904f38675b210 Mon Sep 17 00:00:00 2001 >From: Martin Schwenke <mschwenke@ddn.com> >Date: Thu, 16 Oct 2025 08:17:44 +1100 >Subject: [PATCH 2/5] 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 b9196e3ff63..f1b3119bf34 100644 >--- a/ctdb/server/ctdb_takeover.c >+++ b/ctdb/server/ctdb_takeover.c >@@ -613,7 +613,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 39c5317e984182c1160f40fca47d1edcf1883da5 Mon Sep 17 00:00:00 2001 >From: Martin Schwenke <mschwenke@ddn.com> >Date: Thu, 16 Oct 2025 10:42:22 +1100 >Subject: [PATCH 3/5] 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 061d017d7739ab1839e83bc70a47ec6e1180433b Mon Sep 17 00:00:00 2001 >From: Martin Schwenke <mschwenke@ddn.com> >Date: Thu, 16 Oct 2025 13:51:27 +1100 >Subject: [PATCH 4/5] 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 4139059a3d3..d61852a8161 100755 >--- a/ctdb/config/functions >+++ b/ctdb/config/functions >@@ -594,6 +594,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 445f37243dee3fa905744ca93f1c5362d1286d66 Mon Sep 17 00:00:00 2001 >From: Martin Schwenke <mschwenke@ddn.com> >Date: Thu, 16 Oct 2025 13:54:22 +1100 >Subject: [PATCH 5/5] 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 137d558b3b7..f0545a40455 100755 >--- a/ctdb/config/events/legacy/10.interface.script >+++ b/ctdb/config/events/legacy/10.interface.script >@@ -80,6 +80,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 > } >@@ -216,10 +221,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 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Flags:
anoopcs
:
review+
Actions:
View
Attachments on
bug 15935
:
18760
|
18761
| 18762