Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Remove some DNS lookups to support IPv6 #1934

Closed
wants to merge 2 commits into from

Conversation

silkeh
Copy link
Contributor

@silkeh silkeh commented Feb 22, 2017

The current implementation only does a lookup for an A record. This is troublesome for IPv6-enabled or IPv6-only servers. Removing the lookup lets Twisted do the resolving.

Note that the alternative (doing both A and AAAA lookups) is troublesome as this will break servers having only IPv4 or IPv6 connectivity unless the server's connectivity is also checked.

The current implementation only does a lookup for an A record.
This is troublesome for IPv6-enabled or IPv6-only servers.
Removing the lookup lets Twisted do the resolving.

Signed-off-by: Silke Hofstra <silke@slxh.eu>
@matrixbot
Copy link
Member

Can one of the admins verify this patch?

2 similar comments
@matrixbot
Copy link
Member

Can one of the admins verify this patch?

@matrixbot
Copy link
Member

Can one of the admins verify this patch?

Signed-off-by: Silke Hofstra <silke@slxh.eu>
@alejzeis
Copy link

Does this fix federations on dual-stack servers?

@silkeh
Copy link
Contributor Author

silkeh commented Mar 7, 2017

@jython234: I'm not aware of significant problems with dual stack servers (other than Synapse not binding on :: by default). This does fix federation to IPv6-only servers.

@14mRh4X0r
Copy link
Contributor

14mRh4X0r commented Mar 7, 2017

This patch breaks connections to many hosts. The default config causes servers to only listen on IPv4 (see #1886), but Synapse will try to connect over IPv6 for domains with an AAAA-record, yielding many "Connection refused" messages.

@erikjohnston
Copy link
Member

This is going to require us to properly retry different IP addresses when we get TCP level failures on connection, otherwise its (at the very least) going to cause delays as we back off the reconnects.

Closing for now, but reopen if you manage to fix that :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants