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 4491bde9f4702373128d40ca2fcb9d296056d34c..b8c07ac1de184fd2083a617fa92e430e0e5c0f49 100644 (file)
--- 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 ae82394653c84037932c7a64e44dd170ca087354..177505a6fb686cffe84e95064667530b5f5aaa05 100644 (file)
--- 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;
 }