Hi Anton,
First of all, thanks for the patches!
Just want to let you know that we have a ticket in our internal
system assigned your patches to the developer for review but it'll
take some time due to other tasks and complexity of the patch series.
On 7/10/13 9:17 PM, ToSHiC wrote:
> Hello,
>
> I've split this big patch into several small patches, taking into
> account your comments. I'll send each part in separate email. Here
> is the first one.
>
> commit 597d09e7ae9247c5466b18aa2ef3f5892e61b708
> Author: Anton Kortunov <toshic.toshic@gmail.com
> <mailto:toshic.toshic@gmail.com>>
> Date: Wed Jul 10 13:14:52 2013 +0400
>
> Added new structure ngx_ipaddr_t
>
> This structure contains family field
> and the union of ipv4/ipv6 structures in_addr_t and in6_addr.
>
> diff --git a/src/core/ngx_inet.h b/src/core/ngx_inet.h
> index 6a5a368..077ed34 100644
> --- a/src/core/ngx_inet.h
> +++ b/src/core/ngx_inet.h
> @@ -68,6 +68,16 @@ typedef struct {
>
>
> typedef struct {
> + ngx_uint_t family;
> + union {
> + in_addr_t v4;
> +#if (NGX_HAVE_INET6)
> + struct in6_addr v6;
> +#endif
> + } u;
> +} ngx_ipaddr_t;
> +
> +typedef struct {
> struct sockaddr *sockaddr;
> socklen_t socklen;
> ngx_str_t name;
>
>
>
> On Mon, Jun 17, 2013 at 7:30 PM, Maxim Dounin <mdounin@mdounin.ru
> <mailto:mdounin@mdounin.ru>> wrote:
>
> Hello!
>
> On Fri, Jun 14, 2013 at 09:44:46PM +0400, ToSHiC wrote:
>
> > Hello,
> >
> > We needed this feature in our company, I found that it is in
> milestones of
> > version 1.5 but doesn't exist yet. So I've implemented it
> based in 1.3 code
> > and merged in current 1.5 code. When I wrote this code I
> mostly cared about
> > minimum intrusion into other parts of nginx.
> >
> > IPv6 fallback logic is not a straightforward implementation of
> suggested by
> > RFC. RFC states that IPv6 resolving have priority over IPv4,
> and it's not
> > very good for Internet we have currently. With this patch you
> can specify
> > priority, and in upstream and mail modules I've set IPv4 as
> preferred
> > address family.
> >
> > Patch is pretty big and I hope it'll not break mailing list or
> mail clients.
>
> You may want to try to split the patch into smaller patches to
> simplify review. See also some hints here:
>
> http://nginx.org/en/docs/contributing_changes.html
>
> Some quick comments below.
>
> [...]
>
> > - addr = ntohl(ctx->addr);
> > +failed:
> > +
> > + //addr = ntohl(ctx->addr);
> > + inet_ntop(ctx->addr.family, &ctx->addr.u, text,
> > NGX_SOCKADDR_STRLEN);
> >
> > ngx_log_error(NGX_LOG_ALERT, r->log, 0,
> > - "could not cancel %ud.%ud.%ud.%ud
> resolving",
> > - (addr >> 24) & 0xff, (addr >> 16) & 0xff,
> > - (addr >> 8) & 0xff, addr & 0xff);
> > + "could not cancel %s resolving", text);
>
> 1. Don't use inet_ntop(), there is ngx_sock_ntop() instead.
>
> 2. Don't use C++ style ("//") comments.
>
> 3. If some data is only needed for debug logging, keep relevant
> calculations under #if (NGX_DEBUG).
>
> [...]
>
> > @@ -334,6 +362,7 @@
> > ngx_http_upstream_create_round_robin_peer(ngx_http_request_t *r,
> > peers->peer[i].current_weight = 0;
> > peers->peer[i].max_fails = 1;
> > peers->peer[i].fail_timeout = 10;
> > +
> > }
> > }
> >
>
> Please avoid unrelated changes.
>
> [...]
>
> --
> Maxim Dounin
> http://nginx.org/en/donation.html
>
> _______________________________________________
> nginx-devel mailing list
> nginx-devel@nginx.org <mailto:nginx-devel@nginx.org>
> http://mailman.nginx.org/mailman/listinfo/nginx-devel
>
>
>
>
> _______________________________________________
> nginx-devel mailing list
> nginx-devel@nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel
>
--
Maxim Konovalov
+7 (910) 4293178
http://nginx.com/services.html
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
