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.

Sign up for GitHub

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

Jump to bottom

[Zephyr] Fix potential data race on multicast configuration #20722

Merged
merged 1 commit into from
Jul 15, 2022
Merged

[Zephyr] Fix potential data race on multicast configuration #20722

merged 1 commit into from
Jul 15, 2022

Conversation

Damian-Nordic
Copy link
Contributor

Problem

OpenThread functions for joining or leaving multicast groups would be called without holding the Thread stack lock.

Change overview

Add missing locks.

Testing

Verified that groups add-group/remove-group commands still work as expected.

@Damian-Nordic
[Zephyr] Fix potential data race on multicast configuration
3696b7f 
OpenThread functions for joining or leaving multicast groups
would be called without holding the Thread stack lock.

Signed-off-by: Damian Krolik <[email protected]>
@github-actions
Copy link

github-actions bot commented Jul 14, 2022 •
edited
Loading

PR #20722: Size comparison from 8d07348 to 3696b7f

Increases (15 builds for bl602, cc13x2_26x2, nrfconnect, telink)
platform target config section 8d07348 3696b7f change % change
bl602 lighting-app bl602 .text 1058636 1058640 4 0.0
bl602+rpc (read/write) 1443058 1443066 8 0.0
.text 1090320 1090324 4 0.0
cc13x2_26x2 lock-mtd LP_CC2652R7 (read only) 618583 618591 8 0.0
.text 541936 541944 8 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1175411 1175427 16 0.0
text 811544 811568 24 0.0
all-clusters-minimal-app nrf52840dk_nrf52840 (read/write) 1155607 1155639 32 0.0
text 800988 801012 24 0.0
lighting-app nrf52840dk_nrf52840 (read/write) 1166835 1166867 32 0.0
text 819484 819508 24 0.0
nrf52840dk_nrf52840+rpc (read/write) 1137107 1137123 16 0.0
text 797460 797480 20 0.0
nrf52840dongle_nrf52840 (read/write) 1073411 1073427 16 0.0
text 747524 747548 24 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 1070390 1070406 16 0.0
text 732028 732048 20 0.0
lock-app nrf52840dk_nrf52840 (read/write) 1127639 1127655 16 0.0
text 771512 771532 20 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 1031330 1031346 16 0.0
text 684120 684140 20 0.0
pump-app nrf52840dk_nrf52840 (read/write) 1079671 1079687 16 0.0
text 749092 749116 24 0.0
pump-controller-app nrf52840dk_nrf52840 (read/write) 1067639 1067671 32 0.0
text 740416 740440 24 0.0
telink light-switch-app tlsr9518adk80d (read/write) 797540 797564 24 0.0
text 565732 565756 24 0.0
lighting-app tlsr9518adk80d (read/write) 817364 817396 32 0.0
text 582056 582082 26 0.0
Decreases (2 builds for cc13x2_26x2, cyw30739)
platform target config section 8d07348 3696b7f change % change
cc13x2_26x2 all-clusters-minimal-app LP_CC2652R7 (read only) 632083 632075 -8 -0.0
.text 554380 554372 -8 -0.0
cyw30739 light cyw930739m2evb_01 (read/write) 579622 579614 -8 -0.0
.app_xip_area 458376 458368 -8 -0.0
Full report (40 builds for bl602, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
platform target config section 8d07348 3696b7f change % change
bl602 lighting-app bl602 (read/write) 1397634 1397634 0 0.0
.bss 116978 116978 0 0.0
.data 4480 4480 0 0.0
.text 1058636 1058640 4 0.0
bl602+rpc (read/write) 1443058 1443066 8 0.0
.bss 124418 124418 0 0.0
.data 4600 4600 0 0.0
.text 1090320 1090324 4 0.0
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read only) 666275 666275 0 0.0
(read/write) 184948 184948 0 0.0
.bss 74116 74116 0 0.0
.data 3356 3356 0 0.0
.rodata 88139 88139 0 0.0
.text 577820 577820 0 0.0
all-clusters-minimal-app LP_CC2652R7 (read only) 632083 632075 -8 -0.0
(read/write) 157684 157684 0 0.0
.bss 73412 73412 0 0.0
.data 3356 3356 0 0.0
.rodata 77379 77379 0 0.0
.text 554380 554372 -8 -0.0
lock-ftd LP_CC2652R7 (read only) 669183 669183 0 0.0
(read/write) 172184 172184 0 0.0
.bss 71148 71148 0 0.0
.data 3280 3280 0 0.0
.rodata 76279 76279 0 0.0
.text 592424 592424 0 0.0
lock-mtd LP_CC2652R7 (read only) 618583 618591 8 0.0
(read/write) 144264 144264 0 0.0
.bss 66868 66868 0 0.0
.data 3280 3280 0 0.0
.rodata 76159 76159 0 0.0
.text 541936 541944 8 0.0
pump-app LP_CC2652R7 (read only) 678431 678431 0 0.0
(read/write) 163784 163784 0 0.0
.bss 71228 71228 0 0.0
.data 3280 3280 0 0.0
.rodata 88703 88703 0 0.0
.text 589244 589244 0 0.0
pump-controller-app LP_CC2652R7 (read only) 664255 664255 0 0.0
(read/write) 178080 178080 0 0.0
.bss 71348 71348 0 0.0
.data 3276 3276 0 0.0
.rodata 84567 84567 0 0.0
.text 579208 579208 0 0.0
shell LP_CC2652R7 (read only) 658774 658774 0 0.0
(read/write) 187952 187952 0 0.0
.bss 76420 76420 0 0.0
.data 3360 3360 0 0.0
.rodata 84918 84918 0 0.0
.text 573540 573540 0 0.0
cyw30739 light cyw930739m2evb_01 (read/write) 579622 579614 -8 -0.0
.app_xip_area 458376 458368 -8 -0.0
.bss 64184 64184 0 0.0
.data 716 716 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
lock cyw930739m2evb_01 (read/write) 585574 585574 0 0.0
.app_xip_area 459600 459600 0 0.0
.bss 68912 68912 0 0.0
.data 720 720 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 583018 583018 0 0.0
.app_xip_area 462620 462620 0 0.0
.bss 63392 63392 0 0.0
.data 660 660 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
efr32 lighting-app BRD4161A (read/write) 1081700 1081700 0 0.0
.bss 132996 132996 0 0.0
.data 2048 2048 0 0.0
.text 946636 946636 0 0.0
BRD4161A+rpc (read/write) 1136012 1136012 0 0.0
.bss 149676 149676 0 0.0
.data 2260 2260 0 0.0
.text 984056 984056 0 0.0
BRD4161A+rs911x (read/write) 948172 948172 0 0.0
.bss 140768 140768 0 0.0
.data 2048 2048 0 0.0
.text 805336 805336 0 0.0
lock-app BRD4161A+wf200 (read/write) 1129096 1129096 0 0.0
.bss 144184 144184 0 0.0
.data 2060 2060 0 0.0
.text 982828 982828 0 0.0
window-app BRD4161A (read/write) 1075508 1075508 0 0.0
.bss 134468 134468 0 0.0
.data 2076 2076 0 0.0
.text 938940 938940 0 0.0
esp32 all-clusters-app c3devkit (read only) 1020154 1020154 0 0.0
(read/write) 1485842 1485842 0 0.0
.dram0.bss 70080 70080 0 0.0
.dram0.data 14600 14600 0 0.0
.flash.rodata 215728 215728 0 0.0
.flash.text 1020154 1020154 0 0.0
.iram0.text 62902 62902 0 0.0
m5stack (read only) 1074031 1074031 0 0.0
(read/write) 487904 487904 0 0.0
.dram0.bss 75600 75600 0 0.0
.dram0.data 34144 34144 0 0.0
.flash.rodata 246164 246164 0 0.0
.flash.text 1068647 1068647 0 0.0
.iram0.text 123267 123267 0 0.0
k32w light k32w061+release (read/write) 658832 658832 0 0.0
.bss 69516 69516 0 0.0
.data 1992 1992 0 0.0
.text 581524 581524 0 0.0
lock k32w061+release (read/write) 685684 685684 0 0.0
.bss 69980 69980 0 0.0
.data 2004 2004 0 0.0
.text 607900 607900 0 0.0
linux chip-tool-ipv6only arm64 (read only) 10068812 10068812 0 0.0
(read/write) 686913 686913 0 0.0
.bss 42961 42961 0 0.0
.data 3304 3304 0 0.0
.data.rel.ro 622984 622984 0 0.0
.dynamic 528 528 0 0.0
.got 13736 13736 0 0.0
.init 24 24 0 0.0
.init_array 192 192 0 0.0
.rodata 479628 479628 0 0.0
.text 8026596 8026596 0 0.0
thermostat-no-ble arm64 (read only) 2595668 2595668 0 0.0
(read/write) 158289 158289 0 0.0
.bss 65249 65249 0 0.0
.data 1704 1704 0 0.0
.data.rel.ro 83240 83240 0 0.0
.dynamic 528 528 0 0.0
.got 5072 5072 0 0.0
.init 24 24 0 0.0
.init_array 400 400 0 0.0
.rodata 165668 165668 0 0.0
.text 2190224 2190224 0 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2448312 2448312 0 0.0
.bss 213940 213940 0 0.0
.data 5872 5872 0 0.0
.text 1410956 1410956 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1175411 1175427 16 0.0
bss 142900 142900 0 0.0
rodata 142076 142076 0 0.0
text 811544 811568 24 0.0
all-clusters-minimal-app nrf52840dk_nrf52840 (read/write) 1155607 1155639 32 0.0
bss 142136 142136 0 0.0
rodata 133608 133608 0 0.0
text 800988 801012 24 0.0
lighting-app nrf52840dk_nrf52840 (read/write) 1166835 1166867 32 0.0
bss 144463 144463 0 0.0
rodata 124052 124052 0 0.0
text 819484 819508 24 0.0
nrf52840dk_nrf52840+rpc (read/write) 1137107 1137123 16 0.0
bss 144892 144892 0 0.0
rodata 115524 115524 0 0.0
text 797460 797480 20 0.0
nrf52840dongle_nrf52840 (read/write) 1073411 1073427 16 0.0
bss 151694 151694 0 0.0
rodata 93076 93076 0 0.0
text 747524 747548 24 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 1070390 1070406 16 0.0
bss 140834 140834 0 0.0
rodata 116468 116468 0 0.0
text 732028 732048 20 0.0
lock-app nrf52840dk_nrf52840 (read/write) 1127639 1127655 16 0.0
bss 135431 135431 0 0.0
rodata 141896 141896 0 0.0
text 771512 771532 20 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 1031330 1031346 16 0.0
bss 131834 131834 0 0.0
rodata 134368 134368 0 0.0
text 684120 684140 20 0.0
pump-app nrf52840dk_nrf52840 (read/write) 1079671 1079687 16 0.0
bss 134358 134358 0 0.0
rodata 117400 117400 0 0.0
text 749092 749116 24 0.0
pump-controller-app nrf52840dk_nrf52840 (read/write) 1067639 1067671 32 0.0
bss 134153 134153 0 0.0
rodata 114184 114184 0 0.0
text 740416 740440 24 0.0
p6 all-clusters-app default (read/write) 2566392 2566392 0 0.0
.bss 149120 149120 0 0.0
.data 2776 2776 0 0.0
.text 1524656 1524656 0 0.0
all-clusters-minimal-app default (read/write) 2511688 2511688 0 0.0
.bss 148400 148400 0 0.0
.data 2776 2776 0 0.0
.text 1469952 1469952 0 0.0
light-app default (read/write) 2441616 2441616 0 0.0
.bss 140456 140456 0 0.0
.data 2592 2592 0 0.0
.text 1399880 1399880 0 0.0
lock-app default (read/write) 2468768 2468768 0 0.0
.bss 140304 140304 0 0.0
.data 2600 2600 0 0.0
.text 1427032 1427032 0 0.0
telink light-switch-app tlsr9518adk80d (read/write) 797540 797564 24 0.0
bss 70576 70576 0 0.0
noinit 40416 40416 0 0.0
text 565732 565756 24 0.0
lighting-app tlsr9518adk80d (read/write) 817364 817396 32 0.0
bss 71420 71420 0 0.0
noinit 40416 40416 0 0.0
text 582056 582082 26 0.0

