Skip to content

gnrc_ipv6_nib: handle iface_up/iface_down in IPv6 thread#18708

Merged
benpicco merged 2 commits intoRIOT-OS:masterfrom
miri64:gnrc_ipv6_nib/enh/handle-up-down-in-gnrc_ipv6
Oct 10, 2022
Merged

gnrc_ipv6_nib: handle iface_up/iface_down in IPv6 thread#18708
benpicco merged 2 commits intoRIOT-OS:masterfrom
miri64:gnrc_ipv6_nib/enh/handle-up-down-in-gnrc_ipv6

Conversation

@miri64
Copy link
Copy Markdown
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
@miri64 miri64 requested review from benpicco and jia200x October 7, 2022 10:38
@github-actions github-actions bot added Area: network Area: Networking Area: sys Area: System labels Oct 7, 2022
benpicco
benpicco reviewed Oct 7, 2022
View reviewed changes
@miri64 miri64 mentioned this pull request Oct 7, 2022
Merged
@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
Copy Markdown

riot-ci commented Oct 7, 2022 •
edited
Loading

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
benpicco
benpicco approved these changes Oct 10, 2022
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor

benpicco 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) 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
Copy Markdown
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

Reviewers

@benpicco benpicco benpicco approved these changes

@jia200x jia200x Awaiting requested review from jia200x

@haukepetersen haukepetersen Awaiting requested review from haukepetersen

@PeterKietzmann PeterKietzmann Awaiting requested review from PeterKietzmann

@cgundogan cgundogan Awaiting requested review from cgundogan

Assignees

No one assigned

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

Milestone

Release 2022.10

Development

Successfully merging this pull request may close these issues.

4 participants

@miri64 @riot-ci @benpicco @maribu