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

dnsdist: Refactoring to merge the UDP and TCP paths #7526

Closed
wants to merge 6 commits into from

Conversation

Projects
None yet
3 participants
@rgacogne
Copy link
Member

commented Feb 27, 2019

Short description

This PR refactors the handling of queries and responses a bit so that the existing code for the UDP and TCP paths can be merged into one. This reduces code duplication and will make it easier to add another processing path in the future, like DoH or a more scalable TCP/DoT stack.
It also removes a bit of code duplication in the rules, and create empty objects for the DNSCrypt-related classes when support is disabled, so that we can mostly ignore whether it's enabled in most parts of the code.

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled this code
  • tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added or modified regression test(s)
  • added or modified unit test(s)

@rgacogne rgacogne added this to the dnsdist-1.4.0 milestone Feb 27, 2019

@rgacogne rgacogne referenced this pull request Mar 11, 2019

Merged

dnsdist: Refactoring of the TCP stack #7559

3 of 7 tasks complete

@chbruyand chbruyand self-requested a review Mar 19, 2019

@pieterlexis pieterlexis self-requested a review Mar 19, 2019

@pieterlexis
Copy link
Member

left a comment

Two small comments. As mosts tests pass, I expect the changes to be good. But I'm not well versed in dnsdist internals.

Show resolved Hide resolved pdns/dnsdist-ecs.cc
Show resolved Hide resolved pdns/dnsdist.cc

rgacogne added some commits Feb 25, 2019

dnsdist: Define empty DNSCrypt-related objects when not enabled
This way the rest of the code can mostly ignore whether DNSCrypt
support is enabled.

@rgacogne rgacogne force-pushed the rgacogne:dnsdist-refactor branch from c1ee800 to b2646de Mar 19, 2019

Show resolved Hide resolved pdns/dnsdist-tcp.cc
Show resolved Hide resolved pdns/dnsdist-tcp.cc
Show resolved Hide resolved pdns/dnsdist.cc
@rgacogne

This comment has been minimized.

Copy link
Member Author

commented Apr 4, 2019

Included in #7559.

@rgacogne rgacogne closed this Apr 4, 2019

@rgacogne rgacogne referenced this pull request Apr 15, 2019

Merged

dnsdist: Add DNS over HTTPS support based on libh2o #7726

3 of 7 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.