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

Use keepalives instead of handshakes for timeouts #17

Conversation

tomgroenwoldt
Copy link
Contributor

This PR introduces the use of keepalives instead of handshakes via wireguard. Handshakes are made approx. every minute, keepalives are configurable. This way we can test a peer deletion without waiting for a handshake. This also allows us to introduce tighter timeout intervals, as the lower bound isn't fixed to one minute anymore.

@tomgroenwoldt tomgroenwoldt force-pushed the use-keep-alives-instead-of-handshake-for-timeout branch from 38bcb95 to 0fa0b1a Compare May 4, 2023 12:57
@tomgroenwoldt
Copy link
Contributor Author

I would like to straighten the altered main loop before merging. The idea of this MR is reviewable though.

interactive_test.kdl Outdated Show resolved Hide resolved
@tomgroenwoldt tomgroenwoldt force-pushed the use-keep-alives-instead-of-handshake-for-timeout branch from 0fa0b1a to f580f1b Compare May 4, 2023 22:17
@tomgroenwoldt
Copy link
Contributor Author

I didn't come up with an improvement for the new additions in the main loop. There's still the open question, whether we need a debounce or not, and I'm not sure.

Copy link
Owner

@svenstaro svenstaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be fine to take out the debounce.

src/main.rs Outdated Show resolved Hide resolved
loop {
if args.peer_timeout > Duration::ZERO && last_timeout_removal.elapsed() > timeout_debounce {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should still be possible to disable timeout entirely by setting the duration to zero. Please put this back in.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@tomgroenwoldt tomgroenwoldt force-pushed the use-keep-alives-instead-of-handshake-for-timeout branch from f580f1b to e4d7522 Compare May 7, 2023 12:45
@tomgroenwoldt
Copy link
Contributor Author

After the merge of this PR I will rework #11 .

@svenstaro svenstaro merged commit 65d668e into svenstaro:main May 7, 2023
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