From c8f7976014a479d6c4e96c5ba345d491d327185a Mon Sep 17 00:00:00 2001 From: Francisco Paisana Date: Thu, 6 Jan 2022 13:10:15 +0100 Subject: [PATCH] asn1: use byte buffer pool for temporary variable length field generation instead of stack array --- lib/include/srsran/asn1/asn1_utils.h | 14 +++++----- lib/include/srsran/common/buffer_pool.h | 36 ++++++++++++++++++------- lib/src/asn1/asn1_utils.cc | 20 ++++++++------ lib/test/asn1/asn1_utils_test.cc | 16 +++++++++++ 4 files changed, 63 insertions(+), 23 deletions(-) diff --git a/lib/include/srsran/asn1/asn1_utils.h b/lib/include/srsran/asn1/asn1_utils.h index 2485cda6e..dbce0eb75 100644 --- a/lib/include/srsran/asn1/asn1_utils.h +++ b/lib/include/srsran/asn1/asn1_utils.h @@ -13,6 +13,7 @@ #ifndef SRSASN_COMMON_UTILS_H #define SRSASN_COMMON_UTILS_H +#include "srsran/common/buffer_pool.h" #include "srsran/srslog/srslog.h" #include "srsran/support/srsran_assert.h" #include @@ -21,7 +22,6 @@ #include #include #include -#include #include #include @@ -1320,11 +1320,13 @@ public: ~varlength_field_pack_guard(); private: - bit_ref brefstart; - // bit_ref bref0; - bit_ref* bref_tracker; - uint8_t buffer[4096]; - bool align; + using byte_array_t = std::array; + using byte_array_ptr = srsran::any_pool_ptr; + + bit_ref brefstart; + bit_ref* bref_tracker; + byte_array_ptr buffer_ptr; + bool align; }; class varlength_field_unpack_guard diff --git a/lib/include/srsran/common/buffer_pool.h b/lib/include/srsran/common/buffer_pool.h index 8e270bc95..592102287 100644 --- a/lib/include/srsran/common/buffer_pool.h +++ b/lib/include/srsran/common/buffer_pool.h @@ -157,8 +157,10 @@ private: uint32_t capacity; }; +/// Type of global byte buffer pool using byte_buffer_pool = concurrent_fixed_memory_pool; +/// Function used to generate unique byte buffers inline unique_byte_buffer_t make_byte_buffer() noexcept { return std::unique_ptr(new (std::nothrow) byte_buffer_t()); @@ -197,12 +199,30 @@ inline unique_byte_buffer_t make_byte_buffer(const uint8_t* payload, uint32_t le namespace detail { +template struct byte_buffer_pool_deleter { - void operator()(void* ptr) { byte_buffer_pool::get_instance()->deallocate_node(ptr); } + void operator()(T* ptr) const { byte_buffer_pool::get_instance()->deallocate_node(ptr); } }; } // namespace detail +/// Unique ptr to global byte buffer pool +template +using buffer_pool_ptr = std::unique_ptr >; + +/// Method to create unique_ptrs of type T allocated in global byte buffer pool +template +buffer_pool_ptr make_buffer_pool_obj(CtorArgs&&... args) noexcept +{ + static_assert(sizeof(T) <= byte_buffer_pool::BLOCK_SIZE, "pool_bounded_vector does not fit buffer pool block size"); + void* memblock = byte_buffer_pool::get_instance()->allocate_node(sizeof(T)); + if (memblock == nullptr) { + return buffer_pool_ptr(); + } + new (memblock) T(std::forward(args)...); + return buffer_pool_ptr(static_cast(memblock), detail::byte_buffer_pool_deleter()); +} + /** * Class to wrap objects of type T which get allocated/deallocated using the byte_buffer_pool * @tparam T type of the object being allocated @@ -230,21 +250,19 @@ public: template static byte_buffer_pool_ptr make(CtorArgs&&... args) { - void* memblock = byte_buffer_pool::get_instance()->allocate_node(sizeof(T)); - if (memblock == nullptr) { - return byte_buffer_pool_ptr(); - } - new (memblock) T(std::forward(args)...); byte_buffer_pool_ptr ret; - ret.ptr = std::unique_ptr(static_cast(memblock), - detail::byte_buffer_pool_deleter()); + ret.ptr = make_buffer_pool_obj(std::forward(args)...); return ret; }; private: - std::unique_ptr ptr; + buffer_pool_ptr ptr; }; +/// unique_ptr with virtual deleter, so it can be used by any pool +template +using any_pool_ptr = std::unique_ptr >; + } // namespace srsran #endif // SRSRAN_BUFFER_POOL_H diff --git a/lib/src/asn1/asn1_utils.cc b/lib/src/asn1/asn1_utils.cc index 4d56e641a..bf86ef972 100644 --- a/lib/src/asn1/asn1_utils.cc +++ b/lib/src/asn1/asn1_utils.cc @@ -1412,10 +1412,15 @@ SRSASN_CODE ext_groups_unpacker_guard::unpack(cbit_ref& bref) Open Field *********************/ -varlength_field_pack_guard::varlength_field_pack_guard(bit_ref& bref, bool align_) +varlength_field_pack_guard::varlength_field_pack_guard(bit_ref& bref, bool align_) : + buffer_ptr(srsran::make_buffer_pool_obj()) { + if (buffer_ptr == nullptr) { + // failed to allocate from global byte buffer pool. Fallback to malloc + buffer_ptr = std::unique_ptr(new byte_array_t()); + } brefstart = bref; - bref = bit_ref(&buffer[0], sizeof(buffer)); + bref = bit_ref(buffer_ptr->data(), buffer_ptr->size()); bref_tracker = &bref; align = align_; } @@ -1423,16 +1428,15 @@ varlength_field_pack_guard::varlength_field_pack_guard(bit_ref& bref, bool align varlength_field_pack_guard::~varlength_field_pack_guard() { // fill the spare bits - const bit_ref bref0 = bit_ref(&buffer[0], sizeof(buffer)); - uint32_t leftover = 7 - ((bref_tracker->distance(bref0) - (uint32_t)1) % (uint32_t)8); + uint32_t leftover = 7 - ((bref_tracker->distance() - (uint32_t)1) % (uint32_t)8); bref_tracker->pack(0, leftover); // check how many bytes were written in total - uint32_t nof_bytes = bref_tracker->distance(bref0) / (uint32_t)8; - if (nof_bytes > sizeof(buffer)) { + uint32_t nof_bytes = bref_tracker->distance() / (uint32_t)8; + if (nof_bytes > buffer_ptr->size()) { log_error("The packed variable sized field is too long for the reserved buffer (%zd > %zd)", (size_t)nof_bytes, - sizeof(buffer)); + buffer_ptr->size()); } // go back in time to pack length @@ -1440,7 +1444,7 @@ varlength_field_pack_guard::~varlength_field_pack_guard() // pack encoded bytes for (uint32_t i = 0; i < nof_bytes; ++i) { - brefstart.pack(buffer[i], 8); + brefstart.pack((*buffer_ptr)[i], 8); } *bref_tracker = brefstart; } diff --git a/lib/test/asn1/asn1_utils_test.cc b/lib/test/asn1/asn1_utils_test.cc index 797a5f182..bdedd6f40 100644 --- a/lib/test/asn1/asn1_utils_test.cc +++ b/lib/test/asn1/asn1_utils_test.cc @@ -644,6 +644,21 @@ int test_big_integers() return 0; } +void test_varlength_field_pack() +{ + uint8_t buffer[128]; + bit_ref bref(&buffer[0], sizeof(buffer)); + TESTASSERT_EQ(SRSRAN_SUCCESS, bref.pack(0, 1)); + TESTASSERT_EQ(1, bref.distance()); + { + varlength_field_pack_guard guard(bref); + TESTASSERT_EQ(0, bref.distance()); + bref.pack(0, 8); + TESTASSERT_EQ(1, bref.distance_bytes()); + } + TESTASSERT_EQ(17, bref.distance()); // accounts for length determinant and 1 byte of data +} + int main() { // Setup the log spy to intercept error and warning log entries. @@ -672,6 +687,7 @@ int main() TESTASSERT(test_copy_ptr() == 0); TESTASSERT(test_enum() == 0); TESTASSERT(test_big_integers() == 0); + test_varlength_field_pack(); // TESTASSERT(test_json_writer()==0); srslog::flush();