Travis Cross wrote:
...
>> * TLS and IPv6 can be turned off individually. To be fair though, I
>> realized that the original tcp_dispatcher:get_listenerspecs/0 didn't
>> check for IPv6 being disabled either =).
>
> I thought about this, but in some ways it seemed that enable_v6 and
> tls_disable_server were redundant anyway when one is explicitly
> configuring the service / bind address tuples. Might it just be
> simpler if enable_v6 and tls_disable_server were removed, and users
> could enable ipv6 with:
>
> {listen_on, [{udp6,"0.0.0.0",5060},
> {tcp6,"0.0.0.0",5060}]},
>
> or disable tls just by leaving it out of the listen_on list? In any
> case, doing this would reduce the possibility of specifying
> conflicting or ambiguous configuration parameters.
Good thinking - the syntax would be {udp6, "::", 5060} though. What I'm
really missing here is
1) ability to listen to the same port(s) for all my IP addresses, be
they IPv4 or IPv6
2) sensible defaults
The sensible defaults I think should be as it is today, listen on tcp
and udp port 5060, and tls port 5061. I was going to say 'for both IPv4
and IPv6' but that wasn't actually the case today I noticed =). IPv6 _if
possible_ would be a good default.
>> * TLS is not included in siphost:default_listen_list/0.
>
> Good catch. TLS should indeed go in the default listen list.
>
>> * With your patch, it is possible to listen to multiple different ports
>> of a specific protocol. While this is a good thing in itself, it comes
>> with a whole battery of ... interesting things to look into. When
>> receiving a request where we decide we need to put the port number in
>> the Via header, we should probably take care and put the exact port
>> number we received the request over in there, and likewise when we are
>> routing responses we can no longer just look for a connection to the
>> right remote IP:port, but must make sure that it is also with the
>> right local IP:port.
>
> I did have a sense that something like this might be the case,
> though I wasn't quite sure where to start looking. I'll certainly
> dig a bit deeper now to see how this might be addressed. My feeling
> is that this could be fixed in a post-merge revision, as it doesn't
> break existing functionality, and users could by warned of this in
> the meantime.
It introduces a great way for users to shoot themselves in the foot, so
I think the cautious way forward with your patch would be to only accept
one port per protocol. This means only one port per tcp+tcp6 and
udp+udp6 and so on. Also, I think there is a requirement in RFC3261 to
listen to the same ports for UDP and TCP, but I'm not 100% on this.
Haven't looked.
>> Also, when contributing code it is very much appreciated if you really
>> _really_ try to keep to the same coding style. With your code, the
>> differences are mostly that you don't put spaces after commas and that
>> you format your 'case' statements different than me in a number of
>> places. I know it sucks to have to think about coding style when
>> contributing, but it works the other way around too, when I'm
>> contributing to someone elses project ;)
>
> That's certainly fair. When I was otherwise ready to post my
> patches, I went back through and cleaned up the documentation and
> formatting to better match the ambient style. I still missed some
> elements apparently ;) I'll go back through and clean this up further.
Thanks for not getting discouraged by this - I certainly didn't want you
to =).
...
> As Erlang is growing in
> popularity and use, I'm surprised that this limitation/bug has not
> yet become a major issue [1].
Agree.
> Even if inet:getiflist/0 and inet:ifget/2 worked appropriately [2],
> I believe that the capability to listen on service/address/port
> tuples selectively is a necessary and sound addition to YXA [3].
Definitely.
/Fredrik
_______________________________________________
Yxa-devel mailing list
[email protected]
https://lists.su.se/mailman/listinfo/yxa-devel
