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

Fix inifinte loop in esp32 RIO handling. #12736

Merged
merged 1 commit into from
Dec 8, 2021

Conversation

cecille
Copy link
Contributor

@cecille cecille commented Dec 8, 2021

Problem

If the router is advertising a default prefix (::/0), the route
handler is going into an infinite loop.

Route information options can only have one prefix anyway, so the
loop here is overkill and likely to introduce bad routes.

Change overview

Change for loop to a simple check that the size is sufficient.

Testing

Configured radvd to advertise a default route, saw infinite loop on upstream, no problems with this code. Regular routes are being processed correctly.

If the router is advertising a default prefix (::/0), the route
handler is going into an infinite loop.

Route information options can only have one prefix anyway, so the
loop here is overkill and likely to introduce bad routes. Changing
this to a simple check.

Test: Configured radvd to advertise a default route, saw infinite
      loop on upstream, no problems with this code. Regular routes
      are being processed correctly.
@github-actions
Copy link

github-actions bot commented Dec 8, 2021

PR #12736: Size comparison from 1f4024a to 84dda1e

Decreases (2 builds for esp32)
platform target config section 1f4024a 84dda1e change % change
esp32 all-clusters-app c3devkit (read only) 855952 855942 -10 -0.0
.flash.text 855952 855942 -10 -0.0
m5stack (read only) 958923 958891 -32 -0.0
.flash.text 953539 953507 -32 -0.0
Full report (32 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
platform target config section 1f4024a 84dda1e change % change
efr32 lighting-app BRD4161A (read only) 794956 794956 0 0.0
(read/write) 122996 122996 0 0.0
.bss 121176 121176 0 0.0
.data 1820 1820 0 0.0
.text 794948 794948 0 0.0
BRD4161A+rpc (read only) 782552 782552 0 0.0
(read/write) 139680 139680 0 0.0
.bss 137752 137752 0 0.0
.data 1928 1928 0 0.0
.text 782544 782544 0 0.0
window-app BRD4161A (read only) 771908 771908 0 0.0
(read/write) 121136 121136 0 0.0
.bss 119352 119352 0 0.0
.data 1784 1784 0 0.0
.text 771900 771900 0 0.0
esp32 all-clusters-app c3devkit (read only) 855952 855942 -10 -0.0
(read/write) 1306034 1306034 0 0.0
.dram0.bss 67648 67648 0 0.0
.dram0.data 14124 14124 0 0.0
.flash.rodata 171232 171232 0 0.0
.flash.text 855952 855942 -10 -0.0
.iram0.text 62076 62076 0 0.0
m5stack (read only) 958923 958891 -32 -0.0
(read/write) 450252 450252 0 0.0
.dram0.bss 75000 75000 0 0.0
.dram0.data 34048 34048 0 0.0
.flash.rodata 209416 209416 0 0.0
.flash.text 953539 953507 -32 -0.0
.iram0.text 123451 123451 0 0.0
k32w lighting-app k32w061+se05x+release (read/write) 732132 732132 0 0.0
.bss 81248 81248 0 0.0
.data 1852 1852 0 0.0
.text 643232 643232 0 0.0
lock-app k32w061+debug (read/write) 622716 622716 0 0.0
.bss 71912 71912 0 0.0
.data 1820 1820 0 0.0
.text 543184 543184 0 0.0
shell k32w061+debug (read/write) 685808 685808 0 0.0
.bss 81612 81612 0 0.0
.data 1792 1792 0 0.0
.text 596604 596604 0 0.0
linux chip-tool-ipv6only arm64 (read only) 6665652 6665652 0 0.0
(read/write) 305217 305217 0 0.0
.bss 51665 51665 0 0.0
.data 1048 1048 0 0.0
.data.rel.ro 201888 201888 0 0.0
.dynamic 560 560 0 0.0
.got 46984 46984 0 0.0
.init 24 24 0 0.0
.init_array 160 160 0 0.0
.rodata 338548 338548 0 0.0
.text 5682820 5682820 0 0.0
thermostat-no-ble arm64 (read only) 1906556 1906556 0 0.0
(read/write) 135825 135825 0 0.0
.bss 59457 59457 0 0.0
.data 776 776 0 0.0
.data.rel.ro 69280 69280 0 0.0
.dynamic 560 560 0 0.0
.got 3456 3456 0 0.0
.init 24 24 0 0.0
.init_array 256 256 0 0.0
.rodata 123964 123964 0 0.0
.text 1577744 1577744 0 0.0
mbed all-clusters-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2316496 2316496 0 0.0
.bss 186900 186900 0 0.0
.data 5232 5232 0 0.0
.heap 844312 844312 0 0.0
.text 1279072 1279072 0 0.0
lighting-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2299016 2299016 0 0.0
.bss 175712 175712 0 0.0
.data 5488 5488 0 0.0
.heap 855248 855248 0 0.0
.text 1261616 1261616 0 0.0
lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2271928 2271928 0 0.0
.bss 174752 174752 0 0.0
.data 5488 5488 0 0.0
.heap 856208 856208 0 0.0
.text 1234528 1234528 0 0.0
pigweed-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 1140008 1140008 0 0.0
.bss 11756 11756 0 0.0
.data 4376 4376 0 0.0
.heap 1020312 1020312 0 0.0
.text 103392 103392 0 0.0
shell CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2047808 2047808 0 0.0
.bss 156564 156564 0 0.0
.data 4864 4864 0 0.0
.heap 875016 875016 0 0.0
.text 1010408 1010408 0 0.0
nrfconnect lighting-app nrf52840dk_nrf52840 (read/write) 897027 897027 0 0.0
bss 116184 116184 0 0.0
rodata 100172 100172 0 0.0
text 605116 605116 0 0.0
nrf52840dk_nrf52840+rpc (read/write) 860051 860051 0 0.0
bss 112532 112532 0 0.0
rodata 91468 91468 0 0.0
text 579844 579844 0 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 822974 822974 0 0.0
bss 117560 117560 0 0.0
rodata 95432 95432 0 0.0
text 535496 535496 0 0.0
lock-app nrf52840dk_nrf52840 (read/write) 869499 869499 0 0.0
bss 113448 113448 0 0.0
rodata 96292 96292 0 0.0
text 584376 584376 0 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 795698 795698 0 0.0
bss 114856 114856 0 0.0
rodata 91580 91580 0 0.0
text 514848 514848 0 0.0
pigweed-app nrf52840dk_nrf52840 (read/write) 497463 497463 0 0.0
bss 51820 51820 0 0.0
rodata 45852 45852 0 0.0
text 339492 339492 0 0.0
pump-app nrf52840dk_nrf52840 (read/write) 874347 874347 0 0.0
bss 113360 113360 0 0.0
rodata 97644 97644 0 0.0
text 587892 587892 0 0.0
pump-controller-app nrf52840dk_nrf52840 (read/write) 867587 867587 0 0.0
bss 113236 113236 0 0.0
rodata 95780 95780 0 0.0
text 583112 583112 0 0.0
shell nrf52840dk_nrf52840 (read/write) 781611 781611 0 0.0
bss 109552 109552 0 0.0
rodata 74280 74280 0 0.0
text 523264 523264 0 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 696686 696686 0 0.0
bss 110532 110532 0 0.0
rodata 68920 68920 0 0.0
text 443904 443904 0 0.0
p6 all-clusters-app default (read/write) 2350400 2350400 0 0.0
.bss 113132 113132 0 0.0
.data 2512 2512 0 0.0
.heap 917696 917696 0 0.0
.text 1308664 1308664 0 0.0
light-app default (read/write) 2281464 2281464 0 0.0
.bss 100904 100904 0 0.0
.data 2328 2328 0 0.0
.heap 930112 930112 0 0.0
.text 1239728 1239728 0 0.0
lock-app default (read/write) 2257568 2257568 0 0.0
.bss 99784 99784 0 0.0
.data 2288 2288 0 0.0
.heap 931272 931272 0 0.0
.text 1215832 1215832 0 0.0
qpg lighting-app qpg6100+debug (read only) 511948 511948 0 0.0
(read/write) 122332 122332 0 0.0
.bss 82624 82624 0 0.0
.data 956 956 0 0.0
.text 506628 506628 0 0.0
lock-app qpg6100+debug (read only) 486140 486140 0 0.0
(read/write) 122336 122336 0 0.0
.bss 81760 81760 0 0.0
.data 912 912 0 0.0
.text 480820 480820 0 0.0
persistent-storage-app qpg6100+debug (read only) 108104 108104 0 0.0
(read/write) 122336 122336 0 0.0
.bss 36152 36152 0 0.0
.data 288 288 0 0.0
.text 102784 102784 0 0.0
telink lighting-app tlsr9518adk80d (read/write) 800386 800386 0 0.0
bss 82764 82764 0 0.0
noinit 37160 37160 0 0.0
text 557446 557446 0 0.0

@cecille cecille requested a review from gjc13 December 8, 2021 20:26
@andy31415
Copy link
Contributor

Fast track due to small size.

This is more of a hotfix: this was demostratably causing infinite loops and that was fixed and tested. Any followup reviews will be done separately. For now, pushing the 'stop the ability to busyloop' out of the way.

@andy31415 andy31415 merged commit a5f849c into project-chip:master Dec 8, 2021
@gjc13
Copy link
Contributor

gjc13 commented Dec 9, 2021

It's fine to just use if (rio_data_len >= prefix_len_bytes) to check for the prefix data since there will only be on prefix in each RIO record.

@cecille cecille deleted the rio_bug_esp32 branch December 9, 2021 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants