Skip to content
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

DNS query fails with Nanostack on socket_sendto_control #15118

Closed
rardiol opened this issue Sep 28, 2021 · 2 comments · Fixed by #15147
Closed

DNS query fails with Nanostack on socket_sendto_control #15118

rardiol opened this issue Sep 28, 2021 · 2 comments · Fixed by #15147

Comments

@rardiol
Copy link
Contributor

rardiol commented Sep 28, 2021

Description of defect

Device fails to resolve DNS queries after updating to 6.15 from 6.13. Using an IP address works. Failure occurs using Nanostack and Cellular and PPP.

Target(s) affected by this defect ?

NUCLEO_WB55RG

Toolchain(s) (name and version) displaying this defect ?

gcc-arm-none-eabi-9-2019-q4-major

What version of Mbed-os are you using (tag or sha) ?

mbed-os-6.15.0

What version(s) of tools are you using. List all that apply (E.g. mbed-cli)

mbed-cli 1.10.2

How is this defect reproduced ?

After initializing the network, calling _iface->gethostbyname(url, &_resolved_address, NSAPI_IPv6 fails. The ntp-client lib also fails on apparantly the same NSAPI_UNSUPPORTED error https://github.com/ARMmbed/ntp-client.

GDB backtrace shows it is hitting the unsupported Nanostack::socket_sendto_control

#0  0x08033e8c in Nanostack::socket_sendto_control (this=0x2001500c <Nanostack::get_instance()::nanostack>, handle=0x2000d8cc <app_stack_heap+43348>, 
    address=..., data=0x20025700, size=38, control=0x0, control_size=0) at ./mbed-os/connectivity/nanostack/include/nanostack-interface/Nanostack.h:311
#1  0x080786b6 in InternetDatagramSocket::sendto_control (this=0x2001db48 <_main_stack+7560>, address=..., data=0x20025700, size=38, control=0x0, 
    control_size=0) at ./mbed-os/connectivity/netsocket/source/InternetDatagramSocket.cpp:49
#2  0x0807861a in InternetDatagramSocket::sendto (this=this@entry=0x2001db48 <_main_stack+7560>, address=..., data=data@entry=0x20025700, 
    size=<optimized out>) at ./mbed-os/connectivity/netsocket/source/InternetDatagramSocket.cpp:83
#3  0x08079f66 in nsapi_dns_query_multiple (stack=0x2001500c <Nanostack::get_instance()::nanostack>, host=0x80b9d56 REDACTED, 
    addr=addr@entry=0x2001dc0c <_main_stack+7756>, addr_count=addr_count@entry=1, interface_name=0x0, version=version@entry=NSAPI_IPv6)
    at ./mbed-os/connectivity/netsocket/source/nsapi_dns.cpp:542
#4  0x0807a086 in nsapi_dns_query (stack=<optimized out>, host=<optimized out>, address=0x2001968c <_resolved_address>, interface_name=<optimized out>, 
    version=NSAPI_IPv6) at ./mbed-os/connectivity/netsocket/source/nsapi_dns.cpp:651

I believe the issue is with #15040 . The following patch seems to resolve the issue. Why is NetworkStack::socket_recvfrom_control defined to use the old socket_sendto/socket_recvfrom if control is NULL but Nanostack just fails?

diff --git a/connectivity/nanostack/include/nanostack-interface/Nanostack.h b/connectivity/nanostack/include/nanostack-interface/Nanostack.h
index 09ffc51f1e..b0c0d71465 100644
--- a/connectivity/nanostack/include/nanostack-interface/Nanostack.h
+++ b/connectivity/nanostack/include/nanostack-interface/Nanostack.h
@@ -306,14 +306,24 @@ protected:
                                                 const void *data, nsapi_size_t size,
                                                 nsapi_msghdr_t *control, nsapi_size_t control_size) override
     {
-        return NSAPI_ERROR_UNSUPPORTED;
+        if(control != NULL)
+        {
+            return NSAPI_ERROR_UNSUPPORTED;
+        }
+
+        return this->socket_sendto(handle, address, data, size);
     }
 
     nsapi_size_or_error_t socket_recvfrom_control(nsapi_socket_t handle, SocketAddress *address,
                                                   void *data, nsapi_size_t size,
                                                   nsapi_msghdr_t *control, nsapi_size_t control_size) override
     {
-        return NSAPI_ERROR_UNSUPPORTED;
+        if(control != NULL)
+        {
+            return NSAPI_ERROR_UNSUPPORTED;
+        }
+
+        return this->socket_recvfrom(handle, address, data, size);
     }
 
 private:
@boraozgen
Copy link
Contributor

Same issue for AT_CellularStack. Nice move by the author to leave the API unsupported instead of redirecting it to socket_recvfrom.

nsapi_size_or_error_t socket_sendto_control(nsapi_socket_t handle, const SocketAddress &address,
const void *data, nsapi_size_t size,
nsapi_msghdr_t *control, nsapi_size_t control_size) override
{
return NSAPI_ERROR_UNSUPPORTED;
}
nsapi_size_or_error_t socket_recvfrom_control(nsapi_socket_t handle, SocketAddress *address,
void *data, nsapi_size_t size,
nsapi_msghdr_t *control, nsapi_size_t control_size) override
{
return NSAPI_ERROR_UNSUPPORTED;
}

@boraozgen
Copy link
Contributor

boraozgen commented Oct 20, 2021

Apparently the unimplemented API is actually properly handled by the parent:

virtual nsapi_size_or_error_t socket_sendto_control(nsapi_socket_t handle, const SocketAddress &address,
const void *data, nsapi_size_t size,
nsapi_msghdr_t *control, nsapi_size_t control_size)
{
if (control != NULL) {
return NSAPI_ERROR_UNSUPPORTED;
}
return socket_sendto(handle, address, data, size);
}

virtual nsapi_size_or_error_t socket_recvfrom_control(nsapi_socket_t handle, SocketAddress *address,
void *data, nsapi_size_t size,
nsapi_msghdr_t *control, nsapi_size_t control_size)
{
if (control != NULL) {
return NSAPI_ERROR_UNSUPPORTED;
}
return socket_recvfrom(handle, address, data, size);
}

This means that the dummy implementations returning NSAPI_ERROR_UNSUPPORTED are unnecessary and can be removed. Indeed removing them for AT_CellularStack solves my problem. Can you try it for Nanostack @rardiol ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants