[nginx] SSL: ssl_buffer_size directive.

Ilya Grigorik igrigorik at gmail.com
Fri Dec 20 18:58:43 UTC 2013


Awesome, really glad to see this! A couple of followup questions...

(a) Is there any way to force a packet flush on record end? At the moment
nginx will fragment multiple records across packet boundaries, which is
suboptimal as it means that I need a minimum of two packets to decode any
record - e.g. if I set my record size to 1370 bytes, the first packet will
contain the first full record plus another 20-50 bytes of next record.

(b) Current NGX_SSL_BUFSIZE is set to 16KB which is effectively guaranteed
to overflow the CWND of a new connection and introduce another RTT for
interactive traffic - e.g. HTTP pages. I would love to see a lower starting
record size to mitigate this problem -- defaults matter!

On the subject of optimizing record size, the GFE team at Google recently
landed ~following logic:

- new connections default to small record size
-- each record fits into a TCP packet
-- packets are flushed at record boundaries
- server tracks number of bytes written since reset and timestamp of last
write
-- if bytes written > {configurable byte threshold) then boost record size
to 16KB
-- if last write timestamp > now - {configurable time threshold} then reset
sent byte count

In other words, start with small record size to optimize for delivery of
small/interactive objects (bulk of HTTP traffic). Then, if large file is
being transferred bump record size to 16KB and continue using that until
the connection goes idle.. when communication resumes, start with small
record size and repeat. Overall, this is aimed to optimize delivery of
small files where incremental delivery is a priority, and also for large
downloads where overall throughput is a priority.

Both byte and time thresholds are exposed as configurable flags, and
current defaults in GFE are 1MB and 1s.

This would require a bit more work than the current patch, but I'd love to
see a similar strategy in nginx. Hardcoding a fixed record size will
inevitably lead to suboptimal delivery of either interactive or bulk
traffic. Thoughts?

ig



On Fri, Dec 20, 2013 at 4:19 AM, Maxim Dounin <mdounin at mdounin.ru> wrote:

