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

Async send sigma3 #25695

Merged
merged 35 commits into from
Apr 27, 2023
Merged

Conversation

mlepage-google
Copy link
Contributor

@mlepage-google mlepage-google commented Mar 15, 2023

Break CASESession::SendSigma3 into smaller substeps

This is the send-side change that corresponds to the handle-side change in #24099.

SendSigma3 is subdivided into smaller substeps (3a, 3b, 3c) so costly work can be done in the background, and not block foreground work.

@github-actions
Copy link

PR #25695: Size comparison from 346a817 to 52f4a6f

Increases (2 builds for cc32xx, mbed)
platform target config section 346a817 52f4a6f change % change
cc32xx lock CC3235SF_LAUNCHXL (read only) 645489 645993 504 0.1
.debug_aranges 87384 87416 32 0.0
.debug_frame 300248 300372 124 0.0
.debug_info 20245969 20250634 4665 0.0
.debug_line 2660979 2662802 1823 0.1
.debug_loc 2804909 2807391 2482 0.1
.debug_ranges 283168 283504 336 0.1
.debug_str 3026947 3027585 638 0.0
.strtab 380271 380531 260 0.1
.symtab 257312 257488 176 0.1
.text 537360 537864 504 0.1
mbed lock-app CY8CPROTO_062_4343W+release (read/write) 2468312 2469208 896 0.0
.text 1430956 1431852 896 0.1
Full report (2 builds for cc32xx, mbed)
platform target config section 346a817 52f4a6f change % change
cc32xx lock CC3235SF_LAUNCHXL 0 0 0 0.0
(read only) 645489 645993 504 0.1
(read/write) 203848 203848 0 0.0
.ARM.attributes 44 44 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 197248 197248 0 0.0
.comment 194 194 0 0.0
.data 1480 1480 0 0.0
.debug_abbrev 930293 930293 0 0.0
.debug_aranges 87384 87416 32 0.0
.debug_frame 300248 300372 124 0.0
.debug_info 20245969 20250634 4665 0.0
.debug_line 2660979 2662802 1823 0.1
.debug_loc 2804909 2807391 2482 0.1
.debug_ranges 283168 283504 336 0.1
.debug_str 3026947 3027585 638 0.0
.ramVecs 780 780 0 0.0
.resetVecs 64 64 0 0.0
.rodata 106009 106009 0 0.0
.shstrtab 232 232 0 0.0
.stab 204 204 0 0.0
.stabstr 441 441 0 0.0
.stack 2048 2048 0 0.0
.strtab 380271 380531 260 0.1
.symtab 257312 257488 176 0.1
.text 537360 537864 504 0.1
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2468312 2469208 896 0.0
.bss 215964 215964 0 0.0
.data 5880 5880 0 0.0
.text 1430956 1431852 896 0.1

This supports locking during SignWithOpKeypair, and other operations
that may alter state in the fabric table, while CASESession is
performing work in the background during session establishment.

CASESession registers as a fabric table listener, and when a fabric
is removed (e.g. timeout) it attempts to cancel any outstanding work,
and also clears out the fabric index in the work helper data.

Therefore, if outstanding work has made it into SignWithOpKeypair,
it should be OK until complete.

It still relies on other tasks not altering FabricInfo, or the
configured OperationalKeystore, but that would have had to have been
true before anyways.

The mutex was not made recursive. It's omitted from a few functions,
which should be OK for now, and there should be cleanup on a subsequent
commit (and probably fix up const-ness of member functions, and
factoring of API vs. impl functions). This commit is to flush out
build/test errors on all CI platforms, and to discuss/review/comment on
the general approach.
@github-actions
Copy link

github-actions bot commented Apr 6, 2023

PR #25695: Size comparison from e2fa6ba to 54586ca

Increases above 0.2%:

platform target config section e2fa6ba 54586ca change % change
esp32 all-clusters-app c3devkit (read only) 1052034 1055040 3006 0.3
(read/write) 1585466 1588810 3344 0.2
.flash.rodata 221968 224744 2776 1.3
.flash.text 1052034 1055040 3006 0.3
m5stack (read only) 1102047 1104627 2580 0.2
(read/write) 501355 504315 2960 0.6
.flash.rodata 250520 253440 2920 1.2
.flash.text 1096663 1099243 2580 0.2
linux chip-tool-ipv6only arm64 .got 15504 15560 56 0.4
thermostat-no-ble arm64 (read only) 2500492 2509668 9176 0.4
.data.rel.ro 77648 77888 240 0.3
.got 5360 5416 56 1.0
.text 2108784 2117008 8224 0.4
Increases (4 builds for esp32, linux)
platform target config section e2fa6ba 54586ca change % change
esp32 all-clusters-app c3devkit (read only) 1052034 1055040 3006 0.3
(read/write) 1585466 1588810 3344 0.2
.dram0.bss 77984 78016 32 0.0
.flash.rodata 221968 224744 2776 1.3
.flash.text 1052034 1055040 3006 0.3
m5stack (read only) 1102047 1104627 2580 0.2
(read/write) 501355 504315 2960 0.6
.dram0.bss 83024 83056 32 0.0
.dram0.data 34040 34048 8 0.0
.flash.rodata 250520 253440 2920 1.2
.flash.text 1096663 1099243 2580 0.2
linux chip-tool-ipv6only arm64 (read only) 12113692 1212264 8952 0.1
(read/write) 742616 742904 288 0.0
.data.rel.ro 684496 684736 240 0.0
.got 15504 15560 56 0.4
.rodata 566156 566200 44 0.0
.text 9785268 9793284 8016 0.1
thermostat-no-ble arm64 (read only) 2500492 2509668 9176 0.4
(read/write) 145192 145544 352 0.2
.bss 56344 56408 64 0.1
.data.rel.ro 77648 77888 240 0.3
.got 5360 5416 56 1.0
.rodata 129400 129448 48 0.0
.text 2108784 2117008 8224 0.4
Full report (4 builds for esp32, linux)
platform target config section e2fa6ba 54586ca change % change
esp32 all-clusters-app c3devkit (read only) 1052034 1055040 3006 0.3
(read/write) 1585466 1588810 3344 0.2
.dram0.bss 77984 78016 32 0.0
.dram0.data 13752 13752 0 0.0
.flash.rodata 221968 224744 2776 1.3
.flash.text 1052034 1055040 3006 0.3
.iram0.text 72896 72896 0 0.0
m5stack (read only) 1102047 1104627 2580 0.2
(read/write) 501355 504315 2960 0.6
.dram0.bss 83024 83056 32 0.0
.dram0.data 34040 34048 8 0.0
.flash.rodata 250520 253440 2920 1.2
.flash.text 1096663 1099243 2580 0.2
.iram0.text 124855 124855 0 0.0
linux chip-tool-ipv6only arm64 (read only) 12113692 1212264 8952 0.1
(read/write) 742616 742904 288 0.0
.bss 34392 34392 0 0.0
.data 3008 3008 0 0.0
.data.rel.ro 684496 684736 240 0.0
.dynamic 560 560 0 0.0
.got 15504 15560 56 0.4
.init 24 24 0 0.0
.init_array 216 216 0 0.0
.rodata 566156 566200 44 0.0
.text 9785268 9793284 8016 0.1
thermostat-no-ble arm64 (read only) 2500492 2509668 9176 0.4
(read/write) 145192 145544 352 0.2
.bss 56344 56408 64 0.1
.data 1784 1784 0 0.0
.data.rel.ro 77648 77888 240 0.3
.dynamic 560 560 0 0.0
.got 5360 5416 56 1.0
.init 24 24 0 0.0
.init_array 432 432 0 0.0
.rodata 129400 129448 48 0.0
.text 2108784 2117008 8224 0.4

It's tricky to async background the signing operation, because of the
two ways operational signing occurs.

Legacy way: opkeypair manually added for fabric info
Recommended way: opkeystore handles everything

Removed std::mutex because it wasn't supported by all platforms.

Instead, made background signing occur only if using the operational
keystore (recommended way), since implementors can perform any needed
mutual exclusion in the operational keystore. If using manually added
operational keypairs (legacy way), keep signing in the foreground,
since it's not feasible to mutex the entire fabric table and typically
the operations is simpler anyways.
@github-actions
Copy link

PR #25695: Size comparison from 2ce04b5 to 6b1afb9

Increases above 0.2%:

