From 9ded834adc79599cf2b9d324001798629ef9cffc Mon Sep 17 00:00:00 2001 From: Aurelien DARRAGON Date: Fri, 6 Jan 2023 12:16:28 +0100 Subject: [PATCH] OPTIM: http_ext/7239: introduce c_mode to save some space forwarded header option (rfc7239) deals with sample expressions in two steps: first a sample expression string is extracted from the config file and later in startup sequence this string is converted into the resulting sample_expr. We need to perform these two steps because we cannot compile the expr too early in the parsing sequence. (or we would miss some context) Because of this, we have two dinstinct structure members (expr and expr_s) for each 7239 field supporting sample expressions. This is not cool, because we're bloating the http forwarded config structure, and thus, bloating proxy config structure. To address this, we now merge both expr and expr_s members inside a single union to regain some space. This forces us to perform some additional logic to make sure to use the proper structure member at different parsing steps. Thanks to this, we're also able to free/release related config hints and sample expression strings as soon as the sample expression compilation is finished. --- include/haproxy/http_ext-t.h | 19 ++++++--- src/http_ext.c | 95 ++++++++++++++++++++++++++++-------------- 2 files changed, 77 insertions(+), 37 deletions(-) diff --git a/include/haproxy/http_ext-t.h b/include/haproxy/http_ext-t.h index 22f5633..ab83b35 100644 --- a/include/haproxy/http_ext-t.h +++ b/include/haproxy/http_ext-t.h @@ -73,10 +73,14 @@ enum http_ext_7239_forby_mode { }; struct http_ext_7239_forby { /* nn = nodename, np = nodeport */ - char *nn_expr_s; - struct sample_expr *nn_expr; - char *np_expr_s; - struct sample_expr *np_expr; + union { + char *nn_expr_s; + struct sample_expr *nn_expr; + }; + union { + char *np_expr_s; + struct sample_expr *np_expr; + }; enum http_ext_7239_forby_mode nn_mode; enum http_ext_7239_forby_mode np_mode; }; @@ -86,8 +90,10 @@ enum http_ext_7239_host_mode { HTTP_7239_HOST_SMP = 2 }; struct http_ext_7239_host { - char *expr_s; - struct sample_expr *expr; + union { + char *expr_s; + struct sample_expr *expr; + }; enum http_ext_7239_host_mode mode; }; @@ -100,6 +106,7 @@ struct http_ext_7239 { /* config error hints, used only during configuration parsing */ char *c_file; int c_line; + int c_mode; /* 0: parsed, 1: compiled */ }; enum forwarded_header_field { diff --git a/src/http_ext.c b/src/http_ext.c index 62aa55d..4a9a0b0 100644 --- a/src/http_ext.c +++ b/src/http_ext.c @@ -1049,7 +1049,6 @@ int proxy_http_parse_7239(char **args, int cur_arg, */ int proxy_http_compile_7239(struct proxy *curproxy) { - char *err = NULL; int cfgerr = 0; int loop; @@ -1066,72 +1065,99 @@ int proxy_http_compile_7239(struct proxy *curproxy) curproxy->conf.args.file = curproxy->http.fwd.c_file; curproxy->conf.args.line = curproxy->http.fwd.c_line; + /* it is important that we keep iterating on error to make sure + * all fwd config fields are in the same state (post-parsing state) + */ for (loop = 0; loop < 5; loop++) { - char *expr_str = NULL; + char **expr_str = NULL; struct sample_expr **expr = NULL; + struct sample_expr *cur_expr; + char *err = NULL; int smp = 0; int idx = 0; switch (loop) { case 0: /* host */ - expr_str = curproxy->http.fwd.p_host.expr_s; + expr_str = &curproxy->http.fwd.p_host.expr_s; expr = &curproxy->http.fwd.p_host.expr; smp = (curproxy->http.fwd.p_host.mode == HTTP_7239_HOST_SMP); break; case 1: /* by->node */ - expr_str = curproxy->http.fwd.p_by.nn_expr_s; + expr_str = &curproxy->http.fwd.p_by.nn_expr_s; expr = &curproxy->http.fwd.p_by.nn_expr; smp = (curproxy->http.fwd.p_by.nn_mode == HTTP_7239_FORBY_SMP); break; case 2: /* by->nodeport */ - expr_str = curproxy->http.fwd.p_by.np_expr_s; + expr_str = &curproxy->http.fwd.p_by.np_expr_s; expr = &curproxy->http.fwd.p_by.np_expr; smp = (curproxy->http.fwd.p_by.np_mode == HTTP_7239_FORBY_SMP); break; case 3: /* for->node */ - expr_str = curproxy->http.fwd.p_for.nn_expr_s; + expr_str = &curproxy->http.fwd.p_for.nn_expr_s; expr = &curproxy->http.fwd.p_for.nn_expr; smp = (curproxy->http.fwd.p_for.nn_mode == HTTP_7239_FORBY_SMP); break; case 4: /* for->nodeport */ - expr_str = curproxy->http.fwd.p_for.np_expr_s; + expr_str = &curproxy->http.fwd.p_for.np_expr_s; expr = &curproxy->http.fwd.p_for.np_expr; smp = (curproxy->http.fwd.p_for.np_mode == HTTP_7239_FORBY_SMP); break; } - if (!smp || !expr_str) + if (!smp) continue; /* no expr */ - /* expr cannot be NULL past this point */ - ALREADY_CHECKED(expr); - *expr = - sample_parse_expr((char*[]){expr_str, NULL}, &idx, + /* expr and expr_str cannot be NULL past this point */ + BUG_ON(!expr || !expr_str); + + if (!*expr_str) { + /* should not happen unless system memory exhaustion */ + ha_alert("%s '%s' [%s:%d]: failed to parse 'option forwarded' expression : %s.\n", + proxy_type_str(curproxy), curproxy->id, + curproxy->http.fwd.c_file, curproxy->http.fwd.c_line, + "memory error"); + cfgerr++; + continue; + } + + cur_expr = + sample_parse_expr((char*[]){*expr_str, NULL}, &idx, curproxy->http.fwd.c_file, curproxy->http.fwd.c_line, &err, &curproxy->conf.args, NULL); - if (!*expr) { + if (!cur_expr) { ha_alert("%s '%s' [%s:%d]: failed to parse 'option forwarded' expression '%s' in : %s.\n", proxy_type_str(curproxy), curproxy->id, curproxy->http.fwd.c_file, curproxy->http.fwd.c_line, - expr_str, err); + *expr_str, err); ha_free(&err); cfgerr++; } + /* post parsing individual expr cleanup */ + ha_free(expr_str); + + /* expr assignment */ + *expr = cur_expr; } curproxy->conf.args.file = NULL; curproxy->conf.args.line = 0; + /* post parsing general cleanup */ + ha_free(&curproxy->http.fwd.c_file); + curproxy->http.fwd.c_line = 0; + + curproxy->http.fwd.c_mode = 1; /* parsing completed */ + out: return cfgerr; } @@ -1292,23 +1318,28 @@ int proxy_http_parse_xot(char **args, int cur_arg, void http_ext_7239_clean(struct http_ext_7239 *clean) { - ha_free(&clean->c_file); - ha_free(&clean->p_host.expr_s); - ha_free(&clean->p_by.nn_expr_s); - ha_free(&clean->p_by.np_expr_s); - ha_free(&clean->p_for.nn_expr_s); - ha_free(&clean->p_for.np_expr_s); - - release_sample_expr(clean->p_host.expr); - clean->p_host.expr = NULL; - release_sample_expr(clean->p_by.nn_expr); - clean->p_by.nn_expr = NULL; - release_sample_expr(clean->p_by.np_expr); - clean->p_by.np_expr = NULL; - release_sample_expr(clean->p_for.nn_expr); - clean->p_for.nn_expr = NULL; - release_sample_expr(clean->p_for.np_expr); - clean->p_for.np_expr = NULL; + if (!clean->c_mode) { + /* parsed */ + ha_free(&clean->c_file); + ha_free(&clean->p_host.expr_s); + ha_free(&clean->p_by.nn_expr_s); + ha_free(&clean->p_by.np_expr_s); + ha_free(&clean->p_for.nn_expr_s); + ha_free(&clean->p_for.np_expr_s); + } + else { + /* compiled */ + release_sample_expr(clean->p_host.expr); + clean->p_host.expr = NULL; + release_sample_expr(clean->p_by.nn_expr); + clean->p_by.nn_expr = NULL; + release_sample_expr(clean->p_by.np_expr); + clean->p_by.np_expr = NULL; + release_sample_expr(clean->p_for.nn_expr); + clean->p_for.nn_expr = NULL; + release_sample_expr(clean->p_for.np_expr); + clean->p_for.np_expr = NULL; + } } void http_ext_xff_clean(struct http_ext_xff *clean) @@ -1323,6 +1354,8 @@ void http_ext_xot_clean(struct http_ext_xot *clean) void http_ext_7239_copy(struct http_ext_7239 *dest, const struct http_ext_7239 *orig) { + if (orig->c_mode) + return; /* copy not supported once compiled */ if (orig->c_file) dest->c_file = strdup(orig->c_file); dest->c_line = orig->c_line; -- 1.7.10.4