ctdb-scripts: Track connections for all ports for public IPs

Currently TCP ports like NFS lock manager are not tracked.  It is
easier to track all connections than to add a configuration system to
try to track specified ports, so do that.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15320
RN: Update CTDB to track all TCP connections to public IP addresses

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 590a86dbe4adf45ac8d15497934e25ea98148034)

Autobuild-User(v4-20-test): Jule Anger <janger@samba.org>
Autobuild-Date(v4-20-test): Mon Nov 25 13:58:45 UTC 2024 on atb-devel-224

ctdb/config/events/legacy/10.interface.script

@@ -167,6 +167,8 @@ takeip)
 	ip=$3
 	maskbits=$4
 
+	update_my_public_ip_addresses "takeip" "$ip"
+
 	add_ip_to_iface "$iface" "$ip" "$maskbits" || {
 		exit 1;
 	}
@@ -195,6 +197,8 @@ releaseip)
 
 	kill_tcp_connections "$iface" "$ip"
 
+	update_my_public_ip_addresses "releaseip" "$ip"
+
 	delete_ip_from_iface "$iface" "$ip" "$maskbits" || {
 		ip_unblock "$ip" "$iface"
 		exit 1
@@ -254,8 +258,15 @@ updateip)
 	tickle_tcp_connections "$ip"
 	;;
 
+ipreallocated)
+	# Just to make sure
+	update_my_public_ip_addresses "ipreallocated"
+	;;
+
 monitor)
 	monitor_interfaces || exit 1
+
+	update_tickles
 	;;
 esac
 

ctdb/config/events/legacy/60.nfs.script

@@ -289,7 +289,6 @@ monitor)
 		exit $?
 	fi
 
-	update_tickles 2049
 	nfs_update_lock_info
 
 	nfs_check_services

ctdb/config/functions

@@ -269,6 +269,59 @@ ctdb_get_ip_address()
 	cat "$_ip_addr_file"
 }
 
+# Cache of public IP addresses assigned to this node.  This function
+# exists mainly so statd-callout does not need to talk to ctdbd, so
+# can be run as non-root, but it may be used in other places.  This
+# must be updated/refreshed on failover.  This is done in
+# 10.interface, but doing it in "ipreallocated" isn't enough because
+# clients may connect as soon as "takeip" completes.  Also, the VNN in
+# the daemon is only updated after the "releaseip" event completes, so
+# "ctdb -X ip" can't be relied on there.  Hence, complex updates
+# involving locking for "takeip" & "releaseip".  A future
+# restructuring of the failover model will obsolete all of these
+# moving parts.
+CTDB_MY_PUBLIC_IPS_CACHE="${CTDB_SCRIPT_VARDIR}/my-public-ip-addresses"
+update_my_public_ip_addresses()
+{
+	_event="$1"
+
+	_f="$CTDB_MY_PUBLIC_IPS_CACHE"
+	_lock="${_f}.lock"
+
+	# In private CTDB state directory - no $$ security issue
+	_new="${_f}.new.$$"
+	{
+		flock --timeout 10 9 ||
+			die "ctdb_get_my_public_ip_addresses: timeout"
+
+		case "$_event" in
+		takeip)
+			_ip="$2"
+			# Redirect of stderr guards against initial
+			# missing file
+			cat "$_f" 2>/dev/null >"$_new"
+			echo "$_ip" >>"$_new"
+			;;
+		releaseip)
+			_ip="$2"
+			# Redirect of stderr guards against initial
+			# missing file, which shouldn't happen in
+			# releaseip...
+			grep -Fvx "$_ip" "$_f" 2>/dev/null >"$_new"
+			;;
+		ipreallocated)
+			_pnn=$(ctdb_get_pnn)
+			$CTDB -X ip |
+				awk -F'|' -v pnn="$_pnn" \
+				    '$3 == pnn {print $2}' >"$_new"
+			;;
+		esac
+
+		mv "$_new" "$_f"
+
+	} 9>"$_lock"
+}
+
 # Cached retrieval of database options for use by event scripts.
 #
 # If the variables are already set then they should not be overwritten
@@ -446,7 +499,7 @@ ctdb_check_unix_socket()
 		return 1
 	fi
 
-	_out=$(ss -l -x "src ${_sockpath}" | tail -n +2)
+	_out=$(ss -l -xH "src ${_sockpath}")
 	if [ -z "$_out" ]; then
 		echo "ERROR: ${service_name} not listening on ${_sockpath}"
 		return 1
