net/gnrc/sock: Implement sock_aux_local by maribu · Pull Request #14704 · RIOT-OS/RIOT

maribu · 2020-08-05T07:35:09Z

Contribution description

This PR adds the implementation of the sock_aux_local for GNRC, that provides access to the local address via the extended SOCK API of #14703.

Testing procedure

make -C tests/gnrc_sock_udp flash test
make -C tests/gnrc_sock_ip flash test

Issues/PRs references

Depends on #14703

Same as #14705, but for GNRC

sys/net/gnrc/sock/ip/gnrc_sock_ip.c

maribu · 2020-11-04T09:41:03Z

I changed the implementation a bit (I think it is now more elegant) and used #if IS_USED() over #ifdef in all places.

maribu · 2020-12-03T11:20:24Z

I rebased on master and solved merged conflict (all regarding the documentation fixes in the API, not relevant to this PR).

I did however not squash

maribu · 2020-12-03T11:29:22Z

I messed up the rebase. This time it should have worked.

sys/net/gnrc/sock/gnrc_sock.c

tests/gnrc_sock_ip/Makefile

tests/gnrc_sock_udp/Makefile

maribu · 2020-12-04T08:40:09Z

OK, only the expected failures: The ds3231 test requires hardware to be plugged in to work (and was blacklisted on the CI in #15554). And the issue with libhydrogen on clang is also preexisting.

sys/net/gnrc/sock/gnrc_sock.c

miri64

One last thing:

sys/net/gnrc/sock/ip/gnrc_sock_ip.c

sys/net/gnrc/sock/udp/gnrc_sock_udp.c

miri64 · 2020-12-04T11:02:34Z

Please squash. I will give the tests a final local run.

miri64 · 2020-12-04T11:17:24Z

I ran the tests with the following script on native:

TEST_RUNS=0
for test in tests/gnrc_sock_{ip,udp}; do
    RIOT_CI_BUILD=1 AUX_LOCAL=1 make -C ${test} --no-print-directory -j clean flash test || break
    RIOT_CI_BUILD=1 AUX_LOCAL=0 make -C ${test} --no-print-directory -j clean flash test || break
    TEST_RUNS=$((TEST_RUNS+1))
done
[ ${TEST_RUNS} -eq 2 ] && echo "success" || echo "fail"

Success was printed in the end and the build sizes differed:

Building application "tests_gnrc_sock_ip" for "native" with MCU "native".

/usr/bin/ld: /home/mlenders/Repositories/RIOT-OS/RIOT/tests/gnrc_sock_ip/bin/native/cpu/tramp.o: warning: relocation against `_native_saved_eip' in read-only section `.text'
/usr/bin/ld: warning: creating DT_TEXTREL in a PIE
   text	   data	    bss	    dec	    hex	filename
 148536	    692	  59532	 208760	  32f78	/home/mlenders/Repositories/RIOT-OS/RIOT/tests/gnrc_sock_ip/bin/native/tests_gnrc_sock_ip.elf
true 
r
/home/mlenders/Repositories/RIOT-OS/RIOT/tests/gnrc_sock_ip/bin/native/tests_gnrc_sock_ip.elf /dev/ttyACM0 
RIOT native interrupts/signals initialized.
LED_RED_OFF
LED_GREEN_ON
RIOT native board initialized.
RIOT native hardware initialization complete.

Help: Press s to start test, r to print it is ready
READY
s
START
main(): This is RIOT! (Version: buildtest)
Calling test_sock_ip_create__EAFNOSUPPORT()
Calling test_sock_ip_create__EINVAL_addr()
Calling test_sock_ip_create__EINVAL_netif()
Calling test_sock_ip_create__no_endpoints()
Calling test_sock_ip_create__only_local()
Calling test_sock_ip_create__only_local_reuse_ep()
Calling test_sock_ip_create__only_remote()
Calling test_sock_ip_create__full()
Calling test_sock_ip_recv__EADDRNOTAVAIL()
Calling test_sock_ip_recv__EAGAIN()
Calling test_sock_ip_recv__ENOBUFS()
Calling test_sock_ip_recv__EPROTO()
Calling test_sock_ip_recv__ETIMEDOUT()
 * Calling sock_ip_recv()
 * (timed out with timeout 1000000)
Calling test_sock_ip_recv__socketed()
Calling test_sock_ip_recv__socketed_with_remote()
Calling test_sock_ip_recv__unsocketed()
Calling test_sock_ip_recv__unsocketed_with_remote()
Calling test_sock_ip_recv__with_timeout()
Calling test_sock_ip_recv__non_blocking()
Calling test_sock_ip_recv__aux()
Calling test_sock_ip_recv_buf__success()
Calling test_sock_ip_send__EAFNOSUPPORT_INET()
Calling test_sock_ip_send__EAFNOSUPPORT_UNSPEC()
Calling test_sock_ip_send__EINVAL_addr()
Calling test_sock_ip_send__EINVAL_netif()
Calling test_sock_ip_send__ENOTCONN()
Calling test_sock_ip_send__socketed_no_local_no_netif()
Calling test_sock_ip_send__socketed_no_netif()
Calling test_sock_ip_send__socketed_no_local()
Calling test_sock_ip_send__socketed()
Calling test_sock_ip_send__socketed_other_remote()
Calling test_sock_ip_send__unsocketed_no_local_no_netif()
Calling test_sock_ip_send__unsocketed_no_netif()
Calling test_sock_ip_send__unsocketed_no_local()
Calling test_sock_ip_send__unsocketed()
Calling test_sock_ip_send__no_sock_no_netif()
Calling test_sock_ip_send__no_sock()
ALL TESTS SUCCESSFUL

Building application "tests_gnrc_sock_ip" for "native" with MCU "native".

/usr/bin/ld: /home/mlenders/Repositories/RIOT-OS/RIOT/tests/gnrc_sock_ip/bin/native/cpu/tramp.o: warning: relocation against `_native_saved_eip' in read-only section `.text'
/usr/bin/ld: warning: creating DT_TEXTREL in a PIE
   text	   data	    bss	    dec	    hex	filename
 148328	    692	  59532	 208552	  32ea8	/home/mlenders/Repositories/RIOT-OS/RIOT/tests/gnrc_sock_ip/bin/native/tests_gnrc_sock_ip.elf
true 
r
/home/mlenders/Repositories/RIOT-OS/RIOT/tests/gnrc_sock_ip/bin/native/tests_gnrc_sock_ip.elf /dev/ttyACM0 
RIOT native interrupts/signals initialized.
LED_RED_OFF
LED_GREEN_ON
RIOT native board initialized.
RIOT native hardware initialization complete.

Help: Press s to start test, r to print it is ready
READY
s
START
main(): This is RIOT! (Version: buildtest)
Calling test_sock_ip_create__EAFNOSUPPORT()
Calling test_sock_ip_create__EINVAL_addr()
Calling test_sock_ip_create__EINVAL_netif()
Calling test_sock_ip_create__no_endpoints()
Calling test_sock_ip_create__only_local()
Calling test_sock_ip_create__only_local_reuse_ep()
Calling test_sock_ip_create__only_remote()
Calling test_sock_ip_create__full()
Calling test_sock_ip_recv__EADDRNOTAVAIL()
Calling test_sock_ip_recv__EAGAIN()
Calling test_sock_ip_recv__ENOBUFS()
Calling test_sock_ip_recv__EPROTO()
Calling test_sock_ip_recv__ETIMEDOUT()
 * Calling sock_ip_recv()
 * (timed out with timeout 1000000)
Calling test_sock_ip_recv__socketed()
Calling test_sock_ip_recv__socketed_with_remote()
Calling test_sock_ip_recv__unsocketed()
Calling test_sock_ip_recv__unsocketed_with_remote()
Calling test_sock_ip_recv__with_timeout()
Calling test_sock_ip_recv__non_blocking()
Calling test_sock_ip_recv__aux()
Calling test_sock_ip_recv_buf__success()
Calling test_sock_ip_send__EAFNOSUPPORT_INET()
Calling test_sock_ip_send__EAFNOSUPPORT_UNSPEC()
Calling test_sock_ip_send__EINVAL_addr()
Calling test_sock_ip_send__EINVAL_netif()
Calling test_sock_ip_send__ENOTCONN()
Calling test_sock_ip_send__socketed_no_local_no_netif()
Calling test_sock_ip_send__socketed_no_netif()
Calling test_sock_ip_send__socketed_no_local()
Calling test_sock_ip_send__socketed()
Calling test_sock_ip_send__socketed_other_remote()
Calling test_sock_ip_send__unsocketed_no_local_no_netif()
Calling test_sock_ip_send__unsocketed_no_netif()
Calling test_sock_ip_send__unsocketed_no_local()
Calling test_sock_ip_send__unsocketed()
Calling test_sock_ip_send__no_sock_no_netif()
Calling test_sock_ip_send__no_sock()
ALL TESTS SUCCESSFUL

Building application "tests_gnrc_sock_udp" for "native" with MCU "native".

/usr/bin/ld: /home/mlenders/Repositories/RIOT-OS/RIOT/tests/gnrc_sock_udp/bin/native/cpu/tramp.o: warning: relocation against `_native_saved_eip' in read-only section `.text'
/usr/bin/ld: warning: creating DT_TEXTREL in a PIE
   text	   data	    bss	    dec	    hex	filename
 155796	    692	  67756	 224244	  36bf4	/home/mlenders/Repositories/RIOT-OS/RIOT/tests/gnrc_sock_udp/bin/native/tests_gnrc_sock_udp.elf
true 
r
/home/mlenders/Repositories/RIOT-OS/RIOT/tests/gnrc_sock_udp/bin/native/tests_gnrc_sock_udp.elf /dev/ttyACM0 
RIOT native interrupts/signals initialized.
LED_RED_OFF
LED_GREEN_ON
RIOT native board initialized.
RIOT native hardware initialization complete.

Help: Press s to start test, r to print it is ready
READY
s
START
main(): This is RIOT! (Version: buildtest)
Calling test_sock_udp_create__EADDRINUSE()
Calling test_sock_udp_create__EAFNOSUPPORT()
Calling test_sock_udp_create__EINVAL_addr()
Calling test_sock_udp_create__EINVAL_netif()
Calling test_sock_udp_create__no_endpoints()
Calling test_sock_udp_create__only_local()
Calling test_sock_udp_create__only_local_port0()
Calling test_sock_udp_create__only_local_reuse_ep()
Calling test_sock_udp_create__only_remote()
Calling test_sock_udp_create__full()
Calling test_sock_udp_recv__EADDRNOTAVAIL()
Calling test_sock_udp_recv__EAGAIN()
Calling test_sock_udp_recv__ENOBUFS()
Calling test_sock_udp_recv__EPROTO()
Calling test_sock_udp_recv__ETIMEDOUT()
 * Calling sock_udp_recv()
 * (timed out with timeout 1000000)
Calling test_sock_udp_recv__socketed()
Calling test_sock_udp_recv__socketed_with_remote()
Calling test_sock_udp_recv__socketed_with_port0()
Calling test_sock_udp_recv__unsocketed()
Calling test_sock_udp_recv__unsocketed_with_remote()
Calling test_sock_udp_recv__with_timeout()
Calling test_sock_udp_recv__non_blocking()
Calling test_sock_udp_recv__aux()
Calling test_sock_udp_recv_buf__success()
Calling test_sock_udp_send__EAFNOSUPPORT()
Calling test_sock_udp_send__EINVAL_addr()
Calling test_sock_udp_send__EINVAL_netif()
Calling test_sock_udp_send__EINVAL_port()
Calling test_sock_udp_send__ENOTCONN()
Calling test_sock_udp_send__socketed_no_local_no_netif()
Calling test_sock_udp_send__socketed_no_netif()
Calling test_sock_udp_send__socketed_no_local()
Calling test_sock_udp_send__socketed()
Calling test_sock_udp_send__socketed_other_remote()
Calling test_sock_udp_send__unsocketed_no_local_no_netif()
Calling test_sock_udp_send__unsocketed_no_netif()
Calling test_sock_udp_send__unsocketed_no_local()
Calling test_sock_udp_send__unsocketed()
Calling test_sock_udp_send__no_sock_no_netif()
Calling test_sock_udp_send__no_sock()
ALL TESTS SUCCESSFUL

Building application "tests_gnrc_sock_udp" for "native" with MCU "native".

/usr/bin/ld: /home/mlenders/Repositories/RIOT-OS/RIOT/tests/gnrc_sock_udp/bin/native/cpu/tramp.o: warning: relocation against `_native_saved_eip' in read-only section `.text'
/usr/bin/ld: warning: creating DT_TEXTREL in a PIE
   text	   data	    bss	    dec	    hex	filename
 155556	    692	  67756	 224004	  36b04	/home/mlenders/Repositories/RIOT-OS/RIOT/tests/gnrc_sock_udp/bin/native/tests_gnrc_sock_udp.elf
true 
r
/home/mlenders/Repositories/RIOT-OS/RIOT/tests/gnrc_sock_udp/bin/native/tests_gnrc_sock_udp.elf /dev/ttyACM0 
RIOT native interrupts/signals initialized.
LED_RED_OFF
LED_GREEN_ON
RIOT native board initialized.
RIOT native hardware initialization complete.

Help: Press s to start test, r to print it is ready
READY
s
START
main(): This is RIOT! (Version: buildtest)
Calling test_sock_udp_create__EADDRINUSE()
Calling test_sock_udp_create__EAFNOSUPPORT()
Calling test_sock_udp_create__EINVAL_addr()
Calling test_sock_udp_create__EINVAL_netif()
Calling test_sock_udp_create__no_endpoints()
Calling test_sock_udp_create__only_local()
Calling test_sock_udp_create__only_local_port0()
Calling test_sock_udp_create__only_local_reuse_ep()
Calling test_sock_udp_create__only_remote()
Calling test_sock_udp_create__full()
Calling test_sock_udp_recv__EADDRNOTAVAIL()
Calling test_sock_udp_recv__EAGAIN()
Calling test_sock_udp_recv__ENOBUFS()
Calling test_sock_udp_recv__EPROTO()
Calling test_sock_udp_recv__ETIMEDOUT()
 * Calling sock_udp_recv()
 * (timed out with timeout 1000000)
Calling test_sock_udp_recv__socketed()
Calling test_sock_udp_recv__socketed_with_remote()
Calling test_sock_udp_recv__socketed_with_port0()
Calling test_sock_udp_recv__unsocketed()
Calling test_sock_udp_recv__unsocketed_with_remote()
Calling test_sock_udp_recv__with_timeout()
Calling test_sock_udp_recv__non_blocking()
Calling test_sock_udp_recv__aux()
Calling test_sock_udp_recv_buf__success()
Calling test_sock_udp_send__EAFNOSUPPORT()
Calling test_sock_udp_send__EINVAL_addr()
Calling test_sock_udp_send__EINVAL_netif()
Calling test_sock_udp_send__EINVAL_port()
Calling test_sock_udp_send__ENOTCONN()
Calling test_sock_udp_send__socketed_no_local_no_netif()
Calling test_sock_udp_send__socketed_no_netif()
Calling test_sock_udp_send__socketed_no_local()
Calling test_sock_udp_send__socketed()
Calling test_sock_udp_send__socketed_other_remote()
Calling test_sock_udp_send__unsocketed_no_local_no_netif()
Calling test_sock_udp_send__unsocketed_no_netif()
Calling test_sock_udp_send__unsocketed_no_local()
Calling test_sock_udp_send__unsocketed()
Calling test_sock_udp_send__no_sock_no_netif()
Calling test_sock_udp_send__no_sock()
ALL TESTS SUCCESSFUL

success

Non-native should be covered by the tests on Murdock and we already discussed, that their failures where unrelated.

miri64

Static tests failed because of a missing doc.

miri64 · 2020-12-04T11:41:21Z

sys/net/gnrc/sock/include/gnrc_sock_internal.h

+#if !IS_USED(MODULE_SOCK_AUX_LOCAL)
+    uint8_t this_struct_is_not_empty;
+#endif


Murdock wants this to be documented. Maybe just hide it from it?

Suggested change

#if !IS_USED(MODULE_SOCK_AUX_LOCAL)

uint8_t this_struct_is_not_empty;

#endif

#if !IS_USED(MODULE_SOCK_AUX_LOCAL) && !IS_ACTIVE(DOXYGEN)

uint8_t this_struct_is_not_empty;

#endif

I'm not sure if Doxygen is so smart that it can parse IS_ACTIVE() - it would need a C99 compliant preprocessor for this.

Maybe it is indeed the easiest to just document it.

I'm not sure if Doxygen is so smart that it can parse IS_ACTIVE() - it would need a C99 compliant preprocessor for this.

I think in that case it is just enough to check if it is not defined

Suggested change

#if !IS_USED(MODULE_SOCK_AUX_LOCAL)

uint8_t this_struct_is_not_empty;

#endif

#if !IS_USED(MODULE_SOCK_AUX_LOCAL) && !defined(DOXYGEN)

uint8_t this_struct_is_not_empty;

#endif

Provide address the IP packet / UDP datagram was received on in the auxiliary data, if module sock_aux_local is used.

maribu · 2020-12-04T11:49:42Z

I extended the doc of the struct and squashed right in. See here for the diff.

miri64 · 2020-12-04T11:53:02Z

I think the revert commit can also squashed into the commit it reverts ;-).

maribu · 2020-12-04T11:55:17Z

I think the revert commit can also squashed into the commit it reverts ;-).

Good catch. Maybe I should stop blindly trusting --autosquash.

miri64 · 2020-12-04T11:57:57Z

I think the revert commit can also squashed into the commit it reverts ;-).

Good catch. Maybe I should stop blindly trusting --autosquash.

It's awesome, but I never understood why Revert commits are not included in that :-/

miri64

ACK. Code is sane and I provided tests as well as ran the tests on Murdock.

maribu · 2020-12-04T13:41:17Z

Thanks :-)

maribu added Area: network Area: Networking Type: new feature The issue requests / The PR implemements a new feature for RIOT State: waiting for other PR State: The PR requires another PR to be merged first labels Aug 5, 2020

maribu requested review from jia200x and miri64 August 5, 2020 07:35

maribu requested review from PeterKietzmann, cgundogan and haukepetersen as code owners August 5, 2020 07:35

This was referenced Aug 5, 2020

pkg/lwip: Implement sock_aux_local #14705

Merged

pkg/tinydtls: Implement sock_aux_local #14706

Merged

maribu force-pushed the sock-aux-gnrc branch from 12e9a48 to e472811 Compare August 5, 2020 07:51

maribu requested a review from leandrolanzieri as a code owner August 5, 2020 07:51

maribu mentioned this pull request Aug 5, 2020

net/sock: Add access to auxiliary data #14622

Closed

maribu force-pushed the sock-aux-gnrc branch from e472811 to b9d482e Compare October 23, 2020 15:08

chrysn reviewed Oct 23, 2020

View reviewed changes

sys/net/gnrc/sock/ip/gnrc_sock_ip.c Outdated Show resolved Hide resolved

miri64 reviewed Oct 26, 2020

View reviewed changes

sys/net/gnrc/sock/ip/gnrc_sock_ip.c Outdated Show resolved Hide resolved

sys/net/gnrc/sock/ip/gnrc_sock_ip.c Outdated Show resolved Hide resolved

miri64 mentioned this pull request Nov 4, 2020

pkg/openwsn: add sock_udp [upstream implementation] #15310

Closed

maribu force-pushed the sock-aux-gnrc branch from 7c2ec52 to 0f5cde9 Compare December 3, 2020 11:14

maribu requested review from MichelRottleuthner, aabadie, fjmolinas and smlng as code owners December 3, 2020 11:14

maribu force-pushed the sock-aux-gnrc branch from 0f5cde9 to d927d9d Compare December 3, 2020 11:19

maribu force-pushed the sock-aux-gnrc branch from d927d9d to e111c4b Compare December 3, 2020 11:29

