<p><a href="https://gerrit.osmocom.org/c/osmo-bsc/+/22103">View Change</a></p><p>4 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-bsc/+/22103/1/src/osmo-bsc/assignment_fsm.c">File src/osmo-bsc/assignment_fsm.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-bsc/+/22103/1/src/osmo-bsc/assignment_fsm.c@433">Patch Set #1, Line 433:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;">case GSM_LCHAN_SDCCH:<br>                rate_ctr_inc(&bts->bts_ctrs->ctr[BTS_CTR_ASSIGNMENT_ATTEMPTED_SDCCH]);<br>              break;<br>        case GSM_LCHAN_TCH_H:<br> case GSM_LCHAN_TCH_F:<br>         rate_ctr_inc(&bts->bts_ctrs->ctr[BTS_CTR_ASSIGNMENT_ATTEMPTED_TCH]);<br>                break;<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">I think here conn->lchan still points to the old channel type, so you are incrementing the counters of what the _old_ channel was before the new channel was assigned.  I don't think it's what you want?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-bsc/+/22103/1/src/osmo-bsc/assignment_fsm.c@477">Patch Set #1, Line 477:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;">switch (conn->lchan->type) {<br>                             case GSM_LCHAN_SDCCH:<br>                                 rate_ctr_inc(&bts->bts_ctrs->ctr[BTS_CTR_ASSIGNMENT_COMPLETED_SDCCH]);<br>                                      break;<br>                                case GSM_LCHAN_TCH_H:<br>                         case GSM_LCHAN_TCH_F:<br>                                 rate_ctr_inc(&bts->bts_ctrs->ctr[BTS_CTR_ASSIGNMENT_COMPLETED_TCH]);<br>                                        break;<br>                                default:<br>                                      break;<br>                                }<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">I think I would incremenet both the attempted + completed counters here.  This is about the case where the existing channel type is sufficient to fulfill the MSCs assignment request, and hence no actual assignment happens on the Um interface.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-bsc/+/22103/1/src/osmo-bsc/assignment_fsm.c@543">Patch Set #1, Line 543:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;">case GSM48_CMODE_SIGN:<br>                        rate_ctr_inc(&bts->bts_ctrs->ctr[BTS_CTR_ASSIGNMENT_NO_CHANNEL_SDCCH]);<br>                     break;<br>                case GSM48_CMODE_SPEECH_V1:<br>           case GSM48_CMODE_SPEECH_EFR:<br>          case GSM48_CMODE_SPEECH_AMR:<br>                  rate_ctr_inc(&bts->bts_ctrs->ctr[BTS_CTR_ASSIGNMENT_NO_CHANNEL_TCH]);<br>                       break;<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">the layer3 (04.08) channel mode does not have a 1:1 mapping to channel types.</p><p style="white-space: pre-wrap; word-wrap: break-word;">For example, it is perfectly legal to assign a TCH in SIGN mode.  Your counting would be wrong in this situation, as you'd account it as SDCCH.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-bsc/+/22103/1/src/osmo-bsc/osmo_bsc_filter.c">File src/osmo-bsc/osmo_bsc_filter.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-bsc/+/22103/1/src/osmo-bsc/osmo_bsc_filter.c@122">Patch Set #1, Line 122:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;">case GSM48_MT_MM_LOC_UPD_REQUEST:<br>         rate_ctr_inc(&conn->lchan->ts->trx->bts->bts_ctrs->ctr[BTS_CTR_LOCATION_UPDATE_REQUEST]);<br>               break;<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">at least the comment on top of bsc_scan_msc_msg() hints that this function is only called in the downlink direction. Hence you would never count the LU request?  Do you see the counter incrementing?</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-bsc/+/22103">change 22103</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/+/22103"/><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: I4df275e4770c5ff3643c79ba828e736986f8bb47 </div>
<div style="display:none"> Gerrit-Change-Number: 22103 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: iedemam <michael@kapsulate.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-CC: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Comment-Date: Mon, 11 Jan 2021 18:57:03 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: No </div>
<div style="display:none"> Gerrit-MessageType: comment </div>