From f07a9277a03444ebfb051b619f4eadd7b660cd67 Mon Sep 17 00:00:00 2001 From: Andre Puschmann Date: Mon, 30 Sep 2019 21:03:32 +0200 Subject: [PATCH] fix RLC AM issue where an invalid status PDU was generated happens when very small grant is provided and the status PDU generation fails because of a too small grant add extra check to prevent that ACK_SN is also in NACKS list --- lib/include/srslte/upper/rlc_am.h | 2 +- lib/include/srslte/upper/rlc_common.h | 2 +- lib/src/upper/rlc_am.cc | 18 ++++++++++++++++++ lib/test/upper/rlc_am_test.cc | 9 ++++++++- 4 files changed, 28 insertions(+), 3 deletions(-) diff --git a/lib/include/srslte/upper/rlc_am.h b/lib/include/srslte/upper/rlc_am.h index 48ca8f96a..103aaa3d3 100644 --- a/lib/include/srslte/upper/rlc_am.h +++ b/lib/include/srslte/upper/rlc_am.h @@ -306,6 +306,7 @@ int rlc_am_write_status_pdu(rlc_status_pdu_t *status, uint8_t *payload); uint32_t rlc_am_packed_length(rlc_amd_pdu_header_t *header); uint32_t rlc_am_packed_length(rlc_status_pdu_t *status); uint32_t rlc_am_packed_length(rlc_amd_retx_t retx); +bool rlc_am_is_valid_status_pdu(const rlc_status_pdu_t& status); bool rlc_am_is_control_pdu(byte_buffer_t *pdu); bool rlc_am_is_control_pdu(uint8_t *payload); bool rlc_am_is_pdu_segment(uint8_t *payload); @@ -318,5 +319,4 @@ bool rlc_am_not_start_aligned(const uint8_t fi); } // namespace srslte - #endif // SRSLTE_RLC_AM_H diff --git a/lib/include/srslte/upper/rlc_common.h b/lib/include/srslte/upper/rlc_common.h index 846e8922d..1b3875552 100644 --- a/lib/include/srslte/upper/rlc_common.h +++ b/lib/include/srslte/upper/rlc_common.h @@ -154,7 +154,7 @@ struct rlc_status_nack_t{ // STATUS PDU struct rlc_status_pdu_t{ - uint16_t ack_sn; + uint16_t ack_sn; // SN of the next not received RLC Data PDU uint32_t N_nack; rlc_status_nack_t nacks[RLC_AM_WINDOW_SIZE]; diff --git a/lib/src/upper/rlc_am.cc b/lib/src/upper/rlc_am.cc index a2466dd54..d00dbd473 100644 --- a/lib/src/upper/rlc_am.cc +++ b/lib/src/upper/rlc_am.cc @@ -1629,6 +1629,14 @@ int rlc_am::rlc_am_rx::get_status_pdu(rlc_status_pdu_t* status, const uint32_t m if (status->N_nack >= 1) { log->debug("Removing last NACK SN=%d\n", status->nacks[status->N_nack].nack_sn); status->N_nack--; + // make sure we don't have the current ACK_SN in the NACK list + if (rlc_am_is_valid_status_pdu(*status) == false) { + // No space to send any NACKs + log->debug("Resetting N_nack to zero\n"); + status->N_nack = 0; + } + } else { + log->error("Failed to generate small enough status PDU\n"); } break; } @@ -2024,6 +2032,16 @@ int rlc_am_write_status_pdu(rlc_status_pdu_t *status, uint8_t *payload) return tmp.N_bits/8; } +bool rlc_am_is_valid_status_pdu(const rlc_status_pdu_t& status) +{ + for (uint16_t i = 0; i < status.N_nack; ++i) { + if (status.nacks[i].nack_sn == status.ack_sn) { + return false; + } + } + return true; +} + uint32_t rlc_am_packed_length(rlc_amd_pdu_header_t *header) { uint32_t len = 2; // Fixed part is 2 bytes diff --git a/lib/test/upper/rlc_am_test.cc b/lib/test/upper/rlc_am_test.cc index 51901e776..90a619ec1 100644 --- a/lib/test/upper/rlc_am_test.cc +++ b/lib/test/upper/rlc_am_test.cc @@ -1586,11 +1586,18 @@ bool status_pdu_test() // Read status PDU from RLC2 byte_buffer_t status_buf; - len = rlc2.read_pdu(status_buf.msg, 3); // provide only small grant + len = rlc2.read_pdu(status_buf.msg, 5); // provide only small grant status_buf.N_bytes = len; assert(status_buf.N_bytes != 0); + // check status PDU doesn't contain ACK_SN in NACK list + rlc_status_pdu_t status_pdu = {}; + rlc_am_read_status_pdu(status_buf.msg, status_buf.N_bytes, &status_pdu); + if (rlc_am_is_valid_status_pdu(status_pdu) == false) { + return -1; + } + // Write status PDU to RLC1 rlc1.write_pdu(status_buf.msg, status_buf.N_bytes);