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

sock_udp_send() reports success even though the message was never sent #11551

Closed
benpicco opened this issue May 20, 2019 · 5 comments · Fixed by #12682
Closed

sock_udp_send() reports success even though the message was never sent #11551

benpicco opened this issue May 20, 2019 · 5 comments · Fixed by #12682
Assignees
Labels
Area: network Area: Networking Type: new feature The issue requests / The PR implemements a new feature for RIOT

Comments

@benpicco
Copy link
Contributor

benpicco commented May 20, 2019

Description

When sending the first message to a host with sock_udp_send(), the function will report the number of byes sent, indicating success. But in reality the packet never reached the device driver - it was silently dropped by the IPv6 layer with no error being propagated. (unless you ENABLE_DEBUG is set in gnrc_ipv6.c - then a debug message is printed)

Steps to reproduce the issue

  • configure a custom, non-link-local IP address:
    ipv6_addr_t addr;
    gnrc_netif_t *netif = gnrc_netif_iter(NULL);
    gnrc_netif_ipv6_addrs_get(netif, &addr, sizeof(addr));

    addr.u8[0] = 0xde;
    addr.u8[1] = 0x71;
    addr.u8[2] = 0xc1;
    addr.u8[3] = 0x05;

    gnrc_netif_ipv6_addr_add(netif, &addr, 64, 0);
  • send a message to the remote
    sock_udp_ep_t remote = { .family = AF_INET6, .port = port };
    ipv6_addr_from_str((ipv6_addr_t *)&remote.addr, "de71:c105::1");
    int res = sock_udp_send(NULL, "Hello", 6, &remote);

Expected results

Either res < 0 and the message failed to be send, or res == 6 and the message is being sent.

Actual results

res == 6, the message is not being sent.

When tracing the packet through the network stack it becomes evident that it is dropped in _send_unicast(): no link-layer address or interface for next hop to de71:c105::1.

The Address is subsequently resolved:

ipv6: waiting for incoming message.
ipv6: GNRC_NETAPI_MSG_TYPE_RCV received
ipv6: Received (src = de71:c105::1, dst = de71:c105::8d1b:3202:ae20:bb66, next header = 58, length = 40)
ipv6: forward nh = 58 to other threads
ipv6: handle ICMPv6 packet (nh = 58)

However, _send_unicast does not return an error and the GNRC_NETAPI_MSG_TYPE_SND case will not report an error to the upper layer.

My current hack around this is to send a dummy message after startup, but this is of course not a very nice solution.

Versions

RIOT 2019.04

@PeterKietzmann PeterKietzmann added Area: network Area: Networking Type: new feature The issue requests / The PR implemements a new feature for RIOT labels May 21, 2019
@miri64
Copy link
Member

miri64 commented May 23, 2019

Due to the asynchronous nature of GNRC this is deactivated by default. There is however the poorly utilized, poorly tested and poorly documented gnrc_neterr module which could provide such error reporting for sock. At least most releases in IPv6 are using gnrc_pktbuf_release_error() to signal an error.

@PeterKietzmann
Copy link
Member

I'm wondering if we should consider this issue as 'solved' or feature request. Personally I've never used the gnrc_neterr. @benpicco did you give it a try?

@benpicco
Copy link
Contributor Author

benpicco commented Jun 6, 2019

At the very least the documentation should be updated.
The way I read the description of sock_udp_send, when size is returned, those bytes have been successfully written to the network/Aether/left the device in the intended way.

For the case that the destination is unknown and the packet was therefore dropped without being written to the network, I would have expected to get -EHOSTUNREACH.

@benpicco
Copy link
Contributor Author

something like this?

--- a/sys/include/net/sock/udp.h
+++ b/sys/include/net/sock/udp.h
@@ -419,7 +419,10 @@ ssize_t sock_udp_recv(sock_udp_t *sock, void *data, size_t max_len,
  *                      end point of @p sock provides this information.
  *                      sock_udp_ep_t::port may not be 0.
  *
- * @return  The number of bytes sent on success.
+ * @return  The number of bytes that might have been sent.
+ *          A return value > 0 is not an indication of success.
+ *          To make sure the message has been send, check @ref net_gnrc_neterr
+ *          for the absence of errors.
  * @return  -EADDRINUSE, if `sock` has no local end-point or was `NULL` and the
  *          pool of available ephemeral ports is depleted.
  * @return  -EAFNOSUPPORT, if `remote != NULL` and sock_udp_ep_t::family of

@miri64
Copy link
Member

miri64 commented Jul 19, 2019

The first two sentences are good, the third assumes GNRC as the backend, which in the general doc is a no-go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: network Area: Networking Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants