-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Use refcounter for secure session #17599
Conversation
PR #17599: Size comparison from eeb9434 to 98a4066 Increases above 0.2%:
Increases (26 builds for cc13x2_26x2, cyw30739, k32w, linux, mbed, p6, telink)
Decreases (1 build for telink)
Full report (26 builds for cc13x2_26x2, cyw30739, k32w, linux, mbed, p6, telink)
|
PR #17599: Size comparison from eeb9434 to 9cfdd9e Increases above 0.2%:
Increases (29 builds for cc13x2_26x2, cyw30739, efr32, k32w, linux, mbed, p6, telink)
Decreases (1 build for telink)
Full report (29 builds for cc13x2_26x2, cyw30739, efr32, k32w, linux, mbed, p6, telink)
|
PR #17599: Size comparison from 6109d14 to 2c1e1ae Increases above 0.2%:
Increases (23 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
Decreases (1 build for telink)
Full report (23 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
@kghost The RAM increases here are a lot more than I was expecting, given we only have 16 secure sessions around.... |
Hmm, I need to take a deep look into it |
Here is the layout of class SecureSession : public Session, public ReferenceCounted<chip::Transport::SecureSession, chip::Transport::SecureSessionDeleter, 0> {
/* class Session <ancestor>; */ /* 0 16 */
/* XXX last struct has 3 bytes of padding */
/* class ReferenceCounted<chip::Transport::SecureSession, chip::Transport::SecureSessionDeleter, 0> <ancestor>; */ /* 16 4 */
enum State mState; /* 20 1 */
const enum Type mSecureSessionType; /* 21 1 */
/* XXX 2 bytes hole, try to pack */
class SecureSessionTable & mTable; /* 24 4 */
/* XXX 4 bytes hole, try to pack */
NodeId mPeerNodeId; /* 32 8 */
struct CATValues mPeerCATs; /* 40 12 */
const uint16_t mLocalSessionId; /* 52 2 */
uint16_t mPeerSessionId; /* 54 2 */
class PeerAddress mPeerAddress; /* 56 24 */
/* --- cacheline 1 boundary (64 bytes) was 16 bytes ago --- */
Timestamp mLastActivityTime; /* 80 8 */
Timestamp mLastPeerActivityTime; /* 88 8 */
struct ReliableMessageProtocolConfig mMRPConfig; /* 96 8 */
class CryptoContext mCryptoContext; /* 104 60 */
/* --- cacheline 2 boundary (128 bytes) was 36 bytes ago --- */
class SessionMessageCounter mSessionMessageCounter; /* 164 20 */
/* size: 184, cachelines: 3, members: 15 */
/* sum members: 158, holes: 2, sum holes: 6 */
/* paddings: 1, sum paddings: 3 */
/* last cacheline: 56 bytes */
} This PR adds 9 bytes to the object and also introduce 7 padding. Total increased 16 bytes per object |
PR #17599: Size comparison from 6109d14 to 4ccc30a Increases above 0.2%:
Increases (23 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
Decreases (2 builds for cc13x2_26x2)
Full report (23 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
PR #17599: Size comparison from 6109d14 to e32156d Increases above 0.2%:
Increases (23 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
Decreases (2 builds for cc13x2_26x2)
Full report (23 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
PR #17599: Size comparison from 7ce2a05 to 93042f3 Increases above 0.2%:
Increases (7 builds for cyw30739, linux, mbed, telink)
Full report (7 builds for cyw30739, linux, mbed, telink)
|
PR #17599: Size comparison from 7ce2a05 to f1352f0 Increases above 0.2%:
Increases (32 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
Decreases (2 builds for cc13x2_26x2)
Full report (32 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
PR #17599: Size comparison from 44dd01b to 448656a Increases above 0.2%:
Increases (34 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
Decreases (4 builds for cc13x2_26x2)
Full report (34 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
PR #17599: Size comparison from 2580fb3 to 652f285 Increases above 0.2%:
Increases (25 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
Decreases (4 builds for cc13x2_26x2)
Full report (25 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
PR #17599: Size comparison from a777a80 to 9906f95 Increases above 0.2%:
Increases (37 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
Decreases (5 builds for cc13x2_26x2)
Full report (37 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
PR #17599: Size comparison from f360a70 to c8074e1 Increases above 0.2%:
Increases (31 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
Full report (31 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
PR #17599: Size comparison from d222519 to e40750e Increases above 0.2%:
Increases (37 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
Decreases (5 builds for cc13x2_26x2)
Full report (37 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
fast track because open for 29 days. |
Problem
Fix #17558
Use ref counter for secure session, here are how it works:
SecureSession
starts at 0 refPairingSession::mSecureSessionHolder
is its first refSecureSession::Activate
it obtain a ref for itselfSecureSession::MarkForRemoval
release the ref at step 3.SecureSession::MarkForRemoval
will clear all session holder, and prevent any future holder grabbing the sessionSessionHandle
is released, the ref count of the session will be decreased to 0Change overview
kPairing
,kActive
,kPendingRemoval
kPairing
(waskPending
) fromSessionType
SessionType
is assigned at beginning of pairing, shifted fromActivate
Testing
WIP