[PATCH 05 of 11] SSL: single allocation in session cache on 32-bit platforms

Sergey Kandaurov pluknet at nginx.com
Thu Sep 15 05:41:36 UTC 2022


> On 26 Aug 2022, at 07:01, Maxim Dounin <mdounin at mdounin.ru> wrote:
> 
> # HG changeset patch
> # User Maxim Dounin <mdounin at 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.

> +    ngx_memcpy(sess_id->id, session_id, session_id_length);
> 
>     hash = ngx_crc32_short(session_id, session_id_length);
> 
> @@ -3929,9 +3915,7 @@ ngx_ssl_new_session(ngx_ssl_conn_t *ssl_
> 
>     sess_id->node.key = hash;
>     sess_id->node.data = (u_char) session_id_length;
> -    sess_id->id = id;
>     sess_id->len = len;

With comment in the previous patch#04, this makes the following:

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
@@ -3817,7 +3817,6 @@ ngx_ssl_new_session(ngx_ssl_conn_t *ssl_
     ngx_slab_pool_t          *shpool;
     ngx_ssl_sess_id_t        *sess_id;
     ngx_ssl_session_cache_t  *cache;
-    u_char                    buf[NGX_SSL_MAX_SESSION_SIZE];
 
 #ifdef TLS1_3_VERSION
 
@@ -3843,9 +3842,6 @@ ngx_ssl_new_session(ngx_ssl_conn_t *ssl_
         return 0;
     }
 
-    p = buf;
-    i2d_SSL_SESSION(sess, &p);
-
     session_id = (u_char *) SSL_SESSION_get_id(sess, &session_id_length);
 
     /* do not cache sessions with too long session id */
@@ -3907,7 +3903,10 @@ ngx_ssl_new_session(ngx_ssl_conn_t *ssl_
 
 #endif
 
-    ngx_memcpy(sess_id->session, buf, len);
+    p = sess_id->session;
+    i2d_SSL_SESSION(sess, &p);
+    sess_id->len = len;
+
     ngx_memcpy(sess_id->id, session_id, session_id_length);
 
     hash = ngx_crc32_short(session_id, session_id_length);
@@ -3918,7 +3917,6 @@ ngx_ssl_new_session(ngx_ssl_conn_t *ssl_
 
     sess_id->node.key = hash;
     sess_id->node.data = (u_char) session_id_length;
-    sess_id->len = len;
 
     sess_id->expire = ngx_time() + SSL_CTX_get_timeout(ssl_ctx);
 

> -    sess_id->session = cached_sess;
> 
>     sess_id->expire = ngx_time() + SSL_CTX_get_timeout(ssl_ctx);
> 
> @@ -3945,10 +3929,6 @@ ngx_ssl_new_session(ngx_ssl_conn_t *ssl_
> 
> failed:
> 
> -    if (cached_sess) {
> -        ngx_slab_free_locked(shpool, cached_sess);
> -    }
> -
>     if (sess_id) {
>         ngx_slab_free_locked(shpool, sess_id);
>     }
> @@ -4045,9 +4025,8 @@ ngx_ssl_get_cached_session(ngx_ssl_conn_
> 
>             ngx_rbtree_delete(&cache->session_rbtree, node);
> 
> +#if (NGX_PTR_SIZE == 8)
>             ngx_slab_free_locked(shpool, sess_id->session);
> -#if (NGX_PTR_SIZE == 4)
> -            ngx_slab_free_locked(shpool, sess_id->id);
> #endif
>             ngx_slab_free_locked(shpool, sess_id);
> 
> @@ -4135,9 +4114,8 @@ ngx_ssl_remove_session(SSL_CTX *ssl, ngx
> 
>             ngx_rbtree_delete(&cache->session_rbtree, node);
> 
> +#if (NGX_PTR_SIZE == 8)
>             ngx_slab_free_locked(shpool, sess_id->session);
> -#if (NGX_PTR_SIZE == 4)
> -            ngx_slab_free_locked(shpool, sess_id->id);
> #endif
>             ngx_slab_free_locked(shpool, sess_id);
> 
> @@ -4184,9 +4162,8 @@ ngx_ssl_expire_sessions(ngx_ssl_session_
> 
>         ngx_rbtree_delete(&cache->session_rbtree, &sess_id->node);
> 
> +#if (NGX_PTR_SIZE == 8)
>         ngx_slab_free_locked(shpool, sess_id->session);
> -#if (NGX_PTR_SIZE == 4)
> -        ngx_slab_free_locked(shpool, sess_id->id);
> #endif
>         ngx_slab_free_locked(shpool, sess_id);
>     }
> 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.

-- 
Sergey Kandaurov



More information about the nginx-devel mailing list