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