Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for TCP keepalive via configuration #854

Open
jq-rs opened this issue May 18, 2021 · 5 comments · May be fixed by #1075
Open

Add support for TCP keepalive via configuration #854

jq-rs opened this issue May 18, 2021 · 5 comments · May be fixed by #1075
Labels
feature New feature or request

Comments

@jq-rs
Copy link

jq-rs commented May 18, 2021

Is your feature request related to a problem? Please describe.

In many cases, unused TCP sessions are left to an ESTABLISHED state, e.g. by a firewall that drops the connection. The server endpoint has no way of identifying the situation if no traffic is sent over the TCP session. TCP keepalives RFC1122, sec. 4.2.3.6 is an existing mechanism that handles this issue and drops unused connections when they are identified.

This happens quite often on the internet with public servers so implementing a keepalive configuration mechanism is justified to avoid unnecessary resource consumption by hanging connections. Socket-level keepalives need to be enabled to be able to use OS-level keepalives.

Describe the solution you'd like
TCP sessions should allow keepalive configuration in general.

Describe alternatives you've considered

The alternative would be to implement some kind of monitoring script or similar that tries to identify unused connections from statistics. In most cases, this kind of heuristics is troublesome to implement correctly. Firewalls, like iptables, do not provide the means to drop unused but valid ESTABLISHED TCP connections.

Additional context

#1075 implements TCP keepalive via configuration (thx!)

@jq-rs jq-rs added the feature New feature or request label May 18, 2021
@shish
Copy link

shish commented Aug 20, 2021

I wonder if there's any way to set this flag without running a forked version of warp - right now I'm running a cronjob which restarts my service every 3 hours, because otherwise after 4 hours I hit 65535 ESTABLISHED connections and new connections start failing...

@seanmonstar
Copy link
Owner

You could also configure default keep alive at the operating system level.

@shish
Copy link

shish commented Aug 20, 2021

Searching for how to do that, all the sources I've found (eg [0]) say "you can tweak the default timeout at the OS level, but the application needs to opt-in to having keepalive enabled in the first place" :(

Alternatively if there's some other way to avoid leaking sockets, I'm all ears - I assume that's the end goal of this feature request either way? Briefly looking at eg nginx and varnish, they seem to be proactively closing idle connections after 75 or 60 seconds[1][2] rather than using TCP socket options, but I didn't see an option for warp to use the explicit-timeout approach either. (Apologies if this issue is specifically requesting TCP timeouts and other forms of idle-timeout are off-topic)

[0] https://tldp.org/HOWTO/TCP-Keepalive-HOWTO/usingkeepalive.html
[1] https://nginx.org/en/docs/http/ngx_http_core_module.html#keepalive_timeout
[2] https://varnish-cache.org/docs/6.6/reference/varnishd.html#ref-param-idle-send-timeout

@jq-rs
Copy link
Author

jq-rs commented Aug 20, 2021

Indeed, OS cannot really enable application-level keepalives as they are required to be by default off at the socket. The application needs to set them on explicitly. I tried this way first quite thoroughly.

@bburnichon bburnichon linked a pull request Oct 24, 2023 that will close this issue
@jq-rs
Copy link
Author

jq-rs commented Feb 7, 2024

Pulling #1075 would be a nice fix, reformatting issue.

@jq-rs jq-rs changed the title Add 2h keepalive for TCP sessions by default Add support for TCP keepalive via configuration Feb 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants