From: Aurelien DARRAGON <adarragon@haproxy.com>
Date: Wed, 13 Sep 2023 17:28:34 +0000 (+0200)
Subject: MEDIUM: sink/log: stop relying on AF_UNSPEC for rings
X-Git-Tag: v2.9-dev8~68
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=cc3dfe89edc8ecef008dec802501195b7b9be98f;p=thirdparty%2Fhaproxy.git

MEDIUM: sink/log: stop relying on AF_UNSPEC for rings

Since a5b325f92 ("MINOR: protocol: add a real family for existing FDs"),
we don't rely anymore on AF_UNSPEC for buffer rings in do_send_log.

But we kept it as a parsing hint to differentiate between implicit and
named rings during ring buffer postparsing.

However it is still a bit confusing and forces us to systematically rely
on target->addr, even for named buffer rings where it doesn't make much
sense anymore.

Now that target->addr was made a pointer in a recent commit, we can
choose not to initialize it when not needed (i.e.: named rings) and use
this as a hint to distinguish implicit rings during init since they rely
on the addr struct to temporarily store the ring's address until the ring
is actually created during postparsing step.
---

diff --git a/src/log.c b/src/log.c
index 4491bde9f4..b8c07ac1de 100644
--- a/src/log.c
+++ b/src/log.c
@@ -755,10 +755,12 @@ static int dup_log_target(struct log_target *def, struct log_target *cpy)
 {
 	BUG_ON((def->flags & LOG_TARGET_FL_RESOLVED)); /* postparsing already done, invalid use */
 	init_log_target(cpy);
-	cpy->addr = malloc(sizeof(*cpy->addr));
-	if (!cpy->addr)
-		goto error;
-	*cpy->addr = *def->addr;
+	if (def->addr) {
+		cpy->addr = malloc(sizeof(*cpy->addr));
+		if (!cpy->addr)
+			goto error;
+		*cpy->addr = *def->addr;
+	}
 	if (def->ring_name) {
 		cpy->ring_name = strdup(def->ring_name);
 		if (!cpy->ring_name)
@@ -852,21 +854,22 @@ static int parse_log_target(char *raw, struct log_target *target, char **err)
 	struct sockaddr_storage *sk;
 
 	init_log_target(target);
+	// target addr is NULL at this point
+
+	if (strncmp(raw, "ring@", 5) == 0) {
+		target->type = LOG_TARGET_BUFFER;
+		target->ring_name = strdup(raw + 5);
+		goto done;
+	}
 
-	/* first, try to allocate log target addr */
+	/* try to allocate log target addr */
 	target->addr = malloc(sizeof(*target->addr));
 	if (!target->addr) {
 		memprintf(err, "memory error");
 		goto error;
 	}
 
-	target->type = LOG_TARGET_DGRAM;
-	if (strncmp(raw, "ring@", 5) == 0) {
-		target->addr->ss_family = AF_UNSPEC;
-		target->type = LOG_TARGET_BUFFER;
-		target->ring_name = strdup(raw + 5);
-		goto done;
-	}
+	target->type = LOG_TARGET_DGRAM; // default type
 
 	/* parse the target address */
 	sk = str2sa_range(raw, NULL, &port1, &port2, &fd, &proto,
@@ -886,8 +889,8 @@ static int parse_log_target(char *raw, struct log_target *target, char **err)
 	if (proto && proto->xprt_type == PROTO_TYPE_STREAM) {
 		static unsigned long ring_ids;
 
-		/* Implicit sink buffer will be
-		 * initialized in post_check
+		/* Implicit sink buffer will be initialized in post_check
+		 * (target->addr is set in this case)
 		 */
 		target->type = LOG_TARGET_BUFFER;
 		/* compute unique name for the ring */
diff --git a/src/sink.c b/src/sink.c
index ae82394653..177505a6fb 100644
--- a/src/sink.c
+++ b/src/sink.c
@@ -1242,9 +1242,6 @@ struct sink *sink_new_from_logger(struct logger *logger)
 	if (sink_finalize(sink) & ERR_CODE)
 		goto error_final;
 
-	/* reset family of logger to consider the ring buffer target */
-	logger->target.addr->ss_family = AF_UNSPEC;
-
 	return sink;
  error:
 	srv_drop(srv);
@@ -1286,31 +1283,29 @@ int sink_resolve_logger_buffer(struct logger *logger, char **msg)
 	struct sink *sink;
 
 	BUG_ON(target->type != LOG_TARGET_BUFFER || (target->flags & LOG_TARGET_FL_RESOLVED));
-	sink = sink_find(target->ring_name);
-	if (!sink) {
-		/* LOG_TARGET_BUFFER but !AF_UNSPEC
-		 * means we must allocate a sink
-		 * buffer to send messages to this logger
-		 */
-		if (target->addr->ss_family != AF_UNSPEC) {
-			sink = sink_new_from_logger(logger);
-			if (!sink) {
-				memprintf(msg, "cannot be initialized (failed to create implicit ring)");
-				err_code |= ERR_ALERT | ERR_FATAL;
-				goto end;
-			}
+	if (target->addr) {
+		sink = sink_new_from_logger(logger);
+		if (!sink) {
+			memprintf(msg, "cannot be initialized (failed to create implicit ring)");
+			err_code |= ERR_ALERT | ERR_FATAL;
+			goto end;
 		}
-		else {
+		ha_free(&target->addr); /* we no longer need this */
+	}
+	else {
+		sink = sink_find(target->ring_name);
+		if (!sink) {
 			memprintf(msg, "uses unknown ring named '%s'", target->ring_name);
 			err_code |= ERR_ALERT | ERR_FATAL;
 			goto end;
 		}
+		else if (sink->type != SINK_TYPE_BUFFER) {
+			memprintf(msg, "uses incompatible ring '%s'", target->ring_name);
+			err_code |= ERR_ALERT | ERR_FATAL;
+			goto end;
+		}
 	}
-	else if (sink->type != SINK_TYPE_BUFFER) {
-		memprintf(msg, "uses incompatible ring '%s'", target->ring_name);
-		err_code |= ERR_ALERT | ERR_FATAL;
-		goto end;
-	}
+	/* consistency checks */
 	if (sink && logger->maxlen > ring_max_payload(sink->ctx.ring)) {
 		memprintf(msg, "uses a max length which exceeds ring capacity ('%s' supports %lu bytes at most)",
 		          target->ring_name, (unsigned long)ring_max_payload(sink->ctx.ring));
@@ -1322,7 +1317,6 @@ int sink_resolve_logger_buffer(struct logger *logger, char **msg)
  end:
 	ha_free(&target->ring_name); /* sink is resolved and will replace ring_name hint */
 	target->sink = sink;
-	ha_free(&target->addr); /* we no longer need this */
 	return err_code;
 }