Bug 37087 - Add support for TCP keepalive to SIP server
Summary: Add support for TCP keepalive to SIP server
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: SIP2 (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: David Cook
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords:
Depends on:
Blocks: 37696
  Show dependency tree / graph
 
Reported: 2024-06-14 02:33 UTC by David Cook
Modified: 2024-09-12 22:12 UTC (History)
3 users (show)

See Also:
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
Circulation function:

Attachments
Bug 37087: Add TCP keepalive support to SIP server (7.49 KB, patch)
2024-06-14 04:41 UTC, David Cook 		Details | Diff | Splinter Review
Bug 37087: Add TCP keepalive support to SIP server (7.64 KB, patch)
2024-06-14 13:04 UTC, HKS3 Tadeusz Sośnierz 		Details | Diff | Splinter Review
Bug 37087: Add TCP keepalive support to SIP server (7.71 KB, patch)
2024-07-26 07:11 UTC, Martin Renvoize (ashimema) 		Details | Diff | Splinter Review
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description David Cook 2024-06-14 02:33:16 UTC 
Some computing platforms like Microsoft Azure (and some corporate firewalls) enforce idle timeouts which silently kill long-running/persistent TCP connections. In the case of Microsoft Azure, this idle timeout occurs after 4 minutes. 

In production, this has been massively disruptive to circulation operations using self-checkout kiosks that connect to Koha using SIP2. 

Fortunately, this problem is easily solvable by using TCP keepalives. When TCP keepalives are used, after X seconds a TCP ACK message will be sent from the SIP server to the SIP client, which will then send back a TCP ACK. Nothing happens at the application level. It's just at the lower network level. And this prevents idle timeouts. 

I'll provide more technical information in code comments.

--

I've already successfully implemented this strategy in Azure for managing persistent Elasticsearch TCP connections simply by tweaking the "net.ipv4.tcp_keepalive_time" kernel parameter, which is automatically used by the HTTP::Tiny connection library used by Search::Elasticsearch, since it implements the SO_KEEPALIVE socket option out of the box by default. 

With the SIP server, we have a lot more control than we do with third-party software, so I'm planning to implement a more fine-grained approach, which won't require any kernel parameter changes. Instead, it will be manageable by application configuration alone. (Note: this is partially because testing in koha-testing-docker would be difficult where it's not easy/possible to tweak kernel parameters.)
Comment 1 David Cook 2024-06-14 02:33:52 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.
Comment 2 David Cook 2024-06-14 02:34:28 UTC 
(FYI: note that Net::Stomp and Net::LDAP also have out-of-the-box support for SO_KEEPALIVE.)
Comment 3 David Cook 2024-06-14 02:35:53 UTC 
FYI: Keep in mind that TCP keepalive is different but related to HTTP keepalive.
Comment 4 David Cook 2024-06-14 04:41:21 UTC 
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
Comment 5 David Cook 2024-06-14 04:42:09 UTC 
If anyone has any questions, I've researched this extensively, including reviewing the Linux kernel.
Comment 6 David Cook 2024-06-14 04:50:04 UTC 
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...
Comment 7 HKS3 Tadeusz Sośnierz 2024-06-14 13:04:04 UTC 
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>
Comment 8 David Cook 2024-06-19 05:39:12 UTC 
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.
Comment 9 Martin Renvoize (ashimema) 2024-07-26 07:11:18 UTC 
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>
Comment 10 Martin Renvoize (ashimema) 2024-07-26 07:12:10 UTC 
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
Comment 11 David Cook 2024-08-20 01:52:36 UTC 
RM: let me know if you need anything more for this one.

Just added some release note text...
Comment 12 Katrin Fischer 2024-08-20 07:46:38 UTC 
(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.
Comment 13 Katrin Fischer 2024-08-20 12:53:05 UTC 
(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
Comment 14 Katrin Fischer 2024-08-20 13:33:48 UTC 
Pushed for 24.11!

Well done everyone, thank you!
Comment 15 Katrin Fischer 2024-08-21 12:37:38 UTC 
Thanks for bug 37696, removing additional_work_needed :)
Comment 16 Lucas Gass (lukeg) 2024-09-12 22:12:02 UTC 
Enhancement will not be backported to 24.05.x