Summary: | Add support for TCP keepalive to SIP server | ||
---|---|---|---|
Product: | Koha | Reporter: | David Cook <dcook> |
Component: | SIP2 | Assignee: | David Cook <dcook> |
Status: | RESOLVED FIXED | QA Contact: | Martin Renvoize (ashimema) <martin.renvoize> |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | alexbuckley, fridolin.somers, lucas, martin.renvoize, tadeusz |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: |
This change adds 3 new configuration options to the SIPconfig.xml. These are custom_tcp_keepalive, custom_tcp_keepalive_time, and custom_tcp_keepalive_intvl. Usage is documented in C4/SIP/SIPServer.pm. They are used to control TCP keepalives for the SIP server. Configuration of these parameters is essential for running a SIP server in Microsoft Azure.
|
Version(s) released in: |
24.11.00,24.05.06
|
Circulation function: | |||
Bug Depends on: | |||
Bug Blocks: | 37696 | ||
Attachments: |
Bug 37087: Add TCP keepalive support to SIP server
Bug 37087: Add TCP keepalive support to SIP server Bug 37087: Add TCP keepalive support to SIP server |
Description
David Cook
2024-06-14 02:33:16 UTC
I've already written and tested the code for a MVP and it's working as expected. Now it's just about writing the code in a way that is appropriate for the Koha community. I hope to have this ready very shortly. (FYI: note that Net::Stomp and Net::LDAP also have out-of-the-box support for SO_KEEPALIVE.) FYI: Keep in mind that TCP keepalive is different but related to HTTP keepalive. Created attachment 167706 [details] [review] Bug 37087: Add TCP keepalive support to SIP server This change adds the ability to enable and configure TCP keepalive support for the SIP server using SIPconfig.xml. For the sake of backwards compatibility, it defaults to disabled and additional parameters default match typical kernel defaults. Technical detail can be found in the perldoc for C4/SIP/SIPserver.pm Test plan: 0. Apply the patch 1. koha-sip --restart kohadev 2. apt-get update && apt-get install tcpdump 3. In one window, run "tcpdump -A -n -v -i any 'port 6001'" 4. In another window, run the following: echo -e "9300CNterm1|COterm1|CPCPL|\r" | nc 127.0.0.1 6001 -v 5. Note in tcpdump output that after the initial flood of packets, nothing more is received 6. vi /etc/koha/sites/kohadev/SIPconfig.xml 7. In the "server-params" element, add attributes like the following: custom_tcp_keepalive='1' custom_tcp_keepalive_time='10' custom_tcp_keepalive_intvl='5' 8. koha-sip --restart kohadev 9. In one window, run "tcpdump -A -n -v -i any 'port 6001'" 10. In another window, run the following: echo -e "9300CNterm1|COterm1|CPCPL|\r" | nc 127.0.0.1 6001 -v 11. Note in tcpdump output that after the initial flood of packets, ACK packets are sent out every 10+ seconds for the idle connection If anyone has any questions, I've researched this extensively, including reviewing the Linux kernel. I'm planning to implement this patch in prod next week, as we need the fix ASAP. I've implemented it in the way that I think is best/cleanest for the SIP server. I can understand if people would prefer a per-listener approach for maximum flexibility. However, if folk wanted to go that route, I'd suggest refactoring how we currently handle client_timeout, since post_accept_hook would be a better place than process_request to fetch/set that config. If people don't have strong feelings, we can also always just do it this way, and refactor later if necessary... Created attachment 167713 [details] [review] Bug 37087: Add TCP keepalive support to SIP server This change adds the ability to enable and configure TCP keepalive support for the SIP server using SIPconfig.xml. For the sake of backwards compatibility, it defaults to disabled and additional parameters default match typical kernel defaults. Technical detail can be found in the perldoc for C4/SIP/SIPserver.pm Test plan: 0. Apply the patch 1. koha-sip --restart kohadev 2. apt-get update && apt-get install tcpdump 3. In one window, run "tcpdump -A -n -v -i any 'port 6001'" 4. In another window, run the following: echo -e "9300CNterm1|COterm1|CPCPL|\r" | nc 127.0.0.1 6001 -v 5. Note in tcpdump output that after the initial flood of packets, nothing more is received 6. vi /etc/koha/sites/kohadev/SIPconfig.xml 7. In the "server-params" element, add attributes like the following: custom_tcp_keepalive='1' custom_tcp_keepalive_time='10' custom_tcp_keepalive_intvl='5' 8. koha-sip --restart kohadev 9. In one window, run "tcpdump -A -n -v -i any 'port 6001'" 10. In another window, run the following: echo -e "9300CNterm1|COterm1|CPCPL|\r" | nc 127.0.0.1 6001 -v 11. Note in tcpdump output that after the initial flood of packets, ACK packets are sent out every 10+ seconds for the idle connection Signed-off-by: Tadeusz „tadzik” Sośnierz <tadeusz@sosnierz.com> If your SIP server is behind stunnel, you'll need to add at least the following to your stunnel configuration as well: socket = l:SO_KEEPALIVE=1 socket = r:SO_KEEPALIVE=1 If you've tweaked your kernel params at the OS level, then you can leave it there. Otherwise, you'd also need to add stunnel config for TCP_KEEPIDLE and possibly TCP_KEEPINTVL. Created attachment 169642 [details] [review] Bug 37087: Add TCP keepalive support to SIP server This change adds the ability to enable and configure TCP keepalive support for the SIP server using SIPconfig.xml. For the sake of backwards compatibility, it defaults to disabled and additional parameters default match typical kernel defaults. Technical detail can be found in the perldoc for C4/SIP/SIPserver.pm Test plan: 0. Apply the patch 1. koha-sip --restart kohadev 2. apt-get update && apt-get install tcpdump 3. In one window, run "tcpdump -A -n -v -i any 'port 6001'" 4. In another window, run the following: echo -e "9300CNterm1|COterm1|CPCPL|\r" | nc 127.0.0.1 6001 -v 5. Note in tcpdump output that after the initial flood of packets, nothing more is received 6. vi /etc/koha/sites/kohadev/SIPconfig.xml 7. In the "server-params" element, add attributes like the following: custom_tcp_keepalive='1' custom_tcp_keepalive_time='10' custom_tcp_keepalive_intvl='5' 8. koha-sip --restart kohadev 9. In one window, run "tcpdump -A -n -v -i any 'port 6001'" 10. In another window, run the following: echo -e "9300CNterm1|COterm1|CPCPL|\r" | nc 127.0.0.1 6001 -v 11. Note in tcpdump output that after the initial flood of packets, ACK packets are sent out every 10+ seconds for the idle connection Signed-off-by: Tadeusz „tadzik” Sośnierz <tadeusz@sosnierz.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> QA script doesn't like the head level of the POD here.. but there's basically no other POD in the document.. I think this is fine to let through. Code works and is optional. Passing QA RM: let me know if you need anything more for this one. Just added some release note text... (In reply to David Cook from comment #11) > RM: let me know if you need anything more for this one. > > Just added some release note text... Thanks David. I push in sequence as it's shown on the Dashboard. Getting close to this one. (In reply to Katrin Fischer from comment #12) > (In reply to David Cook from comment #11) > > RM: let me know if you need anything more for this one. > > > > Just added some release note text... > > Thanks David. I push in sequence as it's shown on the Dashboard. Getting > close to this one. On second thought, would not mind a follow-up making the pod validation happier: FAIL C4/SIP/SIPServer.pm FAIL pod *** WARNING: =head3 without preceding higher level in file C4/SIP/SIPServer.pm *** WARNING: =head3 without preceding higher level in file C4/SIP/SIPServer.pm *** WARNING: =head3 without preceding higher level in file C4/SIP/SIPServer.pm Pushed for 24.11! Well done everyone, thank you! Thanks for bug 37696, removing additional_work_needed :) Enhancement will not be backported to 24.05.x Noting: On the Mattermost town square channel someone has requested that this Bug 37087 (https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37087) be considered for backport to 24.05 in the next round of backports. We have noted to consider its backportability in the January rounds of backports :) (In reply to Alex Buckley from comment #17) > Noting: On the Mattermost town square channel someone has requested that > this Bug 37087 > (https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37087) be > considered for backport to 24.05 in the next round of backports. > > We have noted to consider its backportability in the January rounds of > backports :) It should be an easy backport. If anyone is having trouble, happy to lend a hand on this one. Backported to 24.05.x for 24.05.06 Requested in Mattermost Town Square - see comment #17 There is a request in the Mattermost Town Square for this patchset to be backported to 23.11 - see https://chat.koha-community.org/koha-community/pl/h5k3i77jwfft3q315txh9btxdy (In reply to Alex Buckley from comment #20) > There is a request in the Mattermost Town Square for this patchset to be > backported to 23.11 - see > https://chat.koha-community.org/koha-community/pl/h5k3i77jwfft3q315txh9btxdy Mmmm, I fell like this can impact the SIP server at deep level. Since selfcheckout machines are a very used feature and 23.11 is now a very stable branch, I prefer not backport to avoid any unseen impact. (In reply to Fridolin Somers from comment #21) > (In reply to Alex Buckley from comment #20) > > There is a request in the Mattermost Town Square for this patchset to be > > backported to 23.11 - see > > https://chat.koha-community.org/koha-community/pl/h5k3i77jwfft3q315txh9btxdy > > Mmmm, I fell like this can impact the SIP server at deep level. > Since selfcheckout machines are a very used feature and 23.11 is now a very > stable branch, I prefer not backport to avoid any unseen impact. It can have a significant impact at a low/deep level, but for existing systems (ie upgrades), there's no config change made to SIPconfig.xml, so the functionality isn't activated. Most of the patch is just documentation. If you look at the functions, it's a small amount of code, which I think is clearly explained. I've been running this patch in production with 23.11.x since June 2024 with no problems, and it's essential/critical for running a SIP server in the Azure cloud. (In reply to Fridolin Somers from comment #21) > Mmmm, I fell like this can impact the SIP server at deep level. > Since selfcheckout machines are a very used feature and 23.11 is now a very > stable branch, I prefer not backport to avoid any unseen impact. If anything isn't clear about the change, I'm happy to explain it. Overall, it should be a very safe change to make, since it doesn't do anything without adding configuration manually. And even when it's turned on (using custom_tcp_keepalive=1), it will use the default kernel parameters. $ sysctl -a | grep "keepalive" net.ipv4.tcp_keepalive_intvl = 75 net.ipv4.tcp_keepalive_probes = 9 net.ipv4.tcp_keepalive_time = 7200 With these defaults, it won't send a TCP ACK until 2 hours of idle time have passed. If that TCP ACK isn't ACKed back, it will then send up to 9 more ACKs in intervals of 75 seconds. Keep in mind that these are Transport layer packets. They don't contain any actual data in them, so the selfcheckout application and the Koha SIP server application have zero knowledge about them. It's just OS kernel to OS kernel communication. The main use case being to prevent firewalls like Azure's firewall and other corporate firewalls (like Fortinet) from dropping idle persistent SIP TCP connections. OK thanks for the details. I also feel like 23.11.x is too old to have enhancements. Keeping them in upper version encourages people to upgrade, mainly there are several security improvements. And any advanced installation can apply this patch to have the feature. (In reply to Fridolin Somers from comment #24) > OK thanks for the details. > I also feel like 23.11.x is too old to have enhancements. > Keeping them in upper version encourages people to upgrade, mainly there are > several security improvements. > And any advanced installation can apply this patch to have the feature. Hard to argue with that! |