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: Set a correct EDNS OPT RR for self-generated answers #6847

Merged
merged 3 commits into from Sep 4, 2018

Conversation

@rgacogne
Copy link
Member

@rgacogne rgacogne commented Aug 10, 2018

Short description

Fixes #6348 (but at the moment not #4857).
Needs:

  • documentation
  • unit tests would be nice

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 Aug 10, 2018
@rgacogne rgacogne force-pushed the rgacogne:dnsdist-edns-self-generated branch from 4bf9b2c to 856d0d1 Aug 17, 2018
@rgacogne rgacogne changed the title [WIP] dnsdist: Set a correct EDNS OPT RR for self-generated answers dnsdist: Set a correct EDNS OPT RR for self-generated answers Aug 20, 2018
@rgacogne
Copy link
Member Author

@rgacogne rgacogne commented Aug 20, 2018

Should now fix #4857 as well.

@rgacogne rgacogne requested a review from chbruyand Aug 20, 2018
@rgacogne
Copy link
Member Author

@rgacogne rgacogne commented Aug 20, 2018

Ready for review!

@@ -423,6 +424,7 @@ const std::vector<ConsoleKeyword> g_consoleKeywords{
{ "setMaxTCPQueriesPerConnection", true, "n", "set the maximum number of queries in an incoming TCP connection. 0 means unlimited" },
{ "setMaxTCPQueuedConnections", true, "n", "set the maximum number of TCP connections queued (waiting to be picked up by a client thread)" },
{ "setMaxUDPOutstanding", true, "n", "set the maximum number of outstanding UDP queries to a given backend server. This can only be set at configuration time and defaults to 10240" },
{ "setPayloadSizeOnSelfGeneratedAnswers", true, "add", "set the UDP payload size advertised via EDNS on self-generated responses" },

This comment has been minimized.

@chbruyand

chbruyand Aug 20, 2018
Member

payloadSize instead of add ?

@@ -450,6 +457,10 @@ DNSAction::Action SpoofAction::operator()(DNSQuestion* dq, string* ruleresult) c

dq->dh->ancount = htons(dq->dh->ancount);

if (hadEDNS && g_addEDNSToSelfGeneratedResponses) {

This comment has been minimized.

@chbruyand

chbruyand Aug 20, 2018
Member

testing against g_addEDNSToSelfGeneratedResponses is redundant with setting hadEDNS just before.


.. versionadded:: 1.3.3

Set the UDP payload size advertised via EDNS on self-generated responses.

This comment has been minimized.

@chbruyand

chbruyand Aug 20, 2018
Member

Maybe add there that it must be greater than 512 ?

@rgacogne
Copy link
Member Author

@rgacogne rgacogne commented Aug 22, 2018

Comments fixed, thanks for the review!

rgacogne added 3 commits Aug 17, 2018
@rgacogne rgacogne force-pushed the rgacogne:dnsdist-edns-self-generated branch from b1cd1c2 to 8d72bcd Sep 3, 2018
@rgacogne
Copy link
Member Author

@rgacogne rgacogne commented Sep 3, 2018

Rebased to fix a conflict.

@rgacogne rgacogne merged commit 6cb39a3 into PowerDNS:master Sep 4, 2018
4 checks passed
4 checks passed
LGTM analysis: C/C++ No alert changes
Details
LGTM analysis: JavaScript No alert changes
Details
LGTM analysis: Python No alert changes
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@rgacogne rgacogne deleted the rgacogne:dnsdist-edns-self-generated branch Sep 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants
You can’t perform that action at this time.