[PATCH] Core: handle getsockopt(TCP_FASTOPEN) failures.

[Maxim Dounin]

Hello!

On Wed, Jan 29, 2014 at 06:28:05PM +0400, Ruslan Ermilov wrote:

> On Tue, Jan 28, 2014 at 08:19:57PM -0800, Piotr Sikora wrote:
> > # HG changeset patch
> > # User Piotr Sikora <piotr at cloudflare.com>
> > # Date 1390968940 28800
> > #      Tue Jan 28 20:15:40 2014 -0800
> > # Node ID ae13ab8f53fe451d2201a68091ddecc3d36d582d
> > # Parent  fdb67cfc957d110ea887961cc8c08a590df5f62c
> > Core: handle getsockopt(TCP_FASTOPEN) failures.
> > 
> > Linux returns EOPNOTSUPP for non-TCP sockets and ENOPROTOOPT for TCP
> > sockets, because getsockopt(TCP_FASTOPEN) is not implemented so far.
> > 
> > While there, lower the log level from ALERT to NOTICE to match other
> > getsockopt() failures.
> > 
> > Signed-off-by: Piotr Sikora <piotr at cloudflare.com>
> > 
> > diff -r fdb67cfc957d -r ae13ab8f53fe src/core/ngx_connection.c
> > --- a/src/core/ngx_connection.c Tue Jan 28 15:40:46 2014 +0400
> > +++ b/src/core/ngx_connection.c Tue Jan 28 20:15:40 2014 -0800
> > @@ -97,7 +97,7 @@ ngx_set_inherited_sockets(ngx_cycle_t *c
> >      ngx_uint_t                 i;
> >      ngx_listening_t           *ls;
> >      socklen_t                  olen;
> > -#if (NGX_HAVE_DEFERRED_ACCEPT)
> > +#if (NGX_HAVE_DEFERRED_ACCEPT || NGX_HAVE_TCP_FASTOPEN)
> >      ngx_err_t                  err;
> >  #endif
> >  #if (NGX_HAVE_DEFERRED_ACCEPT && defined SO_ACCEPTFILTER)
> > @@ -223,9 +223,13 @@ ngx_set_inherited_sockets(ngx_cycle_t *c
> >                         (void *) &ls[i].fastopen, &olen)
> >              == -1)
> >          {
> > -            ngx_log_error(NGX_LOG_ALERT, cycle->log, ngx_socket_errno,
> > -                          "getsockopt(TCP_FASTOPEN) %V failed, ignored",
> > -                          &ls[i].addr_text);
> > +            err = ngx_errno;

As also pointed by ru@, shouldn't it be ngx_socket_errno instead?  
(It also needs to be fixed in a couple of other places, but anyway 
it's mostly style.)

> > +
> > +            if (err != NGX_EOPNOTSUPP && err != NGX_ENOPROTOOPT) {
> > +                ngx_log_error(NGX_LOG_NOTICE, cycle->log, err,
> > +                              "getsockopt(TCP_FASTOPEN) %V failed, ignored",
> > +                              &ls[i].addr_text);
> > +            }
> 
> I only have a style comment.  I think it should formatted as in the
> TCP_DEFER_ACCEPT case, i.e. the errno check with "continue".

Using "continue" is wrong here, it's only possible in the last 
check.

The patch looks fine for me, modulo ngx_socket_errno.

-- 
Maxim Dounin
http://nginx.org/