-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
netdb:Add macro to control DNS using TCP transmission #15069
Open
yjq91115
wants to merge
1
commit into
apache:master
Choose a base branch
from
yjq91115:dnsstream
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+88
−2
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -237,6 +237,13 @@ config NETDB_DNSSERVER_IPv4ADDR | |
Default DNS server IPv4 address in host byte order. Default value | ||
10.0.0.1. This may be changed via dns_add_nameserver(). | ||
|
||
config NETDB_DNS_STREAM | ||
bool "DNS supports TCP transmission" | ||
depends on LIBC_NETDB | ||
default n | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please include a --help-- to explain exactly what is this feature, when it should be used, etc There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
---help--- | ||
Enable support for DNS Name resolution over TCP. | ||
|
||
if NETDB_DNSSERVER_IPv6 | ||
|
||
config NETDB_DNSSERVER_IPv6ADDR_1 | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the original code already support both TCP/UDP, why do you make this patch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When DNS resolution returns the truncate response, this function will establish TCP connection and return the complete response. However, the complete response can easily exceed the RECV_BUFFER_SIZE, causing the resolution to fail, so I wrote a simple solution to solve this problem. There are two better solutions. One is to recycle the buffer and finally splice the buffer. The second is to use molloc to create a length that matches the response. I would like to ask if there is a better solution. @yamt
ping failed.log
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the simplest solution, if it's acceptable for your configuration, would be to bump your CONFIG_NETDB_DNSCLIENT_MAXRESPONSE.
i agree it makes sense to try to use malloc when it doesn't fit RECV_BUFFER_SIZE.
it shouldn't be too difficult to make our client decide after receiving the record size (the first two bytes of the response) with tcp.
i'm not sure how difficult buffer recycling is.
depending on your configurations and environment, you might also want to implement EDNS buffer size.
some servers "tweak" their responses to fit the specified size.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you for your suggestion!