From 012894592202f43c25b41fec3de2f006eed7139b Mon Sep 17 00:00:00 2001 From: Robert Schmidt Date: Thu, 29 Feb 2024 16:51:31 +0100 Subject: [PATCH] nr_fill_indication(): protect with mutex nr_fill_indication() fills CRC and RX requests, to be sent from L1 to L2. Before this commit, nr_fill_indication() is called when - we finally decoded LDPC (or not) - low signal on PUSCH This might happen at the same time, i.e., nr_fill_indication() might try to fill both CRC and RX, in the same list, from different threads. This can lead to this assertion: Assertion (crc->rnti == rx->rnti) failed! In handle_nr_ulsch() ../../../openair2/NR_PHY_INTERFACE/NR_IF_Module.c:190 mis-match between CRC RNTI e071 and RX RNTI 5e3b e.g., e071 is low energy, and while 5e3b related message are filled in one thread, e071 is being put into the message structure as well. At least that is my understanding. I could not actually reproduce this assertion; to be seen if it still happens. --- openair1/PHY/INIT/nr_init.c | 4 ++++ openair1/SCHED_NR/phy_procedures_nr_gNB.c | 12 +++++++++++- openair2/NR_PHY_INTERFACE/NR_IF_Module.c | 9 +++++++-- openair2/NR_PHY_INTERFACE/NR_IF_Module.h | 8 +++++--- 4 files changed, 27 insertions(+), 6 deletions(-) diff --git a/openair1/PHY/INIT/nr_init.c b/openair1/PHY/INIT/nr_init.c index 1f918086b2..864420f56e 100644 --- a/openair1/PHY/INIT/nr_init.c +++ b/openair1/PHY/INIT/nr_init.c @@ -135,6 +135,8 @@ int phy_init_nr_gNB(PHY_VARS_gNB *gNB) load_LDPClib(NULL, &ldpc_interface); + pthread_mutex_init(&gNB->UL_INFO.crc_rx_mutex, NULL); + if (gNB->ldpc_offload_flag) load_LDPClib("_t2", &ldpc_interface_offload); gNB->max_nb_pdsch = MAX_MOBILES_PER_GNB; @@ -331,6 +333,8 @@ void phy_free_nr_gNB(PHY_VARS_gNB *gNB) const int max_ul_mimo_layers = 4; // taken from phy_init_nr_gNB() const int n_buf = Prx * max_ul_mimo_layers; + pthread_mutex_destroy(&gNB->UL_INFO.crc_rx_mutex); + PHY_MEASUREMENTS_gNB *meas = &gNB->measurements; free_and_zero(meas->n0_subband_power); free_and_zero(meas->n0_subband_power_dB); diff --git a/openair1/SCHED_NR/phy_procedures_nr_gNB.c b/openair1/SCHED_NR/phy_procedures_nr_gNB.c index 4005c7d581..4289c98a57 100644 --- a/openair1/SCHED_NR/phy_procedures_nr_gNB.c +++ b/openair1/SCHED_NR/phy_procedures_nr_gNB.c @@ -471,6 +471,14 @@ void nr_fill_indication(PHY_VARS_gNB *gNB, int frame, int slot_rx, int ULSCH_id, else if (SNRtimes10 > 635) cqi=255; else cqi=(640+SNRtimes10)/5; + // multiple threads might call this function at the same time, or while the + // L2 reads the messages. Hence, if not protected, crc and rx indications + // might not appear pairwise (in the same order) in the same slot, or even in + // separate slots. The L2 does not support this; hence, use the crc_rx_mutex + // to ensure that messages are pairwise. + int rc = pthread_mutex_lock(&gNB->UL_INFO.crc_rx_mutex); + DevAssert(rc == 0); + // crc indication uint16_t num_crc = gNB->UL_INFO.crc_ind.number_crcs; gNB->UL_INFO.crc_ind.crc_list = &gNB->crc_pdu_list[0]; @@ -507,8 +515,10 @@ void nr_fill_indication(PHY_VARS_gNB *gNB, int frame, int slot_rx, int ULSCH_id, gNB->rx_pdu_list[num_rx].pdu_length = harq_process->TBS; gNB->rx_pdu_list[num_rx].pdu = harq_process->b; } - gNB->UL_INFO.rx_ind.number_of_pdus++; + + rc = pthread_mutex_unlock(&gNB->UL_INFO.crc_rx_mutex); + DevAssert(rc == 0); } // Function to fill UL RB mask to be used for N0 measurements diff --git a/openair2/NR_PHY_INTERFACE/NR_IF_Module.c b/openair2/NR_PHY_INTERFACE/NR_IF_Module.c index 37bedddf46..8494a56492 100644 --- a/openair2/NR_PHY_INTERFACE/NR_IF_Module.c +++ b/openair2/NR_PHY_INTERFACE/NR_IF_Module.c @@ -177,6 +177,9 @@ void handle_nr_ulsch(NR_UL_IND_t *UL_info) } if (UL_info->rx_ind.number_of_pdus > 0 && UL_info->crc_ind.number_crcs > 0) { + // see nr_fill_indication about why this mutex is necessary + int rc = pthread_mutex_lock(&UL_info->crc_rx_mutex); + DevAssert(rc == 0); AssertFatal(UL_info->rx_ind.number_of_pdus == UL_info->crc_ind.number_crcs, "number_of_pdus %d, number_crcs %d\n", UL_info->rx_ind.number_of_pdus, UL_info->crc_ind.number_crcs); @@ -210,9 +213,11 @@ void handle_nr_ulsch(NR_UL_IND_t *UL_info) rx->rssi); handle_nr_ul_harq(UL_info->CC_id, UL_info->module_id, UL_info->frame, UL_info->slot, crc); } + UL_info->rx_ind.number_of_pdus = 0; + UL_info->crc_ind.number_crcs = 0; + rc = pthread_mutex_unlock(&UL_info->crc_rx_mutex); + DevAssert(rc == 0); } - UL_info->rx_ind.number_of_pdus = 0; - UL_info->crc_ind.number_crcs = 0; } void handle_nr_srs(NR_UL_IND_t *UL_info) { diff --git a/openair2/NR_PHY_INTERFACE/NR_IF_Module.h b/openair2/NR_PHY_INTERFACE/NR_IF_Module.h index 108e0afe7a..ba230e4f63 100644 --- a/openair2/NR_PHY_INTERFACE/NR_IF_Module.h +++ b/openair2/NR_PHY_INTERFACE/NR_IF_Module.h @@ -64,6 +64,11 @@ typedef struct { /// crc indication list nfapi_nr_crc_indication_t crc_ind; + /// RX indication + nfapi_nr_rx_data_indication_t rx_ind; + /// mutex to protect concurrent access to crc_ind and rx_ind, which the L2 + /// needs to be pairwise + pthread_mutex_t crc_rx_mutex; /// RACH indication list nfapi_nr_rach_indication_t rach_ind; @@ -71,9 +76,6 @@ typedef struct { /// SRS indication list nfapi_nr_srs_indication_t srs_ind; - /// RX indication - nfapi_nr_rx_data_indication_t rx_ind; - /// UCI indication nfapi_nr_uci_indication_t uci_ind;