Add ICD Client Info management support with persistent storage #29783
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
lib
controller
app
support
core
icd
|
PR #29783: Size comparison from 5ef4681 to 654b382 Increases (13 builds for bl702, bl702l, cc32xx, psoc6, telink)
Decreases (3 builds for bl702, telink)
Full report (56 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, k32w, mbed, nrfconnect, psoc6, qpg, telink)
|
yunhanw-google
force-pushed
the
feature/icd_client
branch
from
654b382 to
1688b85
Compare
|
PR #29783: Size comparison from ac5b518 to 1688b85 Increases (12 builds for bl702, bl702l, cc32xx, psoc6)
Decreases (2 builds for bl702)
Full report (37 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, k32w, linux, mbed, nrfconnect, psoc6, qpg)
|
yunhanw-google
force-pushed
the
feature/icd_client
branch
from
1688b85 to
77cfa2f
Compare
|
PR #29783: Size comparison from 46211e9 to 77cfa2f Increases (12 builds for bl702, bl702l, cc32xx, psoc6)
Decreases (2 builds for bl702)
Full report (32 builds for bl602, bl702, bl702l, cc32xx, cyw30739, k32w, linux, mbed, nrfconnect, psoc6, qpg)
|
yunhanw-google
force-pushed
the
feature/icd_client
branch
from
77cfa2f to
284eb75
Compare
|
PR #29783: Size comparison from bc20d34 to 284eb75 Increases (13 builds for bl702, bl702l, cc32xx, psoc6, telink)
Decreases (2 builds for bl702)
Full report (60 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
yunhanw-google
force-pushed
the
feature/icd_client
branch
from
284eb75 to
c377550
Compare
|
PR #29783: Size comparison from 349b795 to c377550 Increases (1 build for cc32xx)
Decreases (1 build for cc32xx)
Full report (2 builds for cc32xx, mbed)
|
yunhanw-google
force-pushed
the
feature/icd_client
branch
from
c377550 to
dbf8117
Compare
|
PR #29783: Size comparison from 349b795 to dbf8117 Increases (12 builds for bl702, bl702l, cc32xx, psoc6)
Decreases (4 builds for bl702, cc32xx)
Full report (36 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, k32w, mbed, psoc6, qpg)
|
yunhanw-google
force-pushed
the
feature/icd_client
branch
from
dbf8117 to
f0e8c33
Compare
|
PR #29783: Size comparison from 349b795 to f0e8c33 Increases (12 builds for bl702, bl702l, cc32xx, psoc6)
Decreases (4 builds for bl702, cc32xx)
Full report (38 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, k32w, linux, mbed, psoc6, qpg)
|
yunhanw-google
force-pushed
the
feature/icd_client
branch
from
f0e8c33 to
41e5500
Compare
yunhanw-google
changed the title
yunhanw-google
force-pushed
the
feature/icd_client
branch
from
41e5500 to
33b24eb
Compare
pullapprove
bot
requested review from
andy31415,
andyg-apple,
anush-apple,
arkq and
bzbarsky-apple
yunhanw-google
force-pushed
the
feature/icd_client
branch
from
2b45538 to
c751af8
Compare
|
PR #29783: Size comparison from ed891e2 to c751af8 Full report (72 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
|
PR #29783: Size comparison from ed891e2 to 1eff6e0 Increases above 0.2%:
Increases (20 builds for cc13x4_26x4, cc32xx, cyw30739, efr32, linux, mbed, psoc6, qpg, telink)
Decreases (13 builds for cc13x4_26x4, cyw30739, efr32, psoc6, telink)
Full report (72 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
bzbarsky-apple
approved these changes
Nov 29, 2023
src/app/icd/client/ICDClientInfo.h
Outdated
Comment on lines
32
to
36
| ScopedNodeId peer_node; | ||
| uint32_t start_icd_counter = 0; | ||
| uint32_t offset = 0; | ||
| uint64_t monitored_subject = static_cast<uint64_t>(0); | ||
| Crypto::Aes128KeyHandle mSharedKey = Crypto::Aes128KeyHandle(); |
There was a problem hiding this comment.
Why the weirdly inconsistent naming? Please use some single naming convention....
There was a problem hiding this comment.
updated, thanks
There was a problem hiding this comment.
@yunhanw-google It's still inconsistent: snake_case vs camelCase. Please pick one thing.
There was a problem hiding this comment.
I forgot to push this change in..sure, would update it.
Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
|
PR #29783: Size comparison from ed891e2 to 5ba2178 Increases above 0.2%:
Increases (3 builds for cc32xx, mbed, qpg)
Decreases (4 builds for nrfconnect, qpg)
Full report (8 builds for cc32xx, mbed, nrfconnect, qpg)
|
This was referenced Nov 29, 2023
|
PR #29783: Size comparison from ed891e2 to cac7caa Increases above 0.2%:
Increases (33 builds for cc13x4_26x4, cc32xx, cyw30739, efr32, linux, mbed, psoc6, qpg, telink)
Decreases (32 builds for cc13x4_26x4, cyw30739, efr32, esp32, k32w, linux, nrfconnect, psoc6, qpg, telink)
Full report (72 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
-- Create ICDClientInfo to store scoped node id, fabric index, startICDCounter, offset, monitorSubject and shared key.
-- Create the Storage class that has fabricId, keystore and other icd client info storage per Identity.
-- Provide ICDClientInfoPersistentStorage as the interface, and ICDClientInfoManagement provides the implementation for ICDClientInfoPersistentStorage, ICDClientInfoManagement manage the vector for Storage.
-- clientInfo has mPeerNodeId, this nodeId is obtained and constructed when executing ICD RegisterClient command, it could be used to remove the ICDInfo from persistent storage when unpairing this icd device..
-- when validating check-in message, I provide the ICDClientInfoIteratorImpl,a iterator, that can traverse all keys/counters.. across storage, and further be validated by check-in message handler
-- Add virtual bool ValidateCheckInPayload(const ByteSpan & aPayload, ICDClientInfo & aClientInfo), which can help hide decryption logic in secure storage.
-- Add SetKey and StoreEntry, SetKey is only called when handling ICD registration during commissioning. StoreEntry would be used to store/update icdInfo during commissioning and during handling check-in message.
fixes #28236