laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/31989 ) Change subject: mgw_fsm: refactor helper function handle_rab_release() ...................................................................... mgw_fsm: refactor helper function handle_rab_release() the function handle_rab_release() is difficult to read and it is not immediately clear what happens and why. Lets split this up and add some comments to improve this. Related: OS#5916 Change-Id: I3595502b98ea5febbde7f2fab3999e2533766b48 --- M src/osmo-hnbgw/mgw_fsm.c 1 file changed, 47 insertions(+), 19 deletions(-) Approvals: Jenkins Builder: Verified pespin: Looks good to me, but someone else must approve laforge: Looks good to me, approved diff --git a/src/osmo-hnbgw/mgw_fsm.c b/src/osmo-hnbgw/mgw_fsm.c index 2ecd7a5..e07ea3b 100644 --- a/src/osmo-hnbgw/mgw_fsm.c +++ b/src/osmo-hnbgw/mgw_fsm.c @@ -633,30 +633,42 @@ }; /* 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) +static int release_mgw_fsm(struct hnbgw_context_map *map, struct msgb *ranap_msg) { - 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_DIRECT_TRANSFER, ranap_msg); + if (rc < 0) { + LOGPFSML(fi, LOGL_DEBUG, "cannot forward RAB-AssignmentRequest to HNB\n"); + return -EINVAL; + } /* Release the FSM normally */ osmo_fsm_inst_state_chg(fi, MGW_ST_RELEASE, 0, 0); + return 0; +} - return rc; +/* Check if the message contains a RAB-ReleaseItem that matches the RAB-ID that is managed by the given context map */ +static bool is_our_rab_release(struct hnbgw_context_map *map, ranap_message *message) +{ + bool rab_release_req; + struct osmo_fsm_inst *fi = map->mgw_fi; + struct mgw_fsm_priv *mgw_fsm_priv = fi->priv; + + /* 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) { + LOGPFSML(map->mgw_fi, LOGL_ERROR, "RAB-AssignmentRequest does not contain any RAB-RelaseItem with RAB-ID %u\n", mgw_fsm_priv->rab_id); + return false; + } + LOGPFSML(map->mgw_fi, LOGL_NOTICE, "MSC asked to release RAB-ID %u\n", mgw_fsm_priv->rab_id); + + return true; } /*! Allocate MGW FSM and handle RANAP RAB AssignmentRequest. @@ -671,7 +683,6 @@ static bool initialized = false; struct mgw_fsm_priv *mgw_fsm_priv; char fsm_name[255]; - int rc; /* Initialize FSM if not done yet */ if (!initialized) { @@ -683,13 +694,16 @@ * it may also be that 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; + /* Check if the RAB-AssignmentRequest contains a RAB-ReleaseItem that matches the RAB-ID we are + * managing in this HNBGW context map. */ + if (is_our_rab_release(map, message)) + return release_mgw_fsm(map, ranap_msg); - LOGPFSML(map->mgw_fi, LOGL_ERROR, - "mgw_fsm_alloc_and_handle_rab_ass_req() unable to handle RAB-AssignmentRequest!\n"); + /* The RAB-ReleaseItem in the incoming message should match the RAB ID we are managing. A mismatch may + * mean that there is an inconsistency between the HNBGW and the MSC state and the MGW FSM on the HNBGW + * side may serve an abandonned connection, which we will now close. However we must also assume that + * the incoming message may still contain a RAB-Assignment for a new RTP stream, so we still must + * continue with the message evaluation. */ osmo_fsm_inst_state_chg(map->mgw_fi, MGW_ST_FAILURE, 0, 0); OSMO_ASSERT(map->mgw_fi == NULL); } -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/31989 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: I3595502b98ea5febbde7f2fab3999e2533766b48 Gerrit-Change-Number: 31989 Gerrit-PatchSet: 4 Gerrit-Owner: dexter &lt;pmaier(a)sysmocom.de&gt; Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge &lt;laforge(a)osmocom.org&gt; Gerrit-Reviewer: neels &lt;nhofmeyr(a)sysmocom.de&gt; Gerrit-Reviewer: pespin &lt;pespin(a)sysmocom.de&gt; Gerrit-MessageType: merged