<p>pespin has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/c/osmo-pcu/+/19023">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">tbf: Drop unneeded method set_tlli_from_ul<br><br>Since commit 322456ed474a733094c9f3e240295b469023ad14 (and previous<br>one), it is expected that a tbf object ALWAYS has a MS object referend<br>to it, even if it's a temporary copy which will later be merged when<br>TLLI/IMSI is retrieved and it is found that several MS objects relate to<br>the same MS.<br><br>The purpose of set_tlli_from_ul was mainly to update TBF's ms() to<br>old_ms before going through usual tbf->update_ms() path. That's not<br>really needed since ms() is already always set and TBFs for old_ms are<br>already freed in update_ms() and children function.<br><br>Change-Id: Ie8795e7a02032336e53febb65c11f9150c36d2a0<br>---<br>M src/tbf.cpp<br>M src/tbf.h<br>M src/tbf_dl.cpp<br>M src/tbf_ul.cpp<br>M tests/tbf/TbfTest.err<br>5 files changed, 2 insertions(+), 52 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/23/19023/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/tbf.cpp b/src/tbf.cpp</span><br><span>index 0cc0129..f77b1e2 100644</span><br><span>--- a/src/tbf.cpp</span><br><span>+++ b/src/tbf.cpp</span><br><span>@@ -392,9 +392,6 @@</span><br><span> </span><br><span> void gprs_rlcmac_tbf::update_ms(uint32_t tlli, enum gprs_rlcmac_tbf_direction dir)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- if (!ms())</span><br><span style="color: hsl(0, 100%, 40%);">- return;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> if (!tlli)</span><br><span> return;</span><br><span> </span><br><span>@@ -1439,49 +1436,6 @@</span><br><span> return 0;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-int gprs_rlcmac_tbf::set_tlli_from_ul(uint32_t new_tlli)</span><br><span style="color: hsl(0, 100%, 40%);">-{</span><br><span style="color: hsl(0, 100%, 40%);">- struct gprs_rlcmac_tbf *dl_tbf = NULL;</span><br><span style="color: hsl(0, 100%, 40%);">- struct gprs_rlcmac_tbf *ul_tbf = NULL;</span><br><span style="color: hsl(0, 100%, 40%);">- GprsMs *old_ms;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- OSMO_ASSERT(direction == GPRS_RLCMAC_UL_TBF);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- old_ms = bts->ms_by_tlli(new_tlli);</span><br><span style="color: hsl(0, 100%, 40%);">- /* Keep the old MS object for the update_ms() */</span><br><span style="color: hsl(0, 100%, 40%);">- GprsMs::Guard guard(old_ms);</span><br><span style="color: hsl(0, 100%, 40%);">- if (old_ms) {</span><br><span style="color: hsl(0, 100%, 40%);">- /* Get them before calling set_ms() */</span><br><span style="color: hsl(0, 100%, 40%);">- dl_tbf = old_ms->dl_tbf();</span><br><span style="color: hsl(0, 100%, 40%);">- ul_tbf = old_ms->ul_tbf();</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- if (!ms())</span><br><span style="color: hsl(0, 100%, 40%);">- set_ms(old_ms);</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%);">- if (dl_tbf && dl_tbf->ms() != ms()) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGPTBFUL(dl_tbf, LOGL_NOTICE,</span><br><span style="color: hsl(0, 100%, 40%);">- "Got RACH from TLLI=0x%08x while TBF still exists: killing pending DL TBF\n", new_tlli);</span><br><span style="color: hsl(0, 100%, 40%);">- tbf_free(dl_tbf);</span><br><span style="color: hsl(0, 100%, 40%);">- dl_tbf = NULL;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">- if (ul_tbf && ul_tbf->ms() != ms()) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGPTBFUL(ul_tbf, LOGL_NOTICE,</span><br><span style="color: hsl(0, 100%, 40%);">- "Got RACH from TLLI=0x%08x while TBF still exists: killing pending UL TBF\n", new_tlli);</span><br><span style="color: hsl(0, 100%, 40%);">- tbf_free(ul_tbf);</span><br><span style="color: hsl(0, 100%, 40%);">- ul_tbf = NULL;</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%);">- /* The TLLI has been taken from an UL message */</span><br><span style="color: hsl(0, 100%, 40%);">- update_ms(new_tlli, GPRS_RLCMAC_UL_TBF);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-#if 0 /* REMOVEME ??? */</span><br><span style="color: hsl(0, 100%, 40%);">- if (ms()->need_dl_tbf())</span><br><span style="color: hsl(0, 100%, 40%);">- establish_dl_tbf_on_pacch();</span><br><span style="color: hsl(0, 100%, 40%);">-#endif</span><br><span style="color: hsl(0, 100%, 40%);">- return 1;</span><br><span style="color: hsl(0, 100%, 40%);">-}</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> const char *tbf_name(gprs_rlcmac_tbf *tbf)</span><br><span> {</span><br><span> return tbf ? tbf->name() : "(no TBF)";</span><br><span>diff --git a/src/tbf.h b/src/tbf.h</span><br><span>index be3336a..528bee3 100644</span><br><span>--- a/src/tbf.h</span><br><span>+++ b/src/tbf.h</span><br><span>@@ -319,7 +319,6 @@</span><br><span> protected:</span><br><span> gprs_rlcmac_bts *bts_data() const;</span><br><span> void enable_egprs();</span><br><span style="color: hsl(0, 100%, 40%);">- int set_tlli_from_ul(uint32_t new_tlli);</span><br><span> void merge_and_clear_ms(GprsMs *old_ms);</span><br><span> </span><br><span> gprs_llc_queue *llc_queue();</span><br><span>diff --git a/src/tbf_dl.cpp b/src/tbf_dl.cpp</span><br><span>index 03149ba..02aaa3a 100644</span><br><span>--- a/src/tbf_dl.cpp</span><br><span>+++ b/src/tbf_dl.cpp</span><br><span>@@ -151,8 +151,6 @@</span><br><span> ul_tbf = ms->ul_tbf();</span><br><span> ta = ms->ta();</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /* TODO: if (!ms) create MS before tbf_alloc is called? */</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> if (ul_tbf && ul_tbf->m_contention_resolution_done</span><br><span> && !ul_tbf->m_final_ack_sent) {</span><br><span> use_trx = ul_tbf->trx->trx_no;</span><br><span>diff --git a/src/tbf_ul.cpp b/src/tbf_ul.cpp</span><br><span>index 3115080..489fc87 100644</span><br><span>--- a/src/tbf_ul.cpp</span><br><span>+++ b/src/tbf_ul.cpp</span><br><span>@@ -305,7 +305,7 @@</span><br><span> LOGPTBFUL(this, LOGL_INFO,</span><br><span> "Decoded premier TLLI=0x%08x of UL DATA TFI=%d.\n",</span><br><span> new_tlli, rlc->tfi);</span><br><span style="color: hsl(0, 100%, 40%);">- set_tlli_from_ul(new_tlli);</span><br><span style="color: hsl(120, 100%, 40%);">+ update_ms(new_tlli, GPRS_RLCMAC_UL_TBF);</span><br><span> } else if (new_tlli && new_tlli != tlli()) {</span><br><span> LOGPTBFUL(this, LOGL_NOTICE,</span><br><span> "TLLI mismatch on UL DATA TFI=%d. (Ignoring due to contention resolution)\n",</span><br><span>diff --git a/tests/tbf/TbfTest.err b/tests/tbf/TbfTest.err</span><br><span>index 827f5ce..0d097ee 100644</span><br><span>--- a/tests/tbf/TbfTest.err</span><br><span>+++ b/tests/tbf/TbfTest.err</span><br><span>@@ -2079,7 +2079,6 @@</span><br><span> TBF(TFI=0 TLLI=0x00000000 DIR=UL STATE=FLOW) BSN 0 storing in window (0..63)</span><br><span> TBF(TFI=0 TLLI=0x00000000 DIR=UL STATE=FLOW) data_length=20, data=f1 22 33 44 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 </span><br><span> TBF(TFI=0 TLLI=0x00000000 DIR=UL STATE=FLOW) Decoded premier TLLI=0xf1223344 of UL DATA TFI=0.</span><br><span style="color: hsl(0, 100%, 40%);">-TBF(TFI=0 TLLI=0xf1223344 DIR=DL STATE=ASSIGN) Got RACH from TLLI=0xf1223344 while TBF still exists: killing pending DL TBF</span><br><span> TBF(TFI=0 TLLI=0xf1223344 DIR=DL STATE=ASSIGN) changes state from ASSIGN to RELEASING</span><br><span> TBF(TFI=0 TLLI=0xf1223344 DIR=DL STATE=RELEASING) free</span><br><span> TBF(TFI=0 TLLI=0xf1223344 DIR=DL STATE=RELEASING) stopping timer T0 [freeing TBF]</span><br><span>@@ -2089,8 +2088,8 @@</span><br><span> Modifying MS object, TLLI = 0x00000000, IMSI '' -> '0011223344'</span><br><span> Modifying MS object, TLLI = 0x00000000, MS class 0 -> 1</span><br><span> Clearing MS object, TLLI: 0xf1223344, IMSI: '0011223344'</span><br><span style="color: hsl(0, 100%, 40%);">-Modifying MS object, UL TLLI: 0x00000000 -> 0xf1223344, not yet confirmed</span><br><span> Destroying MS object, TLLI = 0x00000000</span><br><span style="color: hsl(120, 100%, 40%);">+Modifying MS object, UL TLLI: 0x00000000 -> 0xf1223344, not yet confirmed</span><br><span> TBF(TFI=0 TLLI=0xf1223344 DIR=UL STATE=FLOW) Assembling frames: (len=20)</span><br><span> TBF(TFI=0 TLLI=0xf1223344 DIR=UL STATE=FLOW) Frame 1 starts at offset 4, length=16, is_complete=1</span><br><span> TBF(TFI=0 TLLI=0xf1223344 DIR=UL STATE=FLOW) complete UL frame len=16</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-pcu/+/19023">change 19023</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-pcu/+/19023"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-pcu </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: Ie8795e7a02032336e53febb65c11f9150c36d2a0 </div>
<div style="display:none"> Gerrit-Change-Number: 19023 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>