Bug #5712
Updated by keith over 1 year ago
While working on implementing LCLS Control from a SIP PBX attached to osmo sipcon and osmo-msc, I stumbled here: With an Active (mobile to mobile) Call, and after having sent SIP re-Invites to take the PBX out of the Loop, the resulting LCLS Connect Control Messages are sent from MSC to connect, osmo-bsc has both call legs in ST_LOCALLY_SWITCHED. All Good. :-) Now we will issue SIP re-Invites to put the PBX back into the loop, osmo-bsc will get LCLS-CONN-CTRL LCLC-CONN_CTRL with control set to GSM0808_LCLS_CSC_RELEASE_LCLS (for both legs), and here the FSM works fine, We get the RELEASE for LEG A and go to LOCALLY_SWITCHED_WAIT_OTHER_BREAK then osmo-msc sends the other release and then both legs are in ST_NO_LONGER_LS. Once again, All good. Now, let's issue again a SIP re-Invite to take the PBX back out of the loop (we finished playback of "your credit is low" or whatever): So just to recap: bssmap_handle_lcls_connect_ctrl() is ALWAYS going to call: lcls_update_config() followed by lcls_apply_config(); OK, so now Leg A gets a GSM0808_LCLS_CSC_CONNECT, we update the config and we get to lcls_no_longer_ls_fn() where: We call lcls_enable_possible() which returns false, because: <pre><code class="c"> if (other_conn->lcls.control != GSM0808_LCLS_CSC_CONNECT) { LOGPFSM(conn->lcls.fi, "Not enabling LS due to insufficient other control\n"); return false; } </code></pre> we break out of the FSM function here without having done anything, no state change Next we come to apply the config. As we are still in ST_NO_LONGER_LS, we fall off the end of the switch and hit the ASSERT :-( Now, If I comment that ASSERT it actually works, because when the LCLS_CONN_CTRL for the B-leg comes in right afterwards, we have already changed the config in the A leg (which is now other) and so lcls_enable_possible() returns true, we change to ST_LOCALLY_SWITCHED and fire LCLS_EV_OTHER_ENABLED at the A leg, which, you remember is still in ST_NO_LONGER_LS and now once again we do lcls_enable_possible(), which passes and both legs are now ST_LOCALLY_SWITCHED Yay! I see various solutions? here: # I'm doing something wrong on the MSC side and sending LCLS_CONN_CTRL that osmo-bsc does not and should not expect [1] # The FSM should handle LCLS_EV_APPLY_CFG_CSC in ST_NO_LONGER_LS [2] # We change the state of the A leg to ST_NO_LCLS when lcls_enable_possible() fails? [3] # something else? [1] so BTW, osmo_bsc_lcls.c is peppered with OSMO_ASSERT() - I found out while hacking on this that a misbehaving MSC can very easily crash it with the "wrong" LCLS CONNECT CONTROL messages. Should it not be a little more robust? [2] Isn't this wrong, if there's no handler for LCLS_EV_APPLY_CFG_CSC in lcls_no_longer_ls_fn() ?? <pre><code class="c"> [ST_NO_LONGER_LS] = { .in_event_mask = S(LCLS_EV_UPDATE_CFG_CSC) | S(LCLS_EV_APPLY_CFG_CSC) | /* <- we don't handle this in the action function */ S(LCLS_EV_CORRELATED) | S(LCLS_EV_OTHER_ENABLED) | S(LCLS_EV_OTHER_DEAD), .out_state_mask = S(ST_NO_LONGER_LS) | S(ST_REQ_LCLS_NOT_SUPP) | S(ST_LOCALLY_SWITCHED), .name = "NO_LONGER_LS", .action = lcls_no_longer_ls_fn, </code></pre> [3] currently the FSM won't allow it.