[kamailio/kamailio] core: Add virtual flag to listen config to ignore inactive ip addresses (PR #2985)
<!-- Kamailio Pull Request Template -->
<!-- IMPORTANT: - for detailed contributing guidelines, read: https://github.com/kamailio/kamailio/blob/master/.github/CONTRIBUTING.md - pull requests must be done to master branch, unless they are backports of fixes from master branch to a stable branch - backports to stable branches must be done with 'git cherry-pick -x ...' - code is contributed under BSD for core and main components (tm, sl, auth, tls) - code is contributed GPLv2 or a compatible license for the other components - GPL code is contributed with OpenSSL licensing exception -->
#### Pre-Submission Checklist <!-- Go over all points below, and after creating the PR, tick all the checkboxes that apply --> <!-- All points should be verified, otherwise, read the CONTRIBUTING guidelines from above--> <!-- If you're unsure about any of these, don't hesitate to ask on sr-dev mailing list --> - [X] Commit message has the format required by CONTRIBUTING guide - [X] Commits are split per component (core, individual modules, libs, utils, ...) - [X] Each component has a single commit (if not, squash them into one commit) - [X] No commits to README files for modules (changes must be done to docbook files in `doc/` subfolder, the README file is autogenerated)
#### Type Of Change - [ ] Small bug fix (non-breaking change which fixes an issue) - [X] New feature (non-breaking change which adds new functionality) - [ ] Breaking change (fix or feature that would change existing functionality)
#### Checklist: <!-- Go over all points below, and after creating the PR, tick the checkboxes that apply --> - [ ] PR should be backported to stable branches - [X] Tested changes locally - [X] Related to issue #2984
#### Description
A new option to "listen" has been added called "virtual". This sets a flag on the listening socket to modify the behaviour of grep_sock_info. When this flag is set, grep_sock_info will only consider the listening IP a match if the IP is found in the system's current list of local IP addresses. If the IP is not currently local, then the matching IP is ignored.
If the virtual flag is not set on the socket then existing behaviour used instead.
This feature is intended for scenarios where you have two Kamailio nodes in active/active setup, with virtual IPs, where you need to be able to listen on a virtual IP but it may not always be active locally. With this flag applied to a `listen=` socket, Kamailio will only consider traffic destined to this IP local, when the IP is actually active on the system, and other times the match is ignored. You can view, comment on, or merge this pull request online at:
https://github.com/kamailio/kamailio/pull/2985
-- Commit Summary --
* core: listen can now have a "virtual" flag to check for nonlocal floating IPs. * etc: kamailio.cfg - added example use of listen's new virtual flag. * core: Added virtual flag to output of core.sockets_list * corex: Added virtual flag to output of "corex.list_sockets" * ims_ipsec_pcscf: Added virtual flag to output of sockets list.
-- File Changes --
M etc/kamailio.cfg (2) M src/core/cfg.lex (2) M src/core/cfg.y (83) M src/core/core_cmd.c (10) M src/core/ip_addr.h (1) M src/core/socket_info.c (85) M src/modules/corex/corex_rpc.c (5) M src/modules/ims_ipsec_pcscf/ims_ipsec_pcscf_mod.c (8)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/2985.patch https://github.com/kamailio/kamailio/pull/2985.diff
Some things I am concerned about/would like feedback on if possible: * Right now I'm using DNS caching but would like to not use this if possible, because it would delay how long it takes for Kamailio to know when active IPs change. I am just not sure if using `_resolvehost()` to find local IPs on each call is too heavy or not? * I am not sure if any other code would be caching results based on `grep_host_info` and expecting they never change? This is the only scenario I can see causing problems in future, but I can't see anywhere in the code where this is happening.
@rhys-hanrahan pushed 5 commits.
edb2dcbd6c04ea407c939c3510f263dd579557c1 core: listen can now have a "virtual" flag to check for nonlocal floating IPs. f3df3109667482b74013b1aaa37a741fce33b0ce etc: kamailio.cfg - added example use of listen's new virtual flag. bfbc1c53c0090b1dbc04d50d9dca8b5ba07c27fa core: Added virtual flag to output of core.sockets_list c24c9f48a7019b149dcf710228e3e5f89d517a87 corex: Added virtual flag to output of "corex.list_sockets" c051f4d6b743f77dfca1a06dd00d092ddc833d38 ims_ipsec_pcscf: Added virtual flag to output of sockets list.
Note, I have just re-based with a change to switch from using `dns_resolvehost` to `_resolvehost` to avoid DNS caching, as I think that's more sensible, unless suggested otherwise, for the intended use-case.
@rhys-hanrahan pushed 1 commit.
afe29cfaf45640780ced3d5792ecb14fb5ff2951 Merge branch 'kamailio:master' into add-listen-virtual
I haven't had time to analyze the source code, but the change in `etc/kamailio.cfg` should not be done, the new attribute has to be added to the core cookbook in the wiki (after the merge). The listen has other attributes (e.g., name), not all have to be listed in the default config file and the new one is probably not going to be commonly used.
Also, is it sure it is needed to be added in `ims_ipsec_pcscf`?
Hi Daniel, thanks for this! That's fine by me - both of those changes were just me trying to be thorough about making sure everything is being updated. Happy to remove the change to `kamailio.cfg`.
I don't mind if you don't want the flag added to `ims_ipsec_pcscf`, I just thought it would be good if this flag showed in the output, matching the other flags, for consistency? But if you think not, then I'm happy to remove this too.
@rhys-hanrahan pushed 4 commits.
3f26e912bed771895f52b3089c9b41da715580d6 core: Added virtual flag to output of core.sockets_list a2e43f6e89ab106a0dfdced96016d80655dcd23d corex: Added virtual flag to output of "corex.list_sockets" fc9a46886259bda741082a2e9a885eb6c860d65a ims_ipsec_pcscf: Added virtual flag to output of sockets list. ba55072ca6a7ab87c7fc594ddb1196c51884f4dd uac: fix unknown type name
Just confirming I have rebased and dropped the commit for the cfg file change. Let me know if you want me to drop the commit for `ims_ipsec_pcscf` too. Thanks!
Just wanted to add a note for one limitation I stumbled onto. As per this thread, if your hosts file contains an entry for your local machine name, then my method for listing local IPs does not work: https://stackoverflow.com/questions/1160963/how-to-enumerate-all-ip-addresse...
I just tested this out and confirmed it doesn't work. My machine had default entries for localhost only - so this code has worked fine in my testing. Right now I am just thinking of making a note of this requirement in the cookbook.
``` 127.0.0.1 localhost.localdomain localhost ::1 localhost6.localdomain6 localhost6 ```
If there's any suggestions for a better way of getting a list of local IPs I'm open to that, but so far I haven't seen an alternative that doesn't have the same issue and is multi-platform.
@rhys-hanrahan pushed 1 commit.
9bad0d10d312ee85912752baff3d45176bebc7c2 core: Updated check_local_addresses to use getifaddrs
@rhys-hanrahan pushed 1 commit.
98dd53abc4212d8bc333c4c79e54f7cdc0bd1f20 core: Updated check_local_addresses to use getifaddrs
Would be nice if this could disable (or at least, skip logging an error) for monitoring using OPTIONS in the same scenario where the server do listen to the required socket address. Currently we're relying on some external script enabling/disabling monitoring based on switchover, but that's not so elegant.
@mtryfoss This sounds like something I'd be interested in implementing. Can you provide some more specifics of what you're doing? E.g. Using nathelper or keepalive module? What error is being logged? I am guessing if you have two DMQ nodes that both of them would be sending OPTIONS packets for each usrloc entry, but the inactive node would be timing out, and these are the errors you mean?
I am still not live with Kamailio yet and I'm only using nathelper in my lab, and it looks like timeouts are disabled by default, so I guess this explains why I haven't run into any errors being logged due to OPTIONS ping packets, but this is something I'll have to investigate as well.
Thanks for your input!
I guess multiple modules could benefit from this, but we're using the dispatcher module to distribute calls to static destinations, while monitoring availability of them. I see this PR might not be directly related.
Anyway, if socket is specified in the dispatcher entry and that's not available locally it will throw a lot of errors in the log. Currently we use "kamcmd dispatcher.ping_active <1 or 0>", controlled by the software responsible for moving the VIP between hosts. But this needs to be re-executed after a Kamailio restart.
You'll have a similar issue if udp ping is enabled for registrations and you got those replicated from the live node, as you mentioned.
Aha, makes sense. Yes I think you're right, multiple modules can probably benefit - at least dispatcher and nathelper that I'll be using, I'm sure. Using dispatcher instead of forwarding requests to a single PBX is literally the next item on my list, so I'll be running into this scenario very soon I'm sure.
I will probably start looking at how I could handle this soon, not sure if I'll put it in this PR but probably put it in another PR at least, as it seems like a good fit for this flag.
Thanks, merging! For further enhancements, new PR can be created!
Merged #2985 into master.
-
Daniel-Constantin Mierla -
mtryfoss
-
rhys-hanrahan