From 0f43c2c713e9b9f29a509e5ac69d5841b804806c Mon Sep 17 00:00:00 2001 From: Robert Schmidt Date: Mon, 20 Mar 2023 11:24:08 +0100 Subject: [PATCH] PDCP DL handling: unlock PDCP before forwarding PDU nr_pdcp_entity_recv_sdu() processes PDCP SDUs, and forwards the PDU at the end of the function, while having the PDCP lock. This can lead to deadlocks, e.g., when another thread goes through RLC and tries to access the PDCP while the PDCP tries to lock the RLC. This commit changes the logic: instead of having recv_sdu() forward the PDU directly, it fills a buffer with the PDU (thus the name change to process_sdu()), and returns. We then unlock the PDCP, before forwarding the PDU, thus eliminating potential deadlocks. --- openair2/LAYER2/nr_pdcp/nr_pdcp_entity.c | 17 ++++++++++------- openair2/LAYER2/nr_pdcp/nr_pdcp_entity.h | 4 ++-- openair2/LAYER2/nr_pdcp/nr_pdcp_oai_api.c | 14 ++++++++++++-- openair2/LAYER2/nr_pdcp/nr_pdcp_oai_api.h | 2 ++ openair2/SDAP/nr_sdap/nr_sdap_entity.c | 17 ++++++++++++----- 5 files changed, 38 insertions(+), 16 deletions(-) diff --git a/openair2/LAYER2/nr_pdcp/nr_pdcp_entity.c b/openair2/LAYER2/nr_pdcp/nr_pdcp_entity.c index e632fb2024..326ed49686 100644 --- a/openair2/LAYER2/nr_pdcp/nr_pdcp_entity.c +++ b/openair2/LAYER2/nr_pdcp/nr_pdcp_entity.c @@ -176,14 +176,19 @@ static void nr_pdcp_entity_recv_pdu(nr_pdcp_entity_t *entity, } } -static void nr_pdcp_entity_recv_sdu(nr_pdcp_entity_t *entity, - char *buffer, int size, int sdu_id) +static int nr_pdcp_entity_process_sdu(nr_pdcp_entity_t *entity, + char *buffer, + int size, + int sdu_id, + char *pdu_buffer, + int pdu_max_size) { uint32_t count; int sn; int header_size; int integrity_size; - char buf[size + 3 + 4]; + char *buf = pdu_buffer; + DevAssert(size + 3 + 4 <= pdu_max_size); int dc_bit; entity->stats.rxsdu_pkts++; entity->stats.rxsdu_bytes += size; @@ -243,13 +248,11 @@ static void nr_pdcp_entity_recv_sdu(nr_pdcp_entity_t *entity, entity->tx_next++; - entity->deliver_pdu(entity->deliver_pdu_data, entity, buf, - header_size + size + integrity_size, sdu_id); entity->stats.txpdu_pkts++; entity->stats.txpdu_bytes += header_size + size + integrity_size; entity->stats.txpdu_sn = sn; - + return header_size + size + integrity_size; } /* may be called several times, take care to clean previous settings */ @@ -420,7 +423,7 @@ nr_pdcp_entity_t *new_nr_pdcp_entity( ret->type = type; ret->recv_pdu = nr_pdcp_entity_recv_pdu; - ret->recv_sdu = nr_pdcp_entity_recv_sdu; + ret->process_sdu = nr_pdcp_entity_process_sdu; ret->set_security = nr_pdcp_entity_set_security; ret->set_time = nr_pdcp_entity_set_time; diff --git a/openair2/LAYER2/nr_pdcp/nr_pdcp_entity.h b/openair2/LAYER2/nr_pdcp/nr_pdcp_entity.h index 4ad2508e4a..c693ed81d4 100644 --- a/openair2/LAYER2/nr_pdcp/nr_pdcp_entity.h +++ b/openair2/LAYER2/nr_pdcp/nr_pdcp_entity.h @@ -73,8 +73,8 @@ typedef struct nr_pdcp_entity_t { /* functions provided by the PDCP module */ void (*recv_pdu)(struct nr_pdcp_entity_t *entity, char *buffer, int size); - void (*recv_sdu)(struct nr_pdcp_entity_t *entity, char *buffer, int size, - int sdu_id); + int (*process_sdu)(struct nr_pdcp_entity_t *entity, char *buffer, int size, + int sdu_id, char *pdu_buffer, int pdu_max_size); void (*delete_entity)(struct nr_pdcp_entity_t *entity); void (*get_stats)(struct nr_pdcp_entity_t *entity, nr_pdcp_statistics_t *out); diff --git a/openair2/LAYER2/nr_pdcp/nr_pdcp_oai_api.c b/openair2/LAYER2/nr_pdcp/nr_pdcp_oai_api.c index a91f4e883f..412b348302 100644 --- a/openair2/LAYER2/nr_pdcp/nr_pdcp_oai_api.c +++ b/openair2/LAYER2/nr_pdcp/nr_pdcp_oai_api.c @@ -1173,10 +1173,15 @@ bool nr_pdcp_data_req_srb(ue_id_t ue_id, return 0; } - rb->recv_sdu(rb, (char *)sdu_buffer, sdu_buffer_size, muiP); + int max_size = sdu_buffer_size + 3 + 4; // 3: max header, 4: max integrity + char pdu_buf[max_size]; + int pdu_size = rb->process_sdu(rb, (char *)sdu_buffer, sdu_buffer_size, muiP, pdu_buf, max_size); + deliver_pdu deliver_pdu_cb = rb->deliver_pdu; nr_pdcp_manager_unlock(nr_pdcp_ue_manager); + deliver_pdu_cb(rb->deliver_pdu_data, rb, pdu_buf, pdu_size, muiP); + return 1; } @@ -1223,10 +1228,15 @@ bool nr_pdcp_data_req_drb(protocol_ctxt_t *ctxt_pP, return 0; } - rb->recv_sdu(rb, (char *)sdu_buffer, sdu_buffer_size, muiP); + int max_size = sdu_buffer_size + 3 + 4; // 3: max header, 4: max integrity + char pdu_buf[max_size]; + int pdu_size = rb->process_sdu(rb, (char *)sdu_buffer, sdu_buffer_size, muiP, pdu_buf, max_size); + deliver_pdu deliver_pdu_cb = rb->deliver_pdu; nr_pdcp_manager_unlock(nr_pdcp_ue_manager); + deliver_pdu_cb(rb->deliver_pdu_data, rb, pdu_buf, pdu_size, muiP); + return 1; } diff --git a/openair2/LAYER2/nr_pdcp/nr_pdcp_oai_api.h b/openair2/LAYER2/nr_pdcp/nr_pdcp_oai_api.h index 1b69b9d2e7..84b918529f 100644 --- a/openair2/LAYER2/nr_pdcp/nr_pdcp_oai_api.h +++ b/openair2/LAYER2/nr_pdcp/nr_pdcp_oai_api.h @@ -77,6 +77,8 @@ bool cu_f1u_data_req(protocol_ctxt_t *ctxt_pP, const uint32_t *const sourceL2Id, const uint32_t *const destinationL2Id); +typedef void (*deliver_pdu)(void *data, nr_pdcp_entity_t *entity, + char *buf, int size, int sdu_id); bool nr_pdcp_data_req_srb(ue_id_t ue_id, const rb_id_t rb_id, const mui_t muiP, diff --git a/openair2/SDAP/nr_sdap/nr_sdap_entity.c b/openair2/SDAP/nr_sdap/nr_sdap_entity.c index 357bec24ad..e81490630c 100644 --- a/openair2/SDAP/nr_sdap/nr_sdap_entity.c +++ b/openair2/SDAP/nr_sdap/nr_sdap_entity.c @@ -39,11 +39,18 @@ instance_t *N3GTPUInst = NULL; void nr_pdcp_submit_sdap_ctrl_pdu(ue_id_t ue_id, rb_id_t sdap_ctrl_pdu_drb, nr_sdap_ul_hdr_t ctrl_pdu) { - nr_pdcp_ue_t *ue; - nr_pdcp_ue_manager_t *nr_pdcp_ue_manager; - nr_pdcp_ue_manager = nr_pdcp_sdap_get_ue_manager(); - ue = nr_pdcp_manager_get_ue(nr_pdcp_ue_manager, ue_id); - ue->drb[sdap_ctrl_pdu_drb-1]->recv_sdu(ue->drb[sdap_ctrl_pdu_drb-1], (char*)&ctrl_pdu, SDAP_HDR_LENGTH, RLC_MUI_UNDEFINED); + + protocol_ctxt_t ctxt = { .rntiMaybeUEid = ue_id }; + nr_pdcp_data_req_drb(&ctxt, + SRB_FLAG_NO, + sdap_ctrl_pdu_drb, + RLC_MUI_UNDEFINED, + SDU_CONFIRM_NO, + SDAP_HDR_LENGTH, + (unsigned char *)&ctrl_pdu, + PDCP_TRANSMISSION_MODE_UNKNOWN, + NULL, + NULL); LOG_D(SDAP, "Control PDU - Submitting Control PDU to DRB ID: %ld\n", sdap_ctrl_pdu_drb); LOG_D(SDAP, "QFI: %u\n R: %u\n D/C: %u\n", ctrl_pdu.QFI, ctrl_pdu.R, ctrl_pdu.DC); return;