[PATCH] SO_REUSEPORT support for listen sockets

[Maxim Dounin]

Hello!

On Fri, Jul 26, 2013 at 03:59:47AM -0700, Piotr Sikora wrote:

> Hey,
> 
> > @@ -76,6 +78,13 @@
> >       0,
> >       NULL },
> >
> > +    { ngx_string("so_reuseport"),
> > +      NGX_MAIN_CONF|NGX_DIRECT_CONF|NGX_CONF_TAKE1,
> > +      ngx_set_so_reuseport,
> > +      0,
> > +      0,
> > +      NULL },
> > +
> >     { ngx_string("debug_points"),
> >       NGX_MAIN_CONF|NGX_DIRECT_CONF|NGX_CONF_TAKE1,
> >       ngx_conf_set_enum_slot,
> > @@ -1361,3 +1370,24 @@
> >
> >     return NGX_CONF_OK;
> > }
> > +
> > +
> > +static char *
> > +ngx_set_so_reuseport(ngx_conf_t *cf, ngx_command_t *cmd, void *conf)
> > +{
> > +    ngx_str_t        *value;
> > +    ngx_core_conf_t  *ccf;
> > +
> > +    ccf = (ngx_core_conf_t *) conf;
> > +
> > +    value = (ngx_str_t *) cf->args->elts;
> > +
> > +    if (ngx_strcmp(value[1].data, "on") == 0) {
> > +        ccf->so_reuseport = 1;
> > +    } else if (ngx_strcmp(value[1].data, "off") == 0) {
> > +        ccf->so_reuseport = 0;
> > +    } else {
> > +        return "invalid value";
> > +    }
> > +    return NGX_CONF_OK;
> > +}
> 
> This can be replaced with ngx_conf_set_flag_slot(), i.e.:
> 
> +    { ngx_string("so_reuseport"),
> +      NGX_MAIN_CONF|NGX_DIRECT_CONF|NGX_CONF_FLAG,
> +      ngx_conf_set_flag_slot,
> +      0,
> +      offsetof(ngx_core_conf_t, so_reuseport),
> +      NULL },

If it's kept as a global setting, it would be good idea to move 
this into events module if possible.

> Also:
> 1) like Tom said, you definitely need to guard code to make sure
> SO_REUSEPORT is available,
> 2) this feature should be disabled on DragonFly versions prior to the
> 740d1d9 commit, because it clearly wouldn't do any good there,

I believe SO_REUSEPORT doesn't do accept() load balancing on many 
OSes right now (e.g. FreeBSD doesn't do that), and it might not be 
a good idea to track this in nginx code.  It might be better to 
just allow users to decide whether to use it or not.  Not sure though.

> 3) it might make sense to expose this as an option of "listen"
> directive, instead of a global setting,

Agree.

> 4) how does this (OS-level sharding) play with nginx's upgrade process
> (forking of new binary and passing listening fds)? Are there any
> side-effects of this change that could result in dropped packets /
> requests?

And obvious downside I see is that with the SO_REUSEPORT causes OS 
to allow duplicate bindings from different processes, which makes 
it possible to unintentionally run 2 copies of nginx.  It might be 
also possible that configuration test will start to do bad things 
as a result.

-- 
Maxim Dounin
http://nginx.org/en/donation.html