Skip to content

Return NODATA for IPv6 AAAA question types #1289

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
Jan 10, 2023

Conversation

Nino-K
Copy link
Contributor

@Nino-K Nino-K commented Jan 10, 2023

Previously we were replying with A response for AAAA queries which was causing an error in some clients e.g nginx. The new approach waits for 1 second and then replies with NODATA. This approach allows for more semantic reply even when a client uses the same Transaction ID for both A and AAAA questions.

This PR is meant to address the issue that was reported on Rancher Desktop here: rancher-sandbox/rancher-desktop#3447

Signed-off-by: Nino Kodabande [email protected]

Previously we were replying with A response for AAAA
queries which was causing an error in some clients
e.g nginx. The new approach waits for 1 second and then
replies with NODATA. This approach allows for more semantic
reply even when client uses same Transaction ID for both
A and AAAA questions.

Signed-off-by: Nino Kodabande <[email protected]>
@Nino-K Nino-K force-pushed the return-nodata-ipv6 branch from d2a6077 to 66b2056 Compare January 10, 2023 18:58
Copy link
Member

@jandubois jandubois left a comment

Choose a reason for hiding this comment

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

The change looks ok to me, and should fix the situation from rancher-sandbox/rancher-desktop#3447 (comment).

I'm slightly worried about the arbitrary delay of 1 second (why not 0.5 or 2 seconds?), but I think it should work fine as long as the machine isn't already overloaded.

@AkihiroSuda please merge, if you have no objections!

Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

Thanks

@jandubois jandubois merged commit 45ca822 into lima-vm:master Jan 10, 2023
@AkihiroSuda AkihiroSuda added this to the v0.15 (tentative) milestone Jan 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants