From 03d3e1a5db598915d1a62ddf7e04a67467455a13 Mon Sep 17 00:00:00 2001 From: laurent Date: Tue, 14 Mar 2023 14:48:09 +0100 Subject: [PATCH] code review comments --- ci-scripts/conf_files/gNB_SA_CU.conf | 10 +++++-- ci-scripts/conf_files/gNB_SA_DU.conf | 10 +++++-- ci-scripts/conf_files/gnb-cucp.sa.f1.conf | 23 +++++++-------- ci-scripts/conf_files/gnb-cuup.sa.f1.conf | 18 ++++++------ executables/nr-uesoftmodem.c | 2 +- openair2/COMMON/sctp_messages_types.h | 2 +- openair2/E1AP/e1ap_common.c | 18 ++++++------ openair2/F1AP/f1ap_itti_messaging.c | 3 +- .../NR_MAC_gNB/gNB_scheduler_primitives.c | 14 ++++------ openair2/RRC/NR/MESSAGES/asn1_msg.c | 3 +- openair2/RRC/NR/rrc_gNB_NGAP.c | 2 +- openair2/RRC/NR/rrc_gNB_nsa.c | 5 +++- openair2/X2AP/x2ap_eNB_itti_messaging.c | 3 +- openair3/NGAP/ngap_gNB_itti_messaging.c | 3 +- openair3/NGAP/ngap_gNB_nas_procedures.c | 28 +++++++++---------- openair3/SCTP/sctp_eNB_task.c | 3 +- 16 files changed, 74 insertions(+), 73 deletions(-) diff --git a/ci-scripts/conf_files/gNB_SA_CU.conf b/ci-scripts/conf_files/gNB_SA_CU.conf index a9c6072301..d619c8311b 100644 --- a/ci-scripts/conf_files/gNB_SA_CU.conf +++ b/ci-scripts/conf_files/gNB_SA_CU.conf @@ -1,4 +1,4 @@ -Active_gNBs = ( "gNB-OAI"); +Active_gNBs = ( "gNB-Eurecom-CU"); # Asn1_verbosity, choice in: none, info, annoying Asn1_verbosity = "none"; Num_Threads_PUSCH = 8; @@ -8,11 +8,15 @@ gNBs = { ////////// Identification parameters: gNB_ID = 0xe00; - gNB_name = "gNB-OAI"; + +# cell_type = "CELL_MACRO_GNB"; + + gNB_name = "gNB-Eurecom-CU"; // Tracking area code, 0x0000 and 0xfffe are reserved values tracking_area_code = 1; - plmn_list = ({ mcc = 001; mnc = 01; mnc_length = 2; snssaiList = ({ sst = 1; }) }); + plmn_list = ({ mcc = 208; mnc = 99; mnc_length = 2; snssaiList = ({ sst = 1, sd = 0xffffff }) }); + nr_cellid = 12345678L; force_256qam_off = 1; diff --git a/ci-scripts/conf_files/gNB_SA_DU.conf b/ci-scripts/conf_files/gNB_SA_DU.conf index bdf0826bcb..4013c0d475 100644 --- a/ci-scripts/conf_files/gNB_SA_DU.conf +++ b/ci-scripts/conf_files/gNB_SA_DU.conf @@ -1,4 +1,4 @@ -Active_gNBs = ( "gNB-OAI"); +Active_gNBs = ( "gNB-Eurecom-DU"); # Asn1_verbosity, choice in: none, info, annoying Asn1_verbosity = "none"; @@ -7,11 +7,15 @@ gNBs = { ////////// Identification parameters: gNB_ID = 0xe00; - gNB_name = "gNB-OAI"; + +# cell_type = "CELL_MACRO_GNB"; + + gNB_name = "gNB-Eurecom-DU"; // Tracking area code, 0x0000 and 0xfffe are reserved values tracking_area_code = 1; - plmn_list = ({ mcc = 001; mnc = 01; mnc_length = 2; snssaiList = ({ sst = 1; }) }); + plmn_list = ({ mcc = 208; mnc = 99; mnc_length = 2; snssaiList = ({ sst = 1, sd = 0xffffff }) }); + nr_cellid = 12345678L; diff --git a/ci-scripts/conf_files/gnb-cucp.sa.f1.conf b/ci-scripts/conf_files/gnb-cucp.sa.f1.conf index 0d3808bcb7..f92ef6842c 100644 --- a/ci-scripts/conf_files/gnb-cucp.sa.f1.conf +++ b/ci-scripts/conf_files/gnb-cucp.sa.f1.conf @@ -8,12 +8,16 @@ gNBs = ( { ////////// Identification parameters: - gNB_ID = 0xe00; + gNB_CU_ID = 0xe00; + +# cell_type = "CELL_MACRO_GNB"; + gNB_name = "gNB-OAI"; // Tracking area code, 0x0000 and 0xfffe are reserved values tracking_area_code = 1; - plmn_list = ({ mcc = 001; mnc = 01; mnc_length = 2; snssaiList = ({ sst = 1; }) }); + plmn_list = ({ mcc = 208; mnc = 99; mnc_length = 2; snssaiList = ({ sst = 1, sd = 0xffffff }) }); + nr_cellid = 12345678L; force_256qam_off = 1; @@ -21,8 +25,8 @@ gNBs = tr_s_preference = "f1"; local_s_if_name = "lo"; - local_s_address = "127.0.0.4"; - remote_s_address = "127.0.0.3"; + local_s_address = "192.168.72.140"; + remote_s_address = "192.168.72.143"; local_s_portc = 501; local_s_portd = 2152; remote_s_portc = 500; @@ -167,7 +171,7 @@ gNBs = ////////// AMF parameters: - amf_ip_address = ( { ipv4 = "192.168.70.132"; + amf_ip_address = ( { ipv4 = "192.168.71.132"; ipv6 = "192:168:30::17"; active = "yes"; preference = "ipv4"; @@ -178,9 +182,9 @@ gNBs = ( { type = "cp"; - ipv4_cucp = "127.0.0.2"; + ipv4_cucp = "192.168.72.140"; port_cucp = 38462; - ipv4_cuup = "127.0.0.3"; + ipv4_cuup = "192.168.72.141"; port_cuup = 38462; } ) @@ -188,10 +192,7 @@ gNBs = NETWORK_INTERFACES : { GNB_INTERFACE_NAME_FOR_NG_AMF = "demo-oai"; - GNB_IPV4_ADDRESS_FOR_NG_AMF = "192.168.70.129/24"; - GNB_INTERFACE_NAME_FOR_NGU = "demo-oai"; - GNB_IPV4_ADDRESS_FOR_NGU = "192.168.70.129/24"; - GNB_PORT_FOR_S1U = 2152; # Spec 2152 + GNB_IPV4_ADDRESS_FOR_NG_AMF = "192.168.71.140/24"; }; } ); diff --git a/ci-scripts/conf_files/gnb-cuup.sa.f1.conf b/ci-scripts/conf_files/gnb-cuup.sa.f1.conf index 309756f5c8..6759654e0d 100644 --- a/ci-scripts/conf_files/gnb-cuup.sa.f1.conf +++ b/ci-scripts/conf_files/gnb-cuup.sa.f1.conf @@ -15,14 +15,14 @@ gNBs = // Tracking area code, 0x0000 and 0xfffe are reserved values tracking_area_code = 1; - plmn_list = ({ mcc = 001; mnc = 01; mnc_length = 2; snssaiList = ({ sst = 1; }) }); + plmn_list = ({ mcc = 208; mnc = 99; mnc_length = 2; snssaiList = ({ sst = 1, sd = 0xffffff }) }); tr_s_preference = "f1"; local_s_if_name = "lo"; - local_s_address = "127.0.0.4"; - remote_s_address = "127.0.0.3"; + local_s_address = "192.168.72.141"; + remote_s_address = "192.168.72.142"; local_s_portc = 501; local_s_portd = 2152; remote_s_portc = 500; @@ -39,20 +39,18 @@ gNBs = E1_INTERFACE = ( { - type = "cp"; - ipv4_cucp = "127.0.0.2"; - port_cucp = 38462; - ipv4_cuup = "127.0.0.3"; - port_cuup = 38462; + type = "up"; + ipv4_cucp = "192.168.72.140"; + ipv4_cuup = "192.168.72.141"; } ) NETWORK_INTERFACES : { GNB_INTERFACE_NAME_FOR_NG_AMF = "demo-oai"; - GNB_IPV4_ADDRESS_FOR_NG_AMF = "192.168.70.129/24"; + GNB_IPV4_ADDRESS_FOR_NG_AMF = "192.168.71.141/24"; GNB_INTERFACE_NAME_FOR_NGU = "demo-oai"; - GNB_IPV4_ADDRESS_FOR_NGU = "192.168.70.129/24"; + GNB_IPV4_ADDRESS_FOR_NGU = "192.168.71.141/24"; GNB_PORT_FOR_S1U = 2152; # Spec 2152 }; } diff --git a/executables/nr-uesoftmodem.c b/executables/nr-uesoftmodem.c index 6d8ce1f15e..d3d8090be5 100644 --- a/executables/nr-uesoftmodem.c +++ b/executables/nr-uesoftmodem.c @@ -550,7 +550,7 @@ int main( int argc, char **argv ) { // Sleep a while before checking all parameters have been used // Some are used directly in external threads, asynchronously - sleep(200); + sleep(20); config_check_unknown_cmdlineopt(CONFIG_CHECKALLSECTIONS); while(true) diff --git a/openair2/COMMON/sctp_messages_types.h b/openair2/COMMON/sctp_messages_types.h index 594b9f06e8..c2561c0424 100644 --- a/openair2/COMMON/sctp_messages_types.h +++ b/openair2/COMMON/sctp_messages_types.h @@ -119,7 +119,7 @@ typedef struct sctp_data_ind_s { /* Buffer to send over SCTP */ uint32_t buffer_length; uint8_t *buffer; - bool freeAfterSend; + /* Streams on which data will be sent/received */ uint16_t stream; } sctp_data_ind_t; diff --git a/openair2/E1AP/e1ap_common.c b/openair2/E1AP/e1ap_common.c index e58c4dbc81..caf97b680d 100644 --- a/openair2/E1AP/e1ap_common.c +++ b/openair2/E1AP/e1ap_common.c @@ -220,17 +220,15 @@ int e1ap_encode_send(E1_t type, e1ap_setup_req_t *setupReq, E1AP_E1AP_PDU_t *pdu if (encoded < 0) { LOG_E(E1AP, "%s: Failed to encode E1AP message\n", func); return -1; - } else { - MessageDef *message = itti_alloc_new_message((type==CPtype)?TASK_CUCP_E1:TASK_CUUP_E1, 0, SCTP_DATA_REQ); - sctp_data_req_t *s = &message->ittiMsg.sctp_data_req; - s->assoc_id = setupReq->assoc_id; - s->buffer = buffer; - s->buffer_length = encoded; - s->stream = stream; - s->freeAfterSend=true; - LOG_I(E1AP, "%s: Sending ITTI message to SCTP Task\n", func); - itti_send_msg_to_task(TASK_SCTP, 0 /*unused by callee*/, message); } + MessageDef *message = itti_alloc_new_message((type == CPtype) ? TASK_CUCP_E1 : TASK_CUUP_E1, 0, SCTP_DATA_REQ); + sctp_data_req_t *s = &message->ittiMsg.sctp_data_req; + s->assoc_id = setupReq->assoc_id; + s->buffer = buffer; + s->buffer_length = encoded; + s->stream = stream; + LOG_I(E1AP, "%s: Sending ITTI message to SCTP Task\n", func); + itti_send_msg_to_task(TASK_SCTP, 0 /*unused by callee*/, message); return encoded; } diff --git a/openair2/F1AP/f1ap_itti_messaging.c b/openair2/F1AP/f1ap_itti_messaging.c index 3026a9b25d..12386b69d6 100644 --- a/openair2/F1AP/f1ap_itti_messaging.c +++ b/openair2/F1AP/f1ap_itti_messaging.c @@ -31,8 +31,7 @@ void f1ap_itti_send_sctp_data_req(bool isCu, instance_t instance, uint8_t *buffe sctp_data_req->assoc_id = f1ap_assoc_id(isCu,instance); sctp_data_req->buffer = buffer; sctp_data_req->buffer_length = buffer_length; - sctp_data_req->stream = stream; - sctp_data_req->freeAfterSend=true; + sctp_data_req->stream = stream; LOG_D(F1AP, "Sending ITTI message to SCTP Task\n"); itti_send_msg_to_task(TASK_SCTP, instance, message_p); } diff --git a/openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_primitives.c b/openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_primitives.c index 512e6949bb..b0f2e8ed05 100644 --- a/openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_primitives.c +++ b/openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_primitives.c @@ -2784,17 +2784,13 @@ void nr_mac_update_timers(module_id_t module_id, NR_CellGroupConfig_t *cg = NULL; uper_decode(NULL, - &asn_DEF_NR_CellGroupConfig, //might be added prefix later + &asn_DEF_NR_CellGroupConfig, // might be added prefix later (void **)&cg, (uint8_t *)UE->cg_buf, - (UE->enc_rval.encoded+7)/8, 0, 0); - if (UE->CellGroup) { - NR_CellGroupConfig_t *tmp=UE->CellGroup; - UE->CellGroup = cg; - ASN_STRUCT_FREE(asn_DEF_NR_CellGroupConfig,tmp); - } else - UE->CellGroup = cg; - + (UE->enc_rval.encoded + 7) / 8, + 0, + 0); + UE->CellGroup = cg; if (LOG_DEBUGFLAG(DEBUG_ASN1)) { xer_fprint(stdout, &asn_DEF_NR_CellGroupConfig, (const void *) UE->CellGroup); diff --git a/openair2/RRC/NR/MESSAGES/asn1_msg.c b/openair2/RRC/NR/MESSAGES/asn1_msg.c index 7f99ebe35a..85d0002aac 100644 --- a/openair2/RRC/NR/MESSAGES/asn1_msg.c +++ b/openair2/RRC/NR/MESSAGES/asn1_msg.c @@ -1760,7 +1760,8 @@ do_NR_DLInformationTransfer( asn1cCalloc(infoTransfer->criticalExtensions.choice.dlInformationTransfer, dlInfoTransfer); asn1cCalloc(dlInfoTransfer->dedicatedNAS_Message,msg); - // we will free the caller buffer + // we will free the caller buffer, that is ok in the present code logic (else it will leak memory) but not natural, + // comprehensive code design msg->buf = pdu_buffer; msg->size = pdu_length; diff --git a/openair2/RRC/NR/rrc_gNB_NGAP.c b/openair2/RRC/NR/rrc_gNB_NGAP.c index bc5c93ee4e..262bfec2a9 100644 --- a/openair2/RRC/NR/rrc_gNB_NGAP.c +++ b/openair2/RRC/NR/rrc_gNB_NGAP.c @@ -1317,7 +1317,7 @@ int rrc_gNB_process_NGAP_PDUSESSION_RELEASE_COMMAND(MessageDef *msg_p, instance_ PROTOCOL_CTXT_SET_BY_INSTANCE(&ctxt, instance, GNB_FLAG_YES, UE->rnti, 0, 0); xid = rrc_gNB_get_next_transaction_identifier(ctxt.module_id); LOG_I( - NR_RRC, "PDU Session Release Command: AMF_UE_NGAP_ID %lu GNB_UE_NGAP_ID %u release_pdusessions %d \n", cmd->amf_ue_ngap_id & 0x000000FFFFFFFFFF, gNB_ue_ngap_id, cmd->nb_pdusessions_torelease); + NR_RRC, "PDU Session Release Command: AMF_UE_NGAP_ID %lu GNB_UE_NGAP_ID %u release_pdusessions %d \n", cmd->amf_ue_ngap_id, gNB_ue_ngap_id, cmd->nb_pdusessions_torelease); for (int pdusession = 0; pdusession < cmd->nb_pdusessions_torelease; pdusession++) { rrc_pdu_session_param_t *pduSession = find_pduSession(UE, cmd->pdusession_release_params[pdusession].pdusession_id, false); diff --git a/openair2/RRC/NR/rrc_gNB_nsa.c b/openair2/RRC/NR/rrc_gNB_nsa.c index 54f9251278..89c5b44858 100644 --- a/openair2/RRC/NR/rrc_gNB_nsa.c +++ b/openair2/RRC/NR/rrc_gNB_nsa.c @@ -237,7 +237,10 @@ void rrc_add_nsa_user(gNB_RRC_INST *rrc, rrc_gNB_ue_context_t *ue_context_p, x2a NR_ServingCellConfig_t *scc = UE->spCellConfig ? UE->spCellConfig->spCellConfigDedicated : NULL; fill_default_reconfig(carrier->servingcellconfigcommon, scc, reconfig_ies, UE->secondaryCellGroup, UE->UE_Capability_nr, configuration, ue_context_p->ue_context.gNB_ue_ngap_id); - rrc_gNB_update_ue_context_rnti(UE->secondaryCellGroup->spCellConfig->reconfigurationWithSync->newUE_Identity, rrc, UE->gNB_ue_ngap_id); + // the UE context is not yet inserted in the RRC UE manager + // rrc_gNB_update_ue_context_rnti(UE->secondaryCellGroup->spCellConfig->reconfigurationWithSync->newUE_Identity, rrc, + // UE->gNB_ue_ngap_id); + UE->rnti = UE->secondaryCellGroup->spCellConfig->reconfigurationWithSync->newUE_Identity; NR_CG_Config_t *CG_Config = calloc(1,sizeof(*CG_Config)); memset((void *)CG_Config,0,sizeof(*CG_Config)); // int CG_Config_size = generate_CG_Config(rrc,CG_Config,UE->reconfig,UE->rb_config); diff --git a/openair2/X2AP/x2ap_eNB_itti_messaging.c b/openair2/X2AP/x2ap_eNB_itti_messaging.c index 6468e2134e..66be3520e2 100644 --- a/openair2/X2AP/x2ap_eNB_itti_messaging.c +++ b/openair2/X2AP/x2ap_eNB_itti_messaging.c @@ -43,8 +43,7 @@ void x2ap_eNB_itti_send_sctp_data_req(instance_t instance, int32_t assoc_id, uin sctp_data_req->assoc_id = assoc_id; sctp_data_req->buffer = buffer; sctp_data_req->buffer_length = buffer_length; - sctp_data_req->stream = stream; - sctp_data_req->freeAfterSend=true; + sctp_data_req->stream = stream; itti_send_msg_to_task(TASK_SCTP, instance, message_p); } diff --git a/openair3/NGAP/ngap_gNB_itti_messaging.c b/openair3/NGAP/ngap_gNB_itti_messaging.c index 78751b693a..87b4ba4c73 100644 --- a/openair3/NGAP/ngap_gNB_itti_messaging.c +++ b/openair3/NGAP/ngap_gNB_itti_messaging.c @@ -44,8 +44,7 @@ void ngap_gNB_itti_send_sctp_data_req(instance_t instance, int32_t assoc_id, uin sctp_data_req->assoc_id = assoc_id; sctp_data_req->buffer = buffer; sctp_data_req->buffer_length = buffer_length; - sctp_data_req->stream = stream; - sctp_data_req->freeAfterSend=true; + sctp_data_req->stream = stream; itti_send_msg_to_task(TASK_SCTP, instance, message_p); } diff --git a/openair3/NGAP/ngap_gNB_nas_procedures.c b/openair3/NGAP/ngap_gNB_nas_procedures.c index 024a306e79..fdbe6ce0f9 100644 --- a/openair3/NGAP/ngap_gNB_nas_procedures.c +++ b/openair3/NGAP/ngap_gNB_nas_procedures.c @@ -1193,21 +1193,21 @@ int ngap_gNB_pdusession_release_resp(instance_t instance, ngap_pdusession_releas ie->value.choice.RAN_UE_NGAP_ID = pdusession_release_resp_p->gNB_ue_ngap_id; } - /* optional */ - if (pdusession_release_resp_p->nb_of_pdusessions_released > 0) { - asn1cSequenceAdd(out->protocolIEs.list, NGAP_PDUSessionResourceReleaseResponseIEs_t, ie); - ie->id = NGAP_ProtocolIE_ID_id_PDUSessionResourceReleasedListRelRes; - ie->criticality = NGAP_Criticality_ignore; - ie->value.present = NGAP_PDUSessionResourceReleaseResponseIEs__value_PR_PDUSessionResourceReleasedListRelRes; - - for (i = 0; i < pdusession_release_resp_p->nb_of_pdusessions_released; i++) { - asn1cSequenceAdd(ie->value.choice.PDUSessionResourceReleasedListRelRes.list, NGAP_PDUSessionResourceReleasedItemRelRes_t, item); - item->pDUSessionID = pdusession_release_resp_p->pdusession_release[i].pdusession_id; - allocCopy(&item->pDUSessionResourceReleaseResponseTransfer, pdusession_release_resp_p->pdusession_release[i].data); - NGAP_DEBUG("pdusession_release_resp: pdusession ID %ld\n", item->pDUSessionID); - } + /* optional */ + if (pdusession_release_resp_p->nb_of_pdusessions_released > 0) { + asn1cSequenceAdd(out->protocolIEs.list, NGAP_PDUSessionResourceReleaseResponseIEs_t, ie); + ie->id = NGAP_ProtocolIE_ID_id_PDUSessionResourceReleasedListRelRes; + ie->criticality = NGAP_Criticality_ignore; + ie->value.present = NGAP_PDUSessionResourceReleaseResponseIEs__value_PR_PDUSessionResourceReleasedListRelRes; + + for (i = 0; i < pdusession_release_resp_p->nb_of_pdusessions_released; i++) { + asn1cSequenceAdd(ie->value.choice.PDUSessionResourceReleasedListRelRes.list, NGAP_PDUSessionResourceReleasedItemRelRes_t, item); + item->pDUSessionID = pdusession_release_resp_p->pdusession_release[i].pdusession_id; + allocCopy(&item->pDUSessionResourceReleaseResponseTransfer, pdusession_release_resp_p->pdusession_release[i].data); + NGAP_DEBUG("pdusession_release_resp: pdusession ID %ld\n", item->pDUSessionID); + } } - + if (ngap_gNB_encode_pdu(&pdu, &buffer, &length) < 0) { NGAP_ERROR("Failed to encode release response\n"); /* Encode procedure has failed... */ diff --git a/openair3/SCTP/sctp_eNB_task.c b/openair3/SCTP/sctp_eNB_task.c index ef419556d7..7706137b2e 100644 --- a/openair3/SCTP/sctp_eNB_task.c +++ b/openair3/SCTP/sctp_eNB_task.c @@ -650,8 +650,7 @@ static void sctp_send_data(sctp_data_req_t *sctp_data_req_p) /* TODO: notify upper layer */ return; } - if ( sctp_data_req_p->freeAfterSend) - free(sctp_data_req_p->buffer); + free(sctp_data_req_p->buffer); // assuming it has been malloced SCTP_DEBUG("Successfully sent %u bytes on stream %d for assoc_id %u\n", sctp_data_req_p->buffer_length, sctp_data_req_p->stream, sctp_cnx->assoc_id);