Project

General

Profile

Actions

Bug #5916

closed

MGW FSM code handle_rab_release() looks odd

Added by neels about 1 year ago. Updated about 1 year ago.

Status:
Resolved
Priority:
Normal
Assignee:
Target version:
-
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

Also available in: Atom PDF

Add picture from clipboard (Maximum size: 48.8 MB)