> On 17 Sep 2022, at 01:04, Maxim Dounin <mdou...@mdounin.ru> wrote: > > Hello! > > On Thu, Sep 15, 2022 at 09:41:36AM +0400, Sergey Kandaurov wrote: > >>> On 26 Aug 2022, at 07:01, Maxim Dounin <mdou...@mdounin.ru> wrote: >>> >>> # HG changeset patch >>> # User Maxim Dounin <mdou...@mdounin.ru> >>> # Date 1661481950 -10800 >>> # Fri Aug 26 05:45:50 2022 +0300 >>> # Node ID e88baee178eed529c6170678e373f5e2e0883c37 >>> # Parent f4ae0f4ee928cf20346530e96f1431314ecd0171 >>> SSL: single allocation in session cache on 32-bit platforms. >>> >>> Given the present typical SSL session sizes, on 32-bit platforms it is >>> now beneficial to store all data in a single allocation, since rbtree >>> node + session id + ASN1 representation of a session takes 256 bytes of >>> shared memory (36 + 32 + 150 = about 218 bytes plus SNI server name). >>> >>> Storing all data in a single allocation is beneficial for SNI names up to >>> about 40 characters long and makes it possible to store about 4000 sessions >>> in one megabyte (instead of about 3000 sessions now). This also slightly >>> simplifies the code. >>> >>> diff --git a/src/event/ngx_event_openssl.c b/src/event/ngx_event_openssl.c >>> --- a/src/event/ngx_event_openssl.c >>> +++ b/src/event/ngx_event_openssl.c >>> @@ -3789,9 +3789,9 @@ ngx_ssl_session_cache_init(ngx_shm_zone_ >>> * Typical length of the external ASN1 representation of a session >>> * is about 150 bytes plus SNI server name. >>> * >>> - * On 32-bit platforms we allocate separately an rbtree node, >>> - * a session id, and an ASN1 representation, they take accordingly >>> - * 64, 32, and 256 bytes. >>> + * On 32-bit platforms we allocate an rbtree node, a session id, and >>> + * an ASN1 representation in a single allocation, it typically takes >>> + * 256 bytes. >>> * >>> * On 64-bit platforms we allocate separately an rbtree node + session_id, >>> * and an ASN1 representation, they take accordingly 128 and 256 bytes. >>> @@ -3804,7 +3804,8 @@ static int >>> ngx_ssl_new_session(ngx_ssl_conn_t *ssl_conn, ngx_ssl_session_t *sess) >>> { >>> int len; >>> - u_char *p, *id, *cached_sess, *session_id; >>> + u_char *p, *session_id; >>> + size_t n; >>> uint32_t hash; >>> SSL_CTX *ssl_ctx; >>> unsigned int session_id_length; >>> @@ -3863,23 +3864,13 @@ ngx_ssl_new_session(ngx_ssl_conn_t *ssl_ >>> /* drop one or two expired sessions */ >>> ngx_ssl_expire_sessions(cache, shpool, 1); >>> >>> - cached_sess = ngx_slab_alloc_locked(shpool, len); >>> - >>> - if (cached_sess == NULL) { >>> - >>> - /* drop the oldest non-expired session and try once more */ >>> - >>> - ngx_ssl_expire_sessions(cache, shpool, 0); >>> - >>> - cached_sess = ngx_slab_alloc_locked(shpool, len); >>> - >>> - if (cached_sess == NULL) { >>> - sess_id = NULL; >>> - goto failed; >>> - } >>> - } >>> - >>> - sess_id = ngx_slab_alloc_locked(shpool, sizeof(ngx_ssl_sess_id_t)); >>> +#if (NGX_PTR_SIZE == 8) >>> + n = sizeof(ngx_ssl_sess_id_t); >>> +#else >>> + n = offsetof(ngx_ssl_sess_id_t, session) + len; >>> +#endif >>> + >>> + sess_id = ngx_slab_alloc_locked(shpool, n); >>> >>> if (sess_id == NULL) { >>> >>> @@ -3887,7 +3878,7 @@ ngx_ssl_new_session(ngx_ssl_conn_t *ssl_ >>> >>> ngx_ssl_expire_sessions(cache, shpool, 0); >>> >>> - sess_id = ngx_slab_alloc_locked(shpool, sizeof(ngx_ssl_sess_id_t)); >>> + sess_id = ngx_slab_alloc_locked(shpool, n); >>> >>> if (sess_id == NULL) { >>> goto failed; >>> @@ -3896,30 +3887,25 @@ ngx_ssl_new_session(ngx_ssl_conn_t *ssl_ >>> >>> #if (NGX_PTR_SIZE == 8) >>> >>> - id = sess_id->sess_id; >>> - >>> -#else >>> - >>> - id = ngx_slab_alloc_locked(shpool, session_id_length); >>> - >>> - if (id == NULL) { >>> + sess_id->session = ngx_slab_alloc_locked(shpool, len); >>> + >>> + if (sess_id->session == NULL) { >>> >>> /* drop the oldest non-expired session and try once more */ >>> >>> ngx_ssl_expire_sessions(cache, shpool, 0); >>> >>> - id = ngx_slab_alloc_locked(shpool, session_id_length); >>> - >>> - if (id == NULL) { >>> + sess_id->session = ngx_slab_alloc_locked(shpool, len); >>> + >>> + if (sess_id->session == NULL) { >>> goto failed; >>> } >>> } >>> >>> #endif >>> >>> - ngx_memcpy(cached_sess, buf, len); >>> - >>> - ngx_memcpy(id, session_id, session_id_length); >>> + ngx_memcpy(sess_id->session, buf, len); >> >> Converting to ASN1 looks feasible without intermediate buf[] >> to avoid extra memory copy. > > Quoting the comment before the function: > > * OpenSSL's i2d_SSL_SESSION() and d2i_SSL_SESSION are slow, > * so they are outside the code locked by shared pool mutex > > Hope this helps.
Thanks, somehow missed that. [..] >>> diff --git a/src/event/ngx_event_openssl.h b/src/event/ngx_event_openssl.h >>> --- a/src/event/ngx_event_openssl.h >>> +++ b/src/event/ngx_event_openssl.h >>> @@ -134,14 +134,14 @@ typedef struct ngx_ssl_sess_id_s ngx_ss >>> >>> struct ngx_ssl_sess_id_s { >>> ngx_rbtree_node_t node; >>> - u_char *id; >>> size_t len; >>> - u_char *session; >>> ngx_queue_t queue; >>> time_t expire; >>> + u_char id[32]; >>> #if (NGX_PTR_SIZE == 8) >>> - void *stub; >>> - u_char sess_id[32]; >>> + u_char *session; >>> +#else >>> + u_char session[1]; >>> #endif >>> }; >>> >> >> This reminds me that this structure coupled with ngx_ssl_ticket_key_t >> and ngx_ssl_session_cache_t aren't used outside and can be made private. > > I don't think it worth the effort. Either way, it is completely > unrelated to this patch series. > Sure. -- Sergey Kandaurov _______________________________________________ nginx-devel mailing list -- nginx-devel@nginx.org To unsubscribe send an email to nginx-devel-le...@nginx.org