From 28e3102858149b732ea1330b2ae7cd13f433f337 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=BAben=20Soares=20Silva?= Date: Mon, 7 Oct 2024 17:54:40 +0100 Subject: [PATCH] Address memory leaks found in P7 messages handling Add nr_fapi_p7 library to NFAPI_USER_LIB for access to utility functions Change message handlers to use the utility functions to copy and properly free the messages Change the SRS handler to call the handler for the measurements instead of trying to copy to RC.gNB[0], which does not exist Change nfapi_nr_p7_message_unpack to not allocate unneeded memory --- CMakeLists.txt | 1 + nfapi/oai_integration/nfapi_vnf.c | 219 +++++--------------------- nfapi/open-nFAPI/nfapi/src/nfapi_p7.c | 38 ++--- nfapi/open-nFAPI/vnf/src/vnf_p7.c | 7 + 4 files changed, 62 insertions(+), 203 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 4c13790708..4085e49265 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -759,6 +759,7 @@ set(NFAPI_USER_SRC ) add_library(NFAPI_USER_LIB ${NFAPI_USER_SRC}) target_link_libraries(NFAPI_USER_LIB PRIVATE asn1_lte_rrc_hdrs asn1_nr_rrc_hdrs) +target_link_libraries(NFAPI_USER_LIB PRIVATE nr_fapi_p7) include_directories(${NFAPI_USER_DIR}) # Layer 1 diff --git a/nfapi/oai_integration/nfapi_vnf.c b/nfapi/oai_integration/nfapi_vnf.c index 237a4171e2..76542d3587 100644 --- a/nfapi/oai_integration/nfapi_vnf.c +++ b/nfapi/oai_integration/nfapi_vnf.c @@ -47,6 +47,9 @@ #include "common/ran_context.h" #include "openair2/PHY_INTERFACE/queue_t.h" #include "gnb_ind_vars.h" +#include "nr_fapi_p7_utils.h" +#include + #define TEST @@ -745,35 +748,15 @@ int phy_rach_indication(struct nfapi_vnf_p7_config *config, nfapi_rach_indicatio int phy_nr_rach_indication(nfapi_nr_rach_indication_t *ind) { - if(NFAPI_MODE == NFAPI_MODE_VNF) - { + if (NFAPI_MODE == NFAPI_MODE_VNF) { nfapi_nr_rach_indication_t *rach_ind = CALLOC(1, sizeof(*rach_ind)); - rach_ind->header.message_id = ind->header.message_id; - rach_ind->number_of_pdus = ind->number_of_pdus; - rach_ind->sfn = ind->sfn; - rach_ind->slot = ind->slot; - rach_ind->pdu_list = CALLOC(rach_ind->number_of_pdus, sizeof(*rach_ind->pdu_list)); - AssertFatal(rach_ind->pdu_list != NULL, "Memory not allocated for rach_ind->pdu_list in phy_nr_rach_indication."); - for (int i = 0; i < ind->number_of_pdus; i++) - { - rach_ind->pdu_list[i].num_preamble = ind->pdu_list[i].num_preamble; - rach_ind->pdu_list[i].freq_index = ind->pdu_list[i].freq_index; - rach_ind->pdu_list[i].symbol_index = ind->pdu_list[i].symbol_index; - AssertFatal(rach_ind->pdu_list[i].preamble_list != NULL, "Memory not allocated for rach_ind->pdu_list[i].preamble_list in phy_nr_rach_indication."); - for (int j = 0; j < ind->number_of_pdus; j++) - { - rach_ind->pdu_list[i].preamble_list[j].preamble_index = ind->pdu_list[i].preamble_list[j].preamble_index; - rach_ind->pdu_list[i].preamble_list[j].timing_advance = ind->pdu_list[i].preamble_list[j].timing_advance; - } - } - if (!put_queue(&gnb_rach_ind_queue, rach_ind)) - { + copy_rach_indication(ind, rach_ind); + if (!put_queue(&gnb_rach_ind_queue, rach_ind)) { LOG_E(NR_MAC, "Put_queue failed for rach_ind\n"); - free(rach_ind->pdu_list); + free_rach_indication(rach_ind); free(rach_ind); } - } - else { + } else { LOG_E(NR_MAC, "NFAPI_MODE = %d not NFAPI_MODE_VNF(2)\n", nfapi_getmode()); } return 1; @@ -781,84 +764,18 @@ int phy_nr_rach_indication(nfapi_nr_rach_indication_t *ind) int phy_nr_uci_indication(nfapi_nr_uci_indication_t *ind) { - - LOG_D(NR_MAC, "In %s() NFAPI SFN/SF: %d/%d number_of_pdus :%u\n", - __FUNCTION__,ind->sfn, ind->slot, ind->num_ucis); - if(NFAPI_MODE == NFAPI_MODE_VNF) - { + LOG_D(NR_MAC, "In %s() NFAPI SFN/SF: %d/%d number_of_pdus :%u\n", __FUNCTION__, ind->sfn, ind->slot, ind->num_ucis); + if (NFAPI_MODE == NFAPI_MODE_VNF) { nfapi_nr_uci_indication_t *uci_ind = CALLOC(1, sizeof(*uci_ind)); AssertFatal(uci_ind, "Memory not allocated for uci_ind in phy_nr_uci_indication."); - *uci_ind = *ind; - - uci_ind->uci_list = CALLOC(NFAPI_NR_UCI_IND_MAX_PDU, sizeof(nfapi_nr_uci_t)); - AssertFatal(uci_ind->uci_list != NULL, "Memory not allocated for uci_ind->uci_list in phy_nr_uci_indication."); - for (int i = 0; i < ind->num_ucis; i++) - { - uci_ind->uci_list[i] = ind->uci_list[i]; - - switch (uci_ind->uci_list[i].pdu_type) { - case NFAPI_NR_UCI_PUSCH_PDU_TYPE: - LOG_E(MAC, "%s(): unhandled NFAPI_NR_UCI_PUSCH_PDU_TYPE\n", __func__); - break; - - case NFAPI_NR_UCI_FORMAT_0_1_PDU_TYPE: { - //nfapi_nr_uci_pucch_pdu_format_0_1_t *uci_ind_pdu = &uci_ind->uci_list[i].pucch_pdu_format_0_1; - //nfapi_nr_uci_pucch_pdu_format_0_1_t *ind_pdu = &ind->uci_list[i].pucch_pdu_format_0_1; - //Unused - break; - } - - case NFAPI_NR_UCI_FORMAT_2_3_4_PDU_TYPE: { - nfapi_nr_uci_pucch_pdu_format_2_3_4_t *uci_ind_pdu = &uci_ind->uci_list[i].pucch_pdu_format_2_3_4; - nfapi_nr_uci_pucch_pdu_format_2_3_4_t *ind_pdu = &ind->uci_list[i].pucch_pdu_format_2_3_4; - *uci_ind_pdu = *ind_pdu; - if (ind_pdu->harq.harq_payload) { - uci_ind_pdu->harq.harq_payload = CALLOC(1, sizeof(*uci_ind_pdu->harq.harq_payload)); - AssertFatal(uci_ind_pdu->harq.harq_payload != NULL, "Memory not allocated for uci_ind_pdu->harq.harq_payload in phy_nr_uci_indication."); - *uci_ind_pdu->harq.harq_payload = *ind_pdu->harq.harq_payload; - } - if (ind_pdu->sr.sr_payload) { - uci_ind_pdu->sr.sr_payload = CALLOC(1, sizeof(*uci_ind_pdu->sr.sr_payload)); - AssertFatal(uci_ind_pdu->sr.sr_payload != NULL, "Memory not allocated for uci_ind_pdu->sr.sr_payload in phy_nr_uci_indication."); - *uci_ind_pdu->sr.sr_payload = *ind_pdu->sr.sr_payload; - } - if (ind_pdu->csi_part1.csi_part1_payload) { - uci_ind_pdu->csi_part1.csi_part1_payload = CALLOC(1, sizeof(*uci_ind_pdu->csi_part1.csi_part1_payload)); - AssertFatal(uci_ind_pdu->csi_part1.csi_part1_payload != NULL, "Memory not allocated for uci_ind_pdu->csi_part1.csi_part1_payload in phy_nr_uci_indication."); - *uci_ind_pdu->csi_part1.csi_part1_payload = *ind_pdu->csi_part1.csi_part1_payload; - } - if (ind_pdu->csi_part2.csi_part2_payload) { - uci_ind_pdu->csi_part2.csi_part2_payload = CALLOC(1, sizeof(*uci_ind_pdu->csi_part2.csi_part2_payload)); - AssertFatal(uci_ind_pdu->csi_part2.csi_part2_payload != NULL, "Memory not allocated for uci_ind_pdu->csi_part2.csi_part2_payload in phy_nr_uci_indication."); - *uci_ind_pdu->csi_part2.csi_part2_payload = *ind_pdu->csi_part2.csi_part2_payload; - } - break; - } - } - } - - if (!put_queue(&gnb_uci_ind_queue, uci_ind)) - { + copy_uci_indication(ind, uci_ind); + if (!put_queue(&gnb_uci_ind_queue, uci_ind)) { LOG_E(NR_MAC, "Put_queue failed for uci_ind\n"); - for (int i = 0; i < ind->num_ucis; i++) - { - if (uci_ind->uci_list[i].pdu_type == NFAPI_NR_UCI_FORMAT_0_1_PDU_TYPE) - { - } - if (uci_ind->uci_list[i].pdu_type == NFAPI_NR_UCI_FORMAT_2_3_4_PDU_TYPE) - { - free(uci_ind->uci_list[i].pucch_pdu_format_2_3_4.harq.harq_payload); - free(uci_ind->uci_list[i].pucch_pdu_format_2_3_4.csi_part1.csi_part1_payload); - free(uci_ind->uci_list[i].pucch_pdu_format_2_3_4.csi_part2.csi_part2_payload); - } - } - free(uci_ind->uci_list); - uci_ind->uci_list = NULL; + free_uci_indication(uci_ind); free(uci_ind); uci_ind = NULL; } - } - else { + } else { LOG_E(NR_MAC, "NFAPI_MODE = %d not NFAPI_MODE_VNF(2)\n", nfapi_getmode()); } return 1; @@ -949,43 +866,19 @@ int phy_crc_indication(struct nfapi_vnf_p7_config *config, nfapi_crc_indication_ return 1; } -int phy_nr_crc_indication(nfapi_nr_crc_indication_t *ind) { +int phy_nr_crc_indication(nfapi_nr_crc_indication_t *ind) +{ + LOG_D(NR_MAC, "In %s() NFAPI SFN/SF: %d/%d number_of_pdus :%u\n", __FUNCTION__, ind->sfn, ind->slot, ind->number_crcs); - LOG_D(NR_MAC, "In %s() NFAPI SFN/SF: %d/%d number_of_pdus :%u\n", - __FUNCTION__,ind->sfn, ind->slot, ind->number_crcs); - - if(NFAPI_MODE == NFAPI_MODE_VNF) - { + if (NFAPI_MODE == NFAPI_MODE_VNF) { nfapi_nr_crc_indication_t *crc_ind = CALLOC(1, sizeof(*crc_ind)); - crc_ind->header.message_id = ind->header.message_id; - crc_ind->number_crcs = ind->number_crcs; - crc_ind->sfn = ind->sfn; - crc_ind->slot = ind->slot; - if (ind->number_crcs > 0) { - crc_ind->crc_list = CALLOC(NFAPI_NR_CRC_IND_MAX_PDU, sizeof(nfapi_nr_crc_t)); - AssertFatal(crc_ind->crc_list != NULL, "Memory not allocated for crc_ind->crc_list in phy_nr_crc_indication."); - } - for (int j = 0; j < ind->number_crcs; j++) - { - crc_ind->crc_list[j].handle = ind->crc_list[j].handle; - crc_ind->crc_list[j].harq_id = ind->crc_list[j].harq_id; - crc_ind->crc_list[j].num_cb = ind->crc_list[j].num_cb; - crc_ind->crc_list[j].rnti = ind->crc_list[j].rnti; - crc_ind->crc_list[j].tb_crc_status = ind->crc_list[j].tb_crc_status; - crc_ind->crc_list[j].timing_advance = ind->crc_list[j].timing_advance; - crc_ind->crc_list[j].ul_cqi = ind->crc_list[j].ul_cqi; - LOG_D(NR_MAC, "Received crc_ind.harq_id = %d for %d index SFN SLot %u %u with rnti %x\n", - ind->crc_list[j].harq_id, j, ind->sfn, ind->slot, ind->crc_list[j].rnti); - } - if (!put_queue(&gnb_crc_ind_queue, crc_ind)) - { + copy_crc_indication(ind, crc_ind); + if (!put_queue(&gnb_crc_ind_queue, crc_ind)) { LOG_E(NR_MAC, "Put_queue failed for crc_ind\n"); - free(crc_ind->crc_list); + free_crc_indication(crc_ind); free(crc_ind); } - } - else - { + } else { LOG_E(NR_MAC, "NFAPI_MODE = %d not NFAPI_MODE_VNF(2)\n", nfapi_getmode()); } return 1; @@ -1068,43 +961,25 @@ int phy_rx_indication(struct nfapi_vnf_p7_config *config, nfapi_rx_indication_t return 1; } -int phy_nr_rx_data_indication(nfapi_nr_rx_data_indication_t *ind) { +int phy_nr_rx_data_indication(nfapi_nr_rx_data_indication_t *ind) +{ + LOG_D(NR_MAC, + "In %s() NFAPI SFN/SF: %d/%d number_of_pdus :%u, and pdu %p\n", + __FUNCTION__, + ind->sfn, + ind->slot, + ind->number_of_pdus, + ind->pdu_list[0].pdu); - LOG_D(NR_MAC, "In %s() NFAPI SFN/SF: %d/%d number_of_pdus :%u, and pdu %p\n", - __FUNCTION__,ind->sfn, ind->slot, ind->number_of_pdus, ind->pdu_list[0].pdu); - - if(NFAPI_MODE == NFAPI_MODE_VNF) - { + if (NFAPI_MODE == NFAPI_MODE_VNF) { nfapi_nr_rx_data_indication_t *rx_ind = CALLOC(1, sizeof(*rx_ind)); - rx_ind->header.message_id = ind->header.message_id; - rx_ind->sfn = ind->sfn; - rx_ind->slot = ind->slot; - rx_ind->number_of_pdus = ind->number_of_pdus; - - if (ind->number_of_pdus > 0) { - rx_ind->pdu_list = CALLOC(NFAPI_NR_RX_DATA_IND_MAX_PDU, sizeof(nfapi_nr_rx_data_pdu_t)); - AssertFatal(rx_ind->pdu_list != NULL, "Memory not allocated for rx_ind->pdu_list in phy_nr_rx_data_indication."); - } - for (int j = 0; j < ind->number_of_pdus; j++) - { - rx_ind->pdu_list[j].handle = ind->pdu_list[j].handle; - rx_ind->pdu_list[j].harq_id = ind->pdu_list[j].harq_id; - rx_ind->pdu_list[j].pdu = ind->pdu_list[j].pdu; - rx_ind->pdu_list[j].pdu_length = ind->pdu_list[j].pdu_length; - rx_ind->pdu_list[j].rnti = ind->pdu_list[j].rnti; - rx_ind->pdu_list[j].timing_advance = ind->pdu_list[j].timing_advance; - rx_ind->pdu_list[j].ul_cqi = ind->pdu_list[j].ul_cqi; - rx_ind->pdu_list[j].rssi = ind->pdu_list[j].rssi; - } - if (!put_queue(&gnb_rx_ind_queue, rx_ind)) - { + copy_rx_data_indication(ind, rx_ind); + if (!put_queue(&gnb_rx_ind_queue, rx_ind)) { LOG_E(NR_MAC, "Put_queue failed for rx_ind\n"); - free(rx_ind->pdu_list); + free_rx_data_indication(rx_ind); free(rx_ind); } - } - else - { + } else { LOG_E(NR_MAC, "NFAPI_MODE = %d not NFAPI_MODE_VNF(2)\n", nfapi_getmode()); } return 1; @@ -1265,24 +1140,10 @@ int phy_nr_slot_indication(nfapi_nr_slot_indication_scf_t *ind) { return 1; } - -int phy_nr_srs_indication(nfapi_nr_srs_indication_t *ind) { - struct PHY_VARS_gNB_s *gNB = RC.gNB[0]; - - gNB->UL_INFO.srs_ind = *ind; - - if (ind->number_of_pdus > 0) - gNB->UL_INFO.srs_ind.pdu_list = malloc(sizeof(nfapi_nr_srs_indication_pdu_t)*ind->number_of_pdus); - - for (int i=0; inumber_of_pdus; i++) { - memcpy(&gNB->UL_INFO.srs_ind.pdu_list[i], &ind->pdu_list[i], sizeof(ind->pdu_list[0])); - - LOG_D(MAC, "%s() NFAPI SFN/Slot:%d.%d SRS_IND:number_of_pdus:%d UL_INFO:pdus:%d\n", - __FUNCTION__, - ind->sfn,ind->slot, ind->number_of_pdus, gNB->UL_INFO.srs_ind.number_of_pdus - ); - } - +int phy_nr_srs_indication(nfapi_nr_srs_indication_t *ind) +{ + for (int i = 0; i < ind->number_of_pdus; ++i) + handle_nr_srs_measurements(0, ind->sfn, ind->slot, &ind->pdu_list[i]); return 1; } //end NR phy indication diff --git a/nfapi/open-nFAPI/nfapi/src/nfapi_p7.c b/nfapi/open-nFAPI/nfapi/src/nfapi_p7.c index 655cd69e1a..df554ab599 100644 --- a/nfapi/open-nFAPI/nfapi/src/nfapi_p7.c +++ b/nfapi/open-nFAPI/nfapi/src/nfapi_p7.c @@ -6278,43 +6278,33 @@ int nfapi_nr_p7_message_unpack(void *pMessageBuf, case NFAPI_NR_PHY_MSG_TYPE_RX_DATA_INDICATION: if (check_nr_fapi_unpack_length(NFAPI_NR_PHY_MSG_TYPE_RX_DATA_INDICATION, unpackedBufLen)) { - nfapi_nr_rx_data_indication_t *msg = (nfapi_nr_rx_data_indication_t *)pMessageHeader; - msg->pdu_list = (nfapi_nr_rx_data_pdu_t *)malloc(sizeof(nfapi_nr_rx_data_pdu_t)); - msg->pdu_list->pdu = (uint8_t *)malloc(sizeof(uint8_t)); - result = unpack_nr_rx_data_indication(&pReadPackedMessage, end, msg, config); + result = unpack_nr_rx_data_indication(&pReadPackedMessage, end, pMessageHeader, config); } - break; + break; case NFAPI_NR_PHY_MSG_TYPE_CRC_INDICATION: if (check_nr_fapi_unpack_length(NFAPI_NR_PHY_MSG_TYPE_CRC_INDICATION, unpackedBufLen)) { - nfapi_nr_crc_indication_t *msg = (nfapi_nr_crc_indication_t *)pMessageHeader; - msg->crc_list = (nfapi_nr_crc_t *)malloc(sizeof(nfapi_nr_crc_t)); - result = unpack_nr_crc_indication(&pReadPackedMessage, end, msg, config); - } - break; + result = unpack_nr_crc_indication(&pReadPackedMessage, end, pMessageHeader, config); + } + break; case NFAPI_NR_PHY_MSG_TYPE_UCI_INDICATION: if (check_nr_fapi_unpack_length(NFAPI_NR_PHY_MSG_TYPE_UCI_INDICATION, unpackedBufLen)) { - nfapi_nr_uci_indication_t *msg = (nfapi_nr_uci_indication_t *)pMessageHeader; - msg->uci_list = (nfapi_nr_uci_t *)malloc(sizeof(nfapi_nr_uci_t)); - result = unpack_nr_uci_indication(&pReadPackedMessage, end, msg, config); - } - break; + result = unpack_nr_uci_indication(&pReadPackedMessage, end, pMessageHeader, config); + } + break; case NFAPI_NR_PHY_MSG_TYPE_SRS_INDICATION: if (check_nr_fapi_unpack_length(NFAPI_NR_PHY_MSG_TYPE_SRS_INDICATION, unpackedBufLen)) { - nfapi_nr_srs_indication_t *msg = (nfapi_nr_srs_indication_t *)pMessageHeader; - msg->pdu_list = (nfapi_nr_srs_indication_pdu_t *)malloc(sizeof(nfapi_nr_srs_indication_pdu_t)); - result = unpack_nr_srs_indication(&pReadPackedMessage, end, msg, config); - } - break; + result = unpack_nr_srs_indication(&pReadPackedMessage, end, pMessageHeader, config); + } + break; case NFAPI_NR_PHY_MSG_TYPE_RACH_INDICATION: if (check_nr_fapi_unpack_length(NFAPI_NR_PHY_MSG_TYPE_RACH_INDICATION, unpackedBufLen)) { - nfapi_nr_rach_indication_t *msg = (nfapi_nr_rach_indication_t *)pMessageHeader; - result = unpack_nr_rach_indication(&pReadPackedMessage, end, msg, config); - } - break; + result = unpack_nr_rach_indication(&pReadPackedMessage, end, pMessageHeader, config); + } + break; case NFAPI_NR_PHY_MSG_TYPE_DL_NODE_SYNC: if (check_nr_fapi_unpack_length(NFAPI_NR_PHY_MSG_TYPE_DL_NODE_SYNC, unpackedBufLen)) diff --git a/nfapi/open-nFAPI/vnf/src/vnf_p7.c b/nfapi/open-nFAPI/vnf/src/vnf_p7.c index 07a4593d88..a30efaf47f 100644 --- a/nfapi/open-nFAPI/vnf/src/vnf_p7.c +++ b/nfapi/open-nFAPI/vnf/src/vnf_p7.c @@ -29,6 +29,7 @@ #include "nfapi/oai_integration/aerial/fapi_nvIPC.h" #endif #include "vnf_p7.h" +#include "nr_fapi_p7_utils.h" #ifdef NDEBUG # warning assert is disabled @@ -1511,6 +1512,7 @@ void vnf_handle_nr_slot_indication(void *pRecvMsg, int recvMsgLen, vnf_p7_t* vnf { (vnf_p7->_public.nr_slot_indication)(&ind); } + free_slot_indication(&ind); } } @@ -1537,6 +1539,7 @@ void vnf_handle_nr_rx_data_indication(void *pRecvMsg, int recvMsgLen, vnf_p7_t* { (vnf_p7->_public.nr_rx_data_indication)(&ind); } + free_rx_data_indication(&ind); } } } @@ -1563,6 +1566,7 @@ void vnf_handle_nr_crc_indication(void *pRecvMsg, int recvMsgLen, vnf_p7_t* vnf_ { (vnf_p7->_public.nr_crc_indication)(&ind); } + free_crc_indication(&ind); } } } @@ -1588,6 +1592,7 @@ void vnf_handle_nr_srs_indication(void *pRecvMsg, int recvMsgLen, vnf_p7_t* vnf_ { (vnf_p7->_public.nr_srs_indication)(&ind); } + free_srs_indication(&ind); } } } @@ -1614,6 +1619,7 @@ void vnf_handle_nr_uci_indication(void *pRecvMsg, int recvMsgLen, vnf_p7_t* vnf_ { (vnf_p7->_public.nr_uci_indication)(&ind); } + free_uci_indication(&ind); } } } @@ -1640,6 +1646,7 @@ void vnf_handle_nr_rach_indication(void *pRecvMsg, int recvMsgLen, vnf_p7_t* vnf { (vnf_p7->_public.nr_rach_indication)(&ind); } + free_rach_indication(&ind); } } }