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

gnrc_ipv6_nib: handle iface_up/iface_down in IPv6 thread #18708

Merged

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Oct 7, 2022

Contribution description

The main motivation for this was, that I observed that with #18700 the LoRaWAN thread suddenly used a huge amount of stack space. Given that gnrc_ipv6_nib_iface_up accumulated already uses over 100 bytes on the board I was using (b-l072z-lrwan1), the function is called very deep in the stack, and since it is really weird, that the netif calls a function that configures an IPv6 component, I decided to decouple this call into the gnrc_ipv6 handler, which is actually a cleaner approach.

Testing procedure

I do not have a device that supports this currently in master, but I confirmed that the up-event is still executed when merging this PR (and the changes I will push to #18699 in a moment) by checking if the link-local address was configured, using ifconfig.

Issues/PRs references

Follow-up on #17893

@miri64 miri64 added the Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation label Oct 7, 2022
@github-actions github-actions bot added Area: network Area: Networking Area: sys Area: System labels Oct 7, 2022
@miri64 miri64 force-pushed the gnrc_ipv6_nib/enh/handle-up-down-in-gnrc_ipv6 branch from 79e6591 to 054028f Compare October 7, 2022 11:14
@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Oct 7, 2022
@riot-ci
Copy link

riot-ci commented Oct 7, 2022

Murdock results

✔️ PASSED

a617700 examples/telnet_server: add stm32f7508-dk to BOARD_INSUFFICIENT_MEMORY

Success Failures Total Runtime
1980 0 1980 08m:21s

Artifacts

This only reflects a subset of all builds from https://ci-prod.riot-os.org. Please refer to https://ci.riot-os.org for a complete build for now.

@miri64 miri64 force-pushed the gnrc_ipv6_nib/enh/handle-up-down-in-gnrc_ipv6 branch from 054028f to 47aaa94 Compare October 7, 2022 12:12
Copy link
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stack usage of the wifi thread went down on esp32-wroom-32 with tests/nanocoap_cli:

before

2022-10-10 13:10:40,809 #       pid | name                 | state    Q | pri | stack  ( used) ( free) | base addr  | current
2022-10-10 13:10:40,817 #         1 | sys_evt              | bl mutex _ |   4 |   2616 ( 1228) ( 1388) | 0x3ffbc5b4 | 0x3ffbcd90
2022-10-10 13:10:40,826 #         2 | esp_timer            | sleeping _ |   2 |   3640 (  456) ( 3184) | 0x3ffbcff0 | 0x3ffbdc60
2022-10-10 13:10:40,834 #         3 | idle                 | pending  Q |  31 |   2048 (  516) ( 1532) | 0x3ffb2404 | 0x3ffb2a40
2022-10-10 13:10:40,843 #         4 | main                 | running  Q |  15 |   4096 ( 1716) ( 2380) | 0x3ffb2c04 | 0x3ffb3830
2022-10-10 13:10:40,851 #         5 | ipv6                 | bl rx    _ |  12 |   2048 (  728) ( 1320) | 0x3ffb63d8 | 0x3ffb6960
2022-10-10 13:10:40,860 #         6 | udp                  | bl rx    _ |  13 |   2048 (  664) ( 1384) | 0x3ffb8708 | 0x3ffb8cd0
2022-10-10 13:10:40,869 #         7 | wifi                 | bl mutex _ |   1 |   6200 ( 2188) ( 4012) | 0x3ffbe494 | 0x3ffbfa90
2022-10-10 13:10:40,877 #         8 | netif-esp-wifi       | bl anyfl _ |  10 |   2048 (  996) ( 1052) | 0x3ffb48f4 | 0x3ffb4e80
2022-10-10 13:10:40,884 #           | SUM                  |            |     |  24744 ( 8492) (16252)

with this patch

2022-10-10 13:11:27,727 #       pid | name                 | state    Q | pri | stack  ( used) ( free) | base addr  | current
2022-10-10 13:11:27,736 #         1 | sys_evt              | bl mutex _ |   4 |   2616 ( 1228) ( 1388) | 0x3ffbc5b4 | 0x3ffbcd90
2022-10-10 13:11:27,745 #         2 | esp_timer            | sleeping _ |   2 |   3640 (  456) ( 3184) | 0x3ffbcff0 | 0x3ffbdc60
2022-10-10 13:11:27,753 #         3 | idle                 | pending  Q |  31 |   2048 (  516) ( 1532) | 0x3ffb2404 | 0x3ffb2a40
2022-10-10 13:11:27,762 #         4 | main                 | running  Q |  15 |   4096 ( 1716) ( 2380) | 0x3ffb2c04 | 0x3ffb3630
2022-10-10 13:11:27,770 #         5 | ipv6                 | bl rx    _ |  12 |   2048 (  728) ( 1320) | 0x3ffb63d8 | 0x3ffb6960
2022-10-10 13:11:27,779 #         6 | udp                  | bl rx    _ |  13 |   2048 (  664) ( 1384) | 0x3ffb8708 | 0x3ffb8cd0
2022-10-10 13:11:27,788 #         7 | wifi                 | bl mutex _ |   1 |   6200 ( 1756) ( 4444) | 0x3ffbe494 | 0x3ffbfa90
2022-10-10 13:11:27,796 #         8 | netif-esp-wifi       | bl anyfl _ |  10 |   2048 (  996) ( 1052) | 0x3ffb48f4 | 0x3ffb4e80
2022-10-10 13:11:27,802 #           | SUM                  |            |     |  24744 ( 8060) (16684)

@miri64
Copy link
Member Author

miri64 commented Oct 10, 2022

Stack usage of the wifi thread went down on esp32-wroom-32 with tests/nanocoap_cli:

Great, is it also still working ^^"? As I said, I only was able to test this with gnrc_netif_lorawan in #18700.

@benpicco
Copy link
Contributor

Sure, there is now of course an added point where the message might get lost due to a full queue (e.g. when the interface goes down while the application is sending) and the ROM size increased a bit (which pushed stm32f7508-dk across the limit for examples/telnet_server), but normal operation is not affected by the additional message hop.

@miri64
Copy link
Member Author

miri64 commented Oct 10, 2022

Sure, there is now of course an added point where the message might get lost due to a full queue (e.g. when the interface goes down while the application is sending)

Since I use msg_send() and not msg_try_send() (as we are in thread context here already, this is possible), the netif should block in case the queue is full of IPv6 and only resume, once the queue has space ;-).

@github-actions github-actions bot added the Area: examples Area: Example Applications label Oct 10, 2022
@benpicco benpicco merged commit 988039b into RIOT-OS:master Oct 10, 2022
@miri64 miri64 deleted the gnrc_ipv6_nib/enh/handle-up-down-in-gnrc_ipv6 branch October 11, 2022 06:38
@maribu maribu added this to the Release 2022.10 milestone Oct 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: examples Area: Example Applications Area: network Area: Networking Area: sys Area: System CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants