Hi,
On Wed, Dec 27, 2023 at 06:34:58PM +0400, Sergey Kandaurov wrote:
> On Wed, Dec 13, 2023 at 06:06:59PM +0400, Roman Arutyunyan wrote:
>
> > # HG changeset patch
> > # User Roman Arutyunyan <a...@nginx.com>
> > # Date 1702476295 -14400
> > # Wed Dec 13 18:04:55 2023 +0400
> > # Node ID 844486cdd43a32d10b78493d7e7b80e9e2239d7e
> > # Parent 6c8595b77e667bd58fd28186939ed820f2e55e0e
> > Stream: socket peek in preread phase.
> >
> > Previously, preread buffer was always read out from socket, which made it
> > impossible to terminate SSL on the connection without introducing additional
> > SSL BIOs. The following patches will rely on this.
> >
> > Now, when possible, recv(MSG_PEEK) is used instead, which keeps data in
> > socket.
> > It's called if SSL is not already terminated and if an egde-triggered event
> > method is used. For epoll, EPOLLRDHUP support is also required.
> >
> > diff --git a/src/stream/ngx_stream_core_module.c
> > b/src/stream/ngx_stream_core_module.c
> > --- a/src/stream/ngx_stream_core_module.c
> > +++ b/src/stream/ngx_stream_core_module.c
> > @@ -10,6 +10,10 @@
> > #include <ngx_stream.h>
> >
> >
> > +static ngx_int_t ngx_stream_preread_peek(ngx_stream_session_t *s,
> > + ngx_stream_phase_handler_t *ph);
> > +static ngx_int_t ngx_stream_preread(ngx_stream_session_t *s,
> > + ngx_stream_phase_handler_t *ph);
> > static ngx_int_t ngx_stream_core_preconfiguration(ngx_conf_t *cf);
> > static void *ngx_stream_core_create_main_conf(ngx_conf_t *cf);
> > static char *ngx_stream_core_init_main_conf(ngx_conf_t *cf, void *conf);
> > @@ -203,8 +207,6 @@ ngx_int_t
> > ngx_stream_core_preread_phase(ngx_stream_session_t *s,
> > ngx_stream_phase_handler_t *ph)
> > {
> > - size_t size;
> > - ssize_t n;
> > ngx_int_t rc;
> > ngx_connection_t *c;
> > ngx_stream_core_srv_conf_t *cscf;
> > @@ -217,56 +219,40 @@ ngx_stream_core_preread_phase(ngx_stream
> >
> > if (c->read->timedout) {
> > rc = NGX_STREAM_OK;
> > + goto done;
> > + }
> >
> > - } else if (c->read->timer_set) {
> > - rc = NGX_AGAIN;
> > + if (!c->read->timer_set) {
> > + rc = ph->handler(s);
> >
> > - } else {
> > - rc = ph->handler(s);
> > + if (rc != NGX_AGAIN) {
> > + goto done;
> > + }
> > }
> >
> > - while (rc == NGX_AGAIN) {
> > -
> > + if (c->buffer == NULL) {
> > + c->buffer = ngx_create_temp_buf(c->pool,
> > cscf->preread_buffer_size);
> > if (c->buffer == NULL) {
> > - c->buffer = ngx_create_temp_buf(c->pool,
> > cscf->preread_buffer_size);
> > - if (c->buffer == NULL) {
> > - rc = NGX_ERROR;
> > - break;
> > - }
> > + rc = NGX_ERROR;
> > + goto done;
> > }
> > -
> > - size = c->buffer->end - c->buffer->last;
> > -
> > - if (size == 0) {
> > - ngx_log_error(NGX_LOG_ERR, c->log, 0, "preread buffer full");
> > - rc = NGX_STREAM_BAD_REQUEST;
> > - break;
> > - }
> > + }
> >
> > - if (c->read->eof) {
> > - rc = NGX_STREAM_OK;
> > - break;
> > - }
> > -
> > - if (!c->read->ready) {
> > - break;
> > - }
> > -
> > - n = c->recv(c, c->buffer->last, size);
> > + if (c->ssl == NULL
> > + && (ngx_event_flags & NGX_USE_CLEAR_EVENT)
> > + && ((ngx_event_flags & NGX_USE_EPOLL_EVENT) == 0
> > +#if (NGX_HAVE_EPOLLRDHUP)
> > + || ngx_use_epoll_rdhup
> > +#endif
>
> BTW, c->ssl needs to be guarded under an appropriate macro test.
> Probably, it makes sense to rewrite this in a more readable way.
> For example:
>
> : peak = 0;
> :
> : #if (NGX_HAVE_KQUEUE)
> : if (ngx_event_flags & NGX_USE_KQUEUE_EVENT) {
> : peak = 1;
> : }
> : #endif
> :
> : #if (NGX_HAVE_EPOLLRDHUP)
> : if ((ngx_event_flags & NGX_USE_EPOLL_EVENT) && ngx_use_epoll_rdhup) {
> : peak = 1;
> : }
> : #endif
> :
> : #if (NGX_STREAM_SSL)
> : if (c->ssl) {
> : peak = 0;
> : }
> : #endif
[..]
I think it's still too complicated. I suggest a separate function:
diff --git a/src/stream/ngx_stream_core_module.c
b/src/stream/ngx_stream_core_module.c
--- a/src/stream/ngx_stream_core_module.c
+++ b/src/stream/ngx_stream_core_module.c
@@ -10,6 +10,7 @@
#include <ngx_stream.h>
+static ngx_int_t ngx_stream_preread_can_peek(ngx_connection_t *c);
static ngx_int_t ngx_stream_preread_peek(ngx_stream_session_t *s,
ngx_stream_phase_handler_t *ph);
static ngx_int_t ngx_stream_preread(ngx_stream_session_t *s,
@@ -238,14 +239,7 @@ ngx_stream_core_preread_phase(ngx_stream
}
}
- if (c->ssl == NULL
- && (ngx_event_flags & NGX_USE_CLEAR_EVENT)
- && ((ngx_event_flags & NGX_USE_EPOLL_EVENT) == 0
-#if (NGX_HAVE_EPOLLRDHUP)
- || ngx_use_epoll_rdhup
-#endif
- ))
- {
+ if (ngx_stream_preread_can_peek(c)) {
rc = ngx_stream_preread_peek(s, ph);
} else {
@@ -298,6 +292,35 @@ done:
static ngx_int_t
+ngx_stream_preread_can_peek(ngx_connection_t *c)
+{
+#if (NGX_STREAM_SSL)
+ if (c->ssl) {
+ return 0;
+ }
+#endif
+
+ if ((ngx_event_flags & NGX_USE_CLEAR_EVENT) == 0) {
+ return 0;
+ }
+
+#if (NGX_HAVE_KQUEUE)
+ if (ngx_event_flags & NGX_USE_KQUEUE_EVENT) {
+ return 1;
+ }
+#endif
+
+#if (NGX_HAVE_EPOLLRDHUP)
+ if ((ngx_event_flags & NGX_USE_EPOLL_EVENT) && ngx_use_epoll_rdhup) {
+ return 1;
+ }
+#endif
+
+ return 0;
+}
+
+
+static ngx_int_t
ngx_stream_preread_peek(ngx_stream_session_t *s, ngx_stream_phase_handler_t
*ph)
{
ssize_t n;
--
Roman Arutyunyan
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel
