Skip to content

Add support for tcp keepalive through socket2#981

Draft
sosthene-nitrokey wants to merge 1 commit into
algesten:mainfrom
Nitrokey:socket2
Draft

Add support for tcp keepalive through socket2#981
sosthene-nitrokey wants to merge 1 commit into
algesten:mainfrom
Nitrokey:socket2

Conversation

@sosthene-nitrokey

@sosthene-nitrokey sosthene-nitrokey commented Feb 3, 2025

Copy link
Copy Markdown
Contributor

Not fully implemented, probably should be using a custom structure for TcpKeepalive to avoid exposing socket2 publicly.

In that case, should we have the same feature flag as the socket2 library? Or should we allow each setting and just warn when one is not available for the given platform ?

Fixes #547

Not fully implemented, probably should be using a custom
structure for `TcpKeepalive` to avoid exposing `socket2` publicly.

In that case, should we have the same feature flag as the `socket2` library?
Or should we allow each setting and just warn when one is not available for the
given platform ?
@algesten

algesten commented Feb 3, 2025

Copy link
Copy Markdown
Owner

Are there other things we might want to tweak using socket2? The only thing I'm hesitating about is the name of the keepalive since we might want one more generic tcp-extra or something.

If this lands in std::net eventually, we can make the feature flags a no-op, but it'd be nice to not have many many flags in that case.

@JPDye

JPDye commented Dec 17, 2025

Copy link
Copy Markdown
Contributor

I think tcp-extra or socket-opts is a better feature flag name as I can see support for setting TCP_NODELAY being useful, especially since ureq seems to write requests in a way that is known to occassionally cause issues when Nagle's algorithm is on the client and delayed acknowledgements are used on the server (writing header and body in two separate calls).

EDIT:
I see TCP_NODELAY is already possible to set. Similar logic applies for wanting to set TCP_QUICKACK and others.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add TCP keep-alive?

3 participants