[kamailio] rr: add enable_double_rr_always option (#257)
This patch helps us to handle inadequatness of Linphone mobile apps (both iOS and Android flavours are affected) without reinventing a lot of logics to handle each transport (we use TCP, TLS, WS, WSS). E.g. if we have two TLS-based Linphone clients, even with enable_double_rr for gentle cross-forwarding we would get single RR header without transport= attribute, resulting in Linphone sending ACK and BYE via UDP. This new option, being enabled, results in two RR headers being always inserted, with transport attribute being always added. I believe this cannot harm the installation with enable_double_rr, just makes instructions more explicit. The Linphone's behaviour looks like a set of different bugs, but it would get us much more efforts to fix. I don't know if any other useragent needs such workaround. So I would perfectly understand if this commit wouldn't be accepted upstream. Also this commit currently lacks README doc rebuild, because I don't know how to do that. Only XML doc is updated. You can view, comment on, or merge this pull request online at: https://github.com/kamailio/kamailio/pull/257 -- Commit Summary -- * rr: add enable_double_rr_always option -- File Changes -- M modules/rr/doc/rr_admin.xml (24) M modules/rr/record.c (14) M modules/rr/rr_mod.c (2) M modules/rr/rr_mod.h (1) M tcp_read.c (5) -- Patch Links -- https://github.com/kamailio/kamailio/pull/257.patch https://github.com/kamailio/kamailio/pull/257.diff --- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/257
For clarification enable_double_rr_always still requires that enable_double_rr is set, right? I wonder that instead of adding a new parameter won't be better to consider a new value for enable_double_rr, like: * 0 - double rr disabled * 1 - conditional double based on different sockets for incoming and outgoing (what is now) * 2 - always double rr (what is added by this patch) --- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/257#issuecomment-122935187
For clarification enable_double_rr_always still requires that enable_double_rr is set, right? Yes. This decreases number of extended codelines (no need to change lines with "if (enable_double_rr)").
I agree with your proposition to use new value of enable_double_rr. I will rework the patch soon. --- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/257#issuecomment-122941760
Updated code as agreed, also removed accidentally included unrelated debug code. Please not that there's still no update to plain-text README, just doc xml is updated. Feel free to amend this commit for this. --- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/257#issuecomment-123882881
Please wait before merging this... Retesting... --- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/257#issuecomment-124507238
It turned out that simple record_route() with enable_double_rr=1 (which is default) solves likely all the issues. We have met our problems because we used record_route_preset() initially, then moved to record_route_advertised_address(), but record_route_advertised_address() (with enable_double_rr=1) doesn't append transport attribute when the call is from TLS to WS (record_route() does). So for now, we need either to use this patch, or to investigate why this happens, or to investigate why record_route doesn't pick advertise_address (it really doesn't for tcp and tls connections; for websocket-based it does). Or to use record_route() and just live with Record-Route headers containing internal IP of Kamailio server (it seems to work anyway). --- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/257#issuecomment-124533780
I think worth merging the patch as it is an option anyhow -- if not enabled, it doesn't change anything -- who knows when it can come handy with some UAs. --- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/257#issuecomment-126044105
Ok. Thank you. --- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/257#issuecomment-126064705
Merged #257. --- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/257#event-369303250
Damn, sorry for unrelated change in tcp_read.c in my commit. Thank you for fixing that. --- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/257#issuecomment-127339688
Hello, I have a similar problem (ack problem) Here is the call flow : linphone (tls) --> (tls) kamailio (udp) --> (udp) asterisk I set up kamailio like as : modparam("rr", "enable_full_lr", 1) modparam("rr", "append_fromtag", 1) modparam("rr", "enable_double_rr", 2) ------------------------ kamailio -v version: kamailio 4.4.0-pre0 (x86_64/linux) f39d14 flags: STATS: Off, USE_TCP, USE_TLS, USE_SCTP, TLS_HOOKS, USE_RAW_SOCKS, DISABLE_NAGLE, USE_MCAST, DNS_IP_HACK, SHM_MEM, SHM_MMAP, PKG_MALLOC, Q_MALLOC, F_MALLOC, TLSF_MALLOC, DBG_SR_MEMORY, USE_FUTEX, FAST_LOCK-ADAPTIVE_WAIT, USE_DNS_CACHE, USE_DNS_FAILOVER, USE_NAPTR, USE_DST_BLACKLIST, HAVE_RESOLV_RES ADAPTIVE_WAIT_LOOPS=1024, MAX_RECV_BUFFER_SIZE 262144, MAX_LISTEN 16, MAX_URI_SIZE 1024, BUF_SIZE 65535, DEFAULT PKG_SIZE 8MB poll method support: poll, epoll_lt, epoll_et, sigio_rt, select. id: f39d14 compiled on 13:25:30 Feb 12 2016 with gcc 4.9.1 ------------------------ But I have still the problem. Any help ? Thank you --- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/257#issuecomment-184411651
participants (3)
-
africallshop -
Andrey Utkin
-
Daniel-Constantin Mierla