config, lib-settings: Avoid reading settings blocks' filters into memory

They are accessed directly from the mmaped memory now. This reduces
per-process memory usage.

diff --git a/src/config/config-dump-full.c b/src/config/config-dump-full.c
index 83f64a0a87f99769296863dd88f1de06a6a392fd..4efd8e0ce169ef312807d968ede2d0e7527d4af1 100644 (file)
--- a/src/config/config-dump-full.c
+++ b/src/config/config-dump-full.c
@@ -45,12 +45,16 @@
      <32bit big-endian: filter count>
      Repeat for "filter count":
        <64bit big-endian: filter settings size>
-       <32bit big-endian: event filter string index number>
        <NUL-terminated string: error string>
        Repeat until "filter settings size" is reached:
          <32bit big-endian: key index number>
         [<strlist key>]
         <NUL-terminated string: value>
+     Repeat for "filter count":
+       <32bit big-endian: event filter string index number>
+     Repeat for "filter count":
+       <64bit big-endian: filter settings offset>
+     <trailing safety NUL>
 */
 
 struct dump_context {
@@ -218,9 +222,6 @@ static void config_dump_full_callback(const struct config_export_setting *set,
        if (!ctx->filter_written) {
                uint64_t blob_size = UINT64_MAX;
                o_stream_nsend(ctx->output, &blob_size, sizeof(blob_size));
-               uint32_t filter_idx_be32 = cpu32_to_be(ctx->filter_idx);
-               o_stream_nsend(ctx->output, &filter_idx_be32,
-                              sizeof(filter_idx_be32));
                o_stream_nsend(ctx->output, "", 1); /* no error */
                ctx->filter_written = TRUE;
        }
@@ -308,7 +309,7 @@ config_dump_full_sections(struct config_parsed *config,
 {
        struct config_filter_parser *const *filters;
        struct config_export_context *export_ctx;
-       uint32_t filter_count = 0;
+       uint32_t max_filter_count = 0, filter_count = 0;
        int ret = 0;
 
        filters = config_parsed_get_filter_parsers(config);
@@ -325,6 +326,11 @@ config_dump_full_sections(struct config_parsed *config,
        if (dest != CONFIG_DUMP_FULL_DEST_STDOUT)
                o_stream_nsend(output, &filter_count, sizeof(filter_count));
 
+       while (filters[max_filter_count] != NULL) max_filter_count++;
+
+       uint32_t filter_indexes_be32[max_filter_count];
+       uint64_t filter_offsets_be64[max_filter_count];
+
        for (unsigned int i = 1; filters[i] != NULL && ret == 0; i++) T_BEGIN {
                const struct config_filter_parser *filter = filters[i];
                uoff_t start_offset = output->offset;
@@ -361,22 +367,34 @@ config_dump_full_sections(struct config_parsed *config,
                                ret = -1;
                }
                config_export_free(&export_ctx);
-               if (dump_ctx.filter_written)
+               if (dump_ctx.filter_written) {
+                       filter_indexes_be32[filter_count] = cpu32_to_be(i);
+                       filter_offsets_be64[filter_count] =
+                               cpu64_to_be(start_offset);
                        filter_count++;
+               }
        } T_END;
 
        if (str_len(delayed_filter) > 0) {
-               uint32_t filter_idx = 0; /* empty/global filter */
-               uint64_t blob_size = cpu64_to_be(sizeof(filter_idx) + 1 +
-                                                str_len(delayed_filter));
+               filter_indexes_be32[filter_count] = 0; /* empty/global filter */
+               filter_offsets_be64[filter_count] = cpu64_to_be(output->offset);
+
+               uint64_t blob_size = cpu64_to_be(1 + str_len(delayed_filter));
                o_stream_nsend(output, &blob_size, sizeof(blob_size));
-               o_stream_nsend(output, &filter_idx, sizeof(filter_idx));
                o_stream_nsend(output, "", 1); /* no error */
                o_stream_nsend(output, str_data(delayed_filter),
                               str_len(delayed_filter));
                filter_count++;
        }
 
+       if (dest != CONFIG_DUMP_FULL_DEST_STDOUT) {
+               o_stream_nsend(output, filter_indexes_be32,
+                              sizeof(filter_indexes_be32[0]) * filter_count);
+               o_stream_nsend(output, filter_offsets_be64,
+                              sizeof(filter_offsets_be64[0]) * filter_count);
+               o_stream_nsend(output, "", 1);
+       }
+
        filter_count = cpu32_to_be(filter_count);
        if (dest != CONFIG_DUMP_FULL_DEST_STDOUT &&
            o_stream_pwrite(output, &filter_count, sizeof(filter_count),

diff --git a/src/lib-master/master-service-settings.c b/src/lib-master/master-service-settings.c
index 4a570048774aa689c4325ea7c0bb172c76c4acbb..bdac66a974ee2d29b519b749fb8026b8de2f6b2a 100644 (file)
--- a/src/lib-master/master-service-settings.c
+++ b/src/lib-master/master-service-settings.c
@@ -455,7 +455,9 @@ int master_service_settings_read(struct master_service *service,
                      input->protocol : service->name);
 
        settings_free(service->set);
-       ret = settings_get(event, &master_service_setting_parser_info, 0,
+       ret = settings_get(event, &master_service_setting_parser_info,
+                          !input->no_key_validation ? 0 :
+                          SETTINGS_GET_NO_KEY_VALIDATION,
                           &service->set, error_r);
        event_unref(&event);
        if (ret < 0)

diff --git a/src/lib-master/master-service-settings.h b/src/lib-master/master-service-settings.h
index aa72bc49bc0f84d5479e0bf41d1b9ed2496d3670..e349067e6e3eff3dfdf0678da6e33ff51c29a90a 100644 (file)
--- a/src/lib-master/master-service-settings.h
+++ b/src/lib-master/master-service-settings.h
@@ -41,6 +41,8 @@ struct master_service_settings_input {
        bool check_full_config;
        /* If executing via doveconf, hide warnings about obsolete settings. */
        bool hide_obsolete_warnings;
+       /* unit tests: Enable SETTINGS_GET_NO_KEY_VALIDATION */
+       bool no_key_validation;
        bool reload_config;
        bool never_exec;
        bool always_exec;

diff --git a/src/lib-master/test-master-service-settings.c b/src/lib-master/test-master-service-settings.c
index 6411d18192c7eafdccb0a911d65d63ce3f25e09e..06bd3f582490f73bbf24e563b5fdfad197e671aa 100644 (file)
--- a/src/lib-master/test-master-service-settings.c
+++ b/src/lib-master/test-master-service-settings.c
@@ -158,7 +158,7 @@ static const struct {
               "\x00\x00\x00\x00\x00\x00\x00"), // filter settings size
          "Area too small when reading size of 'filter settings size'" },
 
-       /* filter settings size is zero */
+       /* filter settings is truncated */
        { DATA("DOVECOT-CONFIG\t1.0\n"
               "\x00\x00\x00\x00\x00\x00\x00\x2A" // full size
               "\x00\x00\x00\x01" // event filter count
@@ -170,87 +170,79 @@ static const struct {
               "\x00\x00\x00\x00\x00\x00\x00\x01" // base settings size
               "\x00" // base settings error
               "\x00\x00\x00\x01" // filter count
-              "\x00\x00\x00\x00\x00\x00\x00\x00"), // filter settings size
-         "Area too small when reading uint of 'filter string index'" },
-       /* event filter index is truncated */
-       { DATA("DOVECOT-CONFIG\t1.0\n"
-              "\x00\x00\x00\x00\x00\x00\x00\x2D" // full size
-              "\x00\x00\x00\x01" // event filter count
-              "\x00" // event filter[0]
-              "\x00\x00\x00\x00\x00\x00\x00\x20" // block size
-              "N\x00" // block name
-              "\x00\x00\x00\x01" // settings count
-              "K\x00" // setting[0] key
-              "\x00\x00\x00\x00\x00\x00\x00\x01" // base settings size
-              "\x00" // base settings error
-              "\x00\x00\x00\x01" // filter count
-              "\x00\x00\x00\x00\x00\x00\x00\x03" // filter settings size
-              "\x00\x00\x00"), // event filter index
-         "Area too small when reading uint of 'filter string index'" },
+              "\x00\x00\x00\x00\x00\x00\x10\x00"), // filter settings size
+         "'filter settings size' points outside area" },
        /* filter error is missing */
        { DATA("DOVECOT-CONFIG\t1.0\n"
-              "\x00\x00\x00\x00\x00\x00\x00\x2E" // full size
+              "\x00\x00\x00\x00\x00\x00\x00\x37" // full size
               "\x00\x00\x00\x01" // event filter count
               "\x00" // event filter[0]
-              "\x00\x00\x00\x00\x00\x00\x00\x21" // block size
+              "\x00\x00\x00\x00\x00\x00\x00\x2A" // block size
               "N\x00" // block name
               "\x00\x00\x00\x01" // settings count
               "K\x00" // setting[0] key
               "\x00\x00\x00\x00\x00\x00\x00\x01" // base settings size
               "\x00" // base settings error
               "\x00\x00\x00\x01" // filter count
-              "\x00\x00\x00\x00\x00\x00\x00\x04" // filter settings size
-              "\x00\x00\x00\x00"), // event filter index
-         "'filter settings error' points outside area" },
+              "\x00\x00\x00\x00\x00\x00\x00\x00" // filter settings size
+              "\x00\x00\x00\x00" // event filter index
+              "\x00\x00\x00\x00\x00\x00\x00\x00" // filter settings offset
+              "\x00"), // safety NUL
+         "'filter error string' points outside area" },
        /* filter error is not NUL-terminated */
        { DATA("DOVECOT-CONFIG\t1.0\n"
-              "\x00\x00\x00\x00\x00\x00\x00\x30" // full size
+              "\x00\x00\x00\x00\x00\x00\x00\x45" // full size
               "\x00\x00\x00\x01" // event filter count
               "\x00" // event filter[0]
-              "\x00\x00\x00\x00\x00\x00\x00\x23" // block size
-              "N\x00" // block name
+              "\x00\x00\x00\x00\x00\x00\x00\x38" // block size
+              "master_service\x00" // block name
               "\x00\x00\x00\x01" // settings count
               "K\x00" // setting[0] key
               "\x00\x00\x00\x00\x00\x00\x00\x01" // base settings size
               "\x00" // base settings error
               "\x00\x00\x00\x01" // filter count
-              "\x00\x00\x00\x00\x00\x00\x00\x05" // filter settings size
+              "\x00\x00\x00\x00\x00\x00\x00\x01" // filter settings size
+              "E" // filter error string
               "\x00\x00\x00\x00" // event filter index
-              "E" // filter error
-              "\x00"), // trailing garbage so we can have NUL
-         "'filter settings error' points outside area" },
+              "\x00\x00\x00\x00\x00\x00\x00\x00" // filter settings offset
+              "\x00"), // safety NUL
+         "'filter error string' points outside area" },
        /* invalid filter string */
        { DATA("DOVECOT-CONFIG\t1.0\n"
-              "\x00\x00\x00\x00\x00\x00\x00\x30" // full size
+              "\x00\x00\x00\x00\x00\x00\x00\x39" // full size
               "\x00\x00\x00\x01" // event filter count
               "F\x00" // event filter[0]
-              "\x00\x00\x00\x00\x00\x00\x00\x22" // block size
+              "\x00\x00\x00\x00\x00\x00\x00\x2B" // block size
               "N\x00" // block name
               "\x00\x00\x00\x01" // settings count
               "K\x00" // setting[0] key
               "\x00\x00\x00\x00\x00\x00\x00\x01" // base settings size
               "\x00" // base settings error
               "\x00\x00\x00\x01" // filter count
-              "\x00\x00\x00\x00\x00\x00\x00\x05" // filter settings size
+              "\x00\x00\x00\x00\x00\x00\x00\x01" // filter settings size
+              "\x00" // filter error string
               "\x00\x00\x00\x00" // event filter index
-              "\x00"), // filter error
+              "\x00\x00\x00\x00\x00\x00\x00\x00" // filter settings offset
+              "\x00"), // safety NUL
          "Received invalid filter 'F' at index 0: event filter: syntax error" },
 
        /* Duplicate block name */
        { DATA("DOVECOT-CONFIG\t1.0\n"
-              "\x00\x00\x00\x00\x00\x00\x00\x39" // full size
+              "\x00\x00\x00\x00\x00\x00\x00\x42" // full size
               "\x00\x00\x00\x01" // event filter count
               "\x00" // event filter[0]
-              "\x00\x00\x00\x00\x00\x00\x00\x22" // block size
+              "\x00\x00\x00\x00\x00\x00\x00\x2B" // block size
               "N\x00" // block name
               "\x00\x00\x00\x01" // settings count
               "K\x00" // setting[0] key
               "\x00\x00\x00\x00\x00\x00\x00\x01" // base settings size
               "\x00" // base settings error
               "\x00\x00\x00\x01" // filter count
-              "\x00\x00\x00\x00\x00\x00\x00\x05" // filter settings size
+              "\x00\x00\x00\x00\x00\x00\x00\x01" // filter settings size
+              "\x00" // filter error string
               "\x00\x00\x00\x00" // event filter index
-              "\x00" // filter error
+              "\x00\x00\x00\x00\x00\x00\x00\x00" // filter settings offset
+              "\x00" // safety NUL
               "\x00\x00\x00\x00\x00\x00\x00\x02" // 2nd block size
               "N\x00"), // 2nd block name
          "Duplicate block name 'N'" },
@@ -274,6 +266,7 @@ static void test_master_service_settings_read_binary_corruption(void)
        for (unsigned int i = 0; i < N_ELEMENTS(tests); i++) {
                struct master_service_settings_input input = {
                        .config_fd = test_input_to_fd(tests[i].data, tests[i].size),
+                       .no_key_validation = TRUE,
                };
                struct master_service_settings_output output;
 

diff --git a/src/lib-settings/settings.c b/src/lib-settings/settings.c
index a49690e3a438713c7ecf84352fd7323f7c0e0ab3..0363e28027721a3d7653ee53111978466f05bff6 100644 (file)
--- a/src/lib-settings/settings.c
+++ b/src/lib-settings/settings.c
@@ -23,21 +23,16 @@ struct settings_override {
 };
 ARRAY_DEFINE_TYPE(settings_override, struct settings_override);
 
-struct settings_mmap_filter {
-       /* NULL = empty filter, which matches everything */
-       struct event_filter *filter;
-
-       const char *error; /* if non-NULL, accessing the block must fail */
-       size_t start_offset, end_offset;
-};
-
 struct settings_mmap_block {
        const char *name;
        size_t block_end_offset;
 
        const char *error; /* if non-NULL, accessing the block must fail */
        size_t base_start_offset, base_end_offset;
-       ARRAY(struct settings_mmap_filter) filters;
+
+       uint32_t filter_count;
+       size_t filter_indexes_start_offset;
+       size_t filter_offsets_start_offset;
 
        uint32_t settings_count;
        size_t settings_keys_offset;
@@ -284,63 +279,45 @@ settings_block_read(struct settings_mmap *mmap, uoff_t *_offset,
        offset = block->base_end_offset;
 
        /* <filter count> */
-       uint32_t filter_count;
        if (settings_block_read_uint32(mmap, &offset, block_end_offset,
-                                      "filter count", &filter_count,
+                                      "filter count", &block->filter_count,
                                       error_r) < 0)
                return -1;
-       p_array_init(&block->filters, mmap->pool, filter_count);
 
        /* filters */
-       unsigned int filter_idx = 0;
-       while (offset < block_end_offset) {
+       unsigned int filter_idx;
+       for (filter_idx = 0; filter_idx < block->filter_count; filter_idx++) {
                /* <filter settings size> */
                uint64_t filter_settings_size;
                if (settings_block_read_size(mmap, &offset,
                                block_end_offset, "filter settings size",
                                &filter_settings_size, error_r) < 0)
                        return -1;
-               uint64_t filter_end_offset = offset + filter_settings_size;
 
-               /* <filter string index number> */
-               uint32_t event_filter_idx;
-               if (settings_block_read_uint32(mmap, &offset, filter_end_offset,
-                                              "filter string index",
-                                              &event_filter_idx, error_r) < 0)
-                       return -1;
-               if (event_filter_idx >= mmap->event_filters_count) {
-                       *error_r = t_strdup_printf(
-                               "Event filter index %u higher than count %u",
-                               event_filter_idx, mmap->event_filters_count);
-                       return -1;
-               }
-
-               /* <filter settings error string> */
-               const char *filter_error;
-               if (settings_block_read_str(mmap, &offset,
+               uoff_t tmp_offset = offset;
+               uoff_t filter_end_offset = offset + filter_settings_size;
+               if (settings_block_read_str(mmap, &tmp_offset,
                                            filter_end_offset,
-                                           "filter settings error",
-                                           &filter_error, error_r) < 0)
+                                           "filter error string", &error,
+                                           error_r) < 0)
                        return -1;
 
-               struct settings_mmap_filter *config_filter =
-                       array_append_space(&block->filters);
-               config_filter->filter = mmap->event_filters[event_filter_idx];
-               config_filter->error = filter_error[0] == '\0' ?
-                       NULL : filter_error;
-               config_filter->start_offset = offset;
-               config_filter->end_offset = filter_end_offset;
-
-               /* skip over the key-value pairs */
-               offset = filter_end_offset;
-               filter_idx++;
+               /* skip over the filter contents for now */
+               offset += filter_settings_size;
        }
-       if (filter_idx != filter_count) {
-               *error_r = t_strdup_printf("Filter count mismatch: %u != %u",
-                                          filter_idx, filter_count);
+
+       block->filter_indexes_start_offset = offset;
+       offset += sizeof(uint32_t) * block->filter_count;
+       block->filter_offsets_start_offset = offset;
+       offset += sizeof(uint64_t) * block->filter_count;
+       offset++; /* safety NUL */
+
+       if (offset != block_end_offset) {
+               *error_r = t_strdup_printf(
+                       "Filter end offset mismatch (%"PRIuUOFF_T" != %zu)",
+                       offset, block_end_offset);
                return -1;
        }
-       i_assert(offset == block_end_offset);
        *_offset = offset;
        return 0;
 }
@@ -514,6 +491,7 @@ static int
 settings_mmap_apply(struct settings_mmap *mmap, struct event *event,
                    struct setting_parser_context *parser,
                    const struct setting_parser_info *info,
+                   enum settings_get_flags flags,
                    const char *filter_name, const char **error_r)
 {
        struct settings_mmap_block *block =
@@ -529,7 +507,8 @@ settings_mmap_apply(struct settings_mmap *mmap, struct event *event,
                return -1;
        }
 
-       if (!block->settings_validated) {
+       if (!block->settings_validated &&
+           (flags & SETTINGS_GET_NO_KEY_VALIDATION) == 0) {
                if (settings_mmap_validate(mmap, block, info, error_r) < 0)
                        return -1;
                block->settings_validated = TRUE;
@@ -544,23 +523,44 @@ settings_mmap_apply(struct settings_mmap *mmap, struct event *event,
                .type = LOG_TYPE_DEBUG,
        };
 
-       if (!array_is_created(&block->filters))
-               return 0;
-
        bool seen_filter = FALSE;
-       const struct settings_mmap_filter *config_filter;
-       array_foreach(&block->filters, config_filter) {
-               if (config_filter->filter == NULL ||
-                   event_filter_match(config_filter->filter, event,
-                                      &failure_ctx)) {
-                       if (config_filter->error != NULL) {
-                               *error_r = config_filter->error;
+       for (uint32_t i = 0; i < block->filter_count; i++) {
+               uint32_t event_filter_idx = be32_to_cpu_unaligned(
+                       CONST_PTR_OFFSET(mmap->mmap_base,
+                                        block->filter_indexes_start_offset +
+                                        sizeof(uint32_t) * i));
+               if (event_filter_idx >= mmap->event_filters_count) {
+                       *error_r = t_strdup_printf("event filter idx %u >= %u",
+                               event_filter_idx, mmap->event_filters_count);
+                       return -1;
+               }
+               struct event_filter *event_filter =
+                       mmap->event_filters[event_filter_idx];
+               if (event_filter == NULL ||
+                   event_filter_match(event_filter, event, &failure_ctx)) {
+                       uint64_t filter_offset = be64_to_cpu_unaligned(
+                               CONST_PTR_OFFSET(mmap->mmap_base,
+                                                block->filter_offsets_start_offset +
+                                                sizeof(uint64_t) * i));
+                       uint64_t filter_set_size = be64_to_cpu_unaligned(
+                               CONST_PTR_OFFSET(mmap->mmap_base, filter_offset));
+                       filter_offset += sizeof(filter_set_size);
+                       uint64_t filter_end_offset =
+                               filter_offset + filter_set_size;
+
+                       const char *filter_error =
+                               CONST_PTR_OFFSET(mmap->mmap_base,
+                                                filter_offset);
+                       if (filter_error[0] != '\0') {
+                               *error_r = filter_error;
                                return -1;
                        }
+                       filter_offset += strlen(filter_error) + 1;
+
                        if (filter_name != NULL && !seen_filter) {
                                const char *value =
                                        event_filter_find_field_exact(
-                                               config_filter->filter,
+                                               event_filter,
                                                SETTINGS_EVENT_FILTER_NAME);
                                /* NOTE: The event filter is using
                                   EVENT_FIELD_EXACT, so the value has already
@@ -570,8 +570,7 @@ settings_mmap_apply(struct settings_mmap *mmap, struct event *event,
                                        seen_filter = TRUE;
                        }
                        if (settings_mmap_apply_blob(mmap, block, parser, info,
-                                       config_filter->start_offset,
-                                       config_filter->end_offset,
+                                       filter_offset, filter_end_offset,
                                        error_r) < 0)
                                return -1;
                }
@@ -959,7 +958,7 @@ settings_instance_get(struct event *event,
 
        if (instance->mmap != NULL) {
                ret = settings_mmap_apply(instance->mmap, event, parser, info,
-                                         filter_name, &error);
+                                         flags, filter_name, &error);
                if (ret < 0) {
                        *error_r = t_strdup_printf(
                                "Failed to parse configuration: %s", error);

diff --git a/src/lib-settings/settings.h b/src/lib-settings/settings.h
index 6baf49a9dae57fff7f1eccda98b6f46eb4df4c53..0d9caece73272013657dff344cca0d80fd868358 100644 (file)
--- a/src/lib-settings/settings.h
+++ b/src/lib-settings/settings.h
@@ -28,6 +28,10 @@ enum settings_get_flags {
        /* Mark %settings as expanded without actually doing it. This is needed
           while doing checks for settings before expansion is possible. */
        SETTINGS_GET_FLAG_FAKE_EXPAND = BIT(2),
+
+       /* For unit tests: Don't validate that settings struct keys match
+          th binary config file. */
+       SETTINGS_GET_NO_KEY_VALIDATION = BIT(3),
 };
 
 /* Set struct settings_instance to events so settings_get() can