gnrc_sock: implicitly set netif if there is only a single one

[benpicco]

Contribution description

The interface is already set implicitly for the gnrc_netif_highlander() case - also set it implicitly if there is only a single interface.

Testing procedure

Issues/PRs references

alternative to #17927

[kfessel]

this one seems to meet expetations better than #17927

[kfessel]

this PR in itself seems good to go but i am not sure how to trigger the changes

all users of gnrc_ep_set() seem to check netif for not being SOCK_ADDR_ANY_NETIF (0) prior to calling gnrc_ep_set()

I would like to see example commands or code snippet or script that does not work prior to this PR but works with this PR

eg:

examples/gnrc_networking$ USE_ZEP=1 make all term
..RIOT boot..
> ifconfig
Iface  7  HWaddr: 27:AD  Channel: 26  NID: 0x23  PHY: O-QPSK 
          Long HWaddr: A6:19:9A:54:06:CB:27:AD 
...
> ping fe80::a419:9a54:6cb:1
...
3 packets transmitted, 0 packets received, 100% packet loss << 100% loss is expected 

this one works before this PR -> can't evaluate it
also udp send does not complain about a missing netif

[benpicco]

Huh I can't reproduce the ping issue - ping doesn't even make use of gnrc_sock.

For a test you can use e.g. tests/nanocoap_cli and start aiocoap-fileserver:

With this you can now do

url get coap://[fe80::ac59:5aff:fe33:896e]/

whereas on master you have to specify the interface

url get coap://[fe80::ac59:5aff:fe33:896e%5]/

[kfessel]

url get coap://[fe80::ac59:5aff:fe33:896e]/

should this one error out ? (in master)

[benpicco]

It will just run into a timeout, but the test does not print error return codes (I thought I could just do #17922 instead)

[kfessel]

i added me a

void shell_post_command_hook(int ret, int argc, char **argv)
{
    (void)argv;
    (void)argc;

    if(ret) printf ("ERROR HAPPEND: %d\n", ret);

}

I prints -110 but i seemd to have send something (which is kindof strange)

[benpicco]

I prints -110

Which is -ETIMEDOUT

but i seemd to have send something

What makes you think so?

[kfessel]

but i seemd to have send something

What makes you think so?

With #define ENABLE_DEBUG 1 in nanocoap/sock.c

> url get coap://[fe80::58ca:8ff:fe05:6dfa]/
url get coap://[fe80::58ca:8ff:fe05:6dfa]/
fetching block 0
nanocoap: send 7 bytes (5 tries left)
nanocoap: waiting for response (timeout: 2341766 µs)
nanocoap: timeout
nanocoap: send 7 bytes (4 tries left)
nanocoap: waiting for response (timeout: 4685532 µs)
nanocoap: timeout
nanocoap: send 7 bytes (3 tries left)
nanocoap: waiting for response (timeout: 9371064 µs)
nanocoap: timeout
nanocoap: send 7 bytes (2 tries left)
nanocoap: waiting for response (timeout: 18741128 µs)
nanocoap: timeout
nanocoap: send 7 bytes (1 tries left)
nanocoap: maximum retries reached
error fetching block 0: -110
ERROR HAPPEND: -110

for some reason i think every send should have errored and i should get -22 (EINVAL)

but it sends and waits (which seems wrong)

[benpicco]

i think every send should have errored and i should get -22 (EINVAL)

For this you would need

diff --git a/sys/net/gnrc/sock/udp/gnrc_sock_udp.c b/sys/net/gnrc/sock/udp/gnrc_sock_udp.c
index bc02fd979f..9169130f81 100644
--- a/sys/net/gnrc/sock/udp/gnrc_sock_udp.c
+++ b/sys/net/gnrc/sock/udp/gnrc_sock_udp.c
@@ -133,6 +133,9 @@ int sock_udp_create(sock_udp_t *sock, const sock_udp_ep_t *local,
         }
         gnrc_ep_set((sock_ip_ep_t *)&sock->remote,
                     (sock_ip_ep_t *)remote, sizeof(sock_udp_ep_t));
+        if (sock->remote.netif == SOCK_ADDR_ANY_NETIF) {
+            return -EINVAL;
+        }
     }
     if (local != NULL) {
         /* listen only with local given */

[miri64]

+        if (sock->remote.netif == SOCK_ADDR_ANY_NETIF) {
+            return -EINVAL;
+        }

Shouldn't this only happen if the address is a link-local address?

[kfessel]

some how gnrc purports it send bytes even though it did not. (i think this happen in https://github.com/RIOT-OS/RIOT/blob/master/sys/net/gnrc/sock/gnrc_sock.c#L290 )
where a else clause is missing (at this point it did not copy the pkt into any buffer since it know that will never move anywhere)

I was wrong here thpkt is send without netif information
the netif might thean be added in

sys/net/gnrc/netif/gnrc_netif.c : gnrc_netif_get_by_ipv6_addr or

sys/net/gnrc/network_layer/ipv6/gnrc_ipv6.c : _send () (there is already a loopback check in line 699)

[benpicco]

some how gnrc purports it send bytes even though it did not.

This is a long standing (and quite annoying) issue, see #11551
It looks like that with #15821 there is now a solution on the horizon.

[kfessel]

why not do this in ?:
sys/net/gnrc/netif/gnrc_netif.c : gnrc_netif_get_by_ipv6_addr or

sys/net/gnrc/network_layer/ipv6/gnrc_ipv6.c : _send () (there is already a loopback check in line 699)

[kfessel]

BTW.: This is working like intended, (it reads well and documentation is good ( the function is still doing the same thing just in another way)
I am just not sure if a better position would be lower (avoid sending more bytes send into the void) or higher in stack or if this is just the right place.

[benpicco]

Since this has already a check for gnrc_netif_highlander() I'd say it fits right in.

[kfessel]

more sane default, eases development

[kfessel]

I think its much better to have the package go somewhere especially if it is just one direction then them to be send but not send

[benpicco]

Thank you for the review!