Moved some more packing functions to base class.

lib/include/srslte/upper/pdcp_entity_base.h

@@ -123,8 +123,10 @@ protected:
   void cipher_decrypt(uint8_t* ct, uint32_t ct_len, uint32_t count, uint8_t* msg);
 
   // Common packing functions
-  void extract_mac(const unique_byte_buffer_t& pdu, uint8_t* mac);
+  uint32_t read_data_header(const unique_byte_buffer_t& pdu);
-  void append_mac(const unique_byte_buffer_t& sdu, uint8_t* mac);
+  void     write_data_header(const srslte::unique_byte_buffer_t& sdu, uint32_t count);
+  void     extract_mac(const unique_byte_buffer_t& pdu, uint8_t* mac);
+  void     append_mac(const unique_byte_buffer_t& sdu, uint8_t* mac);
 };
 
 inline uint32_t pdcp_entity_base::HFN(uint32_t count)

lib/include/srslte/upper/pdcp_entity_lte.h

@@ -85,18 +85,5 @@ private:
   void handle_am_drb_pdu(srslte::unique_byte_buffer_t pdu);
 };
 
-/****************************************************************************
- * Pack/Unpack helper functions
- * Ref: 3GPP TS 36.323 v10.1.0
- ***************************************************************************/
-
-void pdcp_pack_control_pdu(uint32_t sn, byte_buffer_t* sdu);
-void pdcp_unpack_control_pdu_sn(byte_buffer_t* sdu, uint32_t* sn);
-
-void pdcp_pack_data_pdu_short_sn(uint32_t sn, byte_buffer_t* sdu);
-void pdcp_unpack_data_pdu_short_sn(byte_buffer_t* sdu, uint32_t* sn);
-void pdcp_pack_data_pdu_long_sn(uint32_t sn, byte_buffer_t* sdu);
-void pdcp_unpack_data_pdu_long_sn(byte_buffer_t* sdu, uint32_t* sn);
-
 } // namespace srslte
 #endif // SRSLTE_PDCP_ENTITY_LTE_H

lib/src/upper/pdcp_entity_base.cc

@@ -20,6 +20,7 @@
  */
 
 #include "srslte/upper/pdcp_entity_base.h"
