]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
Revert "BUG/MEDIUM: random: implement per-thread and per-process random sequences"
authorWilly Tarreau <w@1wt.eu>
Sat, 7 Mar 2020 10:24:39 +0000 (11:24 +0100)
committerWilly Tarreau <w@1wt.eu>
Sat, 7 Mar 2020 10:24:39 +0000 (11:24 +0100)
This reverts commit 1c306aa84d785b9c2240bf7767dcc1f2596cfcfd.

It breaks the build on all non-glibc platforms. I got confused by the
man page (which possibly is the most confusing man page I've ever read
about a standard libc function) and mistakenly understood that random_r
was portable, especially since it appears in latest freebsd source as
well but not in released versions, and with a slightly different API :-/

We need to find a different solution with a fallback. Among the
possibilities, we may reintroduce this one with a fallback relying on
locking around the standard functions, keeping fingers crossed for no
other library function to call them in parallel, or we may also provide
our own PRNG, which is not necessarily more difficult than working
around the totally broken up design of the portable API.

include/common/standard.h
include/types/global.h
src/51d.c
src/backend.c
src/flt_spoe.c
src/flt_trace.c
src/haproxy.c
src/memory.c
src/pattern.c
src/peers.c
src/sample.c

index 56995f4e2c6b3cf81a944aa781e15ae813d40806..a78083318d16110b2cd1a96e1106a37e7104c84f 100644 (file)
@@ -43,7 +43,6 @@
 #include <common/namespace.h>
 #include <eb32tree.h>
 #include <eb32sctree.h>
-#include <types/global.h>
 #include <types/protocol.h>
 
 /* size used for max length of decimal representation of long long int. */
@@ -1531,18 +1530,6 @@ static inline void *my_realloc2(void *ptr, size_t size)
 
 int parse_dotted_uints(const char *s, unsigned int **nums, size_t *sz);
 
-/* returns a positive random from a process-specific and thread-specific
- * sequence initialized by ha_random_init_per_thread(). It's just a wrapper on
- * top of random_r() so it lives with the same limitations (i.e. 31 bits only).
- */
-static inline int32_t ha_random()
-{
-       int32_t r;
-
-       random_r(&ha_rand_data, &r); // no error since our buffer is OK.
-       return r;
-}
-
 /* HAP_STRING() makes a string from a literal while HAP_XSTRING() first
  * evaluates the argument and is suited to pass macros.
  *
index 994927bfd8a834690214bc520e70efdec2f42a4f..82f1011558a001836ea485d6939242f1a8c86cbd 100644 (file)
@@ -240,8 +240,6 @@ extern unsigned int rlim_fd_cur_at_boot;
 extern unsigned int rlim_fd_max_at_boot;
 extern int atexit_flag;
 extern unsigned char boot_seed[20];  // per-boot random seed (160 bits initially)
-extern THREAD_LOCAL char ha_rand_state[32];          /* opaque 256 bits of random state */
-extern THREAD_LOCAL struct random_data ha_rand_data; /* opaque internal random_r() date */
 
 /* bit values to go with "warned" above */
 /* unassigned : 0x00000001 (previously: WARN_BLOCK_DEPRECATED) */
index 42d19296799beef040e7f4a3783d69308e9c2e1f..b00f01844a63861d19aa0e3cabb3af361a37358f 100644 (file)
--- a/src/51d.c
+++ b/src/51d.c
@@ -700,7 +700,7 @@ static int init_51degrees(void)
        free(_51d_property_list);
 
 #ifdef FIFTYONEDEGREES_H_PATTERN_INCLUDED
-       _51d_lru_seed = ha_random();
+       _51d_lru_seed = random();
        if (global_51degrees.cache_size) {
                _51d_lru_tree = lru64_new(global_51degrees.cache_size);
        }