@@ -549,7 +602,7 @@ get_tcp_connections_for_ip()
 {
 	_ip="$1"
 
-	ss -tn state established "src [$_ip]" | awk 'NR > 1 {print $3, $4}'
+	ss -tnH state established "src [$_ip]" | awk '{print $3, $4}'
 }
 
 ########################################################
@@ -1096,49 +1149,39 @@ nfs_callout()
 
 update_tickles()
 {
-	_port="$1"
-
 	tickledir="${CTDB_SCRIPT_VARDIR}/tickles"
 	mkdir -p "$tickledir"
 
-	# What public IPs do I hold?
+	# If not hosting any public IPs then can't have any connections...
-	_pnn=$(ctdb_get_pnn)
+	if [ ! -s "$CTDB_MY_PUBLIC_IPS_CACHE" ]; then
-	_ips=$($CTDB -X ip | awk -F'|' -v pnn="$_pnn" '$3 == pnn {print $2}')
+		return
+	fi
 
-	# IPs and port as ss filters
+	# IPs ss filter
 	_ip_filter=""
-	for _ip in $_ips; do
+	while read -r _ip; do
 		_ip_filter="${_ip_filter}${_ip_filter:+ || }src [${_ip}]"
-	done
+	done <"$CTDB_MY_PUBLIC_IPS_CACHE"
-	_port_filter="sport == :${_port}"
+
+	# Record our current tickles in a temporary file
+	_my_tickles="${tickledir}/all.tickles.$$"
+	while read -r _i; do
+		$CTDB -X gettickles "$_i" |
+			awk -F'|' 'NR > 1 { printf "%s:%s %s:%s\n", $2, $3, $4, $5 }'
+	done <"$CTDB_MY_PUBLIC_IPS_CACHE" |
+		sort >"$_my_tickles"
 
 	# Record connections to our public IPs in a temporary file.
 	# This temporary file is in CTDB's private state directory and
 	# $$ is used to avoid a very rare race involving CTDB's script
 	# debugging.  No security issue, nothing to see here...
-	_my_connections="${tickledir}/${_port}.connections.$$"
+	_my_connections="${tickledir}/all.connections.$$"
-	# Parentheses are needed around the filters for precedence but
+	# Parentheses are needed around the IP filter for precedence but
 	# the parentheses can't be empty!
-	#
+	ss -tnH state established "${_ip_filter:+( ${_ip_filter} )}" |
-	# Recent versions of ss print square brackets around IPv6
+		awk '{print $4, $3}' |
-	# addresses.  While it is desirable to update CTDB's address
-	# parsing and printing code, something needs to be done here
-	# for backward compatibility, so just delete the brackets.
-	ss -tn state established \
-		"${_ip_filter:+( ${_ip_filter} )}" \
-		"${_port_filter:+( ${_port_filter} )}" |
-		awk 'NR > 1 {print $4, $3}' |
-		tr -d '][' |
 		sort >"$_my_connections"
 
-	# Record our current tickles in a temporary file
-	_my_tickles="${tickledir}/${_port}.tickles.$$"
-	for _i in $_ips; do
-		$CTDB -X gettickles "$_i" "$_port" |
-			awk -F'|' 'NR > 1 { printf "%s:%s %s:%s\n", $2, $3, $4, $5 }'
-	done |
-		sort >"$_my_tickles"
-
 	# Add tickles for connections that we haven't already got tickles for
 	comm -23 "$_my_connections" "$_my_tickles" |
 		$CTDB addtickle

ctdb/server/ctdb_takeover.c

@@ -1503,27 +1503,40 @@ static struct ctdb_connection *ctdb_tcp_find(struct ctdb_tcp_array *array,
   clients managing that should tickled with an ACK when IP takeover is
   done
  */
-int32_t ctdb_control_tcp_add(struct ctdb_context *ctdb, TDB_DATA indata, bool tcp_update_needed)
+int32_t ctdb_control_tcp_add(struct ctdb_context *ctdb,
+			     TDB_DATA indata,
+			     bool tcp_update_needed)
 {
 	struct ctdb_connection *p = (struct ctdb_connection *)indata.dptr;
 	struct ctdb_tcp_array *tcparray;
 	struct ctdb_connection tcp;
 	struct ctdb_vnn *vnn;
+	char conn_str[132] = { 0, };
+	int ret;
 
 	/* If we don't have public IPs, tickles are useless */
 	if (ctdb->vnn == NULL) {
 		return 0;
 	}
 
+	ret = ctdb_connection_to_buf(conn_str,
+				     sizeof(conn_str),
+				     p,
+				     false,
+				     " -> ");
+	if (ret != 0) {
+		strlcpy(conn_str, "UNKNOWN", sizeof(conn_str));
+	}
+
 	vnn = find_public_ip_vnn(ctdb, &p->dst);
 	if (vnn == NULL) {
-		DEBUG(DEBUG_INFO,(__location__ " got TCP_ADD control for an address which is not a public address '%s'\n",
+		DBG_INFO("Attempt to add connection %s "
-			ctdb_addr_to_str(&p->dst)));
+			 "but destination is not a public address\n",
+			 conn_str);
 
 		return -1;
 	}
 
-
 	tcparray = vnn->tcp_array;
 
 	/* If this is the first tickle */
@@ -1533,7 +1546,8 @@ int32_t ctdb_control_tcp_add(struct ctdb_context *ctdb, TDB_DATA indata, bool tc
 		vnn->tcp_array = tcparray;
 
 		tcparray->num = 0;
-		tcparray->connections = talloc_size(tcparray, sizeof(struct ctdb_connection));
+		tcparray->connections = talloc_size(tcparray,
+						    sizeof(struct ctdb_connection));
 		CTDB_NO_MEMORY(ctdb, tcparray->connections);
 
 		tcparray->connections[tcparray->num].src = p->src;
@@ -1551,27 +1565,22 @@ int32_t ctdb_control_tcp_add(struct ctdb_context *ctdb, TDB_DATA indata, bool tc
 	tcp.src = p->src;
 	tcp.dst = p->dst;
 	if (ctdb_tcp_find(tcparray, &tcp) != NULL) {
-		DEBUG(DEBUG_DEBUG,("Already had tickle info for %s:%u for vnn:%u\n",
+		DBG_DEBUG("Already had connection %s\n", conn_str);
-			ctdb_addr_to_str(&tcp.dst),
-			ntohs(tcp.dst.ip.sin_port),
-			vnn->pnn));
 		return 0;
 	}
 
 	/* A new tickle, we must add it to the array */
-	tcparray->connections = talloc_realloc(tcparray, tcparray->connections,
+	tcparray->connections = talloc_realloc(tcparray,
-					struct ctdb_connection,
+					       tcparray->connections,
-					tcparray->num+1);
+					       struct ctdb_connection,
+					       tcparray->num + 1);
 	CTDB_NO_MEMORY(ctdb, tcparray->connections);
 
 	tcparray->connections[tcparray->num].src = p->src;
 	tcparray->connections[tcparray->num].dst = p->dst;
 	tcparray->num++;
 
-	DEBUG(DEBUG_INFO,("Added tickle info for %s:%u from vnn %u\n",
+	D_INFO("Added connection %s\n", conn_str);
-		ctdb_addr_to_str(&tcp.dst),
-		ntohs(tcp.dst.ip.sin_port),
-		vnn->pnn));
 
 	if (tcp_update_needed) {
 		vnn->tcp_update_needed = true;
@@ -1581,58 +1590,59 @@ int32_t ctdb_control_tcp_add(struct ctdb_context *ctdb, TDB_DATA indata, bool tc
 }
 
 
-static void ctdb_remove_connection(struct ctdb_vnn *vnn, struct ctdb_connection *conn)
+static void ctdb_remove_connection(struct ctdb_vnn *vnn,
+				   struct ctdb_connection *conn)
 {
 	struct ctdb_connection *tcpp;
+	char conn_str[132] = { 0, };
+	int ret;
 
 	if (vnn == NULL) {
 		return;
 	}
 
-	/* if the array is empty we can't remove it
+	ret = ctdb_connection_to_buf(conn_str,
-	   and we don't need to do anything
+				     sizeof(conn_str),
-	 */
+				     conn,
+				     false,
+				     " -> ");
+	if (ret != 0) {
+		strlcpy(conn_str, "UNKNOWN", sizeof(conn_str));
+	}
+
+	/* If the array is empty there is nothing to remove */
 	if (vnn->tcp_array == NULL) {
-		DEBUG(DEBUG_INFO,("Trying to remove tickle that doesn't exist (array is empty) %s:%u\n",
+		D_INFO("Attempt to remove untracked connection %s (empty)\n",
-			ctdb_addr_to_str(&conn->dst),
+		       conn_str);
-			ntohs(conn->dst.ip.sin_port)));
 		return;
 	}
 
 
-	/* See if we know this connection
-	   if we don't know this connection  then we don't need to do anything
-	 */
 	tcpp = ctdb_tcp_find(vnn->tcp_array, conn);
 	if (tcpp == NULL) {
-		DEBUG(DEBUG_INFO,("Trying to remove tickle that doesn't exist %s:%u\n",
+		D_DEBUG("Attempt to remove untracked connection %s\n", conn_str);
-			ctdb_addr_to_str(&conn->dst),
-			ntohs(conn->dst.ip.sin_port)));
 		return;
 	}
 
 
-	/* We need to remove this entry from the array.
+	/*
-           Instead of allocating a new array and copying data to it
+	 * We need to remove this entry from the array.  Instead of
-	   we cheat and just copy the last entry in the existing array
+	 * allocating a new array and copying data to it, cheat and
-	   to the entry that is to be removed and just shring the 
+	 * just copy the last entry in the existing array to the entry
-	   ->num field
+	 * that is to be removed and just shrink the size.
 	 */
 	*tcpp = vnn->tcp_array->connections[vnn->tcp_array->num - 1];
 	vnn->tcp_array->num--;
 
-	/* If we deleted the last entry we also need to remove the entire array
+	/* Last entry deleted, so remove the entire array */
-	 */
 	if (vnn->tcp_array->num == 0) {
 		talloc_free(vnn->tcp_array);
 		vnn->tcp_array = NULL;
-	}		
+	}
 
 	vnn->tcp_update_needed = true;
 
-	DEBUG(DEBUG_INFO,("Removed tickle info for %s:%u\n",
+	D_INFO("Removed connection %s\n", conn_str);
-		ctdb_addr_to_str(&conn->src),
-		ntohs(conn->src.ip.sin_port)));
 }
 
 
@@ -1652,9 +1662,21 @@ int32_t ctdb_control_tcp_remove(struct ctdb_context *ctdb, TDB_DATA indata)
 
 	vnn = find_public_ip_vnn(ctdb, &conn->dst);
 	if (vnn == NULL) {
-		DEBUG(DEBUG_ERR,
+		char conn_str[132] = { 0, };
-		      (__location__ " unable to find public address %s\n",
+		int ret;
-		       ctdb_addr_to_str(&conn->dst)));
+
+		ret = ctdb_connection_to_buf(conn_str,
+					     sizeof(conn_str),
+					     conn,
+					     false,
+					     " -> ");
+		if (ret != 0) {
+			strlcpy(conn_str, "UNKNOWN", sizeof(conn_str));
+		}
+
+		DBG_ERR("Attempt to remove connection %s "
+			"but destination is not a public address\n",
+			conn_str);
 		return 0;
 	}
 

ctdb/tests/UNIT/eventscripts/scripts/local.sh

@@ -318,6 +318,14 @@ ctdb_set_pnn()
 	CTDB_SCRIPT_VARDIR="${CTDB_TEST_TMP_DIR}/scripts/${FAKE_CTDB_PNN}"
 	export CTDB_SCRIPT_VARDIR
 	mkdir -p "$CTDB_SCRIPT_VARDIR"
+
+	if [ -f "${CTDB_BASE}/public_addresses" ]; then
+		ctdb ip | while read -r _ip _pnn; do
+			if [ "$_pnn" = "$FAKE_CTDB_PNN" ]; then
+				echo "$_ip"
+			fi
+		done >"${CTDB_SCRIPT_VARDIR}/my-public-ip-addresses"
+	fi
 }
 
 ctdb_get_interfaces()

ctdb/tests/UNIT/eventscripts/stubs/ctdb

@@ -195,8 +195,7 @@ ip_reallocate()
 
 ctdb_ip()
 {
-	# If nobody has done any IP-fu then generate a layout.
+	ip_reallocate
-	[ -f "$FAKE_CTDB_IP_LAYOUT" ] || ip_reallocate
 
 	_mypnn=$(ctdb_pnn)