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 Yxa-devel@lists.su.se https://lists.su.se/mailman/listinfo/yxa-devel