index 87e3b9a1c762731a424968ebdcaa679aaa11a98a..251a7a5a47b5f1a2034a041ffd2ad6744a9ded4b 100644 (file)
@@ -541,7 +541,7 @@ static struct server *get_server_rnd(struct stream *s, const struct server *avoi
        do {
                prev = curr;
                /* ensure all 32 bits are covered as long as RAND_MAX >= 65535 */
-               hash = ((uint64_t)ha_random() * ((uint64_t)RAND_MAX + 1)) ^ ha_random();
+               hash = ((uint64_t)random() * ((uint64_t)RAND_MAX + 1)) ^ random();
                curr = chash_get_server_hash(px, hash, avoid);
                if (!curr)
                        break;
index bcdec08e7861f1dfea31ca204aef4385a981b6c7..d54fcd43705dea9c1c13905552cf009ce64cf9ad 100644 (file)
@@ -269,7 +269,7 @@ generate_pseudo_uuid()
 
        while (byte < 4) {
                while (bits < 32) {
-                       last |= (uint64_t)ha_random() << bits;
+                       last |= (uint64_t)random() << bits;
                        bits += rand_max_bits;
                }
                rnd[byte++] = last;
@@ -3109,6 +3109,10 @@ spoe_init_per_thread(struct proxy *p, struct flt_conf *fconf)
        struct spoe_config *conf = fconf->conf;
        struct spoe_agent *agent = conf->agent;
 
+       /* Use a != seed per process */
+       if (relative_pid > 1 && tid == 0)
+               srandom(now_ms * pid);
+
        agent->rt[tid].engine_id = generate_pseudo_uuid();
        if (agent->rt[tid].engine_id == NULL)
                return -1;
index b06ba150b2031b521c3368e39932169b77252c3a..5a26fabea2326ce8a4fb883b79e2ffbfb9496afc 100644 (file)
@@ -468,7 +468,7 @@ trace_http_payload(struct stream *s, struct filter *filter, struct http_msg *msg
                unsigned int data = trace_get_htx_datalen(htxbuf(&msg->chn->buf), offset, len);
 
                if (data) {
-                       ret = ha_random() % (ret+1);
+                       ret = random() % (ret+1);
                        if (!ret || ret >= data)
                                ret = len;
                }
@@ -536,7 +536,7 @@ trace_tcp_payload(struct stream *s, struct filter *filter, struct channel *chn,
                        unsigned int data = trace_get_htx_datalen(htxbuf(&chn->buf), offset, len);
 
                        if (data) {
-                               ret = ha_random() % (ret+1);
+                               ret = random() % (ret+1);
                                if (!ret || ret >= data)
                                        ret = len;
                        }
@@ -554,7 +554,7 @@ trace_tcp_payload(struct stream *s, struct filter *filter, struct channel *chn,
        else {
 
                if (ret && conf->rand_forwarding)
-                       ret = ha_random() % (ret+1);
+                       ret = random() % (ret+1);
 
                FLT_STRM_TRACE(conf, s, "%-25s: channel=%-10s - mode=%-5s (%s) - "
                               "offset=%u - len=%u - forward=%d",
index 8b13d96f48bfac3c841312e3496dc0a344ea5d41..ef9010f958b803a78808982f545159e61d04fc7c 100644 (file)
@@ -238,8 +238,6 @@ unsigned int rlim_fd_max_at_boot = 0;
 
 /* per-boot randomness */
 unsigned char boot_seed[20];        /* per-boot random seed (160 bits initially) */
-THREAD_LOCAL char ha_rand_state[32];          /* opaque 256 bits of random state */
-THREAD_LOCAL struct random_data ha_rand_data; /* opaque internal random_r() date */
 
 struct mworker_proc *proc_self = NULL;
 
@@ -1365,59 +1363,6 @@ static char **copy_argv(int argc, char **argv)
 }
 
 
-/* Initializes the per-thread, per-process random seed for use with random_r().
- *
- * We cannot pass a global state from one thread to another one because we
- * must still call initstate_r() on it to reset the per-thread pointer, and
- * this will reinitialize our state. What we do instead is that we use the
- * *same* seed for all threads so that they start with the exact same internal
- * state, and will loop over random() a different (and large) number of times
- * to make sure their internal state is totally different. This results in 4
- * billion possible *boot* sequences, and each thread may start with a much
- * greater number of sequences as well (we typically add up to 20 bits, giving
- * 4 trillon possible initial sequences).
- */
-static void ha_random_init_per_thread()
-{
-       unsigned int seed;
-       unsigned int loops;
-       uint64_t u64;
-
-       /* recreate a distinct initial state for each process/thread */
-       seed = read_u32(boot_seed);
-
-       /* start with a strictly different seed per thread/process */
-       seed += (relative_pid * MAX_THREADS)+ tid;
-
-       memset(&ha_rand_data, 0, sizeof(ha_rand_data));
-       initstate_r(seed, ha_rand_state, sizeof(ha_rand_state), &ha_rand_data);
-
-       /* make sure all pids and tids have a different count, we'll
-        * loop up to ~1 million times on each thread, with a fairly
-        * different number for each. This should only take a few ms
-        * per thread and will provide ~20 extra bits of randomness
-        * to each thread/process, resulting in ~52 bits per thread per
-        * boot.
-        */
-       loops = read_u32(boot_seed);
-
-       u64 = read_u64(boot_seed + 4);
-       u64 = (u64 << relative_pid) | (u64 >> (63-relative_pid));
-       loops ^= u64 ^ (u64 >> 32);
-
-       u64 = read_u64(boot_seed + 12);
-       u64 = (u64 << tid) | (u64 >> (63-tid));
-       loops ^= u64 ^ (u64 >> 32);
-       loops %= 1048573;
-
-       /* burn some randoms to mix the internal state */
-       while (loops--) {
-               int32_t drop;
-
-               (void)random_r(&ha_rand_data, &drop);
-       }
-}
-
 /* Performs basic random seed initialization. The main issue with this is that
  * srandom_r() only takes 32 bits and purposely provides a reproducible sequence,
  * which means that there will only be 4 billion possible random sequences once
@@ -1429,10 +1374,6 @@ static void ha_random_init_per_thread()
  * We initialize the current process with the first 32 bits before starting the
  * polling loop, where all this will be changed to have process specific and
  * thread specific sequences.
- *
- * Before starting threads, it's still possible to call random() as srandom()
- * is initialized from this, but after threads and/or processes are started,
- * only ha_random() is expected to be used to guarantee distinct sequences.
  */
 static void ha_random_boot(char *const *argv)
 {
@@ -1503,7 +1444,6 @@ static void ha_random_boot(char *const *argv)
        blk_SHA1_Final(boot_seed, &ctx);
 
        srandom(read_u32(boot_seed));
-       ha_random_init_per_thread();
 }
 
 /* considers splicing proxies' maxconn, computes the ideal global.maxpipes
@@ -2840,9 +2780,6 @@ static void *run_thread_poll_loop(void *data)
        ti->clock_id = CLOCK_THREAD_CPUTIME_ID;
 #endif
 #endif
-       /* assign per-process, per-thread randomness */
-       ha_random_init_per_thread();
-
        /* Now, initialize one thread init at a time. This is better since
         * some init code is a bit tricky and may release global resources
         * after reallocating them locally. This will also ensure there is
index 0ff3ea8b8fae05819a6ff194f0912b8e90400947..d1aec5925c327aa7c93e51c5e152b8fa41e5f11b 100644 (file)
@@ -628,7 +628,7 @@ int mem_should_fail(const struct pool_head *pool)
        int n;
 
        if (mem_fail_rate > 0 && !(global.mode & MODE_STARTING)) {
-               int randnb = ha_random() % 100;
+               int randnb = random() % 100;
 
                if (mem_fail_rate > randnb)
                        ret = 1;
index 3ea1f33d44c03b40894450b53755488281d17bae..8dfe3cf29b4af9bbe3921043d60797e01870e8c3 100644 (file)
@@ -2667,7 +2667,7 @@ int pattern_finalize_config(void)
        struct pat_ref *ref, **arr;
        struct list pr = LIST_HEAD_INIT(pr);
 
-       pat_lru_seed = ha_random();
+       pat_lru_seed = random();
 
        /* Count pat_refs with user defined unique_id and totalt count */
        list_for_each_entry(ref, &pattern_reference, list) {
index 640a99f5e9d195ca5925c6c3cbe1fc3a644acc12..f5a4f18655fdf0b0a63333d6f6170bcfc95b7b16 100644 (file)
@@ -2232,7 +2232,7 @@ switchstate:
                                         * retrying otherwise the other end will do the same and we can loop
                                         * for a while.
                                         */
-                                       curpeer->reconnect = tick_add(now_ms, MS_TO_TICKS(50 + ha_random() % 2000));
+                                       curpeer->reconnect = tick_add(now_ms, MS_TO_TICKS(50 + random() % 2000));
                                        peer_session_forceshutdown(curpeer);
                                }
                                if (maj_ver != (unsigned int)-1 && min_ver != (unsigned int)-1) {
@@ -2685,7 +2685,7 @@ static struct task *process_peer_sync(struct task * task, void *context, unsigne
                                                                        ps->reconnect = tick_add(now_ms, MS_TO_TICKS(PEER_RECONNECT_TIMEOUT));
                                                                }
                                                                else  {
-                                                                       ps->reconnect = tick_add(now_ms, MS_TO_TICKS(50 + ha_random() % 2000));
+                                                                       ps->reconnect = tick_add(now_ms, MS_TO_TICKS(50 + random() % 2000));
                                                                        peer_session_forceshutdown(ps);
                                                                        ps->no_hbt++;
                                                                }
@@ -2741,7 +2741,7 @@ static struct task *process_peer_sync(struct task * task, void *context, unsigne
                                 * retrying otherwise the other end will do the same and we can loop
                                 * for a while.
                                 */
-                               ps->reconnect = tick_add(now_ms, MS_TO_TICKS(50 + ha_random() % 2000));
+                               ps->reconnect = tick_add(now_ms, MS_TO_TICKS(50 + random() % 2000));
                                if (ps->appctx) {
                                        peer_session_forceshutdown(ps);
                                }
index fd63902a9f5a6bc966cd673bbc56f5bc104273b0..3c611122404d83ca9f2086f0d939d996108d6bed 100644 (file)
@@ -3124,7 +3124,7 @@ smp_fetch_thread(const struct arg *args, struct sample *smp, const char *kw, voi
 static int
 smp_fetch_rand(const struct arg *args, struct sample *smp, const char *kw, void *private)
 {
-       smp->data.u.sint = ha_random();
+       smp->data.u.sint = random();
 
        /* reduce if needed. Don't do a modulo, use all bits! */
        if (args && args[0].type == ARGT_SINT)
@@ -3336,7 +3336,7 @@ static int smp_fetch_uuid(const struct arg *args, struct sample *smp, const char
 
                while (byte < 4) {
                        while (bits < 32) {
-                               last |= (uint64_t)ha_random() << bits;
+                               last |= (uint64_t)random() << bits;
                                bits += rand_max_bits;
                        }
                        rnd[byte++] = last;