Skip to content

Fix TCP keepalive handling on Haiku and OpenBSD #251

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

Merged
merged 1 commit into from
Jul 28, 2021
Merged

Fix TCP keepalive handling on Haiku and OpenBSD #251

merged 1 commit into from
Jul 28, 2021

Conversation

link2xt
Copy link
Contributor

@link2xt link2xt commented Jul 27, 2021

On Haiku and OpenBSD don't call setsockopt(fd, IPPROTO_TCP, SO_KEEPALIVE, secs), it is incorrect because SO_KEEPALIVE belongs
to SOL_SOCKET layer, because it accepts a boolean instead of the
number of seconds, and because there is no way to set keepalive idle
time per socket on these operating systems.

On Haiku and OpenBSD it is only possible to enable and disable
keepalives, and it is already done with setsockopt(fd, SOL_SOCKET, SO_KEEPALIVE, 1) in a portable way in Socket.set_keepalive()
defined in src/socket.rs.

On Haiku and OpenBSD don't call `setsockopt(fd, IPPROTO_TCP,
SO_KEEPALIVE, secs)`, it is incorrect because `SO_KEEPALIVE` belongs
to `SOL_SOCKET` layer, because it accepts a boolean instead of the
number of seconds, and because there is no way to set keepalive idle
time per socket on these operating systems.

On Haiku and OpenBSD it is only possible to enable and disable
keepalives, and it is already done with `setsockopt(fd, SOL_SOCKET,
SO_KEEPALIVE, 1)` in a portable way in `Socket.set_keepalive()`
defined in `src/socket.rs`.
@Thomasdezeeuw
Copy link
Collaborator

Thanks for looking into this. Does this fix #85?

@Thomasdezeeuw Thomasdezeeuw merged commit b0732a3 into rust-lang:master Jul 28, 2021
@link2xt
Copy link
Contributor Author

link2xt commented Jul 28, 2021

Does this fix #85?

No, there are still two other test failures.

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.

2 participants