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

secondary server, mirage layer #347

Merged
merged 3 commits into from Dec 6, 2023
Merged

Conversation

hannesm
Copy link
Member

@hannesm hannesm commented Nov 28, 2023

If there's a (Dns_server.Secondary.)timer-triggered connection request, do not infinitely try to connect (via request -> fail -> close -> request) in a tight loop, but cancel on first sight (the timer is responsible for re-connecting)

I'm in the process of testing this in production.

If there's a (Dns_server.Secondary.)timer-triggered connection request, do not
infinitely try to connect (via request -> fail -> close -> request) in a tight
loop, but cancel on first sight (the timer is responsible for re-connecting)
Copy link
Member

@reynir reynir left a comment

Choose a reason for hiding this comment

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

This looks good. I have some suggestions.

mirage/server/dns_server_mirage.ml Outdated Show resolved Hide resolved
@hannesm
Copy link
Member Author

hannesm commented Nov 28, 2023

I think the entire dns_server_mirage and dns_mirage logic may need an overhaul with a view on when which failures may arise, and how to properly handle them (to avoid such tight loops). maybe timer is the only thing, though...

@reynir
Copy link
Member

reynir commented Dec 6, 2023

I think this improves the situation so I will merge. If there are unintended effects we can revisit.

@reynir reynir merged commit c67ac0e into mirage:main Dec 6, 2023
1 check passed
@hannesm hannesm deleted the secondary-avoid-synflood branch December 6, 2023 10:16
@hannesm hannesm restored the secondary-avoid-synflood branch December 6, 2023 10:16
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.

None yet

2 participants