> details:   http://hg.nginx.org/nginx/rev/a297b7ad6f94
> branches:
> changeset: 5487:a297b7ad6f94
> user:      Maxim Dounin <mdounin at mdounin.ru>
> date:      Fri Dec 20 16:18:25 2013 +0400
> description:
> SSL: ssl_buffer_size directive.
>
> diffstat:
>
>  src/event/ngx_event_openssl.c          |   9 ++++++---
>  src/event/ngx_event_openssl.h          |   2 ++
>  src/http/modules/ngx_http_ssl_module.c |  13 +++++++++++++
>  src/http/modules/ngx_http_ssl_module.h |   2 ++
>  4 files changed, 23 insertions(+), 3 deletions(-)
>
> diffs (121 lines):
>
> 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
> @@ -190,6 +190,8 @@ ngx_ssl_create(ngx_ssl_t *ssl, ngx_uint_
>          return NGX_ERROR;
>      }
>
> +    ssl->buffer_size = NGX_SSL_BUFSIZE;
> +
>      /* client side options */
>
>      SSL_CTX_set_options(ssl->ctx, SSL_OP_MICROSOFT_SESS_ID_BUG);
> @@ -726,6 +728,7 @@ ngx_ssl_create_connection(ngx_ssl_t *ssl
>      }
>
>      sc->buffer = ((flags & NGX_SSL_BUFFER) != 0);
> +    sc->buffer_size = ssl->buffer_size;
>
>      sc->connection = SSL_new(ssl->ctx);
>
> @@ -1222,7 +1225,7 @@ ngx_ssl_send_chain(ngx_connection_t *c,
>      buf = c->ssl->buf;
>
>      if (buf == NULL) {
> -        buf = ngx_create_temp_buf(c->pool, NGX_SSL_BUFSIZE);
> +        buf = ngx_create_temp_buf(c->pool, c->ssl->buffer_size);
>          if (buf == NULL) {
>              return NGX_CHAIN_ERROR;
>          }
> @@ -1231,14 +1234,14 @@ ngx_ssl_send_chain(ngx_connection_t *c,
>      }
>
>      if (buf->start == NULL) {
> -        buf->start = ngx_palloc(c->pool, NGX_SSL_BUFSIZE);
> +        buf->start = ngx_palloc(c->pool, c->ssl->buffer_size);
>          if (buf->start == NULL) {
>              return NGX_CHAIN_ERROR;
>          }
>
>          buf->pos = buf->start;
>          buf->last = buf->start;
> -        buf->end = buf->start + NGX_SSL_BUFSIZE;
> +        buf->end = buf->start + c->ssl->buffer_size;
>      }
>
>      send = buf->last - buf->pos;
> 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
> @@ -29,6 +29,7 @@
>  typedef struct {
>      SSL_CTX                    *ctx;
>      ngx_log_t                  *log;
> +    size_t                      buffer_size;
>  } ngx_ssl_t;
>
>
> @@ -37,6 +38,7 @@ typedef struct {
>
>      ngx_int_t                   last;
>      ngx_buf_t                  *buf;
> +    size_t                      buffer_size;
>
>      ngx_connection_handler_pt   handler;
>
> diff --git a/src/http/modules/ngx_http_ssl_module.c
> b/src/http/modules/ngx_http_ssl_module.c
> --- a/src/http/modules/ngx_http_ssl_module.c
> +++ b/src/http/modules/ngx_http_ssl_module.c
> @@ -111,6 +111,13 @@ static ngx_command_t  ngx_http_ssl_comma
>        offsetof(ngx_http_ssl_srv_conf_t, ciphers),
>        NULL },
>
> +    { ngx_string("ssl_buffer_size"),
> +      NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_CONF_TAKE1,
> +      ngx_conf_set_size_slot,
> +      NGX_HTTP_SRV_CONF_OFFSET,
> +      offsetof(ngx_http_ssl_srv_conf_t, buffer_size),
> +      NULL },
> +
>      { ngx_string("ssl_verify_client"),
>        NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_CONF_TAKE1,
>        ngx_conf_set_enum_slot,
> @@ -424,6 +431,7 @@ ngx_http_ssl_create_srv_conf(ngx_conf_t
>
>      sscf->enable = NGX_CONF_UNSET;
>      sscf->prefer_server_ciphers = NGX_CONF_UNSET;
> +    sscf->buffer_size = NGX_CONF_UNSET_SIZE;
>      sscf->verify = NGX_CONF_UNSET_UINT;
>      sscf->verify_depth = NGX_CONF_UNSET_UINT;
>      sscf->builtin_session_cache = NGX_CONF_UNSET;
> @@ -465,6 +473,9 @@ ngx_http_ssl_merge_srv_conf(ngx_conf_t *
>                           (NGX_CONF_BITMASK_SET|NGX_SSL_SSLv3|NGX_SSL_TLSv1
>                            |NGX_SSL_TLSv1_1|NGX_SSL_TLSv1_2));
>
> +    ngx_conf_merge_size_value(conf->buffer_size, prev->buffer_size,
> +                         NGX_SSL_BUFSIZE);
> +
>      ngx_conf_merge_uint_value(conf->verify, prev->verify, 0);
>      ngx_conf_merge_uint_value(conf->verify_depth, prev->verify_depth, 1);
>
> @@ -572,6 +583,8 @@ ngx_http_ssl_merge_srv_conf(ngx_conf_t *
>          return NGX_CONF_ERROR;
>      }
>
> +    conf->ssl.buffer_size = conf->buffer_size;
> +
>      if (conf->verify) {
>
>          if (conf->client_certificate.len == 0 && conf->verify != 3) {
> diff --git a/src/http/modules/ngx_http_ssl_module.h
> b/src/http/modules/ngx_http_ssl_module.h
> --- a/src/http/modules/ngx_http_ssl_module.h
> +++ b/src/http/modules/ngx_http_ssl_module.h
> @@ -26,6 +26,8 @@ typedef struct {
>      ngx_uint_t                      verify;
>      ngx_uint_t                      verify_depth;
>
> +    size_t                          buffer_size;
> +
>      ssize_t                         builtin_session_cache;
>
>      time_t                          session_timeout;
>
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.nginx.org/pipermail/nginx-devel/attachments/20131220/de8a35a4/attachment-0001.html>


More information about the nginx-devel mailing list