Skip to content

Commit

Permalink
proc_bsr: fix race condition in BSR reporting
Browse files Browse the repository at this point in the history
fix for #1934

This fixes a race condition between Stack thread and DL
PDU processing that lead to updates of the RLC buffer that
are undetected by the BSR routine.

What happens is that in a UL SCH PDU all outstanding data is transmitted
and and a LBSR with all zero buffers is sent.

14:39:47.327301 [MAC ] [D] [ 3793] BSR:   LCID=3 old_buffer=59
14:39:47.330600 [MAC ] [I] [ 3793] UL LCID=3 len=58 LBSR: b=0 0 0 0

Note that "old_buffer" isn't set to zero here.

At the same time (same TTI), the MAC PDU processing thread handles DL-SCH PDUs
that may generate new UL PDUs:

14:39:47.330749 [RLC ] [I] DRB1 Tx SDU (54 B, tx_sdu_queue_len=1)
14:39:47.330762 [RLC ] [I] DRB1 Tx SDU (54 B, tx_sdu_queue_len=2)
14:39:47.330775 [RLC ] [I] DRB1 Tx SDU (54 B, tx_sdu_queue_len=3)
..

Those PDUs are "new data" since the previous buffer state was zero.

Here is the race now between the threads, at the end of the bsr::step() function
old_buffer of each LCG is updated with the previous new_buffer, so
the buffer state of LCG=2 is now 59.

Now MAC starts the next TTI:

14:39:47.331910 [MAC ] [D] [ 3794] Running MAC tti=3794
14:39:47.331928 [MAC ] [D] [ 3794] Update Bj: lcid=0, Bj=0
14:39:47.331934 [MAC ] [D] [ 3794] Update Bj: lcid=1, Bj=0
14:39:47.331938 [MAC ] [D] [ 3794] Update Bj: lcid=2, Bj=0
14:39:47.331941 [MAC ] [D] [ 3794] Update Bj: lcid=3, Bj=-1752
14:39:47.331951 [MAC ] [D] [ 3794] BSR:   LCID=0 update new buffer=0
14:39:47.331960 [MAC ] [D] [ 3794] BSR:   LCID=1 update new buffer=0
14:39:47.331964 [MAC ] [D] [ 3794] BSR:   LCID=2 update new buffer=0
14:39:47.331971 [MAC ] [D] [ 3794] BSR:   LCID=3 update new buffer=335
14:39:47.331976 [MAC ] [D] [ 3794] BSR:    check_new_data() -> get_buffer_state_lcg(0)=0
14:39:47.331980 [MAC ] [D] [ 3794] BSR:    check_new_data() -> get_buffer_state_lcg(1)=0
14:39:47.331984 [MAC ] [D] [ 3794] BSR:    check_new_data() -> get_buffer_state_lcg(2)=59
14:39:47.331988 [MAC ] [D] [ 3794] BSR:    check_new_data() -> get_buffer_state_lcg(3)=0
14:39:47.331993 [MAC ] [D] [ 3794] BSR:   LCID=0 old_buffer=0
14:39:47.332000 [MAC ] [D] [ 3794] BSR:   LCID=1 old_buffer=0
14:39:47.332003 [MAC ] [D] [ 3794] BSR:   LCID=2 old_buffer=0
14:39:47.332007 [MAC ] [D] [ 3794] BSR:   LCID=3 old_buffer=335

And since the buffer state of LCG=2 isn't zero, the new data for LCID=3 of that LCG is considered.
So effectivly, the BSR missed the "empty" buffer state for a fraction of time and doesn't
consider the outgoing data generated in the same TTI as new. It therefore
doesn't transmit a BSR.

in which a BSR wasn't
  • Loading branch information
andrepuschmann committed Oct 28, 2020
1 parent 465c3d9 commit fd5cde5
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 3 deletions.
6 changes: 5 additions & 1 deletion srsue/hdr/stack/mac/proc_bsr.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ class bsr_interface_mux

/* 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;

/* MUX calls BSR to update buffer state of each LCG after all PDUs for this TTI have been packed */
virtual void update_bsr_tti_end(const bsr_t* bsr) = 0;
};

class bsr_proc : public srslte::timer_callback, public bsr_interface_mux
Expand All @@ -67,6 +70,7 @@ class bsr_proc : public srslte::timer_callback, public bsr_interface_mux
uint32_t get_buffer_state();
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);
void update_bsr_tti_end(const bsr_t* bsr);

private:
const static int QUEUE_STATUS_PERIOD_MS = 1000;
Expand Down Expand Up @@ -99,7 +103,7 @@ class bsr_proc : public srslte::timer_callback, public bsr_interface_mux
void print_state();
void set_trigger(triggered_bsr_type_t new_trigger);
void update_new_data();
void update_buffer_state();
void update_old_buffer();
bool check_highest_channel();
bool check_new_data();
bool check_any_channel();
Expand Down
3 changes: 3 additions & 0 deletions srsue/src/stack/mac/mux.cc
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,9 @@ uint8_t* mux::pdu_get(srslte::byte_buffer_t* payload, uint32_t pdu_sz)
}
}

// Update buffers in BSR procedure after packing entire TTI
bsr_procedure->update_bsr_tti_end(&bsr);

// 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());
Expand Down
22 changes: 20 additions & 2 deletions srsue/src/stack/mac/proc_bsr.cc
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ void bsr_proc::update_new_data()
}
}

void bsr_proc::update_buffer_state()
void bsr_proc::update_old_buffer()
{
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) {
Expand Down Expand Up @@ -269,6 +269,24 @@ bool bsr_proc::generate_bsr(bsr_t* bsr, uint32_t pdu_space)
return send_bsr;
}

/* After packing all UL PDUs for this TTI, the internal buffer state of the BSR procedure needs to be updated with what
* has actually been transmitted in each LCG. We don't ask RLC again as new SDUs could have queued up again. Currently
* we just get the updates per LCG. Since we are only interested when zero outstanding data has been reported, we
* currently just reset the buffer for each LCID of the LCG.
*/
void bsr_proc::update_bsr_tti_end(const bsr_t* bsr)
{
std::lock_guard<std::mutex> lock(mutex);
for (uint32_t i = 0; i < NOF_LCG; i++) {
if (bsr->buff_size[i] == 0) {
for (std::map<uint32_t, lcid_t>::iterator iter = lcgs[i].begin(); iter != lcgs[i].end(); ++iter) {
// Reset buffer state for all LCIDs of that the LCG for which we reported no further data to transmit
iter->second.old_buffer = 0;
}
}
}
}

// Checks if Regular BSR must be assembled, as defined in 5.4.5
// Padding BSR is assembled when called by mux_unit when UL dci is received
// Periodic BSR is triggered by the expiration of the timers
Expand All @@ -288,7 +306,7 @@ void bsr_proc::step(uint32_t tti)
set_trigger(REGULAR);
}

update_buffer_state();
update_old_buffer();
}

char* bsr_proc::bsr_type_tostring(triggered_bsr_type_t type)
Expand Down

0 comments on commit fd5cde5

Please sign in to comment.