@bzbarsky-apple bzbarsky-apple merged commit 0561ad2 into project-chip:master Jul 15, 2022
github-actions bot pushed a commit that referenced this pull request Jul 15, 2022
@Damian-Nordic @web-flow
[Zephyr] Fix potential data race on multicast configuration (#20722)
79f0ebc 
OpenThread functions for joining or leaving multicast groups
would be called without holding the Thread stack lock.

Signed-off-by: Damian Krolik <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andy31415 andy31415 andy31415 approved these changes

@anush-apple anush-apple Awaiting requested review from anush-apple

@arkq arkq Awaiting requested review from arkq

@Byungjoo-Lee Byungjoo-Lee Awaiting requested review from Byungjoo-Lee

@carol-apple carol-apple Awaiting requested review from carol-apple

@chrisdecenzo chrisdecenzo Awaiting requested review from chrisdecenzo

@chshu chshu Awaiting requested review from chshu

@chulspro chulspro Awaiting requested review from chulspro

@dhrishi dhrishi Awaiting requested review from dhrishi

@electrocucaracha electrocucaracha Awaiting requested review from electrocucaracha

@emargolis emargolis Awaiting requested review from emargolis

@erjiaqing erjiaqing Awaiting requested review from erjiaqing

@franck-apple franck-apple Awaiting requested review from franck-apple

@gjc13 gjc13 Awaiting requested review from gjc13

@harimau-qirex harimau-qirex Awaiting requested review from harimau-qirex

@harsha-rajendran harsha-rajendran Awaiting requested review from harsha-rajendran

@hawk248 hawk248 Awaiting requested review from hawk248

@isiu-apple isiu-apple Awaiting requested review from isiu-apple

@jelderton jelderton Awaiting requested review from jelderton

@jepenven-silabs jepenven-silabs Awaiting requested review from jepenven-silabs

@jmartinez-silabs jmartinez-silabs Awaiting requested review from jmartinez-silabs

@jtung-apple jtung-apple Awaiting requested review from jtung-apple

@lazarkov lazarkov Awaiting requested review from lazarkov

@LuDuda LuDuda Awaiting requested review from LuDuda

@mrjerryjohns mrjerryjohns Awaiting requested review from mrjerryjohns

@msandstedt msandstedt Awaiting requested review from msandstedt

@mspang mspang Awaiting requested review from mspang

@rgoliver rgoliver Awaiting requested review from rgoliver

@saurabhst saurabhst Awaiting requested review from saurabhst

@selissia selissia Awaiting requested review from selissia

@tecimovic tecimovic Awaiting requested review from tecimovic

@tehampson tehampson Awaiting requested review from tehampson

@turon turon Awaiting requested review from turon

@vijs vijs Awaiting requested review from vijs

@vivien-apple vivien-apple Awaiting requested review from vivien-apple

@wbschiller wbschiller Awaiting requested review from wbschiller

@woody-apple woody-apple Awaiting requested review from woody-apple

@xylophone21 xylophone21 Awaiting requested review from xylophone21

@yufengwangca yufengwangca Awaiting requested review from yufengwangca

@yunhanw-google yunhanw-google Awaiting requested review from yunhanw-google

@bzbarsky-apple bzbarsky-apple Awaiting requested review from bzbarsky-apple

Assignees
No one assigned
Labels
platform review - approved zephyr
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Damian-Nordic @andy31415 @bzbarsky-apple