diff --git a/src/app/StatusResponse.h b/src/app/StatusResponse.h index 7021e3868bcb5a..93f2557b679854 100644 --- a/src/app/StatusResponse.h +++ b/src/app/StatusResponse.h @@ -22,10 +22,11 @@ #include #include #include +#include namespace chip { namespace app { -static constexpr size_t kMaxSecureSduLengthBytes = 1024; +static constexpr size_t kMaxSecureSduLengthBytes = kMaxAppMessageLen + kMaxTagLen; class StatusResponse { diff --git a/src/app/tests/TestReadInteraction.cpp b/src/app/tests/TestReadInteraction.cpp index 0026b1567abb66..0f907567ee44f4 100644 --- a/src/app/tests/TestReadInteraction.cpp +++ b/src/app/tests/TestReadInteraction.cpp @@ -67,6 +67,9 @@ chip::EndpointId kInvalidTestEndpointId = 3; chip::DataVersion kTestDataVersion1 = 3; chip::DataVersion kTestDataVersion2 = 5; +// Number of items in the list for MockAttributeId(4). +constexpr int kMockAttribute4ListLength = 6; + static chip::System::Clock::Internal::MockClock gMockClock; static chip::System::Clock::ClockBase * gRealClock; @@ -1214,7 +1217,8 @@ void TestReadInteraction::TestReadChunking(nlTestSuite * apSuite, void * apConte NL_TEST_ASSERT(apSuite, !delegate.mGotEventResponse); chip::app::AttributePathParams attributePathParams[1]; - // Mock Attribute 4 is a big attribute, with 6 large OCTET_STRING + // Mock Attribute 4 is a big attribute, with kMockAttribute4ListLength large + // OCTET_STRING elements. attributePathParams[0].mEndpointId = Test::kMockEndpoint3; attributePathParams[0].mClusterId = Test::MockClusterId(2); attributePathParams[0].mAttributeId = Test::MockAttributeId(4); @@ -1234,8 +1238,10 @@ void TestReadInteraction::TestReadChunking(nlTestSuite * apSuite, void * apConte ctx.DrainAndServiceIO(); - // We get one chunk with 3 array elements, and then one chunk per element. - NL_TEST_ASSERT(apSuite, delegate.mNumAttributeResponse == 4); + // We get one chunk with 4 array elements, and then one chunk per + // element, and the total size of the array is + // kMockAttribute4ListLength. + NL_TEST_ASSERT(apSuite, delegate.mNumAttributeResponse == 1 + (kMockAttribute4ListLength - 4)); NL_TEST_ASSERT(apSuite, delegate.mNumArrayItems == 6); NL_TEST_ASSERT(apSuite, delegate.mGotReport); NL_TEST_ASSERT(apSuite, !delegate.mReadError); @@ -1380,13 +1386,16 @@ void TestReadInteraction::TestSetDirtyBetweenChunks(nlTestSuite * apSuite, void ctx.DrainAndServiceIO(); - // We should receive another (3 + 1) = 4 attribute reports represeting 6 - // array items, since the underlying path iterator should be reset to - // the beginning of the cluster it is currently iterating. + // Our list has length kMockAttribute4ListLength. Since the underlying + // path iterator should be reset to the beginning of the cluster it is + // currently iterating, we expect to get another value for our + // attribute. The way the packet boundaries happen to fall, that value + // will encode 4 items in the first IB and then one IB per item. + const int expectedIBs = 1 + (kMockAttribute4ListLength - 4); ChipLogError(DataManagement, "OLD: %d\n", currentAttributeResponsesWhenSetDirty); ChipLogError(DataManagement, "NEW: %d\n", delegate.mNumAttributeResponse); - NL_TEST_ASSERT(apSuite, delegate.mNumAttributeResponse == currentAttributeResponsesWhenSetDirty + 4); - NL_TEST_ASSERT(apSuite, delegate.mNumArrayItems == currentArrayItemsWhenSetDirty + 6); + NL_TEST_ASSERT(apSuite, delegate.mNumAttributeResponse == currentAttributeResponsesWhenSetDirty + expectedIBs); + NL_TEST_ASSERT(apSuite, delegate.mNumArrayItems == currentArrayItemsWhenSetDirty + kMockAttribute4ListLength); NL_TEST_ASSERT(apSuite, delegate.mGotReport); NL_TEST_ASSERT(apSuite, !delegate.mReadError); // By now we should have closed all exchanges and sent all pending acks, so @@ -2341,7 +2350,7 @@ void TestReadInteraction::TestSubscribeWildcard(nlTestSuite * apSuite, void * ap // paths, for a total of 58 attributes. // // Attribute 0xFFFC::0xFFF1'FC02::0xFFF1'0004 (kMockEndpoint3::MockClusterId(2)::MockAttributeId(4)) - // is a list of 6 elements of size 256 bytes each, which cannot fit in a single + // is a list of kMockAttribute4ListLength elements of size 256 bytes each, which cannot fit in a single // packet, so gets list chunking applied to it. // // Because delegate.mNumAttributeResponse counts AttributeDataIB instances, not attributes, @@ -2350,21 +2359,16 @@ void TestReadInteraction::TestSubscribeWildcard(nlTestSuite * apSuite, void * ap // in the response, there will be one AttributeDataIB for the start of the list (which will include // some number of 256-byte elements), then one AttributeDataIB for each of the remaining elements. // - // When EventList is enabled, for the first report for the list attribute we receive two - // of its items in the initial list, then 4 additional items. For the second report we - // receive 3 items in the initial list followed by 3 additional items. - // - // Thus we should receive 29*2 + 4 + 3 = 65 attribute data in total. - constexpr size_t kExpectedAttributeResponse = 65; + // When EventList is enabled, for the first report for the list attribute we receive three + // of its items in the initial list, then the remaining items. For the second report we + // receive 2 items in the initial list followed by the remaining items. + constexpr size_t kExpectedAttributeResponse = 29 * 2 + (kMockAttribute4ListLength - 3) + (kMockAttribute4ListLength - 2); #else // When EventList is not enabled, the packet boundaries shift and for the first - // report for the list attribute we receive two of its items in the initial list, - // then 4 additional items. For the second report we receive 3 items in - // the initial list followed by 3 additional items. - // - // Thus we should receive 29*2 + 4 + 3 = 65 attribute data when the eventlist - // attribute is not available. - constexpr size_t kExpectedAttributeResponse = 65; + // report for the list attribute we receive four of its items in the initial list, + // then additional items. For the second report we receive 4 items in + // the initial list followed by additional items. + constexpr size_t kExpectedAttributeResponse = 29 * 2 + (kMockAttribute4ListLength - 4) + (kMockAttribute4ListLength - 4); #endif NL_TEST_ASSERT(apSuite, delegate.mNumAttributeResponse == kExpectedAttributeResponse); NL_TEST_ASSERT(apSuite, delegate.mNumArrayItems == 12); @@ -3460,9 +3464,10 @@ void TestReadInteraction::TestPostSubscribeRoundtripChunkReport(nlTestSuite * ap gMockClock.AdvanceMonotonic(System::Clock::Milliseconds32(10)); } } - // Two chunked reports carry 4 attributeDataIB: 1 with a list of 3 items, - // and then one per remaining item. - NL_TEST_ASSERT(apSuite, delegate.mNumAttributeResponse == 4); + // We get one chunk with 4 array elements, and then one chunk per + // element, and the total size of the array is + // kMockAttribute4ListLength. + NL_TEST_ASSERT(apSuite, delegate.mNumAttributeResponse == 1 + (kMockAttribute4ListLength - 4)); NL_TEST_ASSERT(apSuite, delegate.mNumArrayItems == 6); NL_TEST_ASSERT(apSuite, engine->GetNumActiveReadClients() == 0); diff --git a/src/app/tests/TestWriteInteraction.cpp b/src/app/tests/TestWriteInteraction.cpp index a8f5d27979c152..19d0d49c940ad9 100644 --- a/src/app/tests/TestWriteInteraction.cpp +++ b/src/app/tests/TestWriteInteraction.cpp @@ -579,8 +579,9 @@ void TestWriteInteraction::TestWriteHandlerReceiveInvalidMessage(nlTestSuite * a err = engine->Init(&ctx.GetExchangeManager(), &ctx.GetFabricTable(), app::reporting::GetDefaultReportScheduler()); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + // Reserve all except the last 128 bytes, so that we make sure to chunk. app::WriteClient writeClient(&ctx.GetExchangeManager(), &writeCallback, Optional::Missing(), - static_cast(900) /* reserved buffer size */); + static_cast(kMaxSecureSduLengthBytes - 128) /* reserved buffer size */); ByteSpan list[5]; @@ -647,8 +648,9 @@ void TestWriteInteraction::TestWriteHandlerInvalidateFabric(nlTestSuite * apSuit err = engine->Init(&ctx.GetExchangeManager(), &ctx.GetFabricTable(), app::reporting::GetDefaultReportScheduler()); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + // Reserve all except the last 128 bytes, so that we make sure to chunk. app::WriteClient writeClient(&ctx.GetExchangeManager(), &writeCallback, Optional::Missing(), - static_cast(900) /* reserved buffer size */); + static_cast(kMaxSecureSduLengthBytes - 128) /* reserved buffer size */); ByteSpan list[5]; diff --git a/src/controller/tests/TestWriteChunking.cpp b/src/controller/tests/TestWriteChunking.cpp index 100ec13b01ae27..8f9fa06336b0da 100644 --- a/src/controller/tests/TestWriteChunking.cpp +++ b/src/controller/tests/TestWriteChunking.cpp @@ -203,21 +203,22 @@ void TestWriteChunking::TestListChunking(nlTestSuite * apSuite, void * apContext app::AttributePathParams attributePath(kTestEndpointId, app::Clusters::UnitTesting::Id, kTestListAttribute); // - // We've empirically determined that by reserving 950 bytes in the packet buffer, we can fit 2 + // We've empirically determined that by reserving all but 75 bytes in the packet buffer, we can fit 2 // AttributeDataIBs into the packet. ~30-40 bytes covers a single write chunk, but let's 2-3x that // to ensure we'll sweep from fitting 2 chunks to 3-4 chunks. // - for (int i = 100; i > 0; i--) + constexpr size_t minReservationSize = kMaxSecureSduLengthBytes - 75 - 100; + for (uint32_t i = 100; i > 0; i--) { CHIP_ERROR err = CHIP_NO_ERROR; TestWriteCallback writeCallback; ChipLogDetail(DataManagement, "Running iteration %d\n", i); - gIterationCount = (uint32_t) i; + gIterationCount = i; app::WriteClient writeClient(&ctx.GetExchangeManager(), &writeCallback, Optional::Missing(), - static_cast(850 + i) /* reserved buffer size */); + static_cast(minReservationSize + i) /* reserved buffer size */); ByteSpan list[kTestListLength]; @@ -358,15 +359,16 @@ void TestWriteChunking::TestConflictWrite(nlTestSuite * apSuite, void * apContex app::AttributePathParams attributePath(kTestEndpointId, app::Clusters::UnitTesting::Id, kTestListAttribute); + /* use a smaller chunk (128 bytes) so we only need a few attributes in the write request. */ + constexpr size_t kReserveSize = kMaxSecureSduLengthBytes - 128; + TestWriteCallback writeCallback1; - app::WriteClient writeClient1( - &ctx.GetExchangeManager(), &writeCallback1, Optional::Missing(), - static_cast(900) /* use a smaller chunk so we only need a few attributes in the write request. */); + app::WriteClient writeClient1(&ctx.GetExchangeManager(), &writeCallback1, Optional::Missing(), + static_cast(kReserveSize)); TestWriteCallback writeCallback2; - app::WriteClient writeClient2( - &ctx.GetExchangeManager(), &writeCallback2, Optional::Missing(), - static_cast(900) /* use a smaller chunk so we only need a few attributes in the write request. */); + app::WriteClient writeClient2(&ctx.GetExchangeManager(), &writeCallback2, Optional::Missing(), + static_cast(kReserveSize)); ByteSpan list[kTestListLength]; @@ -435,15 +437,16 @@ void TestWriteChunking::TestNonConflictWrite(nlTestSuite * apSuite, void * apCon app::AttributePathParams attributePath1(kTestEndpointId, app::Clusters::UnitTesting::Id, kTestListAttribute); app::AttributePathParams attributePath2(kTestEndpointId, app::Clusters::UnitTesting::Id, kTestListAttribute2); + /* use a smaller chunk (128 bytes) so we only need a few attributes in the write request. */ + constexpr size_t kReserveSize = kMaxSecureSduLengthBytes - 128; + TestWriteCallback writeCallback1; - app::WriteClient writeClient1( - &ctx.GetExchangeManager(), &writeCallback1, Optional::Missing(), - static_cast(900) /* use a smaller chunk so we only need a few attributes in the write request. */); + app::WriteClient writeClient1(&ctx.GetExchangeManager(), &writeCallback1, Optional::Missing(), + static_cast(kReserveSize)); TestWriteCallback writeCallback2; - app::WriteClient writeClient2( - &ctx.GetExchangeManager(), &writeCallback2, Optional::Missing(), - static_cast(900) /* use a smaller chunk so we only need a few attributes in the write request. */); + app::WriteClient writeClient2(&ctx.GetExchangeManager(), &writeCallback2, Optional::Missing(), + static_cast(kReserveSize)); ByteSpan list[kTestListLength]; @@ -514,7 +517,8 @@ void RunTest(nlTestSuite * apSuite, TestContext & ctx, Instructions instructions TestWriteCallback writeCallback; std::unique_ptr writeClient = std::make_unique( &ctx.GetExchangeManager(), &writeCallback, Optional::Missing(), - static_cast(900) /* use a smaller chunk so we only need a few attributes in the write request. */); + static_cast(kMaxSecureSduLengthBytes - + 128) /* use a smaller chunk so we only need a few attributes in the write request. */); ConcreteAttributePath onGoingPath = ConcreteAttributePath(); std::vector status; diff --git a/src/controller/tests/data_model/TestRead.cpp b/src/controller/tests/data_model/TestRead.cpp index 30120eca8ad794..59485d46cf971f 100644 --- a/src/controller/tests/data_model/TestRead.cpp +++ b/src/controller/tests/data_model/TestRead.cpp @@ -1063,7 +1063,7 @@ void TestReadInteraction::TestReadSubscribeAttributeResponseWithCache(nlTestSuit } // Read of E2C3A* and E3C2A2, and inject a large amount of event path list, then it would try to apply previous cache - // latest data version and construct data version list but no enough memory, finally fully rollback data version filter. Expect + // latest data version and construct data version list but run out of memory, finally fully rollback data version filter. Expect // E2C3A* attributes in report, and E3C2A2 attribute in report { testId++; @@ -1081,8 +1081,12 @@ void TestReadInteraction::TestReadSubscribeAttributeResponseWithCache(nlTestSuit readPrepareParams.mpAttributePathParamsList = attributePathParams2; readPrepareParams.mAttributePathParamsListSize = 2; - readPrepareParams.mpEventPathParamsList = eventPathParams; - readPrepareParams.mEventPathParamsListSize = 64; + readPrepareParams.mpEventPathParamsList = eventPathParams; + // This size needs to be big enough that we can't fit our + // DataVersionFilterIBs in the same packet. Max size is + // ArraySize(eventPathParams); + static_assert(75 <= ArraySize(eventPathParams)); + readPrepareParams.mEventPathParamsListSize = 75; err = readClient.SendRequest(readPrepareParams); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); @@ -1252,8 +1256,8 @@ void TestReadInteraction::TestReadSubscribeAttributeResponseWithCache(nlTestSuit // Read of E1C2A*(3 attributes) and E2C3A*(5 attributes) and E2C2A*(4 attributes), and inject a large amount of event path // list, then it would try to apply previous cache latest data version and construct data version list with the ordering from - // largest cluster size to smallest cluster size(C2, C3, C1) but no enough memory, finally partially rollback data version - // filter with only C2. Expect E1C2A*, E2C2A* attributes(7 attributes) in report, + // largest cluster size to smallest cluster size(C3, C2, C1) but run out of memory, finally partially rollback data version + // filter with only C3. Expect E1C2A*, E2C2A* attributes(7 attributes) in report, { testId++; ChipLogProgress(DataManagement, "\t -- Running Read with ClusterStateCache Test ID %d", testId); @@ -1275,8 +1279,12 @@ void TestReadInteraction::TestReadSubscribeAttributeResponseWithCache(nlTestSuit readPrepareParams.mpAttributePathParamsList = attributePathParams3; readPrepareParams.mAttributePathParamsListSize = 3; readPrepareParams.mpEventPathParamsList = eventPathParams; - readPrepareParams.mEventPathParamsListSize = 62; - err = readClient.SendRequest(readPrepareParams); + + // This size needs to be big enough that we can only fit our first + // DataVersionFilterIB. Max size is ArraySize(eventPathParams); + static_assert(73 <= ArraySize(eventPathParams)); + readPrepareParams.mEventPathParamsListSize = 73; + err = readClient.SendRequest(readPrepareParams); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); ctx.DrainAndServiceIO(); diff --git a/src/system/SystemConfig.h b/src/system/SystemConfig.h index d0188ca70f46e2..dceac0f8a49479 100644 --- a/src/system/SystemConfig.h +++ b/src/system/SystemConfig.h @@ -293,23 +293,17 @@ #endif // CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING && CHIP_SYSTEM_CONFIG_NO_LOCKING /** - * @def CHIP_SYSTEM_HEADER_RESERVE_SIZE + * @def CHIP_SYSTEM_CRYPTO_HEADER_RESERVE_SIZE * * @brief * The number of bytes to reserve in a network packet buffer to contain - * the CHIP message and exchange headers. - * - * This number was calculated as follows: + * the Matter crypto headers. * - * CHIP Crypto Header: - * - * 4 -- Length of encrypted block - * 4 -- Reserve - * 8 -- Initialization Vector - * 8 -- Encryption Tag + * This is 0, because Matter does not have any crypto headers. This define + * is still here only for backwards compatibility. */ #ifndef CHIP_SYSTEM_CRYPTO_HEADER_RESERVE_SIZE -#define CHIP_SYSTEM_CRYPTO_HEADER_RESERVE_SIZE 24 +#define CHIP_SYSTEM_CRYPTO_HEADER_RESERVE_SIZE 0 #endif /** @@ -317,31 +311,39 @@ * * @brief * The number of bytes to reserve in a network packet buffer to contain - * the CHIP message and exchange headers. + * the CHIP message and payload headers. * * This number was calculated as follows: * - * CHIP Message Header: + * Matter Message Header: * * 2 -- Frame Length - * 2 -- Message Header - * 4 -- Message Id - * 8 -- Source Node Id - * 8 -- Destination Node Id - * 2 -- Key Id + * 1 -- Message Flags + * 2 -- Session ID + * 1 -- Security Flags + * 4 -- Message Counter + * 8 -- Source Node ID + * 8 -- Destination Node ID + * + * Total: 26 bytes. + * + * Matter Payload Header: * - * CHIP Exchange Header: + * 1 -- Exhange Flags + * 1 -- Protocol Opcode + * 2 -- Exchange ID + * 2 -- Protocol Vendor ID + * 2 -- Protocol ID + * 4 -- Acknowledged MEssage Counter * - * 1 -- Application Version - * 1 -- Message Type - * 2 -- Exchange Id - * 4 -- Profile Id - * 4 -- Acknowledged Message Id + * Total: 12 bytes. * - * @note A number of these fields are optional or not presently used. So most headers will be considerably smaller than this. + * @note A number of these fields are optional or not presently used. So most + * headers will be considerably smaller than this. + * @note This calculation assumes ther are no Message Extensions or Secured Extensions. */ #ifndef CHIP_SYSTEM_HEADER_RESERVE_SIZE -#define CHIP_SYSTEM_HEADER_RESERVE_SIZE (38 + CHIP_SYSTEM_CRYPTO_HEADER_RESERVE_SIZE) +#define CHIP_SYSTEM_HEADER_RESERVE_SIZE (26 + 12 + CHIP_SYSTEM_CRYPTO_HEADER_RESERVE_SIZE) #endif /* CHIP_SYSTEM_HEADER_RESERVE_SIZE */ /** diff --git a/src/transport/raw/MessageHeader.h b/src/transport/raw/MessageHeader.h index ec957315714bbb..aa7185d8ff7412 100644 --- a/src/transport/raw/MessageHeader.h +++ b/src/transport/raw/MessageHeader.h @@ -35,15 +35,42 @@ #include #include #include +#include #include #include #include namespace chip { +namespace detail { +// Figure out the max size of a packet we can allocate, including all headers. +static constexpr size_t kMaxIPPacketSizeBytes = 1280; +static constexpr size_t kMaxUDPAndIPHeaderSizeBytes = 48; + +static_assert(kMaxIPPacketSizeBytes >= kMaxUDPAndIPHeaderSizeBytes + CHIP_SYSTEM_HEADER_RESERVE_SIZE, + "Matter headers and IP headers must fit in an MTU."); + +// Max space we have for our Application Payload and MIC, per spec. +static constexpr size_t kMaxPerSpecApplicationPayloadAndMICSizeBytes = + kMaxIPPacketSizeBytes - kMaxUDPAndIPHeaderSizeBytes - CHIP_SYSTEM_HEADER_RESERVE_SIZE; + +// Max space we have for our Application Payload and MIC in our actual packet +// buffers. This is the size _excluding_ the header reserve. +static constexpr size_t kMaxPacketBufferApplicationPayloadAndMICSizeBytes = System::PacketBuffer::kMaxSize; + +static constexpr size_t kMaxApplicationPayloadAndMICSizeBytes = + min(kMaxPerSpecApplicationPayloadAndMICSizeBytes, kMaxPacketBufferApplicationPayloadAndMICSizeBytes); + +} // namespace detail + static constexpr size_t kMaxTagLen = 16; -static constexpr size_t kMaxAppMessageLen = 1200; +static_assert(detail::kMaxApplicationPayloadAndMICSizeBytes > kMaxTagLen, "Need to be able to fit our tag in a message"); + +// This is somewhat of an under-estimate, because in practice any time we have a +// tag we will not have source/destination node IDs, but above we are including +// those in the header sizes. +static constexpr size_t kMaxAppMessageLen = detail::kMaxApplicationPayloadAndMICSizeBytes - kMaxTagLen; static constexpr uint16_t kMsgUnicastSessionIdUnsecured = 0x0000;