IPv6 address parsing + IPv6 lookup for FastCGI and reverse proxy #175

Merged
merged 2 commits into from Apr 16, 2014

Projects

None yet

2 participants

@dumbbell

This pull request contains two commits to improve IPv6 support for communication between Yaws and FastCGI backends/reverse proxies:

  1. Support IPv6 address in reverse proxy URLs and as FastCGI server names, using the [...] syntax.
  2. Allow the user to choose how FastCGI or reverse proxy server names are resolved and connections are made by setting the list of address families to try (e.g. [inet6, inet] if he wants IPv6 first; [inet] remains the default to keep the current behavior).

We need this to play with HHVM: on Linux, we set the net.ipv6.bindv6only sysctl to 1 and, in this case, HHVM only listen on ::1, not 127.0.0.1. It appears to not be configurable.

@vinoski
Collaborator
vinoski commented Apr 14, 2014

Thanks, I'm currently reviewing these changes.

@vinoski vinoski self-assigned this Apr 14, 2014
@dumbbell

Christopher reminds me I forgot to update yaws:setup_gconf/2 function. I'm fixing this and will force-push an amended commit (the second one).

@vinoski
Collaborator
vinoski commented Apr 14, 2014

I'd also like to see some tests for these features included in this pull request, in part to help ensure future changes don't break them.

@dumbbell dumbbell Accept an IPv6 between "[...]" as an fcgi server hostname
Here are some examples:
    php_handler = <fcgi, [::1]:9000>
    fcgi_app_server = [::1]:9000

The square brackets syntax is already used in URLs (http://[::1]:8080/)
and allows to mix an IPv6 address and a port number.

While here, fix a bug in revproxy URL parsing where IPv6 between
brackets were considered as a syntax error.
2e662ff
@dumbbell

Thanks, I fixed the regex as you suggested (though I didn't push anything yet).

Regarding the testsuite, I'm willing to add something, but I don't see how to thoroughly test IPv6 connection. For instance, on Debian, a process which listens on ::1 implicitly listens on 127.0.0.1 by default. This is controlled by the net.ipv6.bindv6only sysctl. So if the testsuite uses http://localhost:8006/ as a revproxy backend and the backend listens on ::1 only, it's reachable through IPv4 anyway. Therefore, such a test can't tell if lookup and connection honored nslookup_pref.

However, I can add a test to check that http://[::1]:8006/ is accepted.

@dumbbell dumbbell Add a nslookup_pref global parameter
This parameter allows to change the name resolution and connection
preference for fcgi servers and revproxy URLs. It takes a list of the
form [inet | inet6].

For instance, to perform only IPv4 resolution:
    nslookup_pref = [inet]

To perform both IPv4 and IPv6, but try IPv6 first:
    nslookup_pref = [inet6, inet]

The default value is [inet]. Therefore, the behavior remains the same
with this addition.

This parameter is used by two new functions:
    o  yaws:tcp_connect/{3,4}
    o  yaws:ssl_connect/{3,4}

They are wrappers around gen_tcp:connect/{3,4} and ssl:connect/{3,4} and
take care of connection retries for each configured family.
53fdbfe
@vinoski
Collaborator
vinoski commented Apr 15, 2014

I agree that making the tests thorough would be difficult, but I should have specified more clearly that I was mostly interested in tests for the config parsing code. If you can add that sort of test, that would be great.

Feel free to push an amended commit for these changes.

@dumbbell

Ok, I pushed the regex fix and a new test, called test_ipv6_address, in t4. In this test, the backend URL uses an IPv6 address between brackets. At least, it verifies that this syntax is accepted.

@vinoski vinoski merged commit 1ff49f3 into klacke:master Apr 16, 2014
@vinoski
Collaborator
vinoski commented Apr 16, 2014

Looks great, thanks!

@dumbbell dumbbell deleted the yakaz:fcgi-ipv6 branch Apr 17, 2014
@dumbbell

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment