-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix DNS resolution and add IPV6 support to Akka.IO.Tcp #3063
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
* added IPV6 support to TCP sockets on Akka.IO
Sitting there thinking about it... UDP isn't going to be able to take advantage of the same design I added for TCP here with falling back between IPV4 / IPV6 upon resolution. We'd need to get an explicit connection failure notification back, which we won't under UDP. |
Had a spec failure with IPV6 on Linux .NET Core... Re-running to see if it's a timeout or something off with DNS on Linux .NET Core... |
Have a failure with IPV6 support on Mono on Linux now too... Looks like this is something at a lower level on the stack than the .NET runtime being used. Might be an issue with the version of Ubuntu we're using not supporting IPV6 properly on DNS (it's come up in the past.) Going to mute those specs on our Linux distributions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Described my changes in detail.
{ | ||
// Aaronontheweb, 9/2/2017 - POSIX-based OSES are still having trouble with IPV6 DNS resolution | ||
#if CORECLR | ||
if(!System.Runtime.InteropServices.RuntimeInformation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check for POSIX operating systems on .NET Core
.IsOSPlatform(OSPlatform.Windows)) | ||
return; | ||
#else | ||
if (RuntimeDetector.IsMono && family == AddressFamily.InterNetworkV6) // same as above |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check for Mono and bail.
@@ -57,7 +58,7 @@ private void ReportConnectFailure(Action thunk) | |||
} | |||
catch (Exception e) | |||
{ | |||
Log.Debug("Could not establish connection to [{0}] due to {1}", _connect.RemoteAddress, e); | |||
Log.Error(e, "Could not establish connection to [{0}].", _connect.RemoteAddress); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Log connection failures as errors.
@@ -24,7 +25,7 @@ internal sealed class TcpOutgoingConnection : TcpConnection | |||
private readonly Tcp.Connect _connect; | |||
|
|||
public TcpOutgoingConnection(TcpExt tcp, IActorRef commander, Tcp.Connect connect) | |||
: base(tcp, new Socket(connect.RemoteAddress.AddressFamily, SocketType.Stream, ProtocolType.Tcp) { Blocking = false }, connect.PullMode) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was the change that caused the most issues with DNS. Passing in a DnsEndpoint
with an unspecified AddressFamily
caused DNS to not work at all, which is how most people use it. We never specified the AddressFamily
in Akka.IO prior to 1.3.0 except on Mono to work around an issue where binding to an IPV6 socket was unsupported. That issue appears to be fixed in the more recent versions of Mono, although as I noted in this PR there appears to be issues at the OS level with DNS resolution and IPV6 still.
else | ||
Register(new IPEndPoint(resolved.Addr, remoteAddress.Port)); | ||
else if(resolved.Ipv4.Any() && resolved.Ipv6.Any()) // one of both families | ||
Register(new IPEndPoint(resolved.Ipv4.FirstOrDefault(), remoteAddress.Port), new IPEndPoint(resolved.Ipv6.FirstOrDefault(), remoteAddress.Port)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a fallback scheme here that only gets invoked when using DNS endpoints. If using DNS, first attempt to connect to a socket is done with IPV4 (default from earlier versions of Akka.IO)... If that fails, second attempt will swap and use IPV6. Third attempt will swap back to IPV4. Fourth will swap back to IPV6... Etc... Idea is to provide some help given the ambiguity of DNS resolution and the address family being used.
As I noted here #3064, this could be improved to use the explicit AddressFamily
provided on the DnsEndPoint
when it's available. We could also extend this in the future to support falling back through multiple resolved addresses until the right one is found. But I decided to leave well-enough alone for now.
@@ -228,7 +227,7 @@ private void ReportConnectFailure(Action thunk) | |||
} | |||
catch (Exception e) | |||
{ | |||
Log.Debug("Failure while connecting UDP channel to remote address [{0}] local address [{1}]: {2}", _connect.RemoteAddress, _connect.LocalAddress, e); | |||
Log.Error(e, "Failure while connecting UDP channel to remote address [{0}] local address [{1}]", _connect.RemoteAddress, _connect.LocalAddress); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Log connection failures as errors.
#endif | ||
var serverHandler = CreateTestProbe(); | ||
var bindCommander = CreateTestProbe(); | ||
bindCommander.Send(Sys.Tcp(), new Tcp.Bind(serverHandler.Ref, new IPEndPoint(family == AddressFamily.InterNetwork ? IPAddress.Loopback |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bind to an IP address which can be resolved via local DNS.
var boundMsg = bindCommander.ExpectMsg<Tcp.Bound>(); | ||
|
||
// setup client to connect | ||
var targetAddress = new DnsEndPoint("localhost", boundMsg.LocalAddress.AsInstanceOf<IPEndPoint>().Port); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Connect via DNS.
var testData = ByteString.FromString(str); | ||
clientEp.Tell(Tcp.Write.Create(testData, Ack.Instance), clientHandler); | ||
clientHandler.ExpectMsg<Ack>(); | ||
var received = serverHandler.ReceiveWhile<Tcp.Received>(o => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
received data from connected DNS endpoint following resolution.
Is this still work in progress @Aaronontheweb or is it ready to merge? |
I think it was ready. Additionally this PR added some behavior that is worth mentioning in the docs. |
@Horusiath this is all finished @Danthar I agree that the docs need some love for Akka.IO.... IMHO most important thing to add is the workflow for how you set up a connection in the first place. |
Still working on UDP