miri64 requested changes Dec 3, 2020

View reviewed changes

sys/net/gnrc/sock/gnrc_sock.c Outdated Show resolved Hide resolved

miri64 reviewed Dec 3, 2020

View reviewed changes

tests/gnrc_sock_ip/Makefile Outdated Show resolved Hide resolved

tests/gnrc_sock_udp/Makefile Outdated Show resolved Hide resolved

miri64 removed the State: waiting for other PR State: The PR requires another PR to be merged first label Dec 3, 2020

maribu commented Dec 4, 2020

View reviewed changes

sys/net/gnrc/sock/gnrc_sock.c Outdated Show resolved Hide resolved

miri64 removed the CI: run tests If set, CI server will run tests on hardware for the labeled PR label Dec 4, 2020

miri64 requested changes Dec 4, 2020

View reviewed changes

sys/net/gnrc/sock/ip/gnrc_sock_ip.c Outdated Show resolved Hide resolved

sys/net/gnrc/sock/udp/gnrc_sock_udp.c Outdated Show resolved Hide resolved

maribu mentioned this pull request Dec 4, 2020

net/gnrc/sock: Implement sock_aux_timestamp for RX #15560

Merged

miri64 requested changes Dec 4, 2020

View reviewed changes

maribu added 3 commits December 4, 2020 12:47

