Hi, On Tue, Nov 28, 2017 at 09:08:59AM +0100, Maciej Zdeb wrote: > Hi Willy, > > Thanks for your response. In change > http://git.haproxy.org/?p=haproxy-1.8.git;a=commit;h=4f45bb9c461f462290b77bf2511badb7a4453c0a > William made shctx more generic. Right now it's calculated: > > shctx = (struct shared_context *)mmap(NULL, sizeof(struct shared_context) + > extra + (maxblocks * (sizeof(struct shared_block) + blocksize)), > > before it was: > > shctx = (struct shared_context *)mmap(NULL, sizeof(struct > shared_context)+(size*sizeof(struct shared_block)), > > Still, William is the best person to ask. :-) >
Thanks for the report, the bug was elsewhere, in fact the shctx was initialized for each ssl bind. Patch attached. -- William Lallemand
>From 6798b9d884096f17f0f4863dc8ce52445865f90d Mon Sep 17 00:00:00 2001 From: William Lallemand <[email protected]> Date: Tue, 28 Nov 2017 11:04:43 +0100 Subject: [PATCH] BUG/MEDIUM: ssl: don't allocate shctx several time The shctx_init() function does not check anymore if the pointer is not NULL, this check must be done is the caller. The consequence was to allocate one shctx per ssl bind. Bug introduced by 4f45bb9 ("MEDIUM: shctx: separate ssl and shctx") Thanks to Maciej Zdeb for reporting this bug. Must be backported to 1.8. --- src/ssl_sock.c | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/src/ssl_sock.c b/src/ssl_sock.c index 84804f734..da6f2ded6 100644 --- a/src/ssl_sock.c +++ b/src/ssl_sock.c @@ -324,7 +324,7 @@ const char *SSL_SOCK_KEYTYPE_NAMES[] = { #define SSL_SOCK_NUM_KEYTYPES 1 #endif -static struct shared_context *ssl_shctx; /* ssl shared session cache */ +static struct shared_context *ssl_shctx = NULL; /* ssl shared session cache */ static struct eb_root *sh_ssl_sess_tree; /* ssl shared session tree */ #define sh_ssl_sess_tree_delete(s) ebmb_delete(&(s)->key); @@ -4705,24 +4705,24 @@ int ssl_sock_prepare_bind_conf(struct bind_conf *bind_conf) return -1; } } - - alloc_ctx = shctx_init(&ssl_shctx, global.tune.sslcachesize, - sizeof(struct sh_ssl_sess_hdr) + SHSESS_BLOCK_MIN_SIZE, - sizeof(*sh_ssl_sess_tree), - ((global.nbthread > 1) || (!global_ssl.private_cache && (global.nbproc > 1))) ? 1 : 0); - if (alloc_ctx < 0) { - if (alloc_ctx == SHCTX_E_INIT_LOCK) - ha_alert("Unable to initialize the lock for the shared SSL session cache. You can retry using the global statement 'tune.ssl.force-private-cache' but it could increase CPU usage due to renegotiations if nbproc > 1.\n"); - else - ha_alert("Unable to allocate SSL session cache.\n"); - return -1; + if (!ssl_shctx) { + alloc_ctx = shctx_init(&ssl_shctx, global.tune.sslcachesize, + sizeof(struct sh_ssl_sess_hdr) + SHSESS_BLOCK_MIN_SIZE, + sizeof(*sh_ssl_sess_tree), + ((global.nbthread > 1) || (!global_ssl.private_cache && (global.nbproc > 1))) ? 1 : 0); + if (alloc_ctx < 0) { + if (alloc_ctx == SHCTX_E_INIT_LOCK) + ha_alert("Unable to initialize the lock for the shared SSL session cache. You can retry using the global statement 'tune.ssl.force-private-cache' but it could increase CPU usage due to renegotiations if nbproc > 1.\n"); + else + ha_alert("Unable to allocate SSL session cache.\n"); + return -1; + } + /* free block callback */ + ssl_shctx->free_block = sh_ssl_sess_free_blocks; + /* init the root tree within the extra space */ + sh_ssl_sess_tree = (void *)ssl_shctx + sizeof(struct shared_context); + *sh_ssl_sess_tree = EB_ROOT_UNIQUE; } - /* free block callback */ - ssl_shctx->free_block = sh_ssl_sess_free_blocks; - /* init the root tree within the extra space */ - sh_ssl_sess_tree = (void *)ssl_shctx + sizeof(struct shared_context); - *sh_ssl_sess_tree = EB_ROOT_UNIQUE; - err = 0; /* initialize all certificate contexts */ err += ssl_sock_prepare_all_ctx(bind_conf); -- 2.13.6