+#include "srslte/common/int_helpers.h"
 #include "srslte/common/security.h"
 
 namespace srslte {
@@ -227,6 +228,69 @@ void pdcp_entity_base::cipher_decrypt(uint8_t* ct, uint32_t ct_len, uint32_t cou
 /****************************************************************************
  * Common pack functions
  ***************************************************************************/
+uint32_t pdcp_entity_base::read_data_header(const unique_byte_buffer_t& pdu)
+{
+  // Check PDU is long enough to extract header
+  if (pdu->N_bytes <= cfg.hdr_len_bytes) {
+    log->error("PDU too small to extract header\n");
+    return 0;
+  }
+
+  // Extract RCVD_SN
+  uint16_t rcvd_sn_16 = 0;
+  uint32_t rcvd_sn_32 = 0;
+  switch (cfg.sn_len) {
+    case PDCP_SN_LEN_5:
+      rcvd_sn_32 = SN(pdu->msg[0]);
+      break;
+    case PDCP_SN_LEN_12:
+      srslte::uint8_to_uint16(pdu->msg, &rcvd_sn_16);
+      rcvd_sn_32 = SN(rcvd_sn_16);
+      break;
+    case PDCP_SN_LEN_18:
+      srslte::uint8_to_uint24(pdu->msg, &rcvd_sn_32);
+      rcvd_sn_32 = SN(rcvd_sn_32);
+      break;
+    default:
+      log->error("Cannot extract RCVD_SN, invalid SN length configured: %d\n", cfg.sn_len);
+  }
+
+  // Discard header
+  pdu->msg += cfg.hdr_len_bytes;
+  pdu->N_bytes -= cfg.hdr_len_bytes;
+  return rcvd_sn_32;
+}
+
+void pdcp_entity_base::write_data_header(const srslte::unique_byte_buffer_t& sdu, uint32_t count)
+{
+  // Add room for header
+  if (cfg.hdr_len_bytes > sdu->get_headroom()) {
+    log->error("Not enough space to add header\n");
+    return;
+  }
+  sdu->msg -= cfg.hdr_len_bytes;
+  sdu->N_bytes += cfg.hdr_len_bytes;
+
+  // Add SN
+  switch (cfg.sn_len) {
+    case PDCP_SN_LEN_5:
+      sdu->msg[0] = SN(count); // Data PDU and SN LEN 5 implies SRB, D flag must not be present 
+      break;
+    case PDCP_SN_LEN_12:
+      srslte::uint16_to_uint8(SN(count), sdu->msg);
+      if (is_drb()) {
+        sdu->msg[0] |= 0x80; // On Data PDUs for DRBs we must set the D flag.
+      }
+      break;
+    case PDCP_SN_LEN_18:
+      srslte::uint24_to_uint8(SN(count), sdu->msg);
+      sdu->msg[0] |= 0x80; // Data PDU and SN LEN 18 implies DRB, D flag must be present
+      break;
+    default:
+      log->error("Invalid SN length configuration: %d bits\n", cfg.sn_len);
+  }
+}
+
 void pdcp_entity_base::extract_mac(const unique_byte_buffer_t& pdu, uint8_t* mac)
 {
   // Check enough space for MAC

lib/src/upper/pdcp_entity_lte.cc

@@ -110,19 +110,16 @@ void pdcp_entity_lte::write_sdu(unique_byte_buffer_t sdu, bool blocking)
   {
     std::unique_lock<std::mutex> lock(mutex);
 
-    if (is_srb()) {
+    write_data_header(sdu, tx_count);
-      pdcp_pack_control_pdu(tx_count, sdu.get());
+
-      if (do_integrity) {
+    uint8_t mac[4] = {};
-        integrity_generate(sdu->msg, sdu->N_bytes - 4, tx_count, &sdu->msg[sdu->N_bytes - 4]);
+    if (do_integrity) {
-      }
+      integrity_generate(sdu->msg, sdu->N_bytes, tx_count, mac);
     }
 
-    if (is_drb()) {
+    // Append MAC-I
-      if (12 == cfg.sn_len) {
+    if (is_srb()) {
-        pdcp_pack_data_pdu_long_sn(tx_count, sdu.get());
+      append_mac(sdu, mac);
-      } else {
-        pdcp_pack_data_pdu_short_sn(tx_count, sdu.get());
-      }
     }
 
     if (do_encryption) {
@@ -149,6 +146,7 @@ void pdcp_entity_lte::write_pdu(unique_byte_buffer_t pdu)
 
   // Sanity check
   if (pdu->N_bytes <= cfg.hdr_len_bytes) {
+    log->error("PDCP PDU smaller than required header size.\n");
     return;
   }
 
@@ -227,12 +225,7 @@ void pdcp_entity_lte::handle_srb_pdu(srslte::unique_byte_buffer_t pdu)
 // DRBs mapped on RLC UM (5.1.2.1.3)
 void pdcp_entity_lte::handle_um_drb_pdu(srslte::unique_byte_buffer_t pdu)
 {
-  uint32_t sn;
+  uint32_t sn = read_data_header(pdu);
-  if (12 == cfg.sn_len) {
-    pdcp_unpack_data_pdu_long_sn(pdu.get(), &sn);
-  } else {
-    pdcp_unpack_data_pdu_short_sn(pdu.get(), &sn);
-  }
 
   if (sn < next_pdcp_rx_sn) {
     rx_hfn++;
@@ -259,8 +252,7 @@ void pdcp_entity_lte::handle_um_drb_pdu(srslte::unique_byte_buffer_t pdu)
 // DRBs mapped on RLC AM, without re-ordering (5.1.2.1.2)
 void pdcp_entity_lte::handle_am_drb_pdu(srslte::unique_byte_buffer_t pdu)
 {
-  uint32_t sn, count;
+  uint32_t sn = read_data_header(pdu);
-  pdcp_unpack_data_pdu_long_sn(pdu.get(), &sn);
 
   int32_t last_submit_diff_sn     = last_submitted_pdcp_rx_sn - sn;
   int32_t sn_diff_last_submit     = sn - last_submitted_pdcp_rx_sn;
@@ -273,6 +265,7 @@ void pdcp_entity_lte::handle_am_drb_pdu(srslte::unique_byte_buffer_t pdu)
              next_pdcp_rx_sn);
 
   // Handle PDU
+  uint32_t count;
   if ((0 <= sn_diff_last_submit && sn_diff_last_submit > (int32_t)reordering_window) ||
       (0 <= last_submit_diff_sn && last_submit_diff_sn < (int32_t)reordering_window)) {
     log->warning("|SN - last_submitted_sn| is larger than re-ordering window.\n");
@@ -351,64 +344,4 @@ void pdcp_entity_lte::get_bearer_status(uint16_t* dlsn, uint16_t* dlhfn, uint16_
   }
 }
 
-/****************************************************************************
- * Pack/Unpack helper functions
- * Ref: 3GPP TS 36.323 v10.1.0
- ***************************************************************************/
-void pdcp_pack_control_pdu(uint32_t sn, byte_buffer_t* sdu)
-{
-  // Make room and add header
-  sdu->msg--;
-  sdu->N_bytes++;
-  *sdu->msg = sn & 0x1F;
-
-  // Add MAC
-  sdu->msg[sdu->N_bytes++] = (PDCP_CONTROL_MAC_I >> 24) & 0xFF;
-  sdu->msg[sdu->N_bytes++] = (PDCP_CONTROL_MAC_I >> 16) & 0xFF;
-  sdu->msg[sdu->N_bytes++] = (PDCP_CONTROL_MAC_I >> 8) & 0xFF;
-  sdu->msg[sdu->N_bytes++] = PDCP_CONTROL_MAC_I & 0xFF;
-}
-
-void pdcp_unpack_control_pdu_sn(byte_buffer_t* sdu, uint32_t* sn)
-{
-  // Strip header
-  *sn = sdu->msg[0] & 0x1Fu;
-  sdu->msg++;
-  sdu->N_bytes--;
-}
-
-void pdcp_pack_data_pdu_short_sn(uint32_t sn, byte_buffer_t* sdu)
-{
-  // Make room and add header
-  sdu->msg--;
-  sdu->N_bytes++;
-  sdu->msg[0] = (PDCP_D_C_DATA_PDU << 7) | (sn & 0x7Fu);
-}
-
-
-void pdcp_unpack_data_pdu_short_sn(byte_buffer_t* sdu, uint32_t* sn)
-{
-  // Strip header
-  *sn = sdu->msg[0] & 0x7Fu;
-  sdu->msg++;
-  sdu->N_bytes--;
-}
-
-void pdcp_pack_data_pdu_long_sn(uint32_t sn, byte_buffer_t* sdu)
-{
-  // Make room and add header
-  sdu->msg -= 2;
-  sdu->N_bytes += 2;
-  sdu->msg[0] = (PDCP_D_C_DATA_PDU << 7) | ((sn >> 8) & 0x0Fu);
-  sdu->msg[1] = sn & 0xFFu;
-}
-
-void pdcp_unpack_data_pdu_long_sn(byte_buffer_t* sdu, uint32_t* sn)
-{
-  // Strip header
-  *sn = (sdu->msg[0] & 0x0Fu) << 8;
-  *sn |= sdu->msg[1];
-  sdu->msg += 2;
-  sdu->N_bytes -= 2;
-}
 } // namespace srslte

lib/src/upper/pdcp_entity_nr.cc

@@ -20,7 +20,6 @@
  */
 
 #include "srslte/upper/pdcp_entity_nr.h"
-#include "srslte/common/int_helpers.h"
 #include "srslte/common/security.h"
 
 namespace srslte {
@@ -228,62 +227,6 @@ void pdcp_entity_nr::write_pdu(unique_byte_buffer_t pdu)
 /*
  * Packing / Unpacking Helpers
  */
-uint32_t pdcp_entity_nr::read_data_header(const unique_byte_buffer_t& pdu)
-{
-  // Check PDU is long enough to extract header
-  if (pdu->N_bytes <= cfg.hdr_len_bytes) {
-    log->error("PDU too small to extract header\n");
-    return 0;
-  }
-
-  // Extract RCVD_SN
-  uint16_t rcvd_sn_16 = 0;
-  uint32_t rcvd_sn_32 = 0;
-  switch (cfg.sn_len) {
-    case PDCP_SN_LEN_12:
-      srslte::uint8_to_uint16(pdu->msg, &rcvd_sn_16);
-      rcvd_sn_32 = SN(rcvd_sn_16);
-      break;
-    case PDCP_SN_LEN_18:
-      srslte::uint8_to_uint24(pdu->msg, &rcvd_sn_32);
-      rcvd_sn_32 = SN(rcvd_sn_32);
-      break;
-    default:
-      log->error("Cannot extract RCVD_SN, invalid SN length configured: %d\n", cfg.sn_len);
-  }
-
-  // Discard header
-  pdu->msg += cfg.hdr_len_bytes;
-  pdu->N_bytes -= cfg.hdr_len_bytes;
-  return rcvd_sn_32;
-}
-
-void pdcp_entity_nr::write_data_header(const srslte::unique_byte_buffer_t& sdu, uint32_t count)
-{
-  // Add room for header
-  if (cfg.hdr_len_bytes > sdu->get_headroom()) {
-    log->error("Not enough space to add header\n");
-    return;
-  }
-  sdu->msg -= cfg.hdr_len_bytes;
-  sdu->N_bytes += cfg.hdr_len_bytes;
-
-  // Add SN
-  switch (cfg.sn_len) {
-    case PDCP_SN_LEN_12:
-      srslte::uint16_to_uint8(SN(count), sdu->msg);
-      if (is_drb()) {
-        sdu->msg[0] |= 0x80; // On Data PDUs for DRBs we must set the D flag.
-      }
-      break;
-    case PDCP_SN_LEN_18:
-      srslte::uint24_to_uint8(SN(count), sdu->msg);
-      sdu->msg[0] |= 0x80; // Data PDU and SN LEN 18 implies DRB, D flag must be present
-      break;
-    default:
-      log->error("Invalid SN length configuration: %d bits\n", cfg.sn_len);
-  }
-}
 
 
 // Deliver all consecutivly associated COUNTs.