platform target config section 2ce04b5 6b1afb9 change % change
cc32xx lock CC3235SF_LAUNCHXL (read only) 642953 644705 1752 0.3
.debug_loc 2824782 2834099 9317 0.3
.debug_ranges 286208 286936 728 0.3
.debug_str 3039567 3076227 36660 1.2
.strtab 377533 379325 1792 0.5
.symtab 256800 257392 592 0.2
.text 536480 538160 1680 0.3
Increases (1 build for cc32xx)
platform target config section 2ce04b5 6b1afb9 change % change
cc32xx lock CC3235SF_LAUNCHXL (read only) 642953 644705 1752 0.3
(read/write) 203848 203872 24 0.0
.bss 197248 197272 24 0.0
.debug_abbrev 933129 933257 128 0.0
.debug_aranges 87616 87712 96 0.1
.debug_frame 301352 301712 360 0.1
.debug_info 20303348 20339293 35945 0.2
.debug_line 2679969 2684970 5001 0.2
.debug_loc 2824782 2834099 9317 0.3
.debug_ranges 286208 286936 728 0.3
.debug_str 3039567 3076227 36660 1.2
.rodata 104353 104425 72 0.1
.strtab 377533 379325 1792 0.5
.symtab 256800 257392 592 0.2
.text 536480 538160 1680 0.3
Full report (1 build for cc32xx)
platform target config section 2ce04b5 6b1afb9 change % change
cc32xx lock CC3235SF_LAUNCHXL 0 0 0 0.0
(read only) 642953 644705 1752 0.3
(read/write) 203848 203872 24 0.0
.ARM.attributes 44 44 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 197248 197272 24 0.0
.comment 194 194 0 0.0
.data 1480 1480 0 0.0
.debug_abbrev 933129 933257 128 0.0
.debug_aranges 87616 87712 96 0.1
.debug_frame 301352 301712 360 0.1
.debug_info 20303348 20339293 35945 0.2
.debug_line 2679969 2684970 5001 0.2
.debug_loc 2824782 2834099 9317 0.3
.debug_ranges 286208 286936 728 0.3
.debug_str 3039567 3076227 36660 1.2
.ramVecs 780 780 0 0.0
.resetVecs 64 64 0 0.0
.rodata 104353 104425 72 0.1
.shstrtab 232 232 0 0.0
.stab 204 204 0 0.0
.stabstr 441 441 0 0.0
.stack 2048 2048 0 0.0
.strtab 377533 379325 1792 0.5
.symtab 256800 257392 592 0.2
.text 536480 538160 1680 0.3

@github-actions
Copy link

PR #25695: Size comparison from 7a92beb to e719521

Increases above 0.2%:

platform target config section 7a92beb e719521 change % change
cc32xx lock CC3235SF_LAUNCHXL (read only) 643081 644857 1776 0.3
.debug_loc 2838361 2847585 9224 0.3
.debug_ranges 288040 288768 728 0.3
.debug_str 3042379 3079039 36660 1.2
.strtab 377626 379418 1792 0.5
.symtab 256832 257424 592 0.2
.text 536568 538272 1704 0.3
Increases (1 build for cc32xx)
platform target config section 7a92beb e719521 change % change
cc32xx lock CC3235SF_LAUNCHXL (read only) 643081 644857 1776 0.3
(read/write) 203848 203872 24 0.0
.bss 197248 197272 24 0.0
.debug_abbrev 933224 933352 128 0.0
.debug_aranges 87760 87856 96 0.1
.debug_frame 302028 302388 360 0.1
.debug_info 20326061 20362008 35947 0.2
.debug_line 2687403 2692416 5013 0.2
.debug_loc 2838361 2847585 9224 0.3
.debug_ranges 288040 288768 728 0.3
.debug_str 3042379 3079039 36660 1.2
.rodata 104393 104465 72 0.1
.strtab 377626 379418 1792 0.5
.symtab 256832 257424 592 0.2
.text 536568 538272 1704 0.3
Full report (1 build for cc32xx)
platform target config section 7a92beb e719521 change % change
cc32xx lock CC3235SF_LAUNCHXL 0 0 0 0.0
(read only) 643081 644857 1776 0.3
(read/write) 203848 203872 24 0.0
.ARM.attributes 44 44 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 197248 197272 24 0.0
.comment 194 194 0 0.0
.data 1480 1480 0 0.0
.debug_abbrev 933224 933352 128 0.0
.debug_aranges 87760 87856 96 0.1
.debug_frame 302028 302388 360 0.1
.debug_info 20326061 20362008 35947 0.2
.debug_line 2687403 2692416 5013 0.2
.debug_loc 2838361 2847585 9224 0.3
.debug_ranges 288040 288768 728 0.3
.debug_str 3042379 3079039 36660 1.2
.ramVecs 780 780 0 0.0
.resetVecs 64 64 0 0.0
.rodata 104393 104465 72 0.1
.shstrtab 232 232 0 0.0
.stab 204 204 0 0.0
.stabstr 441 441 0 0.0
.stack 2048 2048 0 0.0
.strtab 377626 379418 1792 0.5
.symtab 256832 257424 592 0.2
.text 536568 538272 1704 0.3

@github-actions
Copy link

PR #25695: Size comparison from 2b3d5c3 to 6479997

Increases above 0.2%:

platform target config section 2b3d5c3 6479997 change % change
cc32xx lock CC3235SF_LAUNCHXL (read only) 643249 645033 1784 0.3
.debug_loc 2838930 2848145 9215 0.3
.debug_ranges 288072 288800 728 0.3
.debug_str 3042155 3078815 36660 1.2
.strtab 377963 379755 1792 0.5
.symtab 256976 257568 592 0.2
.text 536728 538440 1712 0.3
Increases (1 build for cc32xx)
platform target config section 2b3d5c3 6479997 change % change
cc32xx lock CC3235SF_LAUNCHXL (read only) 643249 645033 1784 0.3
(read/write) 203848 203872 24 0.0
.bss 197248 197272 24 0.0
.debug_abbrev 933224 933352 128 0.0
.debug_aranges 87792 87888 96 0.1
.debug_frame 302140 302500 360 0.1
.debug_info 20328411 20364358 35947 0.2
.debug_line 2687890 2692919 5029 0.2
.debug_loc 2838930 2848145 9215 0.3
.debug_ranges 288072 288800 728 0.3
.debug_str 3042155 3078815 36660 1.2
.rodata 104401 104473 72 0.1
.strtab 377963 379755 1792 0.5
.symtab 256976 257568 592 0.2
.text 536728 538440 1712 0.3
Full report (1 build for cc32xx)
platform target config section 2b3d5c3 6479997 change % change
cc32xx lock CC3235SF_LAUNCHXL 0 0 0 0.0
(read only) 643249 645033 1784 0.3
(read/write) 203848 203872 24 0.0
.ARM.attributes 44 44 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 197248 197272 24 0.0
.comment 194 194 0 0.0
.data 1480 1480 0 0.0
.debug_abbrev 933224 933352 128 0.0
.debug_aranges 87792 87888 96 0.1
.debug_frame 302140 302500 360 0.1
.debug_info 20328411 20364358 35947 0.2
.debug_line 2687890 2692919 5029 0.2
.debug_loc 2838930 2848145 9215 0.3
.debug_ranges 288072 288800 728 0.3
.debug_str 3042155 3078815 36660 1.2
.ramVecs 780 780 0 0.0
.resetVecs 64 64 0 0.0
.rodata 104401 104473 72 0.1
.shstrtab 232 232 0 0.0
.stab 204 204 0 0.0
.stabstr 441 441 0 0.0
.stack 2048 2048 0 0.0
.strtab 377963 379755 1792 0.5
.symtab 256976 257568 592 0.2
.text 536728 538440 1712 0.3

OperationalKeystore declares whether it supports this capability.
If so, then CASE session establishment may take advantage of it.
If not, then CASE session establishment must use foreground.
@github-actions
Copy link

PR #25695: Size comparison from b8368a1 to db76a70

Increases above 0.2%:

platform target config section b8368a1 db76a70 change % change
cc32xx lock CC3235SF_LAUNCHXL (read only) 643249 645057 1808 0.3
.debug_loc 2838960 2848319 9359 0.3
.debug_ranges 288072 288816 744 0.3
.debug_str 3042335 3079112 36777 1.2
.strtab 377963 379834 1871 0.5
.symtab 256976 257600 624 0.2
.text 536728 538456 1728 0.3
Increases (1 build for cc32xx)
platform target config section b8368a1 db76a70 change % change
cc32xx lock CC3235SF_LAUNCHXL (read only) 643249 645057 1808 0.3
(read/write) 203848 203872 24 0.0
.bss 197248 197272 24 0.0
.debug_abbrev 933224 933352 128 0.0
.debug_aranges 87792 87904 112 0.1
.debug_frame 302140 302532 392 0.1
.debug_info 20330829 20367869 37040 0.2
.debug_line 2687904 2693014 5110 0.2
.debug_loc 2838960 2848319 9359 0.3
.debug_ranges 288072 288816 744 0.3
.debug_str 3042335 3079112 36777 1.2
.rodata 104401 104481 80 0.1
.strtab 377963 379834 1871 0.5
.symtab 256976 257600 624 0.2
.text 536728 538456 1728 0.3
Full report (1 build for cc32xx)
platform target config section b8368a1 db76a70 change % change
cc32xx lock CC3235SF_LAUNCHXL 0 0 0 0.0
(read only) 643249 645057 1808 0.3
(read/write) 203848 203872 24 0.0
.ARM.attributes 44 44 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 197248 197272 24 0.0
.comment 194 194 0 0.0
.data 1480 1480 0 0.0
.debug_abbrev 933224 933352 128 0.0
.debug_aranges 87792 87904 112 0.1
.debug_frame 302140 302532 392 0.1
.debug_info 20330829 20367869 37040 0.2
.debug_line 2687904 2693014 5110 0.2
.debug_loc 2838960 2848319 9359 0.3
.debug_ranges 288072 288816 744 0.3
.debug_str 3042335 3079112 36777 1.2
.ramVecs 780 780 0 0.0
.resetVecs 64 64 0 0.0
.rodata 104401 104481 80 0.1
.shstrtab 232 232 0 0.0
.stab 204 204 0 0.0
.stabstr 441 441 0 0.0
.stack 2048 2048 0 0.0
.strtab 377963 379834 1871 0.5
.symtab 256976 257600 624 0.2
.text 536728 538456 1728 0.3

