From: Willy Tarreau Date: Sat, 2 Feb 2019 12:14:34 +0000 (+0100) Subject: MEDIUM: listener: keep a single thread-mask and warn on "process" misuse X-Git-Tag: v2.0-dev2~165 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=a36b324777d5230f1eb0d77b01b5d54642d791ca;p=thirdparty%2Fhaproxy.git MEDIUM: listener: keep a single thread-mask and warn on "process" misuse Now that nbproc and nbthread are exclusive, we can still provide more detailed explanations about what we've found in the config when a bind line appears on multiple threads and processes at the same time, then ignore the setting. This patch reduces the listener's thread mask to a single mask instead of an array of masks per process. Now we have only one thread mask and one process mask per bind-conf. This removes ~504 bytes of RAM per bind-conf and will simplify handling of thread masks. If a "bind" line only refers to process numbers not found by its parent frontend or not covered by the global nbproc directive, or to a thread not covered by the global nbthread directive, a warning is emitted saying what will be used instead. --- diff --git a/doc/configuration.txt b/doc/configuration.txt index 1c42d065d9..aedced58d5 100644 --- a/doc/configuration.txt +++ b/doc/configuration.txt @@ -11323,7 +11323,7 @@ prefer-client-ciphers the client cipher list. process [/] - This restricts the list of processes and/or threads on which this listener is + This restricts the list of processes or threads on which this listener is allowed to run. It does not enforce any process but eliminates those which do not match. If the frontend uses a "bind-process" setting, the intersection between the two is applied. If in the end the listener is not allowed to run @@ -11331,9 +11331,11 @@ process [/] run on the first process of the listener if a single process was specified, or on all of its processes if multiple processes were specified. If a thread set is specified, it limits the threads allowed to process incoming - connections for this listener, for the corresponding process set. For the - unlikely case where several ranges are needed, this directive may be - repeated. and must use the format + connections for this listener, for the the process set. If multiple processes + and threads are configured, a warning is emitted, as it either results from a + configuration error or a misunderstanding of these models. For the unlikely + case where several ranges are needed, this directive may be repeated. + and must use the format all | odd | even | number[-[number]] diff --git a/include/types/listener.h b/include/types/listener.h index 9c0ebde069..1203d17016 100644 --- a/include/types/listener.h +++ b/include/types/listener.h @@ -166,7 +166,7 @@ struct bind_conf { int is_ssl; /* SSL is required for these listeners */ int generate_certs; /* 1 if generate-certificates option is set, else 0 */ unsigned long bind_proc; /* bitmask of processes allowed to use these listeners */ - unsigned long bind_thread[MAX_PROCS]; /* bitmask of threads (per processes) allowed to use these listeners */ + unsigned long bind_thread; /* bitmask of threads allowed to use these listeners */ struct { /* UNIX socket permissions */ uid_t uid; /* -1 to leave unchanged */ gid_t gid; /* -1 to leave unchanged */ diff --git a/src/cfgparse.c b/src/cfgparse.c index 85d8dd53aa..16d92512cd 100644 --- a/src/cfgparse.c +++ b/src/cfgparse.c @@ -2304,8 +2304,7 @@ int check_config_validity() #endif /* detect and address thread affinity inconsistencies */ - nbproc = my_ffsl(proc_mask(bind_conf->bind_proc)) - 1; - mask = thread_mask(bind_conf->bind_thread[nbproc]); + mask = thread_mask(bind_conf->bind_thread); if (!(mask & all_threads_mask)) { unsigned long new_mask = 0; @@ -2314,33 +2313,28 @@ int check_config_validity() mask >>= global.nbthread; } - for (nbproc = 0; nbproc < MAX_PROCS; nbproc++) { - if (!bind_conf->bind_proc || (bind_conf->bind_proc & (1UL << nbproc))) - bind_conf->bind_thread[nbproc] = new_mask; - } + bind_conf->bind_thread = new_mask; ha_warning("Proxy '%s': the thread range specified on the 'process' directive of 'bind %s' at [%s:%d] only refers to thread numbers out of the range defined by the global 'nbthread' directive. The thread numbers were remapped to existing threads instead (mask 0x%lx).\n", curproxy->id, bind_conf->arg, bind_conf->file, bind_conf->line, new_mask); } /* detect process and nbproc affinity inconsistencies */ - if (!bind_conf->bind_proc) - continue; - - mask = proc_mask(curproxy->bind_proc) & all_proc_mask; - /* mask cannot be null here thanks to the previous checks */ - - nbproc = my_popcountl(bind_conf->bind_proc); - bind_conf->bind_proc &= mask; - - if (!bind_conf->bind_proc && nbproc == 1) { - ha_warning("Proxy '%s': the process number specified on the 'process' directive of 'bind %s' at [%s:%d] refers to a process not covered by the proxy. This has been fixed by forcing it to run on the proxy's first process only.\n", - curproxy->id, bind_conf->arg, bind_conf->file, bind_conf->line); - bind_conf->bind_proc = mask & ~(mask - 1); - } - else if (!bind_conf->bind_proc && nbproc > 1) { - ha_warning("Proxy '%s': the process range specified on the 'process' directive of 'bind %s' at [%s:%d] only refers to processes not covered by the proxy. The directive was ignored so that all of the proxy's processes are used.\n", - curproxy->id, bind_conf->arg, bind_conf->file, bind_conf->line); - bind_conf->bind_proc = 0; + mask = proc_mask(bind_conf->bind_proc) & proc_mask(curproxy->bind_proc); + if (!(mask & all_proc_mask)) { + mask = proc_mask(curproxy->bind_proc) & all_proc_mask; + nbproc = my_popcountl(bind_conf->bind_proc); + bind_conf->bind_proc = proc_mask(bind_conf->bind_proc) & mask; + + if (!bind_conf->bind_proc && nbproc == 1) { + ha_warning("Proxy '%s': the process number specified on the 'process' directive of 'bind %s' at [%s:%d] refers to a process not covered by the proxy. This has been fixed by forcing it to run on the proxy's first process only.\n", + curproxy->id, bind_conf->arg, bind_conf->file, bind_conf->line); + bind_conf->bind_proc = mask & ~(mask - 1); + } + else if (!bind_conf->bind_proc && nbproc > 1) { + ha_warning("Proxy '%s': the process range specified on the 'process' directive of 'bind %s' at [%s:%d] only refers to processes not covered by the proxy. The directive was ignored so that all of the proxy's processes are used.\n", + curproxy->id, bind_conf->arg, bind_conf->file, bind_conf->line); + bind_conf->bind_proc = 0; + } } } diff --git a/src/listener.c b/src/listener.c index f8745e3360..117b9c43f2 100644 --- a/src/listener.c +++ b/src/listener.c @@ -954,7 +954,6 @@ static int bind_parse_process(char **args, int cur_arg, struct proxy *px, struct { char *slash; unsigned long proc = 0, thread = 0; - int i; if ((slash = strchr(args[cur_arg + 1], '/')) != NULL) *slash = 0; @@ -973,11 +972,7 @@ static int bind_parse_process(char **args, int cur_arg, struct proxy *px, struct } conf->bind_proc |= proc; - if (thread) { - for (i = 0; i < MAX_PROCS; i++) - if (!proc || (proc & (1UL << i))) - conf->bind_thread[i] |= thread; - } + conf->bind_thread |= thread; return 0; } diff --git a/src/proto_sockpair.c b/src/proto_sockpair.c index 9105dd2f9e..713da370ab 100644 --- a/src/proto_sockpair.c +++ b/src/proto_sockpair.c @@ -153,7 +153,7 @@ static int sockpair_bind_listener(struct listener *listener, char *errmsg, int e listener->state = LI_LISTEN; fd_insert(fd, listener, listener->proto->accept, - thread_mask(listener->bind_conf->bind_thread[relative_pid-1])); + thread_mask(listener->bind_conf->bind_thread)); return err; diff --git a/src/proto_tcp.c b/src/proto_tcp.c index e4b8f63d75..28b77503e5 100644 --- a/src/proto_tcp.c +++ b/src/proto_tcp.c @@ -1076,7 +1076,7 @@ int tcp_bind_listener(struct listener *listener, char *errmsg, int errlen) listener->state = LI_LISTEN; fd_insert(fd, listener, listener->proto->accept, - thread_mask(listener->bind_conf->bind_thread[relative_pid-1])); + thread_mask(listener->bind_conf->bind_thread)); tcp_return: if (msg && errlen) { diff --git a/src/proto_uxst.c b/src/proto_uxst.c index 7fc145b8cf..d454d4ca13 100644 --- a/src/proto_uxst.c +++ b/src/proto_uxst.c @@ -340,7 +340,7 @@ static int uxst_bind_listener(struct listener *listener, char *errmsg, int errle listener->state = LI_LISTEN; fd_insert(fd, listener, listener->proto->accept, - thread_mask(listener->bind_conf->bind_thread[relative_pid-1])); + thread_mask(listener->bind_conf->bind_thread)); return err;