From: Aurelien DARRAGON Date: Fri, 6 Jan 2023 11:16:28 +0000 (+0100) Subject: OPTIM: http_ext/7239: introduce c_mode to save some space X-Git-Tag: v2.8-dev3~64 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=9ded834adc79599cf2b9d324001798629ef9cffc;p=thirdparty%2Fhaproxy.git 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. --- diff --git a/include/haproxy/http_ext-t.h b/include/haproxy/http_ext-t.h index 22f5633ec3..ab83b35441 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 62aa55dd47..4a9a0b091a 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;