<p>laforge <strong>merged</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/osmo-bts/+/14337">View Change</a></p><div style="white-space:pre-wrap">Approvals:
  laforge: Looks good to me, approved
  Jenkins Builder: Verified

</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">[correctly] use the LAPDm T200 values received via OML<br><br>As per GSM TS 12.21, the LAPDm timers (T200) of the LAPDm instances<br>in the BTS are configured via OML from the BSC.  While OsmoBSC<br>is sending them and OsmoBTS is parsing them, OsmoBTS stopped to<br>make use of them from commit 3ca59512d2f4eb1f87699e8fada67f33674918b4<br>(January 2016) onwards.<br><br>The cause for this has been documented and discovered in May 2017<br>in https://osmocom.org/issues/2294 and it is quite obvious:  LAPDm<br>timers are supposed to start when a given frame is actually transmitted<br>on the radio interface.  PH-RTS.ind and PH-DATA.req are suppsed to be<br>used over a synchronous interface in some deeply embedded processor.<br><br>With OsmoBTS however, we have an asynchronous L1/L2 interface between<br>a DSP (osmo-bts-{sysmo,lc15,oc2g,octphy}) or OsmoTRX (osmo-bts-trx)<br>and we receive PH-RTS.ind quite some time ahead.  So if we start T200<br>at that point, then it will start running way before it has been sent<br>or before the MS has had a chance to receive the message.<br><br>The "correct" way to handle this is to actually measure the difference<br>between frame numbers in PH-RTS.ind (uplink, advanced) and PH-DATA.ind<br>(downlink) or PH-TIME.ind, and then add that amount to the actual<br>timeout value.  This ensures that the timers will time-out the<br>user-specified amount of time after the actual transmit.<br><br>Change-Id: If8babda9e3e5e219908911ddd9c0756b5ea0bca4<br>Closes: OS#2294<br>Closes: OS#3906<br>---<br>M src/common/oml.c<br>1 file changed, 13 insertions(+), 16 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/common/oml.c b/src/common/oml.c</span><br><span>index d264a6e..ad58cb9 100644</span><br><span>--- a/src/common/oml.c</span><br><span>+++ b/src/common/oml.c</span><br><span>@@ -488,8 +488,15 @@</span><br><span>     [T200_SACCH_TCH_SAPI3]  = 2000,</span><br><span> };</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-static void dl_set_t200(struct lapdm_datalink *dl, unsigned int t200_msec)</span><br><span style="color: hsl(120, 100%, 40%);">+static void dl_set_t200(struct gsm_bts *bts, struct lapdm_datalink *dl, unsigned int t200_msec)</span><br><span> {</span><br><span style="color: hsl(120, 100%, 40%);">+     int32_t fn_advance = bts_get_avg_fn_advance(bts);</span><br><span style="color: hsl(120, 100%, 40%);">+     int32_t fn_advance_us = fn_advance * 4615; /* 4.615 ms per frame */</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* we have to compensate for the "RTS advance" due to the asynchronous interface between</span><br><span style="color: hsl(120, 100%, 40%);">+     * the BTS (LAPDm) and the PHY/L1 (OsmoTRX or DSP in case of osmo-bts-{sysmo,lc15,oc2g,octphy} */</span><br><span style="color: hsl(120, 100%, 40%);">+     t200_msec += fn_advance_us / 1000;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span>         dl->dl.t200_sec = t200_msec / 1000;</span><br><span>       dl->dl.t200_usec = (t200_msec % 1000) * 1000;</span><br><span> }</span><br><span>@@ -528,10 +535,10 @@</span><br><span>        LOGPLCHAN(lchan, DLLAPD, LOGL_DEBUG, "Setting T200 D0=%u, D3=%u, S0=%u, S3=%u (all in ms)\n",</span><br><span>                t200_dcch, t200_dcch_sapi3, t200_acch, t200_acch_sapi3);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-  dl_set_t200(&lc->lapdm_dcch.datalink[DL_SAPI0], t200_dcch);</span><br><span style="color: hsl(0, 100%, 40%);">-      dl_set_t200(&lc->lapdm_dcch.datalink[DL_SAPI3], t200_dcch_sapi3);</span><br><span style="color: hsl(0, 100%, 40%);">-        dl_set_t200(&lc->lapdm_acch.datalink[DL_SAPI0], t200_acch);</span><br><span style="color: hsl(0, 100%, 40%);">-      dl_set_t200(&lc->lapdm_acch.datalink[DL_SAPI3], t200_acch_sapi3);</span><br><span style="color: hsl(120, 100%, 40%);">+      dl_set_t200(bts, &lc->lapdm_dcch.datalink[DL_SAPI0], t200_dcch);</span><br><span style="color: hsl(120, 100%, 40%);">+       dl_set_t200(bts, &lc->lapdm_dcch.datalink[DL_SAPI3], t200_dcch_sapi3);</span><br><span style="color: hsl(120, 100%, 40%);">+ dl_set_t200(bts, &lc->lapdm_acch.datalink[DL_SAPI0], t200_acch);</span><br><span style="color: hsl(120, 100%, 40%);">+       dl_set_t200(bts, &lc->lapdm_acch.datalink[DL_SAPI3], t200_acch_sapi3);</span><br><span> </span><br><span>    return 0;</span><br><span> }</span><br><span>@@ -674,20 +681,10 @@</span><br><span>               payload = TLVP_VAL(&tp, NM_ATT_T200);</span><br><span>            for (i = 0; i < ARRAY_SIZE(bts->t200_ms); i++) {</span><br><span>                       uint32_t t200_ms = payload[i] * abis_nm_t200_ms[i];</span><br><span style="color: hsl(0, 100%, 40%);">-#if 0</span><br><span>                     bts->t200_ms[i] = t200_ms;</span><br><span>                        DEBUGPFOH(DOML, foh, "T200[%u]: OML=%u, mult=%u => %u ms\n",</span><br><span style="color: hsl(0, 100%, 40%);">-                                 i, payload[i], abis_nm_t200_mult[i],</span><br><span style="color: hsl(120, 100%, 40%);">+                                  i, payload[i], abis_nm_t200_ms[i],</span><br><span>                                   bts->t200_ms[i]);</span><br><span style="color: hsl(0, 100%, 40%);">-#else</span><br><span style="color: hsl(0, 100%, 40%);">-                     /* we'd rather use the 1s/2s (long) defaults by</span><br><span style="color: hsl(0, 100%, 40%);">-                      * libosmocore, as we appear to have some bug(s)</span><br><span style="color: hsl(0, 100%, 40%);">-                         * related to handling T200 expiration in</span><br><span style="color: hsl(0, 100%, 40%);">-                        * libosmogsm lapd(m) code? */</span><br><span style="color: hsl(0, 100%, 40%);">-                  LOGPFOH(DOML, LOGL_NOTICE, foh, "Ignoring T200[%u] (%u ms) "</span><br><span style="color: hsl(0, 100%, 40%);">-                          "as sent by BSC due to suspected LAPDm bug!\n",</span><br><span style="color: hsl(0, 100%, 40%);">-                               i, t200_ms);</span><br><span style="color: hsl(0, 100%, 40%);">-#endif</span><br><span>           }</span><br><span>    }</span><br><span> </span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-bts/+/14337">change 14337</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-bts/+/14337"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: osmo-bts </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: If8babda9e3e5e219908911ddd9c0756b5ea0bca4 </div>
<div style="display:none"> Gerrit-Change-Number: 14337 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: laforge <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>