You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2020/04/27 05:17:53 UTC
[GitHub] [incubator-nuttx] juniskane opened a new pull request #892: Netdb multiple dns servers pr
juniskane opened a new pull request #892:
URL: https://github.com/apache/incubator-nuttx/pull/892
## Summary
PR adds capability to add multiple nameservers on run-time even when not using resolv.conf and reset the list of nameservers back to default setting (no nameserver at all or single predefined nameserver in Kconfig). This is useful for applications that change their cellular network frequently.
Also make getaddrinfo re-entrant when doing service name query
## Impact
## Testing
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #892: Netdb multiple dns servers
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #892:
URL: https://github.com/apache/incubator-nuttx/pull/892#discussion_r415608827
##########
File path: libs/libc/netdb/lib_dnsaddserver.c
##########
@@ -198,10 +198,24 @@ int dns_add_nameserver(FAR const struct sockaddr *addr, socklen_t addrlen)
{
FAR uint16_t *pport;
size_t copylen;
+ int nservers;
+ int idx;
DEBUGASSERT(addr != NULL);
- /* Copy the new server IP address into our private global data structure */
+ /* Get the index of the next free nameserver slot. */
+
+ dns_semtake();
+ if (g_dns_nservers == CONFIG_NETDB_DNSSERVER_NAMESERVERS)
Review comment:
> Perhaps, but note that the resolv.conf version of the function appends the new nameserver at the end of the file. It should be changed to match for consistency. I would leave that to some future PR.
>
If so, it is better to append the new server to the end of list and iterate through the server backward.
> I think the most common use case is to get DNS1, DNS2,... IPs from operator and then add entire list by first calling dns_default_nameserver() and then add the DNS1, DNS2, DNS3... and maybe add some generic fallback server like Google's 8.8.8.8 as _last_ in case operator's preferred servers are down.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #892: Netdb multiple dns servers pr
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #892:
URL: https://github.com/apache/incubator-nuttx/pull/892#discussion_r415522746
##########
File path: libs/libc/netdb/lib_dnsaddserver.c
##########
@@ -198,10 +198,24 @@ int dns_add_nameserver(FAR const struct sockaddr *addr, socklen_t addrlen)
{
FAR uint16_t *pport;
size_t copylen;
+ int nservers;
+ int idx;
DEBUGASSERT(addr != NULL);
- /* Copy the new server IP address into our private global data structure */
+ /* Get the index of the next free nameserver slot. */
+
+ dns_semtake();
+ if (g_dns_nservers == CONFIG_NETDB_DNSSERVER_NAMESERVERS)
Review comment:
Should we pretend the new server to the beginning of the list? so we alwasy eliminate the oldest server and try the newest server in for each loop,
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] juniskane commented on a change in pull request #892: Netdb multiple dns servers pr
Posted by GitBox <gi...@apache.org>.
juniskane commented on a change in pull request #892:
URL: https://github.com/apache/incubator-nuttx/pull/892#discussion_r415533560
##########
File path: libs/libc/netdb/lib_dnsaddserver.c
##########
@@ -198,10 +198,24 @@ int dns_add_nameserver(FAR const struct sockaddr *addr, socklen_t addrlen)
{
FAR uint16_t *pport;
size_t copylen;
+ int nservers;
+ int idx;
DEBUGASSERT(addr != NULL);
- /* Copy the new server IP address into our private global data structure */
+ /* Get the index of the next free nameserver slot. */
+
+ dns_semtake();
+ if (g_dns_nservers == CONFIG_NETDB_DNSSERVER_NAMESERVERS)
Review comment:
Perhaps, but note that the resolv.conf version of the function appends the new nameserver at the end of the file. It should be changed to match for consistency. I would leave that to some future PR.
I think the most common use case is to get DNS1, DNS2,... IPs from operator and then add entire list by first calling dns_default_nameserver() and then add the DNS1, DNS2, DNS3... and maybe add some generic fallback server like Google's 8.8.8.8 as *last* in case operator's preferred servers are down.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #892: Netdb multiple dns servers
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #892:
URL: https://github.com/apache/incubator-nuttx/pull/892#discussion_r415608827
##########
File path: libs/libc/netdb/lib_dnsaddserver.c
##########
@@ -198,10 +198,24 @@ int dns_add_nameserver(FAR const struct sockaddr *addr, socklen_t addrlen)
{
FAR uint16_t *pport;
size_t copylen;
+ int nservers;
+ int idx;
DEBUGASSERT(addr != NULL);
- /* Copy the new server IP address into our private global data structure */
+ /* Get the index of the next free nameserver slot. */
+
+ dns_semtake();
+ if (g_dns_nservers == CONFIG_NETDB_DNSSERVER_NAMESERVERS)
Review comment:
> Perhaps, but note that the resolv.conf version of the function appends the new nameserver at the end of the file. It should be changed to match for consistency. I would leave that to some future PR.
>
If so, it is better to append the new server to the end of list and iterate through the server backward. It's fine to enhance it in the furture.
> I think the most common use case is to get DNS1, DNS2,... IPs from operator and then add entire list by first calling dns_default_nameserver() and then add the DNS1, DNS2, DNS3... and maybe add some generic fallback server like Google's 8.8.8.8 as _last_ in case operator's preferred servers are down.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] juniskane commented on a change in pull request #892: Netdb multiple dns servers pr
Posted by GitBox <gi...@apache.org>.
juniskane commented on a change in pull request #892:
URL: https://github.com/apache/incubator-nuttx/pull/892#discussion_r415533560
##########
File path: libs/libc/netdb/lib_dnsaddserver.c
##########
@@ -198,10 +198,24 @@ int dns_add_nameserver(FAR const struct sockaddr *addr, socklen_t addrlen)
{
FAR uint16_t *pport;
size_t copylen;
+ int nservers;
+ int idx;
DEBUGASSERT(addr != NULL);
- /* Copy the new server IP address into our private global data structure */
+ /* Get the index of the next free nameserver slot. */
+
+ dns_semtake();
+ if (g_dns_nservers == CONFIG_NETDB_DNSSERVER_NAMESERVERS)
Review comment:
Perhaps, but note that the resolv.conf version of the function appends the new nameserver at the end of the file. It should be changed to match for consistency. I would leave that to some future PR.
I think the most common use case is to get DNS1, DNS2,... IPs from operator and then add entire list by first calling dns_clear_servers() and then add the DNS1, DNS2, DNS3... and maybe add some generic fallback server like Google's 8.8.8.8 as *last* in case operator's preferred servers are down.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #892: Netdb multiple dns servers pr
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #892:
URL: https://github.com/apache/incubator-nuttx/pull/892#issuecomment-619734749
@juniskane there are two minor nxstyle, please fix them, thanks.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #892: Netdb multiple dns servers
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #892:
URL: https://github.com/apache/incubator-nuttx/pull/892#issuecomment-619818778
@juniskane , since the 2nd patch generate the nxstyle issues, it's better to merge the 3rd patch into the 2nd one:
```
git rebase --interactive
change pick to squach for the 3rd patch
git push -f origin netdb_multiple_dns_servers_pr
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org