Welcome! Log In Create A New Profile

Advanced

Re: [Patch] Allow listening on Unix domain sockets

All files from this thread

File Name File Size   Posted by Date  
patch.listen.unix.txt 2.8 KB open | download Igor Sysoev 10/23/2009 Read message
patch.listen.unix1.txt 6.5 KB open | download Igor Sysoev 10/25/2009 Read message
Maxim Dounin
October 23, 2009 06:44PM
Hello!

On Fri, Oct 23, 2009 at 03:12:33PM +0200, Hongli Lai wrote:

> Hi.
>
> The attached patch allows Nginx to listen on Unix domain sockets, like
> this:
>
> server {
> listen unix:/tmp/nginx.sock;
> server_name foobar.com;
> root /webapps/foobar;
> }
>
> We use it to improve proxying performance; Unix domain sockets are
> faster than TCP sockets.
>
> I'd like to see this patch getting merged upstream. Could a maintainer
> please review this patch?

I'm not really maintainer, but here is the review.

[...]

> diff --git a/src/core/ngx_inet.c b/src/core/ngx_inet.c
> index 4c18036..59998be 100644
> --- a/src/core/ngx_inet.c
> +++ b/src/core/ngx_inet.c
> @@ -68,7 +68,9 @@ ngx_sock_ntop(struct sockaddr *sa, u_char *text, size_t len, ngx_uint_t port)
> size_t n;
> struct sockaddr_in6 *sin6;
> #endif
> -
> + struct sockaddr_un *sun;

This should be protected by #if (NGX_HAVE_UNIX_DOMAIN).

> + size_t path_len;

Using 'plen' here is just enough to be understood. And actually
it may be a good idea to use just one 'n' shared with INET6 case.

> +

Whitespace damage here.

> switch (sa->sa_family) {
>
> case AF_INET:
> @@ -108,6 +110,17 @@ ngx_sock_ntop(struct sockaddr *sa, u_char *text, size_t len, ngx_uint_t port)
> return n;
> #endif
>
> + case AF_UNIX:
> +
> + sun = (struct sockaddr_un *) sa;

This should be protected by #if (NGX_HAVE_UNIX_DOMAIN).

> + path_len = strlen(sun->sun_path);
> + if (path_len == 0) {
> + return ngx_snprintf(text, len, "(unknown)") - text;
> + } else {
> + ngx_copy(text, (const u_char *) sun->sun_path, path_len);

Using const is wrong here, as it's not in prototype at least in
some cases.

And it looks like you just trashed memory if buffer pointed by
text isn't big enough, no?

Note well that buffers used in the code for ngx_sock_ntop() result
are sized against NGX_SOCKADDR_STRLEN, which is big enough to hold
ipv6 address, but may be smaller than needed for unix sockets.

> + return path_len;
> + }
> +
> default:
> return 0;
> }
> diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c
> index 4930b50..5e3493e 100644
> --- a/src/http/ngx_http_request.c
> +++ b/src/http/ngx_http_request.c
> @@ -2426,7 +2426,11 @@ ngx_http_set_keepalive(ngx_http_request_t *r)
>
> if (tcp_nodelay
> && clcf->tcp_nodelay
> - && c->tcp_nodelay == NGX_TCP_NODELAY_UNSET)
> + && c->tcp_nodelay == NGX_TCP_NODELAY_UNSET
> + #if (NGX_HAVE_UNIX_DOMAIN)
> + && c->sockaddr->sa_family != AF_UNIX
> + #endif

Please keep preprocessor directive starting at position 0.

And note that ngx_tcp_nopush() / ngx_tcp_push() invocations
probably should be protected too.

Maxim Dounin
Subject Author Posted

[Patch] Allow listening on Unix domain sockets

Hongli Lai October 23, 2009 09:18AM

Re: [Patch] Allow listening on Unix domain sockets

Cliff Wells October 23, 2009 04:22PM

Re: [Patch] Allow listening on Unix domain sockets

Nick Pearson October 23, 2009 04:42PM

Re: Allow listening on Unix domain sockets

Hongli Lai October 23, 2009 04:56PM

Re: [Patch] Allow listening on Unix domain sockets

slact October 23, 2009 04:56PM

Re: [Patch] Allow listening on Unix domain sockets Attachments

Igor Sysoev October 23, 2009 05:26PM

Re: Allow listening on Unix domain sockets

Hongli Lai October 23, 2009 08:04PM

Re: Allow listening on Unix domain sockets Attachments

Igor Sysoev October 25, 2009 06:32AM

Re: Allow listening on Unix domain sockets

Hongli Lai November 17, 2009 05:56AM

Re: Allow listening on Unix domain sockets

Hongli Lai November 17, 2009 06:00AM

Re: Allow listening on Unix domain sockets

Igor Sysoev November 17, 2009 09:16AM

Re: [Patch] Allow listening on Unix domain sockets

Maxim Dounin October 23, 2009 06:44PM



Sorry, only registered users may post in this forum.

Click here to login

Online Users

Guests: 110
Record Number of Users: 6 on February 13, 2018
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready