mux,bsr: refactor UL buffer status reporting

this patch fixes the UL BSR as per TS 36.321, it includes following
main changes:

* report UL buffer state to reflect the UEs transmit buffer after
  the MAC UL PDU containing the BSR has been built.
  In other words, if the UE, for example, can transmit all outstanding
  data in an UL grant, it will not report any pending data to transmit.

* refactor MUX routines and subheader space calculation

srsue/hdr/stack/mac/mux.h

@@ -44,8 +44,8 @@ typedef struct {
   uint32_t bucket_size;
   uint32_t BSD;
   uint32_t priority;
-  int      sched_len;
-  int      buffer_len;
+  int      sched_len;  // scheduled upper layer payload for this LCID
+  int      buffer_len; // outstanding bytes for this LCID
 } logical_channel_config_t;
 
 namespace srsue {

srsue/hdr/stack/mac/proc_bsr.h

@@ -45,8 +45,8 @@ public:
     uint32_t     buff_size[4];
   } bsr_t;
 
-  /* MUX calls BSR to check if it can fit a BSR into PDU */
-  virtual bool need_to_send_bsr_on_ul_grant(uint32_t grant_size, bsr_t* bsr) = 0;
+  /* MUX calls BSR to check if it should send (and can fit) a BSR into PDU */
+  virtual bool need_to_send_bsr_on_ul_grant(uint32_t grant_size, uint32_t total_data, bsr_t* bsr) = 0;
 
   /* MUX calls BSR to let it generate a padding BSR if there is space in PDU */
   virtual bool generate_padding_bsr(uint32_t nof_padding_bytes, bsr_t* bsr) = 0;
@@ -65,7 +65,7 @@ public:
   void     setup_lcid(uint32_t lcid, uint32_t lcg, uint32_t priority);
   void     timer_expired(uint32_t timer_id);
   uint32_t get_buffer_state();
-  bool     need_to_send_bsr_on_ul_grant(uint32_t grant_size, bsr_t* bsr);
+  bool     need_to_send_bsr_on_ul_grant(uint32_t grant_size, uint32_t total_data, bsr_t* bsr);
   bool     generate_padding_bsr(uint32_t nof_padding_bytes, bsr_t* bsr);
 
 private:
@@ -73,14 +73,14 @@ private:
 
   std::mutex mutex;
 
-  srslte::ext_task_sched_handle* task_sched;
+  srslte::ext_task_sched_handle* task_sched = nullptr;
   srslte::log_ref                log_h;
-  rlc_interface_mac*             rlc;
-  sr_proc*                       sr;
+  rlc_interface_mac*             rlc = nullptr;
+  sr_proc*                       sr  = nullptr;
 
   srslte::bsr_cfg_t bsr_cfg;
 
-  bool initiated;
+  bool initiated = false;
 
   const static int NOF_LCG = 4;
 
@@ -94,10 +94,7 @@ private:
 
   uint32_t find_max_priority_lcg_with_data();
   typedef enum { NONE, REGULAR, PADDING, PERIODIC } triggered_bsr_type_t;
-  triggered_bsr_type_t triggered_bsr_type;
-
-  uint32_t current_tti;
-  uint32_t trigger_tti;
+  triggered_bsr_type_t triggered_bsr_type = NONE;
 
   void     set_trigger(triggered_bsr_type_t new_trigger);
   void     update_new_data();

srsue/src/stack/mac/mux.cc

@@ -169,11 +169,6 @@ uint8_t* mux::pdu_get(srslte::byte_buffer_t* payload, uint32_t pdu_sz)
 {
   std::lock_guard<std::mutex> lock(mutex);
 
-  // Reset sched_len and update Bj
-  for (auto& channel : logical_channels) {
-    channel.sched_len = 0;
-  }
-
   // Logical Channel Procedure
   payload->clear();
   pdu_msg.init_tx(payload, pdu_sz, true);
@@ -194,14 +189,35 @@ uint8_t* mux::pdu_get(srslte::byte_buffer_t* payload, uint32_t pdu_sz)
   }
   pending_crnti_ce = 0;
 
-  bsr_proc::bsr_t bsr;
-  bool            regular_bsr = bsr_procedure->need_to_send_bsr_on_ul_grant(pdu_msg.rem_size(), &bsr);
+  // Calculate pending UL data per LCID and LCG as well as the total amount
+  bsr_proc::bsr_t bsr                = {}; // pending data per LCG
+  int             total_pending_data = 0;
+  int             last_sdu_len       = 0;
+  for (auto& channel : logical_channels) {
+    channel.sched_len  = 0; // reset sched_len for LCID
+    channel.buffer_len = rlc->get_buffer_state(channel.lcid);
+    total_pending_data += channel.buffer_len + sch_pdu::size_header_sdu(channel.buffer_len);
+    last_sdu_len = channel.buffer_len;
+    bsr.buff_size[channel.lcg] += channel.buffer_len;
+  }
+  if (total_pending_data > 0) {
+    // remove length byte(s) of last SDU
+    total_pending_data -= sch_pdu::size_header_sdu(last_sdu_len);
+    total_pending_data++; // the R/R/E/LCID subheader is still needed
+  }
+
+  bool regular_bsr = bsr_procedure->need_to_send_bsr_on_ul_grant(pdu_msg.rem_size(), total_pending_data, &bsr);
+  int  bsr_idx     = -1; // subheader index of BSR (needed to update content later)
 
   // MAC control element for BSR, with exception of BSR included for padding;
   if (regular_bsr) {
     if (pdu_msg.new_subh()) {
       if (pdu_msg.get()->set_bsr(bsr.buff_size, bsr_format_convert(bsr.format)) == false) {
         pdu_msg.del_subh();
+        regular_bsr = false; // make sure we don't update the BSR content later on
+      } else {
+        // BSR allocation was successful, store subheader index
+        bsr_idx = pdu_msg.get_current_idx();
       }
     }
   }
@@ -218,29 +234,36 @@ uint8_t* mux::pdu_get(srslte::byte_buffer_t* payload, uint32_t pdu_sz)
     }
   }
 
-  // Update buffer states for all logical channels
-  for (auto& channel : logical_channels) {
-    channel.buffer_len = rlc->get_buffer_state(channel.lcid);
-  }
-
-  int sdu_space = pdu_msg.get_sdu_space();
+  int32_t sdu_space = pdu_msg.get_sdu_space(); // considers first SDU's header is only one byte
 
   // data from any Logical Channel, except data from UL-CCCH;
   // first only those with positive Bj
+  uint32_t last_sdu_subheader_len = 0; // needed to keep track of added SDUs and actual required subheades
   for (auto& channel : logical_channels) {
     int max_sdu_sz = (channel.PBR < 0) ? -1 : channel.Bj; // this can be zero if no PBR has been allocated
     if (max_sdu_sz != 0) {
       if (sched_sdu(&channel, &sdu_space, max_sdu_sz)) {
         channel.Bj -= channel.sched_len;
+        // account for (possible) subheader needed for next SDU
+        last_sdu_subheader_len = SRSLTE_MIN((uint32_t)sdu_space, sch_pdu::size_header_sdu(channel.sched_len));
+        sdu_space -= last_sdu_subheader_len;
       }
     }
   }
+  if (last_sdu_subheader_len > 0) {
+    // Unused last subheader, give it back ..
+    sdu_space += last_sdu_subheader_len;
+  }
 
   print_logical_channel_state("First round of allocation:");
 
   // If resources remain, allocate regardless of their Bj value
   for (auto& channel : logical_channels) {
     if (channel.lcid != 0) {
+      // allocate subheader if this LCID has not been scheduled yet but there is data to send
+      if (channel.sched_len == 0 && channel.buffer_len > 0) {
+        sdu_space -= last_sdu_subheader_len;
+      }
       sched_sdu(&channel, &sdu_space, -1);
     }
   }
@@ -250,10 +273,16 @@ uint8_t* mux::pdu_get(srslte::byte_buffer_t* payload, uint32_t pdu_sz)
   for (auto& channel : logical_channels) {
     if (channel.sched_len != 0) {
       allocate_sdu(channel.lcid, &pdu_msg, channel.sched_len);
+
+      // update BSR according to allocation
+      bsr.buff_size[channel.lcg] -= channel.sched_len;
     }
   }
 
-  if (!regular_bsr) {
+  if (regular_bsr && bsr_idx >= 0 && pdu_msg.get(bsr_idx) != nullptr) {
+    // update BSR content
+    pdu_msg.get(bsr_idx)->update_bsr(bsr.buff_size, bsr_format_convert(bsr.format));
+  } else {
     // Insert Padding BSR if not inserted Regular/Periodic BSR
     if (bsr_procedure->generate_padding_bsr(pdu_msg.rem_size(), &bsr)) {
       if (pdu_msg.new_subh()) {
@@ -264,10 +293,10 @@ uint8_t* mux::pdu_get(srslte::byte_buffer_t* payload, uint32_t pdu_sz)
     }
   }
 
-  log_h->debug("Assembled MAC PDU msg size %d/%d bytes\n", pdu_msg.get_pdu_len() - pdu_msg.rem_size(), pdu_sz);
-
-  /* Generate MAC PDU and save to buffer */
+  // Generate MAC PDU and save to buffer
   uint8_t* ret = pdu_msg.write_packet(log_h);
+  Info("%s\n", pdu_msg.to_string().c_str());
+  Debug("Assembled MAC PDU msg size %d/%d bytes\n", pdu_msg.get_pdu_len() - pdu_msg.rem_size(), pdu_sz);
 
   return ret;
 }
@@ -290,20 +319,11 @@ bool mux::sched_sdu(logical_channel_config_t* ch, int* sdu_space, int max_sdu_sz
         sched_len = *sdu_space;
       }
 
-      log_h->debug("SDU:   scheduled lcid=%d, rlc_buffer=%d, allocated=%d/%d\n",
-                   ch->lcid,
-                   ch->buffer_len,
-                   sched_len,
-                   sdu_space ? *sdu_space : 0);
+      *sdu_space -= sched_len; // UL-SCH subheader size is accounted for in the calling function
 
-      *sdu_space -= sched_len;
-      ch->buffer_len -= sched_len;
-
-      if (ch->sched_len == 0) {
-        // account for header for the first time
-        *sdu_space -= sch_pdu::size_header_sdu(sched_len);
-      }
+      Debug("SDU:   scheduled lcid=%d, rlc_buffer=%d, allocated=%d\n", ch->lcid, ch->buffer_len, sched_len);
 
+      ch->buffer_len -= sched_len;
       ch->sched_len += sched_len;
       return true;
     }

srsue/src/stack/mac/proc_bsr.cc

@@ -25,13 +25,7 @@
 
 namespace srsue {
 
-bsr_proc::bsr_proc()
-{
-  initiated          = false;
-  current_tti        = 0;
-  trigger_tti        = 0;
-  triggered_bsr_type = NONE;
-}
+bsr_proc::bsr_proc() {}
 
 void bsr_proc::init(sr_proc*                       sr_,
                     rlc_interface_mac*             rlc_,
@@ -71,7 +65,6 @@ void bsr_proc::init(sr_proc*                       sr_,
 void bsr_proc::set_trigger(srsue::bsr_proc::triggered_bsr_type_t new_trigger)
 {
   triggered_bsr_type = new_trigger;
-  trigger_tti        = current_tti;
 
   // Trigger SR always when Regular BSR is triggered in the current TTI. Will be cancelled if a grant is received
   if (triggered_bsr_type == REGULAR) {
@@ -86,8 +79,6 @@ void bsr_proc::reset()
   timer_retx.stop();
 
   triggered_bsr_type = NONE;
-
-  trigger_tti = 0;
 }
 
 void bsr_proc::set_config(srslte::bsr_cfg_t& bsr_cfg_)
@@ -141,7 +132,6 @@ uint32_t bsr_proc::get_buffer_state()
 // Checks if data is available for a a channel with higher priority than others
 bool bsr_proc::check_highest_channel()
 {
-
   for (int i = 0; i < NOF_LCG; i++) {
     for (std::map<uint32_t, lcid_t>::iterator iter = lcgs[i].begin(); iter != lcgs[i].end(); ++iter) {
       // If new data available
@@ -219,15 +209,15 @@ uint32_t bsr_proc::get_buffer_state_lcg(uint32_t lcg)
   return n;
 }
 
+// Checks if a BSR needs to be generated and, if so, configures the BSR format
+// It does not update the BSR values of the LCGs
 bool bsr_proc::generate_bsr(bsr_t* bsr, uint32_t pdu_space)
 {
   bool     send_bsr = false;
   uint32_t nof_lcg = 0;
-  bzero(bsr, sizeof(bsr_t));
 
-  // Calculate buffer size for each LCG
+  // Check if more than one LCG has data to sned
   for (int i = 0; i < NOF_LCG; i++) {
-    bsr->buff_size[i] = get_buffer_state_lcg(i);
     if (bsr->buff_size[i] > 0) {
       nof_lcg++;
     }
@@ -245,7 +235,7 @@ bool bsr_proc::generate_bsr(bsr_t* bsr, uint32_t pdu_space)
   } else if (pdu_space >= CE_SUBHEADER_LEN + ce_size(srslte::ul_sch_lcid::SHORT_BSR)) {
     // we can only fit a short or truncated BSR
     if (nof_lcg > 1) {
-      // only send truncated BSR for a padding BSR
+      // send truncated BSR
       bsr->format           = TRUNC_BSR;
       uint32_t max_prio_lcg = find_max_priority_lcg_with_data();
       for (uint32_t i = 0; i < NOF_LCG; i++) {
@@ -260,14 +250,6 @@ bool bsr_proc::generate_bsr(bsr_t* bsr, uint32_t pdu_space)
   }
 
   if (send_bsr) {
-    Info("BSR:   Type %s, Format %s, Value=%d,%d,%d,%d\n",
-         bsr_type_tostring(triggered_bsr_type),
-         bsr_format_tostring(bsr->format),
-         bsr->buff_size[0],
-         bsr->buff_size[1],
-         bsr->buff_size[2],
-         bsr->buff_size[3]);
-
     // Restart or Start Periodic timer every time a BSR is generated and transmitted in an UL grant
     if (timer_periodic.duration() && bsr->format != TRUNC_BSR) {
       timer_periodic.run();
@@ -289,8 +271,6 @@ void bsr_proc::step(uint32_t tti)
     return;
   }
 
-  current_tti = tti;
-
   update_new_data();
 
   // Regular BSR triggered if new data arrives or channel with high priority has new data
@@ -330,31 +310,18 @@ char* bsr_proc::bsr_format_tostring(bsr_format_t format)
   }
 }
 
-bool bsr_proc::need_to_send_bsr_on_ul_grant(uint32_t grant_size, bsr_t* bsr)
+bool bsr_proc::need_to_send_bsr_on_ul_grant(uint32_t grant_size, uint32_t total_data, bsr_t* bsr)
 {
   std::lock_guard<std::mutex> lock(mutex);
 
-  bool bsr_included = false;
-
+  bool send_bsr = false;
   if (triggered_bsr_type == PERIODIC || triggered_bsr_type == REGULAR) {
-    bsr_included = generate_bsr(bsr, grant_size);
-  }
-
-  // All triggered BSRs shall be cancelled in case the UL grant can accommodate all pending data available for
-  // transmission but is not sufficient to additionally accommodate the BSR MAC control element plus its subheader. All
-  // triggered BSRs shall be cancelled when a BSR is included in a MAC PDU for transmission
-  int total_data = 0;
-  if (!bsr_included) {
-    for (int i = 0; i < NOF_LCG; i++) {
-      for (std::map<uint32_t, lcid_t>::iterator iter = lcgs[i].begin(); iter != lcgs[i].end(); ++iter) {
-        total_data += srslte::sch_pdu::size_header_sdu(iter->second.old_buffer) + iter->second.old_buffer;
-      }
+    // All triggered BSRs shall be cancelled in case the UL grant can accommodate all pending data
+    if (grant_size >= total_data) {
+      triggered_bsr_type = NONE;
+    } else {
+      send_bsr = generate_bsr(bsr, grant_size);
     }
-    total_data--; // Because last SDU has no size header
-  }
-  // If all data fits in the grant or BSR has been included, cancel the trigger
-  if (total_data < (int)grant_size || bsr_included) {
-    set_trigger(NONE);
   }
 
   // Cancel SR if an Uplink grant is received
@@ -366,25 +333,23 @@ bool bsr_proc::need_to_send_bsr_on_ul_grant(uint32_t grant_size, bsr_t* bsr)
     timer_retx.run();
     Debug("BSR:   Started retxBSR-Timer\n");
   }
-  return bsr_included;
+  return send_bsr;
 }
 
+// This function is called by MUX only if Regular BSR has not been triggered before
 bool bsr_proc::generate_padding_bsr(uint32_t nof_padding_bytes, bsr_t* bsr)
 {
   std::lock_guard<std::mutex> lock(mutex);
 
-  bool ret = false;
-
-  // This function is called by MUX only if Regular BSR has not been triggered before
   if (nof_padding_bytes >= CE_SUBHEADER_LEN + ce_size(srslte::ul_sch_lcid::SHORT_BSR)) {
     // generate padding BSR
     set_trigger(PADDING);
     generate_bsr(bsr, nof_padding_bytes);
     set_trigger(NONE);
-    ret = true;
+    return true;
   }
 
-  return ret;
+  return false;
 }
 
 void bsr_proc::setup_lcid(uint32_t lcid, uint32_t new_lcg, uint32_t priority)