mac: downgrade logging of invalid MAC PDUs from warning to info level

Author: frankist

lib/mac/mac_ul/mac_ul_sch_pdu.cpp

@@ -13,18 +13,17 @@
 
 using namespace srsran;
 
-bool mac_ul_sch_subpdu::unpack(const byte_buffer& subpdu)
+error_type<std::string> mac_ul_sch_subpdu::unpack(const byte_buffer& subpdu)
 {
   byte_buffer_reader reader = subpdu;
   return unpack(reader);
 }
 
-bool mac_ul_sch_subpdu::unpack(byte_buffer_reader& subpdu_reader)
+error_type<std::string> mac_ul_sch_subpdu::unpack(byte_buffer_reader& subpdu_reader)
 {
   unsigned subpdu_len = subpdu_reader.length();
   if (subpdu_len == 0) {
-    srslog::fetch_basic_logger("MAC", true).info("Invalid UL MAC PDU. Cause: Empty subPDU.");
-    return false;
+    return std::string{"Empty subPDU."};
   }
   payload_view = {};
 
@@ -35,18 +34,15 @@ bool mac_ul_sch_subpdu::unpack(byte_buffer_reader& subpdu_reader)
   header_length = 1;
 
   if (not lcid_val.is_valid_lcid()) {
-    srslog::fetch_basic_logger("MAC").info("Invalid UL MAC PDU. Cause: Unrecognized lcid={}.", lcid_val);
-    return false;
+    return fmt::format("Unrecognized lcid={}.", lcid_val);
   }
 
   uint32_t sdu_length = 0;
   if (lcid_val.has_length_field()) {
     // Variable-sized MAC CEs or SDUs
 
     if (subpdu_len < (F_bit ? 3 : 2)) {
-      srslog::fetch_basic_logger("MAC").info(
-          "Invalid UL MAC PDU. Cause: Not enough bytes remaining in PDU to decode length prefix.");
-      return false;
+      return fmt::format("Not enough bytes remaining in PDU to decode length prefix.");
     }
 
     // Read first length byte
@@ -62,11 +58,8 @@ bool mac_ul_sch_subpdu::unpack(byte_buffer_reader& subpdu_reader)
     }
 
     if (subpdu_len < header_length + sdu_length) {
-      srslog::fetch_basic_logger("MAC").info(
-          "Invalid UL MAC PDU. Cause: Not enough bytes remaining in PDU to decode SDU payload ({} < {}).",
-          subpdu_len - header_length,
-          sdu_length);
-      return false;
+      return fmt::format(
+          "Not enough bytes remaining in PDU to decode SDU payload ({} < {}).", subpdu_len - header_length, sdu_length);
     }
     payload_view = subpdu_reader.split_and_advance(sdu_length);
   } else {
@@ -80,39 +73,39 @@ bool mac_ul_sch_subpdu::unpack(byte_buffer_reader& subpdu_reader)
       sdu_length = lcid_val.sizeof_ce();
 
       if (subpdu_len < header_length + sdu_length) {
-        srslog::fetch_basic_logger("MAC").info(
-            "Invalid UL MAC PDU. Cause: Not enough bytes remaining in PDU to decode CE payload ({} < {}).",
-            subpdu_len - header_length,
-            sdu_length);
-        return false;
+        return fmt::format("Not enough bytes remaining in PDU to decode CE payload ({} < {}).",
+                           subpdu_len - header_length,
+                           sdu_length);
       }
       payload_view = subpdu_reader.split_and_advance(sdu_length);
     }
   }
-  return true;
+  return {};
 }
 
 void mac_ul_sch_pdu::clear()
 {
   subpdus.clear();
 }
 
-bool mac_ul_sch_pdu::unpack(const byte_buffer& payload)
+error_type<std::string> mac_ul_sch_pdu::unpack(const byte_buffer& payload)
 {
   byte_buffer_reader reader = payload;
   while (not reader.empty()) {
     if (subpdus.full()) {
       srslog::fetch_basic_logger("MAC", true)
           .warning("Maximum number of subPDUs per UL MAC PDU limit of {} was reached.", (unsigned)MAX_SUBPDUS_PER_PDU);
-      return true;
+      return {};
     }
-    mac_ul_sch_subpdu& subpdu = subpdus.emplace_back();
-    if (not subpdu.unpack(reader)) {
+
+    mac_ul_sch_subpdu&      subpdu = subpdus.emplace_back();
+    error_type<std::string> ret    = subpdu.unpack(reader);
+    if (ret.is_error()) {
       // Discard all decoded subPDUs.
       clear();
-      return false;
+      return ret;
     }
   }
 
-  return true;
+  return {};
 }

lib/mac/mac_ul/mac_ul_sch_pdu.h

@@ -24,10 +24,13 @@ namespace srsran {
 class mac_ul_sch_subpdu
 {
 public:
-  /// Returns buffer view with begin() pointing at the first byte after the decoded subPDU.
-  bool unpack(byte_buffer_reader& subpdu_reader);
-  bool unpack(const byte_buffer& subpdu);
+  /// \brief Unpacks a subPDU from a byte buffer reader.
+  error_type<std::string> unpack(byte_buffer_reader& subpdu_reader);
 
+  /// \brief Unpacks a subPDU from a byte buffer containing an subPDU.
+  error_type<std::string> unpack(const byte_buffer& subpdu);
+
+  /// Get LCID of the unpacked subPDU.
   lcid_ul_sch_t    lcid() const { return lcid_val; }
   uint32_t         total_length() const { return header_length + payload().length(); }
   byte_buffer_view payload() const { return payload_view; }
@@ -55,7 +58,7 @@ class mac_ul_sch_pdu
 
   void clear();
 
-  bool unpack(const byte_buffer& payload);
+  error_type<std::string> unpack(const byte_buffer& payload);
 
   mac_ul_sch_subpdu&       subpdu(size_t i) { return subpdus[i]; }
   const mac_ul_sch_subpdu& subpdu(size_t i) const { return subpdus[i]; }

lib/mac/mac_ul/pdu_rx_handler.cpp

@@ -76,9 +76,10 @@ bool pdu_rx_handler::handle_rx_pdu(slot_point sl_rx, du_cell_index_t cell_index,
   du_ue_index_t ue_index = rnti_table[pdu.rnti];
 
   // > Decode MAC UL PDU.
-  decoded_mac_rx_pdu ctx{sl_rx, cell_index, std::move(pdu), ue_index};
-  if (not ctx.decoded_subpdus.unpack(ctx.pdu_rx.pdu)) {
-    logger.warning("{}: Failed to decode PDU", create_prefix(ctx));
+  decoded_mac_rx_pdu      ctx{sl_rx, cell_index, std::move(pdu), ue_index};
+  error_type<std::string> ret = ctx.decoded_subpdus.unpack(ctx.pdu_rx.pdu);
+  if (ret.is_error()) {
+    logger.info("{}: Failed to decode MAC UL PDU. Cause: {}", create_prefix(ctx), ret.error());
     return false;
   }
 

tests/unittests/mac/mac_ul_pdu_test.cpp

@@ -590,7 +590,7 @@ TEST(mac_ul_pdu, handle_the_case_when_a_pdu_has_too_many_subpdus)
   }
 
   mac_ul_sch_pdu pdu;
-  pdu.unpack(msg); // Should not crash.
+  ASSERT_TRUE(pdu.unpack(msg)); // Should not crash.
   ASSERT_GT(pdu.nof_subpdus(), 0);
 }