Skip to content

Commit

Permalink
Improve size calculation for our packets.
Browse files Browse the repository at this point in the history
We were hardcoding kMaxAppMessageLen and kMaxSecureSduLengthBytes, but those
defines were not even consistent with each other, and both were overly
conservative.  Specific changes:

1. Fix computation of CHIP_SYSTEM_HEADER_RESERVE_SIZE: we don't have any "crypto
   headers".
2. Fix computation of kMaxAppMessageLen to use our actual packet buffer sizes
   and defined header sizes.
3. Fix kMaxSecureSduLengthBytes to be consistent with kMaxAppMessageLen.
  • Loading branch information
bzbarsky-apple committed Aug 7, 2023
1 parent 6bcd87c commit f2d8f4c
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 28 deletions.
3 changes: 2 additions & 1 deletion src/app/StatusResponse.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,11 @@
#include <messaging/ExchangeContext.h>
#include <protocols/interaction_model/Constants.h>
#include <system/TLVPacketBufferBackingStore.h>
#include <transport/raw/MessageHeader.h>

namespace chip {
namespace app {
static constexpr size_t kMaxSecureSduLengthBytes = 1024;
static constexpr size_t kMaxSecureSduLengthBytes = kMaxAppMessageLen + kMaxTagLen;

class StatusResponse
{
Expand Down
54 changes: 28 additions & 26 deletions src/system/SystemConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -293,55 +293,57 @@
#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

/**
* @def CHIP_SYSTEM_HEADER_RESERVE_SIZE
*
* @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 */

/**
Expand Down
28 changes: 27 additions & 1 deletion src/transport/raw/MessageHeader.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,41 @@
#include <lib/core/PeerId.h>
#include <lib/support/BitFlags.h>
#include <lib/support/BufferReader.h>
#include <lib/support/CodeUtils.h>
#include <lib/support/TypeTraits.h>
#include <protocols/Protocols.h>
#include <system/SystemPacketBuffer.h>

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 kMaxPacketBufferSizeBytes = System::PacketBuffer::kMaxSizeWithoutReserve;
static constexpr size_t kMaxPacketSizeBytes = min(kMaxIPPacketSizeBytes, kMaxPacketBufferSizeBytes);

// Figure out the max size of our headers.
// System::PacketBuffer::kDefaultHeaderReserve may or may not include space for
// UDP headers, depending on the situation. Make sure we always have enough
// space for UDP headers plus Matter headers.
static constexpr size_t kMaxUDPHeaderSizeBytes = 48;
static constexpr size_t kMaxPacketbufferHeaderSizeBytes = System::PacketBuffer::kDefaultHeaderReserve;
static constexpr size_t kMaxHeaderSizeBytes =
max(kMaxUDPHeaderSizeBytes + CHIP_SYSTEM_HEADER_RESERVE_SIZE, kMaxPacketbufferHeaderSizeBytes);

static_assert(kMaxPacketSizeBytes > kMaxHeaderSizeBytes, "Need to be able to fit our headers in a packet.");

static constexpr size_t kMaxMessageSizeBytes = detail::kMaxPacketSizeBytes - detail::kMaxHeaderSizeBytes;
} // namespace detail

static constexpr size_t kMaxTagLen = 16;

static constexpr size_t kMaxAppMessageLen = 1200;
static_assert(detail::kMaxMessageSizeBytes > 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 size.
static constexpr size_t kMaxAppMessageLen = detail::kMaxMessageSizeBytes - kMaxTagLen;

static constexpr uint16_t kMsgUnicastSessionIdUnsecured = 0x0000;

Expand Down

0 comments on commit f2d8f4c

Please sign in to comment.