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

Re-pack a few structures to see impact on code and RAM usage size #36995

Closed
wants to merge 9 commits into from
16 changes: 8 additions & 8 deletions src/access/SubjectDescriptor.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,23 +29,23 @@ namespace Access {

struct SubjectDescriptor
{
// CASE Authenticated Tags (CATs) only valid if auth mode is CASE.
CATValues cats;

// Holds FabricIndex of fabric, 0 if no fabric.
FabricIndex fabricIndex = kUndefinedFabricIndex;

// Holds AuthMode of subject(s), kNone if no access.
AuthMode authMode = AuthMode::kNone;

// NOTE: due to packing there should be free bytes here

// Holds subject according to auth mode.
NodeId subject = kUndefinedNodeId;

// CASE Authenticated Tags (CATs) only valid if auth mode is CASE.
CATValues cats;

// Whether the subject is currently a pending commissionee. See `IsCommissioning`
// definition in Core Specification's ACL Architecture pseudocode.
bool isCommissioning = false;

// NOTE: due to packing there should be 1 free byte here

// Holds subject according to auth mode.
NodeId subject = kUndefinedNodeId;
};

} // namespace Access
Expand Down
60 changes: 30 additions & 30 deletions src/access/tests/TestAccessControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1014,65 +1014,65 @@ constexpr CheckData checkData1[] = {
.privilege = Privilege::kOperate,
.allow = false },
// Checks for entry 6
{ .subjectDescriptor = { .fabricIndex = 2,
.authMode = AuthMode::kCase,
.cats = { kCASEAuthTag0, kUndefinedCAT, kUndefinedCAT } },
{ .subjectDescriptor = { .cats = { kCASEAuthTag0, kUndefinedCAT, kUndefinedCAT },
.fabricIndex = 2,
.authMode = AuthMode::kCase },
.requestPath = { .cluster = kLevelControlCluster, .endpoint = 1 },
.privilege = Privilege::kOperate,
.allow = false },
{ .subjectDescriptor = { .fabricIndex = 1,
.authMode = AuthMode::kCase,
.cats = { kCASEAuthTag0, kUndefinedCAT, kUndefinedCAT } },
{ .subjectDescriptor = { .cats = { kCASEAuthTag0, kUndefinedCAT, kUndefinedCAT },
.fabricIndex = 1,
.authMode = AuthMode::kCase },
.requestPath = { .cluster = kLevelControlCluster, .endpoint = 1 },
.privilege = Privilege::kOperate,
.allow = true },
{ .subjectDescriptor = { .fabricIndex = 1,
.authMode = AuthMode::kCase,
.cats = { kCASEAuthTag1, kUndefinedCAT, kUndefinedCAT } },
{ .subjectDescriptor = { .cats = { kCASEAuthTag1, kUndefinedCAT, kUndefinedCAT },
.fabricIndex = 1,
.authMode = AuthMode::kCase },
.requestPath = { .cluster = kLevelControlCluster, .endpoint = 1 },
.privilege = Privilege::kOperate,
.allow = false },
// Checks for entry 7
{ .subjectDescriptor = { .fabricIndex = 2,
.authMode = AuthMode::kCase,
.cats = { kCASEAuthTag0, kUndefinedCAT, kUndefinedCAT } },
{ .subjectDescriptor = { .cats = { kCASEAuthTag0, kUndefinedCAT, kUndefinedCAT },
.fabricIndex = 2,
.authMode = AuthMode::kCase },
.requestPath = { .cluster = kOnOffCluster, .endpoint = 1 },
.privilege = Privilege::kOperate,
.allow = false },
{ .subjectDescriptor = { .fabricIndex = 2,
.authMode = AuthMode::kCase,
.cats = { kCASEAuthTag0, kCASEAuthTag2, kUndefinedCAT } },
{ .subjectDescriptor = { .cats = { kCASEAuthTag0, kCASEAuthTag2, kUndefinedCAT },
.fabricIndex = 2,
.authMode = AuthMode::kCase },
.requestPath = { .cluster = kOnOffCluster, .endpoint = 1 },
.privilege = Privilege::kOperate,
.allow = false },
{ .subjectDescriptor = { .fabricIndex = 2,
.authMode = AuthMode::kCase,
.cats = { kCASEAuthTag0, kCASEAuthTag3, kUndefinedCAT } },
{ .subjectDescriptor = { .cats = { kCASEAuthTag0, kCASEAuthTag3, kUndefinedCAT },
.fabricIndex = 2,
.authMode = AuthMode::kCase },
.requestPath = { .cluster = kOnOffCluster, .endpoint = 1 },
.privilege = Privilege::kOperate,
.allow = true },
{ .subjectDescriptor = { .fabricIndex = 2,
.authMode = AuthMode::kCase,
.cats = { kCASEAuthTag0, kCASEAuthTag4, kUndefinedCAT } },
{ .subjectDescriptor = { .cats = { kCASEAuthTag0, kCASEAuthTag4, kUndefinedCAT },
.fabricIndex = 2,
.authMode = AuthMode::kCase },
.requestPath = { .cluster = kOnOffCluster, .endpoint = 1 },
.privilege = Privilege::kManage,
.allow = true },
// Checks for entry 8
{ .subjectDescriptor = { .fabricIndex = 2,
.authMode = AuthMode::kCase,
.cats = { kCASEAuthTag0, kCASEAuthTag3, kUndefinedCAT } },
{ .subjectDescriptor = { .cats = { kCASEAuthTag0, kCASEAuthTag3, kUndefinedCAT },
.fabricIndex = 2,
.authMode = AuthMode::kCase },
.requestPath = { .cluster = kLevelControlCluster, .endpoint = 1 },
.privilege = Privilege::kOperate,
.allow = false },
{ .subjectDescriptor = { .fabricIndex = 2,
.authMode = AuthMode::kCase,
.cats = { kCASEAuthTag0, kCASEAuthTag4, kUndefinedCAT } },
{ .subjectDescriptor = { .cats = { kCASEAuthTag0, kCASEAuthTag4, kUndefinedCAT },
.fabricIndex = 2,
.authMode = AuthMode::kCase },
.requestPath = { .cluster = kLevelControlCluster, .endpoint = 2 },
.privilege = Privilege::kOperate,
.allow = true },
{ .subjectDescriptor = { .fabricIndex = 2,
.authMode = AuthMode::kCase,
.cats = { kCASEAuthTag1, kUndefinedCAT, kUndefinedCAT } },
{ .subjectDescriptor = { .cats = { kCASEAuthTag1, kUndefinedCAT, kUndefinedCAT },
.fabricIndex = 2,
.authMode = AuthMode::kCase },
.requestPath = { .cluster = kLevelControlCluster, .endpoint = 2 },
.privilege = Privilege::kOperate,
.allow = true },
Expand Down
4 changes: 2 additions & 2 deletions src/app/server/CommissioningWindowManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,6 @@ class CommissioningWindowManager : public Messaging::UnsolicitedMessageHandler,

bool mIsBLE = true;

PASESession mPairingSession;

uint8_t mFailedCommissioningAttempts = 0;

bool mUseECM = false;
Expand Down Expand Up @@ -221,6 +219,8 @@ class CommissioningWindowManager : public Messaging::UnsolicitedMessageHandler,
// removed.
app::DataModel::Nullable<VendorId> mOpenerVendorId;
app::DataModel::Nullable<FabricIndex> mOpenerFabricIndex;

PASESession mPairingSession;
};

} // namespace chip
9 changes: 7 additions & 2 deletions src/app/tests/TestThreadBorderRouterManagementCluster.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,13 @@ class TestCommandHandler : public CommandHandler

Access::SubjectDescriptor GetSubjectDescriptor() const
{
Access::SubjectDescriptor subjectDescriptor = { kUndefinedFabricIndex, Access::AuthMode::kNone, kUndefinedNodeId,
kUndefinedCATs };
Access::SubjectDescriptor subjectDescriptor = {
.cats = kUndefinedCATs,
.fabricIndex = kUndefinedFabricIndex,
.authMode = Access::AuthMode::kNone,
.isCommissioning = false,
.subject = kUndefinedNodeId,
};
return subjectDescriptor;
}

Expand Down
4 changes: 2 additions & 2 deletions src/credentials/GroupDataProviderImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -536,15 +536,15 @@ struct KeyMapData : public GroupDataProvider::GroupKey, LinkedData
}
};

struct EndpointData : GroupDataProvider::GroupEndpoint, PersistentData<kPersistentBufferMax>
struct EndpointData : PersistentData<kPersistentBufferMax>, GroupDataProvider::GroupEndpoint
{
static constexpr TLV::Tag TagEndpoint() { return TLV::ContextTag(1); }
static constexpr TLV::Tag TagNext() { return TLV::ContextTag(2); }

chip::FabricIndex fabric_index = kUndefinedFabricIndex;
uint16_t index = 0;
chip::EndpointId next = 0;
chip::EndpointId prev = 0;
chip::FabricIndex fabric_index = kUndefinedFabricIndex;
bool first = true;

EndpointData() = default;
Expand Down
4 changes: 2 additions & 2 deletions src/lib/core/TLVReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ static const uint8_t sTagSizes[] = { 0, 1, 2, 4, 2, 4, 6, 8 };

TLVReader::TLVReader() :
ImplicitProfileId(kProfileIdNotSpecified), AppData(nullptr), mElemLenOrVal(0), mBackingStore(nullptr), mReadPoint(nullptr),
mBufEnd(nullptr), mLenRead(0), mMaxLen(0), mContainerType(kTLVType_NotSpecified), mControlByte(kTLVControlByte_NotSpecified),
mContainerOpen(false)
mBufEnd(nullptr), mLenRead(0), mMaxLen(0), mContainerType(kTLVType_NotSpecified), mContainerOpen(false),
mControlByte(kTLVControlByte_NotSpecified)
{}

void TLVReader::Init(const uint8_t * data, size_t dataLen)
Expand Down
5 changes: 4 additions & 1 deletion src/lib/core/TLVReader.h
Original file line number Diff line number Diff line change
Expand Up @@ -961,11 +961,14 @@ class DLL_EXPORT TLVReader
uint32_t mLenRead;
uint32_t mMaxLen;
TLVType mContainerType;
uint16_t mControlByte;

// NOTE: odd nesting to optimize size for ARM32 (save 8 bytes of struct size)
private:
bool mContainerOpen;

protected:
uint16_t mControlByte;

protected:
bool IsContainerOpen() const { return mContainerOpen; }
void SetContainerOpen(bool aContainerOpen) { mContainerOpen = aContainerOpen; }
Expand Down
2 changes: 1 addition & 1 deletion src/messaging/ReliableMessageContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ using namespace chip::DeviceLayer;
namespace chip {
namespace Messaging {

ReliableMessageContext::ReliableMessageContext() : mNextAckTime(0), mPendingPeerAckMessageCounter(0) {}
ReliableMessageContext::ReliableMessageContext() : mPendingPeerAckMessageCounter(0), mNextAckTime(0) {}

ExchangeContext * ReliableMessageContext::GetExchangeContext()
{
Expand Down
9 changes: 6 additions & 3 deletions src/messaging/ReliableMessageContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -186,8 +186,6 @@ class ReliableMessageContext
kFlagWaitingForResponseOrAck = (1u << 11),
};

BitFlags<Flags> mFlags; // Internal state flags

private:
void HandleRcvdAck(uint32_t ackMessageCounter);
CHIP_ERROR HandleNeedsAck(uint32_t messageCounter, BitFlags<MessageFlagValues> messageFlags);
Expand All @@ -214,8 +212,13 @@ class ReliableMessageContext
friend class ::chip::app::TestReadInteraction;
friend class ::chip::app::TestWriteInteraction;

System::Clock::Timestamp mNextAckTime; // Next time for triggering Solo Ack
uint32_t mPendingPeerAckMessageCounter;
// NOTE: odd nesting of members to save structure size due to padding. On ARM32:
// mFlags is 2 bytes, mPendingPeerAckMessageCounter is 4 bytes, mNextAckTime is 8 bytes
protected:
BitFlags<Flags> mFlags; // Internal state flags
private:
System::Clock::Timestamp mNextAckTime; // Next time for triggering Solo Ack
};

inline bool ReliableMessageContext::AutoRequestAck() const
Expand Down
16 changes: 8 additions & 8 deletions src/transport/UnauthenticatedSessionTable.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,10 @@ class UnauthenticatedSession : public Session, public ReferenceCounted<Unauthent
protected:
UnauthenticatedSession(SessionRole sessionRole, NodeId ephemeralInitiatorNodeID, const Transport::PeerAddress & peerAddress,
const ReliableMessageProtocolConfig & config) :
mEphemeralInitiatorNodeId(ephemeralInitiatorNodeID),
mSessionRole(sessionRole), mPeerAddress(peerAddress), mLastActivityTime(System::SystemClock().GetMonotonicTimestamp()),
mLastPeerActivityTime(System::Clock::kZero), // Start at zero to default to IDLE state
mRemoteSessionParams(config)
mPeerAddress(peerAddress),
mRemoteSessionParams(config), mSessionRole(sessionRole), mEphemeralInitiatorNodeId(ephemeralInitiatorNodeID),
mLastActivityTime(System::SystemClock().GetMonotonicTimestamp()),
mLastPeerActivityTime(System::Clock::kZero) // Start at zero to default to IDLE state
{}
~UnauthenticatedSession() override { VerifyOrDie(GetReferenceCount() == 0); }

Expand Down Expand Up @@ -182,13 +182,13 @@ class UnauthenticatedSession : public Session, public ReferenceCounted<Unauthent
#endif // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP

private:
const NodeId mEphemeralInitiatorNodeId;
const SessionRole mSessionRole;
PeerAddress mPeerAddress;
System::Clock::Timestamp mLastActivityTime; ///< Timestamp of last tx or rx
System::Clock::Timestamp mLastPeerActivityTime; ///< Timestamp of last rx
SessionParameters mRemoteSessionParams;
PeerMessageCounter mPeerMessageCounter;
const SessionRole mSessionRole;
const NodeId mEphemeralInitiatorNodeId;
System::Clock::Timestamp mLastActivityTime; ///< Timestamp of last tx or rx
System::Clock::Timestamp mLastPeerActivityTime; ///< Timestamp of last rx
};

template <size_t kMaxSessionCount>
Expand Down
16 changes: 8 additions & 8 deletions src/transport/raw/MessageHeader.h
Original file line number Diff line number Diff line change
Expand Up @@ -738,21 +738,21 @@ class PayloadHeader

constexpr bool HaveVendorId() const { return mExchangeFlags.Has(Header::ExFlagValues::kExchangeFlag_VendorIdPresent); }

/// Packet type (application data, security control packets, e.g. pairing,
/// configuration, rekey etc)
uint8_t mMessageType = 0;

/// Security session identifier
uint16_t mExchangeID = 0;
/// Message counter of a previous message that is being acknowledged by the current message
Optional<uint32_t> mAckMessageCounter;

/// Protocol identifier
Protocols::Id mProtocolID = Protocols::NotSpecified;

/// Security session identifier
uint16_t mExchangeID = 0;

/// Bit flag indicators for CHIP Exchange header
Header::ExFlags mExchangeFlags;

/// Message counter of a previous message that is being acknowledged by the current message
Optional<uint32_t> mAckMessageCounter;
/// Packet type (application data, security control packets, e.g. pairing,
/// configuration, rekey etc)
uint8_t mMessageType = 0;
};

/** Handles encoding/decoding of CHIP message headers */
Expand Down
Loading