Changed default outgoing_s2s_families to IPv6

[jpds]

Servers are within datacenters where IPv6 is more commonly enabled (contrary to clients), and if it's not present - it'll just fall back to IPv4.

[p1bot]

Hi @jpds, many thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes.

[p1bot]

You did it @jpds!

Thank you for signing the ProcessOne Contribution License Agreement.

We will have a look at your contribution!

[Neustradamus]

@badlop, @prefiks, @weiss: What do you think?

[nosnilmot]

Nothing in this PR actually changes the behavior of ejabberd, most of it is changes to things where order has no significance.

The default for outgoing_s2s_families is set here and not touched by this PR: https://github.com/processone/ejabberd/blob/3b50cd36ba7e915b13da24d33d4466e60a26bae1/src/ejabberd_options.erl#L614

[jpds]

@nosnilmot I reverted the changes, but I still feel they fit better with the documentation change as everything is consistent that way.

[coveralls]

Coverage increased (+0.03%) to 33.275% when pulling 9d37a86bdd26c8a72e03beb9a852f35fcb3c3985 on jpds:outgoing-s2s-families-ipv6-first into bc063b8f9734e523030290175e61cef2610d6f7e on processone:master.

[prefiks]

As is i don't think this change do anything, we really on data that we receive from dns srv requests for server (first sorting them by weights associated with each server and then really on order in which they are returned).

Values that are changed here are mostly used to filter ipv4/ipv6 addresses, so if you know you server can handle only one type you can tell it to completely ignore parts of them.

I could see some flag that would make us preffer one address above other, but i would also like to respect what dns srv records says (so probably only use that order for same weights?)

[nosnilmot]

I could see some flag that would make us preffer one address above other

That's exactly what outgoing_s2s_families is.

The (current) documentation for outgoing_s2s_families states:

Specify which address families to try, in what order. The default is [ipv4, ipv6] which means it first tries connecting with IPv4, if that fails it tries using IPv6.

This change is to switch to prefer IPv6 connections over IPv4, to bring the behavior more in line with the norm.

Arguably this should be left up to the OS to fully support address selection using RFC 3484 by using getaddrinfo(), but it looks like net:getaddrinfo/2 has only been available in Erlang since OTP 22.0+ (!)

SRV records are not relevant here, as they specify server priorities using hostname/port combinations, not IP addresses or address families. To try to achieve this using SRV records would require address-family specific host entries everywhere, which is not feasible or desirable.

[nosnilmot]

Arguably this should be left up to the OS to fully support address selection using RFC 3484 by using getaddrinfo()

https://github.com/processone/xmpp/pull/71 does just that for OTP 24.3+, which would remove the need for this PR

[badlop]

processone/xmpp#71 does just that for OTP 24.3+, which would remove the need for this PR

Ok, then now that processone/xmpp#71 is merged, this PR can be closed, right?

[nosnilmot]

Ok, then now that processone/xmpp#71 is merged, this PR can be closed, right?

correct - especially as processone/xmpp#72 (also merged) extends the use of getaddrinfo() to OTP 22+

[jpds]

I guess we'll need a PR that removes this user-facing option instead now.

[nosnilmot]

I guess we'll need a PR that removes this user-facing option instead now.

Only when ejabberd drops support for OTP < 22.

I'm sure a PR updating the ejabberd documentation to indicate this option is deprecated/irrelevant on OTP 22+ would be appreciated though.

[badlop]

I guess we'll need a PR that removes this user-facing option instead now.

Only when ejabberd drops support for OTP < 22.

I'm sure a PR updating the ejabberd documentation to indicate this option is deprecated/irrelevant on OTP 22+ would be appreciated though.

Something like this?

diff --git a/src/ejabberd_options_doc.erl b/src/ejabberd_options_doc.erl
index 03aa78fb2..2558991b4 100644
--- a/src/ejabberd_options_doc.erl
+++ b/src/ejabberd_options_doc.erl
@@ -969,7 +969,9 @@ doc() ->
         desc =>
             ?T("Specify which address families to try, in what order. "
                "The default is '[ipv4, ipv6]' which means it first tries "
-               "connecting with IPv4, if that fails it tries using IPv6.")}},
+               "connecting with IPv4, if that fails it tries using IPv6."
+               "This option is obsolete and irrelevant when using ejabberd 23.xx "
+               "and Erlang/OTP 22, or newer versions of them.")}},
      {outgoing_s2s_ipv4_address,
       #{value => "Address",
         note => "added in 20.12",

[nosnilmot]

Something like this?

yes, something like that :)