sys/net/gnrc: Implement sock_aux_local

1d69f06

Provide address the IP packet / UDP datagram was received on in the auxiliary data, if module sock_aux_local is used.

tests/gnrc_sock_udp: Added test for sock_aux_local

f97c190

tests/gnrc_sock_ip: Added test for sock_aux_local

3e1417c

maribu force-pushed the sock-aux-gnrc branch from eb90d8a to 742a96e Compare December 4, 2020 11:48

maribu force-pushed the sock-aux-gnrc branch from 742a96e to 3e1417c Compare December 4, 2020 11:54

miri64 approved these changes Dec 4, 2020

View reviewed changes

miri64 merged commit ba89d86 into RIOT-OS:master Dec 4, 2020

maribu deleted the sock-aux-gnrc branch December 4, 2020 13:41

Conversation

maribu commented Aug 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution description

Testing procedure

Issues/PRs references

Uh oh!

Uh oh!

Uh oh!

Uh oh!

maribu commented Nov 4, 2020

Uh oh!

maribu commented Dec 3, 2020

Uh oh!

maribu commented Dec 3, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

maribu commented Dec 4, 2020

Uh oh!

Uh oh!

miri64 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

miri64 commented Dec 4, 2020

Uh oh!

miri64 commented Dec 4, 2020

Uh oh!

miri64 left a comment

Choose a reason for hiding this comment

Uh oh!

miri64 Dec 4, 2020

Choose a reason for hiding this comment

Uh oh!

maribu Dec 4, 2020

Choose a reason for hiding this comment

Uh oh!

miri64 Dec 4, 2020

Choose a reason for hiding this comment

Uh oh!

maribu commented Dec 4, 2020

Uh oh!

miri64 commented Dec 4, 2020

Uh oh!

maribu commented Dec 4, 2020

Uh oh!

miri64 commented Dec 4, 2020

Uh oh!

miri64 left a comment

Choose a reason for hiding this comment

Uh oh!

maribu commented Dec 4, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

maribu commented Aug 5, 2020 •

edited

Loading