From: Willy Tarreau <w@1wt.eu>
Date: Tue, 9 Apr 2024 06:41:06 +0000 (+0200)
Subject: BUG/MINOR: listener: always assign distinct IDs to shards
X-Git-Tag: v3.0-dev8~114
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0db8b6034d87abe33792b8ff69693c9ca3900d2f;p=thirdparty%2Fhaproxy.git

BUG/MINOR: listener: always assign distinct IDs to shards

When sharded listeners were introdcued in 2.5 with commit 6dfbef4145
("MEDIUM: listener: add the "shards" bind keyword"), a point was
overlooked regarding how IDs are assigned to listeners: they are just
duplicated! This means that if a "option socket-stats" is set and a
shard is configured, or multiple thread groups are enabled, then a stats
dump will produce several lines with exactly the same socket name and ID.

This patch tries to address this by trying to assign consecutive numbers
to these sockets. The usual algo is maintained, but with a preference for
the next number in a shard. This will help users reserve ranges for each
socket, for example by using multiples of 100 or 1000 on each bind line,
leaving enough room for all shards to be assigned.

The mechanism however is quite tricky, because the configured listener
currently ends up being the last one of the shard. This helps insert them
before the current position without having to revisit them. But here it
causes a difficulty which is that we'd like to restart from the current
ID and assign new ones on top of it. What is done is that the number is
passed between shards and the current one is cleared (and removed from
the tree) so that we instead insert the new one. It's tricky because of
the situation which depends whether it's the listener that was already
assigned on the bind line or not. But overall, always removing the entry,
always adding the new one when the ID is not zero, and passing them from
the reference to the next one does the trick.

This may be backported to all versions till 2.6.
---

diff --git a/src/cfgparse.c b/src/cfgparse.c
index cdddfc2ed1..b159a5a039 100644
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -4178,6 +4178,11 @@ init_proxies_list_stage2:
 				/* listener ID not set, use automatic numbering with first
 				 * spare entry starting with next_luid.
 				 */
+				if (listener->by_fe.p != &curproxy->conf.listeners) {
+					struct listener *prev_li = LIST_PREV(&listener->by_fe, typeof(prev_li), by_fe);
+					if (prev_li->luid)
+						next_id = prev_li->luid + 1;
+				}
 				next_id = get_next_id(&curproxy->conf.used_listener_id, next_id);
 				listener->conf.id.key = listener->luid = next_id;
 				eb32_insert(&curproxy->conf.used_listener_id, &listener->conf.id);
diff --git a/src/listener.c b/src/listener.c
index 07113ff6ec..8b20b55b04 100644
--- a/src/listener.c
+++ b/src/listener.c
@@ -916,6 +916,7 @@ struct listener *clone_listener(struct listener *src)
 		goto oom1;
 	memcpy(l, src, sizeof(*l));
 
+	l->luid = 0; // don't dup the listener's ID!
 	if (l->name) {
 		l->name = strdup(l->name);
 		if (!l->name)
@@ -1810,6 +1811,12 @@ int bind_complete_thread_setup(struct bind_conf *bind_conf, int *err_code)
 						*err_code |= ERR_FATAL | ERR_ALERT;
 						return cfgerr;
 					}
+					/* assign the ID to the first one only */
+					new_li->luid = new_li->conf.id.key = tmp_li->luid;
+					tmp_li->luid = 0;
+					eb32_delete(&tmp_li->conf.id);
+					if (tmp_li->luid)
+						eb32_insert(&fe->conf.used_listener_id, &new_li->conf.id);
 					new_li = tmp_li;
 				}
 			}
@@ -1828,6 +1835,12 @@ int bind_complete_thread_setup(struct bind_conf *bind_conf, int *err_code)
 				*err_code |= ERR_FATAL | ERR_ALERT;
 				return cfgerr;
 			}
+			/* assign the ID to the first one only */
+			new_li->luid = new_li->conf.id.key = li->luid;
+			li->luid = 0;
+			eb32_delete(&li->conf.id);
+			if (li->luid)
+				eb32_insert(&fe->conf.used_listener_id, &new_li->conf.id);
 		}
 	}