Copy link
Contributor

@samadDotDev samadDotDev left a comment

Choose a reason for hiding this comment

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

Great work @mlepage-google !

src/protocols/secure_channel/CASESession.cpp Show resolved Hide resolved
src/credentials/FabricTable.h Outdated Show resolved Hide resolved
src/protocols/secure_channel/CASESession.cpp Outdated Show resolved Hide resolved
src/protocols/secure_channel/CASESession.cpp Show resolved Hide resolved
src/protocols/secure_channel/CASESession.cpp Show resolved Hide resolved
src/protocols/secure_channel/CASESession.cpp Outdated Show resolved Hide resolved
src/protocols/secure_channel/CASESession.cpp Show resolved Hide resolved
src/protocols/secure_channel/CASESession.cpp Show resolved Hide resolved
src/protocols/secure_channel/CASESession.cpp Show resolved Hide resolved
src/protocols/secure_channel/CASESession.cpp Show resolved Hide resolved
@github-actions
Copy link

PR #25695: Size comparison from c1108d7 to 7fc0cd6

Increases above 0.2%:

platform target config section c1108d7 7fc0cd6 change % change
cc32xx lock CC3235SF_LAUNCHXL (read only) 601114 602714 1600 0.3
.debug_aranges 103416 103664 248 0.2
.debug_frame 349704 350552 848 0.2
.debug_loclists 1501882 1506147 4265 0.3
.debug_rnglists 96008 96278 270 0.3
.debug_str 3024877 3059490 34613 1.1
.strtab 477531 481887 4356 0.9
.symtab 285936 287248 1312 0.5
.text 494824 496340 1516 0.3
Increases (1 build for cc32xx)
platform target config section c1108d7 7fc0cd6 change % change
cc32xx lock CC3235SF_LAUNCHXL (read only) 601114 602714 1600 0.3
(read/write) 204132 204156 24 0.0
.bss 197544 197568 24 0.0
.debug_abbrev 956756 956984 228 0.0
.debug_aranges 103416 103664 248 0.2
.debug_frame 349704 350552 848 0.2
.debug_info 19489250 19521972 32722 0.2
.debug_line 2678035 2682751 4716 0.2
.debug_loclists 1501882 1506147 4265 0.3
.debug_rnglists 96008 96278 270 0.3
.debug_str 3024877 3059490 34613 1.1
.rodata 104170 104250 80 0.1
.strtab 477531 481887 4356 0.9
.symtab 285936 287248 1312 0.5
.text 494824 496340 1516 0.3
Full report (1 build for cc32xx)
platform target config section c1108d7 7fc0cd6 change % change
cc32xx lock CC3235SF_LAUNCHXL 0 0 0 0.0
(read only) 601114 602714 1600 0.3
(read/write) 204132 204156 24 0.0
.ARM.attributes 44 44 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 197544 197568 24 0.0
.comment 206 206 0 0.0
.data 1468 1468 0 0.0
.debug_abbrev 956756 956984 228 0.0
.debug_aranges 103416 103664 248 0.2
.debug_frame 349704 350552 848 0.2
.debug_info 19489250 19521972 32722 0.2
.debug_line 2678035 2682751 4716 0.2
.debug_line_str 513 513 0 0.0
.debug_loc 33340 33340 0 0.0
.debug_loclists 1501882 1506147 4265 0.3
.debug_ranges 4984 4984 0 0.0
.debug_rnglists 96008 96278 270 0.3
.debug_str 3024877 3059490 34613 1.1
.ramVecs 780 780 0 0.0
.resetVecs 64 64 0 0.0
.rodata 104170 104250 80 0.1
.shstrtab 265 265 0 0.0
.stack 2048 2048 0 0.0
.strtab 477531 481887 4356 0.9
.symtab 285936 287248 1312 0.5
.text 494824 496340 1516 0.3

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.

5 participants