Actions
Bug #5916
closedMGW FSM code handle_rab_release() looks odd
Start date:
02/21/2023
Due date:
% Done:
100%
Spec Reference:
Description
Looking at this code:
/* The MSC may ask to release a specific RAB within a RAB-AssignmentRequest */ static int handle_rab_release(struct hnbgw_context_map *map, struct msgb *ranap_msg, ranap_message *message) { bool rab_release_req; struct osmo_fsm_inst *fi = map->mgw_fi; struct mgw_fsm_priv *mgw_fsm_priv = fi->priv; int rc; /* Check if the RAB that is handled by this FSM is addressed by the release request */ rab_release_req = ranap_rab_ass_req_ies_check_release(&message->msg.raB_AssignmentRequestIEs, mgw_fsm_priv->rab_id); if (!rab_release_req) return -EINVAL; LOGPFSML(map->mgw_fi, LOGL_NOTICE, "MSC asked to release RAB-ID %u\n", mgw_fsm_priv->rab_id); /* Forward the unmodifed RAB-AssignmentRequest to HNB, so that the HNB is informed about the RAB release as * well */ LOGPFSML(fi, LOGL_DEBUG, "forwarding unmodified RAB-AssignmentRequest to HNB\n"); rc = map_rua_dispatch(map, MAP_RUA_EV_TX_RANAP_MSG, ranap_msg); /* Release the FSM normally */ osmo_fsm_inst_state_chg(fi, MGW_ST_RELEASE, 0, 0); return rc; } int handle_rab_ass_req(struct hnbgw_context_map *map, struct msgb *ranap_msg, ranap_message *message) { [...] /* The RTP stream negotiation usually begins with a RAB-AssignmentRequest and ends with an IU-Release, however * it may also be thet the MSC decides to release the RAB with a dedicated RAB-AssignmentRequest that contains * a ReleaseList. In this case an FSM will already be present. */ if (map->mgw_fi) { /* A RAB Release might be in progress, handle it */ rc = handle_rab_release(map, ranap_msg, message); if (rc >= 0) return rc; LOGPFSML(map->mgw_fi, LOGL_ERROR, "mgw_fsm_alloc_and_handle_rab_ass_req() unable to handle RAB-AssignmentRequest!\n"); osmo_fsm_inst_state_chg(map->mgw_fi, MGW_ST_FAILURE, 0, 0); OSMO_ASSERT(map->mgw_fi == NULL); }
This code looks like it has these problems:
- The rc >= 0 is used to indicate that release IEs were present. But in handle_rab_release(), dispatching the RUA message may also return rc < 0, after a release IE was actually detected and should have returned rc == 0 to indicate so.
- If, upon receiving a RAB Assignment message, an mgw_fwm != NULL, we always fail the CS RAB:
- if rc >= 0, handle_rab_release() has changed the state to MGW_ST_RELEASE
- if rc < 0, we log an error and go to MGW_ST_FAILURE.
- Upon rc < 0, after error logging and state change, we do not return, but continue to evaluate the RAB Assignment, even though mgw_fi == NULL now.
IIUC HNBGW_Tests.TC_rab_release tests for this behavior, so it is somehow working, but the code seems very odd to me.
Could you please check this and see what you think?
Actions