From fab75592ecfc55ed8011a895e3861d904dfb8405 Mon Sep 17 00:00:00 2001
From: Andre Puschmann <andre@softwareradiosystems.com>
Date: Tue, 25 Jun 2019 14:56:54 +0200
Subject: [PATCH] fix MAC PDU padding with only padding and add test
 accordingly

---
 lib/src/common/pdu.cc       | 14 ++++-----
 lib/test/common/pdu_test.cc | 57 +++++++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+), 7 deletions(-)

diff --git a/lib/src/common/pdu.cc b/lib/src/common/pdu.cc
index 162240113..70ce739b8 100644
--- a/lib/src/common/pdu.cc
+++ b/lib/src/common/pdu.cc
@@ -70,11 +70,6 @@ uint8_t* sch_pdu::write_packet()
 /* Writes the MAC PDU in the packet, including the MAC headers and CE payload. Section 6.1.2 */
 uint8_t* sch_pdu::write_packet(srslte::log* log_h)
 {
-  if (nof_subheaders <= 0 && nof_subheaders < (int)max_subheaders) {
-    log_h->error("Trying to write packet with invalid number of subheaders (nof_subheaders=%d).\n", nof_subheaders);
-    return nullptr;
-  }
-
   // set padding to remaining length in PDU
   uint32_t num_padding = rem_len;
 
@@ -128,9 +123,9 @@ uint8_t* sch_pdu::write_packet(srslte::log* log_h)
   uint8_t* ptr = buffer_tx->msg;
 
   // Add single/two byte padding first
-  sch_subh padding;
-  padding.set_padding();
   for (uint32_t i = 0; i < onetwo_padding; i++) {
+    sch_subh padding;
+    padding.set_padding();
     padding.write_subheader(&ptr, false);
   }
 
@@ -174,6 +169,11 @@ uint8_t* sch_pdu::write_packet(srslte::log* log_h)
     buffer_tx->N_bytes += num_padding;
   }
 
+  // Print warning if we have padding only
+  if (nof_subheaders <= 0 && nof_subheaders < (int)max_subheaders) {
+    log_h->warning("Writing MAC PDU with padding only (%d B)\n", pdu_len);
+  }
+
   /* Sanity check and print if error */
   if (log_h) {
     log_h->debug(
diff --git a/lib/test/common/pdu_test.cc b/lib/test/common/pdu_test.cc
index c9c89034c..18517fd44 100644
--- a/lib/test/common/pdu_test.cc
+++ b/lib/test/common/pdu_test.cc
@@ -361,6 +361,58 @@ int mac_sch_pdu_pack_test3()
   return SRSLTE_SUCCESS;
 }
 
+// Test for padding-only MAC PDU
+int mac_sch_pdu_pack_test4()
+{
+  static uint8_t tv[] = {0x1f, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
+
+  srslte::log_filter rlc_log("RLC");
+  rlc_log.set_level(srslte::LOG_LEVEL_DEBUG);
+  rlc_log.set_hex_limit(100000);
+
+  rlc_dummy rlc;
+
+  srslte::log_filter mac_log("MAC");
+  mac_log.set_level(srslte::LOG_LEVEL_DEBUG);
+  mac_log.set_hex_limit(100000);
+
+  const uint32_t  pdu_size = 10;
+  srslte::sch_pdu pdu(10);
+
+  byte_buffer_t buffer;
+  pdu.init_tx(&buffer, pdu_size, true);
+
+  TESTASSERT(pdu.rem_size() == pdu_size);
+  TESTASSERT(pdu.get_pdu_len() == pdu_size);
+  TESTASSERT(pdu.get_sdu_space() == pdu_size - 1);
+  TESTASSERT(pdu.get_current_sdu_ptr() == buffer.msg);
+
+  // Try to add SDU
+  TESTASSERT(pdu.new_subh());
+  TESTASSERT(pdu.get()->set_sdu(2, 5, &rlc) == 0);
+
+  // Adding SDU failed, remove subheader again
+  pdu.del_subh();
+
+  // write PDU
+  pdu.write_packet(&mac_log);
+
+  // make sure full PDU has been written
+  TESTASSERT(buffer.N_bytes == pdu_size);
+
+  // log
+  mac_log.info_hex(buffer.msg, buffer.N_bytes, "MAC PDU (%d B):\n", buffer.N_bytes);
+
+#if HAVE_PCAP
+  pcap_handle->write_ul_crnti(buffer.msg, buffer.N_bytes, 0x1001, true, 1);
+#endif
+
+  // compare with TV
+  TESTASSERT(memcmp(buffer.msg, tv, sizeof(tv)) == 0);
+
+  return SRSLTE_SUCCESS;
+}
+
 // Test for checking error cases
 int mac_sch_pdu_pack_error_test()
 {
@@ -463,6 +515,11 @@ int main(int argc, char** argv)
     return SRSLTE_ERROR;
   }
 
+  if (mac_sch_pdu_pack_test4()) {
+    fprintf(stderr, "mac_sch_pdu_pack_test4 failed.\n");
+    return SRSLTE_ERROR;
+  }
+
   if (mac_sch_pdu_pack_error_test()) {
     fprintf(stderr, "mac_sch_pdu_pack_error_test failed.\n");
     return SRSLTE_ERROR;