Skip to content

Commit

Permalink
[dns-sd] Optimize memory usage of SRP client (#8370)
Browse files Browse the repository at this point in the history
* [dns-sd] Optimize memory usage of SRP client

Allocate all SRP service data from a single flat buffer
instead of using an array of arrays. That way, less memory
is used when entries of the same kind differ in size.

Also, don't duplicate TXT entry keys as they are constants.

* Address code review comments

* Add TODO

* Restyled by clang-format

Co-authored-by: Restyled.io <commits@restyled.io>
  • Loading branch information
Damian-Nordic and restyled-commits authored Jul 17, 2021
1 parent 4ef1ead commit 37df562
Show file tree
Hide file tree
Showing 9 changed files with 313 additions and 72 deletions.
33 changes: 22 additions & 11 deletions src/lib/mdns/Advertiser.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include <inet/InetLayer.h>
#include <lib/support/Span.h>
#include <support/CHIPMemString.h>
#include <support/SafeString.h>

namespace chip {
namespace Mdns {
Expand All @@ -34,6 +35,11 @@ static constexpr uint16_t kMdnsPort = 5353;
// Need 8 bytes to fit a thread mac.
static constexpr size_t kMaxMacSize = 8;

// Operational node TXT entries
static constexpr size_t kTxtRetryIntervalIdleMaxLength = 7; // [CRI] 0-3600000
static constexpr size_t kTxtRetryIntervalActiveMaxLength = 7; // [CRA] 0-3600000

// Commissionable/commissioner node TXT entries
static constexpr size_t kKeyDiscriminatorMaxLength = 5;
static constexpr size_t kKeyVendorProductMaxLength = 11;
static constexpr size_t kKeyAdditionalPairingMaxLength = 1;
Expand All @@ -44,16 +50,16 @@ static constexpr size_t kKeyRotatingIdMaxLength = 100;
static constexpr size_t kKeyPairingInstructionMaxLength = 128;
static constexpr size_t kKeyPairingHintMaxLength = 10;

// Commissionable/commissioner node subtypes
static constexpr size_t kSubTypeShortDiscriminatorMaxLength = 4; // _S<dd>
static constexpr size_t kSubTypeLongDiscriminatorMaxLength = 6; // _L<dddd>
static constexpr size_t kSubTypeVendorMaxLength = 7; // _V<ddddd>
static constexpr size_t kSubTypeDeviceTypeMaxLength = 5; // _T<ddd>
static constexpr size_t kSubTypeCommissioningModeMaxLength = 3; // _C<d>
static constexpr size_t kSubTypeAdditionalPairingMaxLength = 3; // _A<d>
static constexpr size_t kSubTypeMaxNumber = 6;
static constexpr size_t kSubTypeMaxLength =
std::max({ kSubTypeShortDiscriminatorMaxLength, kSubTypeLongDiscriminatorMaxLength, kSubTypeVendorMaxLength,
kSubTypeDeviceTypeMaxLength, kSubTypeCommissioningModeMaxLength, kSubTypeAdditionalPairingMaxLength });
static constexpr size_t kSubTypeTotalLength = kSubTypeShortDiscriminatorMaxLength + kSubTypeLongDiscriminatorMaxLength +
kSubTypeVendorMaxLength + kSubTypeDeviceTypeMaxLength + kSubTypeCommissioningModeMaxLength + kSubTypeAdditionalPairingMaxLength;

enum class CommssionAdvertiseMode : uint8_t
{
Expand Down Expand Up @@ -98,10 +104,10 @@ class BaseAdvertisingParams
class OperationalAdvertisingParameters : public BaseAdvertisingParams<OperationalAdvertisingParameters>
{
public:
// Amount of mDNS text entries required for this advertising type
static constexpr uint8_t kNumAdvertisingTxtEntries = 2;
static constexpr uint8_t kTxtMaxKeySize = 3 + 1; // "CRI"/"CRA" as possible keys
static constexpr uint8_t kTxtMaxValueSize = 7 + 1; // Max for text representation of the 32-bit MRP intervals
static constexpr uint8_t kTxtMaxNumber = 2;
static constexpr uint8_t kTxtMaxKeySize = MaxStringLength("CRI", "CRA"); // possible keys
static constexpr uint8_t kTxtMaxValueSize = std::max({ kTxtRetryIntervalIdleMaxLength, kTxtRetryIntervalActiveMaxLength });
static constexpr size_t kTxtTotalValueSize = kTxtRetryIntervalIdleMaxLength + kTxtRetryIntervalActiveMaxLength;

OperationalAdvertisingParameters & SetPeerId(const PeerId & peerId)
{
Expand Down Expand Up @@ -131,10 +137,15 @@ class OperationalAdvertisingParameters : public BaseAdvertisingParams<Operationa
class CommissionAdvertisingParameters : public BaseAdvertisingParams<CommissionAdvertisingParameters>
{
public:
// Amount of mDNS text entries required for this advertising type
static constexpr uint8_t kNumAdvertisingTxtEntries = 8; // Min 1 - Max 8
static constexpr uint8_t kTxtMaxKeySize = 2 + 1; // "D"/"VP"/"CM"/"DT"/"DN"/"RI"/"PI"/"PH" as possible keys
static constexpr uint8_t kTxtMaxValueSize = 128; // Max from PI - Pairing Instruction
static constexpr uint8_t kTxtMaxNumber = 9;
static constexpr uint8_t kTxtMaxKeySize = MaxStringLength("D", "VP", "CM", "DT", "DN", "RI", "PI", "PH"); // possible keys
static constexpr uint8_t kTxtMaxValueSize =
std::max({ kKeyDiscriminatorMaxLength, kKeyVendorProductMaxLength, kKeyAdditionalPairingMaxLength,
kKeyCommissioningModeMaxLength, kKeyDeviceTypeMaxLength, kKeyDeviceNameMaxLength, kKeyRotatingIdMaxLength,
kKeyPairingInstructionMaxLength, kKeyPairingHintMaxLength });
static constexpr size_t kTxtTotalValueSize = kKeyDiscriminatorMaxLength + kKeyVendorProductMaxLength +
kKeyAdditionalPairingMaxLength + kKeyCommissioningModeMaxLength + kKeyDeviceTypeMaxLength + kKeyDeviceNameMaxLength +
kKeyRotatingIdMaxLength + kKeyPairingInstructionMaxLength + kKeyPairingHintMaxLength;

CommissionAdvertisingParameters & SetShortDiscriminator(uint8_t discriminator)
{
Expand Down
4 changes: 2 additions & 2 deletions src/lib/mdns/Discovery_ImplPlatform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ CHIP_ERROR DiscoveryImplPlatform::Advertise(const CommissionAdvertisingParameter
char pairingHintBuf[kKeyPairingHintMaxLength + 1];
char pairingInstrBuf[kKeyPairingInstructionMaxLength + 1];
// size of textEntries array should be count of Bufs above
TextEntry textEntries[9];
TextEntry textEntries[CommissionAdvertisingParameters::kTxtMaxNumber];
size_t textEntrySize = 0;
// add null-character to the subtypes
char shortDiscriminatorSubtype[kSubTypeShortDiscriminatorMaxLength + 1];
Expand Down Expand Up @@ -345,7 +345,7 @@ CHIP_ERROR DiscoveryImplPlatform::Advertise(const OperationalAdvertisingParamete
constexpr uint8_t kMaxMRPRetryBufferSize = 7 + 1;
char mrpRetryIntervalIdleBuf[kMaxMRPRetryBufferSize];
char mrpRetryIntervalActiveBuf[kMaxMRPRetryBufferSize];
TextEntry mrpRetryIntervalEntries[OperationalAdvertisingParameters::kNumAdvertisingTxtEntries];
TextEntry mrpRetryIntervalEntries[OperationalAdvertisingParameters::kTxtMaxNumber];
size_t textEntrySize = 0;
uint32_t mrpRetryIntervalIdle, mrpRetryIntervalActive;
int writtenCharactersNumber;
Expand Down
2 changes: 2 additions & 0 deletions src/lib/support/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ static_library("support") {
"ErrorStr.h",
"FibonacciUtils.cpp",
"FibonacciUtils.h",
"FixedBufferAllocator.cpp",
"FixedBufferAllocator.h",
"LifetimePersistedCounter.cpp",
"LifetimePersistedCounter.h",
"PersistedCounter.cpp",
Expand Down
52 changes: 52 additions & 0 deletions src/lib/support/FixedBufferAllocator.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/*
*
* Copyright (c) 2021 Project CHIP Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#include "FixedBufferAllocator.h"

#include <cstring>

namespace chip {
uint8_t * FixedBufferAllocator::Alloc(size_t count)
{
if (mBegin + count > mEnd)
{
mAnyAllocFailed = true;
return nullptr;
}

uint8_t * ptr = mBegin;
mBegin += count;
return ptr;
}

uint8_t * FixedBufferAllocator::Clone(const void * data, size_t dataLen)
{
uint8_t * ptr = Alloc(dataLen);

if (ptr != nullptr)
{
memcpy(ptr, data, dataLen);
}

return ptr;
}

char * FixedBufferAllocator::Clone(const char * str)
{
return reinterpret_cast<char *>(Clone(str, strlen(str) + 1));
}
} // namespace chip
96 changes: 96 additions & 0 deletions src/lib/support/FixedBufferAllocator.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
/*
*
* Copyright (c) 2021 Project CHIP Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#pragma once

#include <cstddef>
#include <cstdint>

namespace chip {
/**
* Memory allocator that uses a fixed-size buffer.
*
* This class allocates subsequent memory regions out of a fixed-size buffer.
* Deallocation of specific regions is unsupported and it is assumed that the entire
* buffer will be released at once.
*/
class FixedBufferAllocator
{
public:
FixedBufferAllocator() = default;
FixedBufferAllocator(uint8_t * buffer, size_t capacity) { Init(buffer, capacity); }

template <size_t N>
explicit FixedBufferAllocator(uint8_t (&buffer)[N])
{
Init(buffer);
}

void Init(uint8_t * buffer, size_t capacity)
{
mBegin = buffer;
mEnd = buffer + capacity;
mAnyAllocFailed = false;
}

template <size_t N>
void Init(uint8_t (&buffer)[N])
{
Init(buffer, N);
}

/**
* Allocate a specified number of bytes.
*
* @param count Number of bytes to allocate.
* @return Pointer to the allocated memory region or nullptr on failure.
*/
uint8_t * Alloc(size_t count);

/**
* Allocate memory for the specified data and copy the data into the allocated region.
*
* @param data Pointer to the data to be copied into the allocated memory region.
* @param dataLen Size of the data to be copied into the allocated memory region.
* @return Pointer to the allocated memory region or nullptr on failure.
*/
uint8_t * Clone(const void * data, size_t dataLen);

/**
* Allocate memory for the specified string and copy the string, including
* the null-character, into the allocated region.
*
* @param str Pointer to the string to be copied into the allocated memory region.
* @return Pointer to the allocated memory region or nullptr on failure.
*/
char * Clone(const char * str);

/**
* Returns whether any allocation has failed so far.
*/
bool AnyAllocFailed() const { return mAnyAllocFailed; }

private:
FixedBufferAllocator(const FixedBufferAllocator &) = delete;
void operator=(const FixedBufferAllocator &) = delete;

uint8_t * mBegin = nullptr;
uint8_t * mEnd = nullptr;
bool mAnyAllocFailed = false;
};

} // namespace chip
1 change: 1 addition & 0 deletions src/lib/support/tests/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ chip_test_suite("tests") {
"TestCHIPCounter.cpp",
"TestCHIPMem.cpp",
"TestErrorStr.cpp",
"TestFixedBufferAllocator.cpp",
"TestOwnerOf.cpp",
"TestPool.cpp",
"TestPrivateHeap.cpp",
Expand Down
79 changes: 79 additions & 0 deletions src/lib/support/tests/TestFixedBufferAllocator.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
/*
*
* Copyright (c) 2021 Project CHIP Authors
* All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#include <support/FixedBufferAllocator.h>
#include <support/UnitTestRegistration.h>

#include <cstring>
#include <nlunit-test.h>

using namespace chip;

namespace {

void TestClone(nlTestSuite * inSuite, void * inContext)
{
uint8_t buffer[128];
FixedBufferAllocator alloc(buffer);

const char * kTestString = "Test string";
const char * allocatedString = alloc.Clone(kTestString);

NL_TEST_ASSERT(inSuite, allocatedString != nullptr);
NL_TEST_ASSERT(inSuite, allocatedString != kTestString);
NL_TEST_ASSERT(inSuite, strcmp(allocatedString, kTestString) == 0);

const uint8_t kTestData[] = { 0xDE, 0xAD, 0xBE, 0xEF };
const uint8_t * allocatedData = alloc.Clone(kTestData, sizeof(kTestData));

NL_TEST_ASSERT(inSuite, allocatedData != nullptr);
NL_TEST_ASSERT(inSuite, allocatedData != kTestData);
NL_TEST_ASSERT(inSuite, memcmp(allocatedData, kTestData, sizeof(kTestData)) == 0);
}

void TestOutOfMemory(nlTestSuite * inSuite, void * inContext)
{
uint8_t buffer[16];
FixedBufferAllocator alloc(buffer);

const char * kTestData = "0123456789abcdef";

// Allocating 16 bytes still works...
NL_TEST_ASSERT(inSuite, alloc.Clone(kTestData, 16) != nullptr);
NL_TEST_ASSERT(inSuite, !alloc.AnyAllocFailed());

// ...but cannot allocate even one more byte...
NL_TEST_ASSERT(inSuite, alloc.Clone(kTestData, 1) == nullptr);
NL_TEST_ASSERT(inSuite, alloc.AnyAllocFailed());
}

const nlTest sTests[] = { NL_TEST_DEF("Test successfull clone", TestClone), NL_TEST_DEF("Test out of memory", TestOutOfMemory),
NL_TEST_SENTINEL() };

} // namespace

int TestFixedBufferAllocator()
{
nlTestSuite theSuite = { "CHIP FixedBufferAllocator tests", &sTests[0], nullptr, nullptr };

// Run test suit againt one context.
nlTestRunner(&theSuite, nullptr);
return nlTestRunnerStats(&theSuite);
}

CHIP_REGISTER_TEST_SUITE(TestFixedBufferAllocator)
Loading

0 comments on commit 37df562

Please sign in to comment.