Skip to content
This repository has been archived by the owner on Jun 20, 2024. It is now read-only.

DNS responses incomplete when TCP query contains EDNS record #1332

Closed
rade opened this issue Aug 19, 2015 · 0 comments · Fixed by #1348
Closed

DNS responses incomplete when TCP query contains EDNS record #1332

rade opened this issue Aug 19, 2015 · 0 comments · Fixed by #1348
Assignees
Milestone

Comments

@rade
Copy link
Member

rade commented Aug 19, 2015

$ weave launch
$ weave run --name=foo -ti ubuntu
e14c663b3e5996ac94288a947d5985ff89450ffcdc35e04fde23ca25f2f27d8f
$ for i in $(seq 50); do weave dns-add 10.2.1.$i foo -h bar.weave.local ; done
$ dig +short @$(weave docker-bridge-ip) bar.weave.local A | wc -l
50
$ dig +tcp +short @$(weave docker-bridge-ip) bar.weave.local A | wc -l
50
$ dig +bufsize=700 +short @$(weave docker-bridge-ip) bar.weave.local A | wc -l
22
$ dig +tcp +bufsize=700 +short @$(weave docker-bridge-ip) bar.weave.local A | wc -l
21

Looking at this a bit more deeply, dig by default adds an EDNS OPT RR record indicating the max buffer size is 4k. The +bufsize option shrinks that. dig also falls back to tcp when getting a truncated response.

The reason we get truncation even then is the nameserver takes into account the EDNS-supplied UDP buffer size even when interacting over TCP.

I am pretty sure this is wrong.

@rade rade added this to the current milestone Aug 19, 2015
@tomwilkie tomwilkie self-assigned this Aug 19, 2015
@rade rade removed the in progress label Aug 20, 2015
tomwilkie pushed a commit that referenced this issue Aug 20, 2015
Also ignore EDNS on tcp requests. Fixes #1332

Also fixes the case where recursive dns entries would be considered
too big and compressed when using TCP.
rade added a commit that referenced this issue Aug 21, 2015
Unify logic to tell if DNS response is too big.

Fixes #1345, fixes #1332
@rade rade modified the milestones: current, 1.1.0 Aug 29, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants