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 22f5633ec3ed5f9eea638f176ae9d91dc9a8fafe..ab83b3544167d9f9139746bdbcf4759cd5d459c8 100644 (file)
--- 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 62aa55dd479535d5f43c3d9bdba921fee61fd52e..4a9a0b091a6ec2258a51a82027749e0faa4a5c6c 100644 (file)
--- 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;