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

Implement required check for adding an already-existing fabric. #16404

Merged
merged 1 commit into from
Mar 22, 2022
Merged

Implement required check for adding an already-existing fabric. #16404

merged 1 commit into from
Mar 22, 2022

Conversation

bzbarsky-apple
Copy link
Contributor

Per spec, AddNOC should fail if the fabric id and root public key of
the new NOC match an existing fabric. We were not implementing this
check.

Problem

See above.

Change overview

Free up some error codes, add a new error code to represent "fabric exists", add check for existing fabric.

Testing

Adds YAML tests for this scenario.

@bzbarsky-apple
Implement required check for adding an already-existing fabric.
d592182 
Per spec, AddNOC should fail if the fabric id and root public key of
the new NOC match an existing fabric.  We were not implementing this
check.
@github-actions
Copy link

github-actions bot commented Mar 18, 2022 •
edited
Loading

PR #16404: Size comparison from c4d81c8 to d592182

Increases (18 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
platform target config section c4d81c8 d592182 change % change
cyw30739 light cyw930739m2evb_01 (read/write) 603538 603826 288 0.0
.app_xip_area 510636 510924 288 0.1
lock cyw930739m2evb_01 (read/write) 561326 561606 280 0.0
.app_xip_area 469952 470232 280 0.1
ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 571050 571338 288 0.1
.app_xip_area 470028 470316 288 0.1
efr32 lighting-app BRD4161A (read only) 921384 921800 416 0.0
.text 921376 921792 416 0.0
BRD4161A+rpc (read only) 950188 950604 416 0.0
.text 950180 950596 416 0.0
window-app BRD4161A (read only) 851944 852360 416 0.0
.text 851936 852352 416 0.0
esp32 all-clusters-app c3devkit (read only) 961498 961736 238 0.0
.flash.text 961498 961736 238 0.0
m5stack (read only) 1017639 1017967 328 0.0
.flash.text 1012255 1012583 328 0.0
k32w light k32w061+release (read/write) 700516 700804 288 0.0
.text 615196 615484 288 0.0
lock k32w061+release (read/write) 700652 700924 272 0.0
.text 615320 615592 272 0.0
linux chip-tool-ipv6only arm64 (read only) 9765964 9772828 6864 0.1
(read/write) 476113 476225 112 0.0
.data.rel.ro 371824 371880 56 0.0
.got 55328 55384 56 0.1
.rodata 493620 493892 272 0.1
.text 8220980 8227284 6304 0.1
thermostat-no-ble arm64 (read only) 2221092 2221284 192 0.0
.text 1863072 1863472 400 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read/write) 2352508 2352828 320 0.0
.text 1315108 1315428 320 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1155239 1155511 272 0.0
text 787588 787864 276 0.0
p6 all-clusters-app default (read/write) 2492120 2492552 432 0.0
.text 1450384 1450816 432 0.0
light-app default (read/write) 2395704 2396120 416 0.0
.text 1353968 1354384 416 0.0
lock-app default (read/write) 2359248 2359664 416 0.0
.text 1317512 1317928 416 0.0
telink lighting-app tlsr9518adk80d (read/write) 895306 895610 304 0.0
text 632968 633270 302 0.0
Decreases (1 build for linux)
platform target config section c4d81c8 d592182 change % change
linux thermostat-no-ble arm64 .rodata 137956 137748 -208 -0.2
Full report (18 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
platform target config section c4d81c8 d592182 change % change
cyw30739 light cyw930739m2evb_01 (read/write) 603538 603826 288 0.0
.app_xip_area 510636 510924 288 0.1
.bss 75656 75656 0 0.0
.data 596 596 0 0.0
.rodata 0 0 0 0.0
.text 0 0 0 0.0
lock cyw930739m2evb_01 (read/write) 561326 561606 280 0.0
.app_xip_area 469952 470232 280 0.1
.bss 74160 74160 0 0.0
.data 560 560 0 0.0
.rodata 0 0 0 0.0
.text 0 0 0 0.0
ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 571050 571338 288 0.1
.app_xip_area 470028 470316 288 0.1
.bss 83464 83464 0 0.0
.data 520 520 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
efr32 lighting-app BRD4161A (read only) 921384 921800 416 0.0
(read/write) 128752 128752 0 0.0
.bss 126744 126744 0 0.0
.data 2008 2008 0 0.0
.text 921376 921792 416 0.0
BRD4161A+rpc (read only) 950188 950604 416 0.0
(read/write) 144708 144708 0 0.0
.bss 142520 142520 0 0.0
.data 2188 2188 0 0.0
.text 950180 950596 416 0.0
window-app BRD4161A (read only) 851944 852360 416 0.0
(read/write) 126704 126704 0 0.0
.bss 124840 124840 0 0.0
.data 1864 1864 0 0.0
.text 851936 852352 416 0.0
esp32 all-clusters-app c3devkit (read only) 961498 961736 238 0.0
(read/write) 1395034 1395034 0 0.0
.dram0.bss 64048 64048 0 0.0
.dram0.data 14188 14188 0 0.0
.flash.rodata 197888 197888 0 0.0
.flash.text 961498 961736 238 0.0
.iram0.text 62016 62016 0 0.0
m5stack (read only) 1017639 1017967 328 0.0
(read/write) 462892 462892 0 0.0
.dram0.bss 69576 69576 0 0.0
.dram0.data 34016 34016 0 0.0
.flash.rodata 227464 227464 0 0.0
.flash.text 1012255 1012583 328 0.0
.iram0.text 123107 123107 0 0.0
k32w light k32w061+release (read/write) 700516 700804 288 0.0
.bss 77632 77632 0 0.0
.data 1888 1888 0 0.0
.text 615196 615484 288 0.0
lock k32w061+release (read/write) 700652 700924 272 0.0
.bss 77624 77624 0 0.0
.data 1908 1908 0 0.0
.text 615320 615592 272 0.0
linux chip-tool-ipv6only arm64 (read only) 9765964 9772828 6864 0.1
(read/write) 476113 476225 112 0.0
.bss 44033 44033 0 0.0
.data 1128 1128 0 0.0
.data.rel.ro 371824 371880 56 0.0
.dynamic 560 560 0 0.0
.got 55328 55384 56 0.1
.init 24 24 0 0.0
.init_array 192 192 0 0.0
.rodata 493620 493892 272 0.1
.text 8220980 8227284 6304 0.1
thermostat-no-ble arm64 (read only) 2221092 2221284 192 0.0
(read/write) 149809 149809 0 0.0
.bss 65713 65713 0 0.0
.data 1024 1024 0 0.0
.data.rel.ro 75728 75728 0 0.0
.dynamic 560 560 0 0.0
.got 4352 4352 0 0.0
.init 24 24 0 0.0
.init_array 360 360 0 0.0
.rodata 137956 137748 -208 -0.2
.text 1863072 1863472 400 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2352508 2352828 320 0.0
.bss 186644 186644 0 0.0
.data 5752 5752 0 0.0
.text 1315108 1315428 320 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1155239 1155511 272 0.0
bss 146724 146724 0 0.0
rodata 145768 145768 0 0.0
text 787588 787864 276 0.0
p6 all-clusters-app default (read/write) 2492120 2492552 432 0.0
.bss 120072 120072 0 0.0
.data 2632 2632 0 0.0
.text 1450384 1450816 432 0.0
light-app default (read/write) 2395704 2396120 416 0.0
.bss 113536 113536 0 0.0
.data 2488 2488 0 0.0
.text 1353968 1354384 416 0.0
lock-app default (read/write) 2359248 2359664 416 0.0
.bss 113280 113280 0 0.0
.data 2448 2448 0 0.0
.text 1317512 1317928 416 0.0
telink lighting-app tlsr9518adk80d (read/write) 895306 895610 304 0.0
bss 87432 87432 0 0.0
noinit 37160 37160 0 0.0
text 632968 633270 302 0.0

@bzbarsky-apple bzbarsky-apple mentioned this pull request Mar 18, 2022
Merged
@bzbarsky-apple bzbarsky-apple merged commit ca56d6b into project-chip:master Mar 22, 2022
@bzbarsky-apple bzbarsky-apple deleted the fix-fabric-double-add branch March 22, 2022 18:08
andrei-menzopol pushed a commit to andrei-menzopol/connectedhomeip that referenced this pull request Apr 14, 2022
@bzbarsky-apple @andrei-menzopol
Implement required check for adding an already-existing fabric. (proj…
26e9a97 
…ect-chip#16404)

Per spec, AddNOC should fail if the fabric id and root public key of
the new NOC match an existing fabric.  We were not implementing this
check.
@bzbarsky-apple bzbarsky-apple mentioned this pull request May 4, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@carol-apple carol-apple carol-apple approved these changes

@andy31415 andy31415 andy31415 approved these changes

@tcarmelveilleux tcarmelveilleux tcarmelveilleux approved these changes

@krypton36 krypton36 Awaiting requested review from krypton36

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

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

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

@chrisdecenzo chrisdecenzo Awaiting requested review from chrisdecenzo

@chshu chshu Awaiting requested review from chshu

@chulspro chulspro Awaiting requested review from chulspro

@Damian-Nordic Damian-Nordic Awaiting requested review from Damian-Nordic

@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

@hawk248 hawk248 Awaiting requested review from hawk248

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

@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

@lazarkov lazarkov Awaiting requested review from lazarkov

@LuDuda LuDuda Awaiting requested review from LuDuda

@lzgrablic02 lzgrablic02 Awaiting requested review from lzgrablic02

@mlepage-google mlepage-google Awaiting requested review from mlepage-google

@msandstedt msandstedt Awaiting requested review from msandstedt

@mspang mspang Awaiting requested review from mspang

@robszewczyk robszewczyk Awaiting requested review from robszewczyk

@sagar-apple sagar-apple Awaiting requested review from sagar-apple

@saurabhst saurabhst Awaiting requested review from saurabhst

@selissia selissia Awaiting requested review from selissia

@tecimovic tecimovic Awaiting requested review from tecimovic

@turon turon Awaiting requested review from turon

@vijs vijs Awaiting requested review from vijs

@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

Assignees
No one assigned
Labels
app controller darwin examples lib review - approved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bzbarsky-apple @tcarmelveilleux @andy31415 @carol-apple