BUG/MINOR: backend: fix balance hash calculation when using hash-type none

The "hash-type xxx none" is broken for keys that are not in type string
because the sample fetch call casts them to SMP_T_BIN, that tends to
preserve the original format (integers, IP addresses etc), but the
gen_hash() function in case of BE_LB_HFCN_NONE expects to read a string
representing a number, that it parses to retrieve the value, and just
fails on many binary types. For example, the following just always
returns key 0:

    balance hash rand()
    hash type consistent none

An ugly workaround is to make sure the expression returns a string, for
example this:

    balance hash rand(),concat()
    hash type consistent none

In order to fix most cases here, we force the conversion to type string
when using BE_LB_HFCN_NONE, but a better approach would require a larger
rework and split gen_hash() or change it to accept an integer as well,
so that the caller could cast to SMP_T_INT for BE_LB_HFCN_NONE and pass
the resulting number already parsed with the least information loss. In
this case even IPv4 addresses would be preserved.

The current approach at least addresses the initially envisioned use
cases, and the limitations have been added to the doc. This can be
backported to 3.0 though it's not really important.

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 65dafcb5d58d0ac17b09aab08eef26b41091b167..c430a47961295d99d17edac7cf7e4c940bc266dd 100644 (file)
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -8260,7 +8260,10 @@ hash-type <method> <function> <modifier>
 
        none   don't hash the key, the key will be used as a hash, this can be
               useful to manually hash the key using a converter for that purpose
-              and let haproxy use the result directly.
+              and let haproxy use the result directly. The operation will
+              convert the key to a string if it is not already, and parse it as
+              an integer whose value will be used as the key. Some input key
+              types might not be relevant here (e.g. IP addresses).
 
     <modifier> indicates an optional method applied after hashing the key :
 

diff --git a/src/backend.c b/src/backend.c
index f460b261c2c6f4beedd203ec78f9e37bd6ef994d..97085be2586560dee3444b9c9f7d14e3c923ad74 100644 (file)
--- a/src/backend.c
+++ b/src/backend.c
@@ -87,7 +87,7 @@ unsigned int gen_hash(const struct proxy* px, const char* key, unsigned long len
                hash = hash_crc32(key, len);
                break;
        case BE_LB_HFCN_NONE:
-               /* use key as a hash */
+               /* use key as a hash. It MUST be in string format */
                {
                        const char *_key = key;
 
@@ -545,7 +545,14 @@ struct server *get_server_expr(struct stream *s, const struct server *avoid)
        if (px->lbprm.tot_used == 1)
                goto hash_done;
 
-       smp = sample_fetch_as_type(px, s->sess, s, SMP_OPT_DIR_REQ | SMP_OPT_FINAL, px->lbprm.expr, SMP_T_BIN);
+       /* Note that if the hash-type doesn't hash the key, we must provide it
+        * as a string representing a number as it will be parsed by read_int64().
+        * Otherwise it's binary. The difference happens on samples returing
+        * ints (e.g. rand()) as well as IP addresses, which, when turned to
+        * binary, are just binary-encoded and cannot be parsed.
+        */
+       smp = sample_fetch_as_type(px, s->sess, s, SMP_OPT_DIR_REQ | SMP_OPT_FINAL, px->lbprm.expr,
+                                  ((px->lbprm.algo & BE_LB_HASH_FUNC) == BE_LB_HFCN_NONE) ? SMP_T_STR : SMP_T_BIN);
        if (!smp)
                return NULL;