<p>neels <strong>submitted</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/osmo-bsc/+/18505">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Jenkins Builder: Verified
laforge: Looks good to me, approved
pespin: Looks good to me, but someone else must approve
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">refactor bsc_find_msc()'s round-robin<br><br>Prepare for MSC pooling by NRI. Before introducing actual NRI decoding and MSC<br>matching, fix the bsc_find_msc() implementation.<br>(Indicate the places relevant for NRI by "TODO" comments).<br><br>bsc_find_msc() puts an MSC to the end of the internal list of MSCs when it was<br>used. This has problems:<br><br>- Modifying the list affects VTY output, e.g. 'show running-config' and<br> 'show mscs' change their order in which MSCs are shown, depending on how<br> often a round-robin selection has taken place.<br><br>- Emergency calls and normal calls potentially pick quite different sets of<br> eligible MSCs. When the round-robin choices between these sets affect each<br> other, the choice is not balanced. For example, if only the first MSC is<br> allow_emerg == true, every emergency call would reset the round-robin state<br> to the first MSC in the list, also for normal calls. If there are regular<br> emergency calls, normal calls will then tend to load more onto the first few<br> MSCs after those picked for emergency calls.<br><br>Fix: Never affect the ordering of MSCs in the internal list of MSCs. Instead,<br>keep a "next_nr" MSC index and determine the next round-robin target like that.<br>Keep a separate "next_emerg_nr" MSC index so that emergency call round-robin<br>does no longer cause normal round-robin to skip MSCs.<br><br>Further problems in current bsc_find_msc():<br><br>- The "blind:" label should also do round-robin.<br>- The "paging:" part should not attempt to use disconnected MSCs.<br>- Both should also heed NRI matches (when they are added).<br><br>Fix: instead of code dup, determine Paging Response matching with an earlier<br>Paging Request right at the start. If that yields no usable MSC, continue into<br>the normal NRI and round-robin selection.<br><br>The loop in this patch is inspired by the upcoming implementation of MSC<br>pooling by NRI, as indicated by the two TODO comments. The point is that, in<br>the presence of an NRI from a TMSI identity, we always need to iterate all of<br>the MSCs to find possible NRI matches. The two round-robin sets (Emergency and<br>non-Emergency) are determined in the same loop iteration for cases that have no<br>or match no NRI, or where a matching MSC is currently disconnected.<br><br>Change-Id: Idf71f07ba5a17d5b870dc1a5a2875b6fedb61291<br>---<br>M include/osmocom/bsc/gsm_data.h<br>M src/osmo-bsc/gsm_08_08.c<br>2 files changed, 71 insertions(+), 60 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/include/osmocom/bsc/gsm_data.h b/include/osmocom/bsc/gsm_data.h</span><br><span>index 1e7e88f..52ff5e4 100644</span><br><span>--- a/include/osmocom/bsc/gsm_data.h</span><br><span>+++ b/include/osmocom/bsc/gsm_data.h</span><br><span>@@ -1651,6 +1651,10 @@</span><br><span> </span><br><span> /* msc configuration */</span><br><span> struct llist_head mscs;</span><br><span style="color: hsl(120, 100%, 40%);">+ uint8_t mscs_round_robin_next_nr;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Emergency calls potentially select a different set of MSCs, so to not mess up the normal round-robin</span><br><span style="color: hsl(120, 100%, 40%);">+ * behavior, emergency calls need a separate round-robin counter. */</span><br><span style="color: hsl(120, 100%, 40%);">+ uint8_t mscs_round_robin_next_emerg_nr;</span><br><span> </span><br><span> /* rf ctl related bits */</span><br><span> int mid_call_timeout;</span><br><span>diff --git a/src/osmo-bsc/gsm_08_08.c b/src/osmo-bsc/gsm_08_08.c</span><br><span>index d8e33d6..a252203 100644</span><br><span>--- a/src/osmo-bsc/gsm_08_08.c</span><br><span>+++ b/src/osmo-bsc/gsm_08_08.c</span><br><span>@@ -36,6 +36,9 @@</span><br><span> </span><br><span> #include <osmocom/bsc/osmo_bsc_sigtran.h></span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+#define LOG_COMPL_L3(pdisc, mtype, loglevel, format, args...) \</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DRSL, loglevel, "%s %s: " format, gsm48_pdisc_name(pdisc), gsm48_pdisc_msgtype_name(pdisc, mtype), ##args)</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /* Check if we have a proper connection to the MSC */</span><br><span> static bool msc_connected(struct gsm_subscriber_connection *conn)</span><br><span> {</span><br><span>@@ -159,6 +162,21 @@</span><br><span> return subscr;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+static bool is_msc_usable(struct bsc_msc_data *msc, bool is_emerg)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ if (is_emerg && !msc->allow_emerg)</span><br><span style="color: hsl(120, 100%, 40%);">+ return false;</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!a_reset_conn_ready(msc))</span><br><span style="color: hsl(120, 100%, 40%);">+ return false;</span><br><span style="color: hsl(120, 100%, 40%);">+ return true;</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+/* Decide which MSC to forward this Complete Layer 3 request to.</span><br><span style="color: hsl(120, 100%, 40%);">+ * a) If the subscriber was previously paged from a particular MSC, that MSC shall receive the Paging Response.</span><br><span style="color: hsl(120, 100%, 40%);">+ * b) If the message contains an NRI indicating a particular MSC and the MSC is connected, that MSC shall handle this</span><br><span style="color: hsl(120, 100%, 40%);">+ * conn.</span><br><span style="color: hsl(120, 100%, 40%);">+ * c) All other cases distribute the messages across connected MSCs in a round-robin fashion.</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span> static struct bsc_msc_data *bsc_find_msc(struct gsm_subscriber_connection *conn,</span><br><span> struct msgb *msg)</span><br><span> {</span><br><span>@@ -166,9 +184,13 @@</span><br><span> struct gsm48_hdr *gh;</span><br><span> int8_t pdisc;</span><br><span> uint8_t mtype;</span><br><span style="color: hsl(0, 100%, 40%);">- struct bsc_msc_data *msc, *pag_msc;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct bsc_msc_data *msc;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct bsc_msc_data *msc_target = NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct bsc_msc_data *msc_round_robin_next = NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct bsc_msc_data *msc_round_robin_first = NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+ uint8_t round_robin_next_nr;</span><br><span> struct bsc_subscr *subscr;</span><br><span style="color: hsl(0, 100%, 40%);">- int is_emerg = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ bool is_emerg = false;</span><br><span> </span><br><span> if (msgb_l3len(msg) < sizeof(*gh)) {</span><br><span> LOGP(DMSC, LOGL_ERROR, "There is no GSM48 header here.\n");</span><br><span>@@ -179,72 +201,57 @@</span><br><span> pdisc = gsm48_hdr_pdisc(gh);</span><br><span> mtype = gsm48_hdr_msg_type(gh);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /*</span><br><span style="color: hsl(0, 100%, 40%);">- * We are asked to select a MSC here but they are not equal. We</span><br><span style="color: hsl(0, 100%, 40%);">- * want to respond to a paging request on the MSC where we got the</span><br><span style="color: hsl(0, 100%, 40%);">- * request from. This is where we need to decide where this connection</span><br><span style="color: hsl(0, 100%, 40%);">- * will go.</span><br><span style="color: hsl(0, 100%, 40%);">- */</span><br><span style="color: hsl(0, 100%, 40%);">- if (pdisc == GSM48_PDISC_RR && mtype == GSM48_MT_RR_PAG_RESP)</span><br><span style="color: hsl(0, 100%, 40%);">- goto paging;</span><br><span style="color: hsl(0, 100%, 40%);">- else if (pdisc == GSM48_PDISC_MM && mtype == GSM48_MT_MM_CM_SERV_REQ) {</span><br><span style="color: hsl(0, 100%, 40%);">- is_emerg = is_cm_service_for_emerg(msg);</span><br><span style="color: hsl(0, 100%, 40%);">- goto round_robin;</span><br><span style="color: hsl(0, 100%, 40%);">- } else</span><br><span style="color: hsl(0, 100%, 40%);">- goto round_robin;</span><br><span style="color: hsl(120, 100%, 40%);">+ is_emerg = (pdisc == GSM48_PDISC_MM && mtype == GSM48_MT_MM_CM_SERV_REQ) && is_cm_service_for_emerg(msg);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-round_robin:</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Has the subscriber been paged from a connected MSC? */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (pdisc == GSM48_PDISC_RR && mtype == GSM48_MT_RR_PAG_RESP) {</span><br><span style="color: hsl(120, 100%, 40%);">+ subscr = extract_sub(conn, msg);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (subscr) {</span><br><span style="color: hsl(120, 100%, 40%);">+ msc_target = paging_get_msc(conn_get_bts(conn), subscr);</span><br><span style="color: hsl(120, 100%, 40%);">+ bsc_subscr_put(subscr);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (is_msc_usable(msc_target, is_emerg))</span><br><span style="color: hsl(120, 100%, 40%);">+ return msc_target;</span><br><span style="color: hsl(120, 100%, 40%);">+ msc_target = NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* TODO: extract NRI from MI */</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Iterate MSCs to find one that matches the extracted NRI, and the next round-robin target for the case no NRI</span><br><span style="color: hsl(120, 100%, 40%);">+ * match is found. */</span><br><span style="color: hsl(120, 100%, 40%);">+ round_robin_next_nr = (is_emerg ? net->mscs_round_robin_next_emerg_nr : net->mscs_round_robin_next_nr);</span><br><span> llist_for_each_entry(msc, &net->mscs, entry) {</span><br><span style="color: hsl(0, 100%, 40%);">- if (is_emerg && !msc->allow_emerg)</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!is_msc_usable(msc, is_emerg))</span><br><span> continue;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /* force round robin by moving it to the end */</span><br><span style="color: hsl(0, 100%, 40%);">- llist_move_tail(&msc->entry, &net->mscs);</span><br><span style="color: hsl(0, 100%, 40%);">- return msc;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* TODO: return msc when extracted NRI matches this MSC */</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Figure out the next round-robin MSC. The MSCs may appear unsorted in net->mscs. Make sure to linearly</span><br><span style="color: hsl(120, 100%, 40%);">+ * round robin the MSCs by number: pick the lowest msc->nr >= round_robin_next_nr, and also remember the</span><br><span style="color: hsl(120, 100%, 40%);">+ * lowest available msc->nr to wrap back to that in case no next MSC is left. */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!msc_round_robin_first || msc->nr < msc_round_robin_first->nr)</span><br><span style="color: hsl(120, 100%, 40%);">+ msc_round_robin_first = msc;</span><br><span style="color: hsl(120, 100%, 40%);">+ if (msc->nr >= round_robin_next_nr</span><br><span style="color: hsl(120, 100%, 40%);">+ && (!msc_round_robin_next || msc->nr < msc_round_robin_next->nr))</span><br><span style="color: hsl(120, 100%, 40%);">+ msc_round_robin_next = msc;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- return NULL;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-paging:</span><br><span style="color: hsl(0, 100%, 40%);">- subscr = extract_sub(conn, msg);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- if (!subscr) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DMSC, LOGL_INFO, "Got paging response but no subscriber found, will now (blindly) deliver the paging response to the first configured MSC!\n");</span><br><span style="color: hsl(0, 100%, 40%);">- goto blind;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* No dedicated MSC found. Choose by round-robin.</span><br><span style="color: hsl(120, 100%, 40%);">+ * If msc_round_robin_next is NULL, there are either no more MSCs at/after mscs_round_robin_next_nr, or none of</span><br><span style="color: hsl(120, 100%, 40%);">+ * them are usable -- wrap to the start. */</span><br><span style="color: hsl(120, 100%, 40%);">+ msc_target = msc_round_robin_next ? : msc_round_robin_first;</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!msc_target) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_COMPL_L3(pdisc, mtype, LOGL_ERROR, "%sNo suitable MSC for this Complete Layer 3 request found\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ is_emerg ? "FOR EMERGENCY CALL: " : "");</span><br><span style="color: hsl(120, 100%, 40%);">+ return NULL;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- pag_msc = paging_get_msc(conn_get_bts(conn), subscr);</span><br><span style="color: hsl(0, 100%, 40%);">- bsc_subscr_put(subscr);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- llist_for_each_entry(msc, &net->mscs, entry) {</span><br><span style="color: hsl(0, 100%, 40%);">- if (msc != pag_msc)</span><br><span style="color: hsl(0, 100%, 40%);">- continue;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- /*</span><br><span style="color: hsl(0, 100%, 40%);">- * We don't check if the MSC is connected. In case it</span><br><span style="color: hsl(0, 100%, 40%);">- * is not the connection will be dropped.</span><br><span style="color: hsl(0, 100%, 40%);">- */</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- /* force round robin by moving it to the end */</span><br><span style="color: hsl(0, 100%, 40%);">- llist_move_tail(&msc->entry, &net->mscs);</span><br><span style="color: hsl(0, 100%, 40%);">- return msc;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DMSC, LOGL_INFO, "Got paging response but no request found, will now (blindly) deliver the paging response to the first configured MSC!\n");</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-blind:</span><br><span style="color: hsl(0, 100%, 40%);">- /* In the case of an MT CSFB call we will get a paging response from</span><br><span style="color: hsl(0, 100%, 40%);">- * the BTS without a preceding paging request via A-Interface. In those</span><br><span style="color: hsl(0, 100%, 40%);">- * cases the MSC will page the subscriber via SGs interface, so the BSC</span><br><span style="color: hsl(0, 100%, 40%);">- * can not know about the paging in advance. In those cases we can not</span><br><span style="color: hsl(0, 100%, 40%);">- * know the MSC which is in charge. The only meaningful option we have</span><br><span style="color: hsl(0, 100%, 40%);">- * is to deliver the paging response to the first configured MSC</span><br><span style="color: hsl(0, 100%, 40%);">- * blindly. */</span><br><span style="color: hsl(0, 100%, 40%);">- msc = llist_first_entry_or_null(&net->mscs, struct bsc_msc_data, entry);</span><br><span style="color: hsl(0, 100%, 40%);">- if (msc)</span><br><span style="color: hsl(0, 100%, 40%);">- return msc;</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DMSC, LOGL_ERROR, "Unable to find any suitable MSC to deliver paging response!\n");</span><br><span style="color: hsl(0, 100%, 40%);">- return NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* An MSC was picked by round-robin, so update the next round-robin nr to pick */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (is_emerg)</span><br><span style="color: hsl(120, 100%, 40%);">+ net->mscs_round_robin_next_emerg_nr = msc_target->nr + 1;</span><br><span style="color: hsl(120, 100%, 40%);">+ else</span><br><span style="color: hsl(120, 100%, 40%);">+ net->mscs_round_robin_next_nr = msc_target->nr + 1;</span><br><span style="color: hsl(120, 100%, 40%);">+ return msc_target;</span><br><span> }</span><br><span> </span><br><span> static int handle_page_resp(struct gsm_subscriber_connection *conn, struct msgb *msg)</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-bsc/+/18505">change 18505</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/osmo-bsc/+/18505"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-bsc </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: Idf71f07ba5a17d5b870dc1a5a2875b6fedb61291 </div>
<div style="display:none"> Gerrit-Change-Number: 18505 </div>
<div style="display:none"> Gerrit-PatchSet: 6 </div>
<div style="display:none"> Gerrit-Owner: neels <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Reviewer: neels <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-CC: fixeria <vyanitskiy@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>