From 9950278a12f8317d4c0e3c41b367b1f39ba3fc09 Mon Sep 17 00:00:00 2001 From: Andre Puschmann Date: Thu, 8 Apr 2021 17:49:03 +0200 Subject: [PATCH] nr,mac_sch_pdu,mux: fix packing of MAC subPDUs with 16bit L field the created RLC PDU was too large to fit inside the MAC grant because only the header room for the short L field was used. The patch determines the correct size before passing the opportunity to RLC. It also improves logging in error case by using the MAC logger instead of stderr/stdout when error occurs. --- lib/include/srsran/mac/mac_sch_pdu_nr.h | 8 ++++++-- lib/src/mac/mac_sch_pdu_nr.cc | 14 ++++++-------- srsue/src/stack/mac_nr/mux_nr.cc | 12 +++++++++--- 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/lib/include/srsran/mac/mac_sch_pdu_nr.h b/lib/include/srsran/mac/mac_sch_pdu_nr.h index abbd979a5..c3e64841f 100644 --- a/lib/include/srsran/mac/mac_sch_pdu_nr.h +++ b/lib/include/srsran/mac/mac_sch_pdu_nr.h @@ -51,7 +51,10 @@ public: PADDING = 0b111111, } nr_lcid_sch_t; - mac_sch_subpdu_nr(mac_sch_pdu_nr* parent_); + // SDUs up to 256 B can use the short 8-bit L field + static const int32_t MAC_SUBHEADER_LEN_THRESHOLD = 256; + + mac_sch_subpdu_nr(mac_sch_pdu_nr* parent_) : parent(parent_), logger(&srslog::fetch_basic_logger("MAC")){}; nr_lcid_sch_t get_type(); bool is_sdu(); @@ -100,7 +103,8 @@ public: static uint32_t sizeof_ce(uint32_t lcid, bool is_ul); private: - // protected: + srslog::basic_logger* logger; + uint32_t lcid = 0; int header_length = 0; int sdu_length = 0; diff --git a/lib/src/mac/mac_sch_pdu_nr.cc b/lib/src/mac/mac_sch_pdu_nr.cc index 0d7c1888c..6c9878011 100644 --- a/lib/src/mac/mac_sch_pdu_nr.cc +++ b/lib/src/mac/mac_sch_pdu_nr.cc @@ -14,8 +14,6 @@ namespace srsran { -mac_sch_subpdu_nr::mac_sch_subpdu_nr(mac_sch_pdu_nr* parent_) : parent(parent_) {} - mac_sch_subpdu_nr::nr_lcid_sch_t mac_sch_subpdu_nr::get_type() { if (lcid >= 32) { @@ -85,11 +83,11 @@ void mac_sch_subpdu_nr::set_sdu(const uint32_t lcid_, const uint8_t* payload_, c F_bit = false; sdu_length = sizeof_ce(lcid, parent->is_ulsch()); if (len_ != static_cast(sdu_length)) { - srslog::fetch_basic_logger("MAC").warning("Invalid SDU length of UL-SCH SDU (%d != %d)", len_, sdu_length); + logger->warning("Invalid SDU length of UL-SCH SDU (%d != %d)", len_, sdu_length); } } - if (sdu_length >= 256) { + if (sdu_length >= MAC_SUBHEADER_LEN_THRESHOLD) { F_bit = true; header_length += 1; } @@ -160,7 +158,7 @@ uint32_t mac_sch_subpdu_nr::write_subpdu(const uint8_t* start_) } else if (header_length == 1) { // do nothing } else { - srslog::fetch_basic_logger("MAC").warning("Error while packing PDU. Unsupported header length (%d)", header_length); + logger->error("Error while packing PDU. Unsupported header length (%d)", header_length); } // copy SDU payload @@ -301,7 +299,7 @@ void mac_sch_pdu_nr::unpack(const uint8_t* payload, const uint32_t& len) while (offset < len) { mac_sch_subpdu_nr sch_pdu(this); if (sch_pdu.read_subheader(payload + offset) == SRSRAN_ERROR) { - fprintf(stderr, "Error parsing NR MAC PDU (len=%d, offset=%d)\n", len, offset); + logger.error("Error parsing NR MAC PDU (len=%d, offset=%d)\n", len, offset); return; } offset += sch_pdu.get_total_length(); @@ -314,7 +312,7 @@ void mac_sch_pdu_nr::unpack(const uint8_t* payload, const uint32_t& len) subpdus.push_back(sch_pdu); } if (offset != len) { - fprintf(stderr, "Error parsing NR MAC PDU (len=%d, offset=%d)\n", len, offset); + logger.error("Error parsing NR MAC PDU (len=%d, offset=%d)\n", len, offset); } } @@ -373,7 +371,7 @@ uint32_t mac_sch_pdu_nr::add_sdu(const uint32_t lcid_, const uint8_t* payload_, { int header_size = size_header_sdu(lcid_, len_); if (header_size + len_ > remaining_len) { - printf("Header and SDU exceed space in PDU (%d > %d).\n", header_size + len_, remaining_len); + logger.error("Header and SDU exceed space in PDU (%d + %d > %d)", header_size, len_, remaining_len); return SRSRAN_ERROR; } diff --git a/srsue/src/stack/mac_nr/mux_nr.cc b/srsue/src/stack/mac_nr/mux_nr.cc index 6f6290720..64a955624 100644 --- a/srsue/src/stack/mac_nr/mux_nr.cc +++ b/srsue/src/stack/mac_nr/mux_nr.cc @@ -72,9 +72,14 @@ srsran::unique_byte_buffer_t mux_nr::get_pdu(uint32_t max_pdu_len) while (tx_pdu.get_remaing_len() >= MIN_RLC_PDU_LEN) { // read RLC PDU rlc_buff->clear(); - uint8_t* rd = rlc_buff->msg; - int pdu_len = 0; - pdu_len = rlc->read_pdu(lc.lcid, rd, tx_pdu.get_remaing_len() - 2); + uint8_t* rd = rlc_buff->msg; + + // Determine space for RLC + uint32_t rlc_opportunity = tx_pdu.get_remaing_len(); + rlc_opportunity -= tx_pdu.get_remaing_len() >= srsran::mac_sch_subpdu_nr::MAC_SUBHEADER_LEN_THRESHOLD ? 3 : 2; + + // Read PDU from RLC + int pdu_len = rlc->read_pdu(lc.lcid, rd, rlc_opportunity); // Add SDU if RLC has something to tx if (pdu_len > 0) { @@ -84,6 +89,7 @@ srsran::unique_byte_buffer_t mux_nr::get_pdu(uint32_t max_pdu_len) // add to MAC PDU and pack if (tx_pdu.add_sdu(lc.lcid, rlc_buff->msg, rlc_buff->N_bytes) != SRSRAN_SUCCESS) { logger.error("Error packing MAC PDU"); + break; } } else { break;