-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[feature-wisun] Enable Nanostack DNS cache usage #13507
[feature-wisun] Enable Nanostack DNS cache usage #13507
Conversation
return -1; | ||
} | ||
|
||
interface_id = atoi(&interface_name[3]); |
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.
Do all interfaces have exactly 4 leters ? Mesh ETH? TUN? is there possibility to search first number and then atoi it instead of assuming
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.
I agree, this is problematic. Interface name is created https://github.com/ARMmbed/mbed-os/blob/feature-wisun/features/nanostack/mbed-mesh-api/source/MeshInterfaceNanostack.cpp#L132 and but it can also be given from application.
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.
Application returns interface name that the nanostack interfaces have returned, so nanostack is in full control of those. Nanostack interface naming pattern is to have always 3 letters and a number.
if (nanostack_dns_query_result_check(name, address, interface_name) == 0) { | ||
return 0; | ||
} | ||
|
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.
Should have the "if (version == NSAPI_UNSPEC) { " and in that case set version ipv6. Resolving ipv4 addresses with nanostack is bit weird since stack does not have ipv4, but I guess no reason to force reject for those either.
|
||
// try nanostack DNS cache, if not found then fallback to dns query | ||
if (nanostack_dns_query_result_check(name, address, interface_name) == 0) { | ||
return 0; |
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.
return NSAPI_ERROR_OK
callback(NSAPI_ERROR_OK, &address); | ||
return NSAPI_ERROR_OK; | ||
} | ||
|
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.
also check for unspecified version
@amarjeet-arm would you please check. This PR contains API that can be used to set DNS query results to Nanostack cache. |
Test run: FAILEDSummary: 1 of 3 test jobs failed Failed test jobs:
|
One API is missing from the |
Pull request has been modified.
This PR cannot be merged due to conflicts. Please rebase to resolve them. |
Inherit methods gethostbyname, gethostbyname_async and get_dns_server to Nanostack class. Methods will try to find DNS server address or DNS query results from Nanostack DNS cache.
-Check address version NSAPI_UNSPEC -Disable traces -More specific interface ID parsing -Harmonize return values
Add set_dns_query_result to WisunBorderRouter.
bf95a0b
to
22c3c35
Compare
Rebased on top of the latest feature-wisun, please re-review. |
CI started |
Test run: FAILEDSummary: 1 of 3 test jobs failed Failed test jobs:
|
CI restarted |
Test run: SUCCESSSummary: 11 of 11 test jobs passed |
Summary of changes
Enable usage of Nanostack DNS cache. The cache may contain DNS server address and DNS query results.
Inherit methods gethostbyname, gethostbyname_async and get_dns_server to Nanostack class.
Methods will try to find DNS server address or DNS query results from Nanostack DNS cache.
Impact of changes
Migration actions required
Documentation
Pull request type
Test results
Reviewers