<p>pespin has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/c/osmo-pcu/+/23907">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">ul_tbf: Clean up handle_tbf_reject()<br><br>Document the function, make it look similar to usual TBF creation path<br>tbf_alloc_ul()->tbf_alloc_ul_tbf->tbf::setup(), which it mimics with<br>some differences.<br>Get rid of unneeded stuff like creating MS and settings its TLLI (that's<br>already done in only caller of the function). There's no need for<br>calling update_ms() either.<br><br>Change-Id: I61df2e4f0f0df1f8db941741a2d35a2319252c5e<br>---<br>M src/pdch.cpp<br>M src/tbf_ul.cpp<br>M src/tbf_ul.h<br>M tests/tbf/TbfTest.cpp<br>M tests/tbf/TbfTest.err<br>5 files changed, 21 insertions(+), 21 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/07/23907/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/pdch.cpp b/src/pdch.cpp</span><br><span>index 76ca1d0..5389763 100644</span><br><span>--- a/src/pdch.cpp</span><br><span>+++ b/src/pdch.cpp</span><br><span>@@ -685,8 +685,7 @@</span><br><span> </span><br><span>                ul_tbf = tbf_alloc_ul(bts(), ms, trx_no(), tlli);</span><br><span>            if (!ul_tbf) {</span><br><span style="color: hsl(0, 100%, 40%);">-                  handle_tbf_reject(bts(), ms, tlli,</span><br><span style="color: hsl(0, 100%, 40%);">-                              trx_no(), ts_no);</span><br><span style="color: hsl(120, 100%, 40%);">+                     handle_tbf_reject(bts(), ms, trx_no(), ts_no);</span><br><span>                       goto return_unref;</span><br><span>           }</span><br><span> </span><br><span>diff --git a/src/tbf_ul.cpp b/src/tbf_ul.cpp</span><br><span>index 7a4e3a3..29d79d6 100644</span><br><span>--- a/src/tbf_ul.cpp</span><br><span>+++ b/src/tbf_ul.cpp</span><br><span>@@ -162,30 +162,23 @@</span><br><span>   return tbf;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+/* Create a temporary dummy TBF to Tx a ImmAssReject if allocating a new one during</span><br><span style="color: hsl(120, 100%, 40%);">+ * packet resource Request failed. This is similar as tbf_alloc_ul() but without</span><br><span style="color: hsl(120, 100%, 40%);">+ * calling tbf->setup() (in charge of TFI/USF allocation), and reusing resources</span><br><span style="color: hsl(120, 100%, 40%);">+ * from Packet Resource Request we received. */</span><br><span> struct gprs_rlcmac_ul_tbf *handle_tbf_reject(struct gprs_rlcmac_bts *bts,</span><br><span style="color: hsl(0, 100%, 40%);">-                  GprsMs *ms, uint32_t tlli, uint8_t trx_no, uint8_t ts)</span><br><span style="color: hsl(120, 100%, 40%);">+                        GprsMs *ms, uint8_t trx_no, uint8_t ts)</span><br><span> {</span><br><span>         struct gprs_rlcmac_ul_tbf *ul_tbf = NULL;</span><br><span>    struct gprs_rlcmac_trx *trx = &bts->trx[trx_no];</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%);">-                ms = bts_alloc_ms(bts, 0, 0);</span><br><span style="color: hsl(0, 100%, 40%);">-   ms_set_tlli(ms, tlli);</span><br><span style="color: hsl(120, 100%, 40%);">+        OSMO_ASSERT(ms);</span><br><span> </span><br><span>         ul_tbf = talloc(tall_pcu_ctx, struct gprs_rlcmac_ul_tbf);</span><br><span>    if (!ul_tbf)</span><br><span>                 return ul_tbf;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span>       talloc_set_destructor(ul_tbf, ul_tbf_dtor);</span><br><span>  new (ul_tbf) gprs_rlcmac_ul_tbf(bts, ms);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-   llist_add(tbf_bts_list((struct gprs_rlcmac_tbf *)ul_tbf), &bts->ul_tbfs);</span><br><span style="color: hsl(0, 100%, 40%);">-        bts_do_rate_ctr_inc(ul_tbf->bts, CTR_TBF_UL_ALLOCATED);</span><br><span style="color: hsl(0, 100%, 40%);">-      TBF_SET_ASS_ON(ul_tbf, GPRS_RLCMAC_FLAG_PACCH, false);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-  ms_attach_tbf(ms, ul_tbf);</span><br><span style="color: hsl(0, 100%, 40%);">-      ul_tbf->update_ms(tlli, GPRS_RLCMAC_UL_TBF);</span><br><span style="color: hsl(0, 100%, 40%);">- TBF_SET_ASS_STATE_UL(ul_tbf, GPRS_RLCMAC_UL_ASS_SEND_ASS_REJ);</span><br><span>       ul_tbf->control_ts = ts;</span><br><span>  ul_tbf->trx = trx;</span><br><span>        ul_tbf->m_ctrs = rate_ctr_group_alloc(ul_tbf, &tbf_ctrg_desc, next_tbf_ctr_group_id++);</span><br><span>@@ -201,6 +194,12 @@</span><br><span>                return NULL;</span><br><span>         }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ ms_attach_tbf(ms, ul_tbf);</span><br><span style="color: hsl(120, 100%, 40%);">+    llist_add(tbf_bts_list((struct gprs_rlcmac_tbf *)ul_tbf), &bts->ul_tbfs);</span><br><span style="color: hsl(120, 100%, 40%);">+      bts_do_rate_ctr_inc(ul_tbf->bts, CTR_TBF_UL_ALLOCATED);</span><br><span style="color: hsl(120, 100%, 40%);">+    TBF_SET_ASS_ON(ul_tbf, GPRS_RLCMAC_FLAG_PACCH, false);</span><br><span style="color: hsl(120, 100%, 40%);">+        TBF_SET_ASS_STATE_UL(ul_tbf, GPRS_RLCMAC_UL_ASS_SEND_ASS_REJ);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span>     return ul_tbf;</span><br><span> }</span><br><span> </span><br><span>diff --git a/src/tbf_ul.h b/src/tbf_ul.h</span><br><span>index a2ad25e..53c7ad8 100644</span><br><span>--- a/src/tbf_ul.h</span><br><span>+++ b/src/tbf_ul.h</span><br><span>@@ -121,7 +121,7 @@</span><br><span> struct gprs_rlcmac_ul_tbf *tbf_alloc_ul(struct gprs_rlcmac_bts *bts, GprsMs *ms,</span><br><span>                                       int8_t use_trx, uint32_t tlli);</span><br><span> struct gprs_rlcmac_ul_tbf *handle_tbf_reject(struct gprs_rlcmac_bts *bts,</span><br><span style="color: hsl(0, 100%, 40%);">-    GprsMs *ms, uint32_t tlli, uint8_t trx_no, uint8_t ts_no);</span><br><span style="color: hsl(120, 100%, 40%);">+    GprsMs *ms, uint8_t trx_no, uint8_t ts_no);</span><br><span> </span><br><span> #else /* ifdef __cplusplus */</span><br><span> struct gprs_rlcmac_ul_tbf;</span><br><span>diff --git a/tests/tbf/TbfTest.cpp b/tests/tbf/TbfTest.cpp</span><br><span>index c839ee0..cf8e083 100644</span><br><span>--- a/tests/tbf/TbfTest.cpp</span><br><span>+++ b/tests/tbf/TbfTest.cpp</span><br><span>@@ -3160,7 +3160,8 @@</span><br><span>      int ts_no = 7;</span><br><span>       uint8_t trx_no = 0;</span><br><span>  uint32_t tlli = 0xffeeddcc;</span><br><span style="color: hsl(0, 100%, 40%);">-     struct gprs_rlcmac_ul_tbf *ul_tbf = NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+     struct gprs_rlcmac_ul_tbf *ul_tbf;</span><br><span style="color: hsl(120, 100%, 40%);">+    struct GprsMs *ms;</span><br><span> </span><br><span>       fprintf(stderr, "=== start %s ===\n", __func__);</span><br><span> </span><br><span>@@ -3168,8 +3169,9 @@</span><br><span> </span><br><span>   int rc = 0;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- ul_tbf = handle_tbf_reject(bts, NULL, tlli,</span><br><span style="color: hsl(0, 100%, 40%);">-                             trx_no, ts_no);</span><br><span style="color: hsl(120, 100%, 40%);">+       ms = bts_alloc_ms(bts, 0, 0);</span><br><span style="color: hsl(120, 100%, 40%);">+ ms_set_tlli(ms, tlli);</span><br><span style="color: hsl(120, 100%, 40%);">+        ul_tbf = handle_tbf_reject(bts, ms, trx_no, ts_no);</span><br><span> </span><br><span>      OSMO_ASSERT(ul_tbf != 0);</span><br><span> </span><br><span>diff --git a/tests/tbf/TbfTest.err b/tests/tbf/TbfTest.err</span><br><span>index 4b38aec..e700fe2 100644</span><br><span>--- a/tests/tbf/TbfTest.err</span><br><span>+++ b/tests/tbf/TbfTest.err</span><br><span>@@ -7956,8 +7956,8 @@</span><br><span> [UL] algo A <multi> (suggested TRX: 0): failed to allocate a TS, no USF available</span><br><span> TBF(TFI=0 TLLI=0xffeeddd3 DIR=UL STATE=NULL EGPRS) Timeslot Allocation failed: trx = 0, single_slot = 0</span><br><span> MS(TLLI=0xffeeddd3, IMSI=, TA=7, 11/11,) No PDCH resource</span><br><span style="color: hsl(120, 100%, 40%);">+MS(TLLI=0xffeeddd3, IMSI=, TA=7, 11/11,) Attaching UL TBF: TBF(TFI=0 TLLI=0xffeeddd3 DIR=UL STATE=NULL)</span><br><span> TBF(TFI=0 TLLI=0xffeeddd3 DIR=UL STATE=NULL) changes state from NULL to ASSIGN</span><br><span style="color: hsl(0, 100%, 40%);">-MS(TLLI=0xffeeddd3, IMSI=, TA=7, 11/11,) Attaching UL TBF: TBF(TFI=0 TLLI=0xffeeddd3 DIR=UL STATE=ASSIGN)</span><br><span> TBF(TFI=0 TLLI=0xffeeddd3 DIR=UL STATE=ASSIGN) changes UL ASS state from GPRS_RLCMAC_UL_ASS_NONE to GPRS_RLCMAC_UL_ASS_SEND_ASS_REJ</span><br><span> PDCH(bts=0,trx=0,ts=7) Expiring FN=82 but previous FN=2654231 is still reserved!</span><br><span> PDCH(bts=0,trx=0,ts=7) Timeout for registered POLL (FN=2654231): TBF(TFI=6 TLLI=0xffeeddd2 DIR=UL STATE=ASSIGN EGPRS)</span><br><span>@@ -7974,8 +7974,8 @@</span><br><span> === start test_packet_access_rej_prr_no_other_tbfs ===</span><br><span> Creating MS object, TLLI = 0xffffffff</span><br><span> Modifying MS object, UL TLLI: 0xffffffff -> 0xffeeddcc, not yet confirmed</span><br><span style="color: hsl(120, 100%, 40%);">+MS(TLLI=0xffeeddcc, IMSI=, TA=220, 0/0,) Attaching UL TBF: TBF(TFI=0 TLLI=0xffeeddcc DIR=UL STATE=NULL)</span><br><span> TBF(TFI=0 TLLI=0xffeeddcc DIR=UL STATE=NULL) changes state from NULL to ASSIGN</span><br><span style="color: hsl(0, 100%, 40%);">-MS(TLLI=0xffeeddcc, IMSI=, TA=220, 0/0,) Attaching UL TBF: TBF(TFI=0 TLLI=0xffeeddcc DIR=UL STATE=ASSIGN)</span><br><span> TBF(TFI=0 TLLI=0xffeeddcc DIR=UL STATE=ASSIGN) changes UL ASS state from GPRS_RLCMAC_UL_ASS_NONE to GPRS_RLCMAC_UL_ASS_SEND_ASS_REJ</span><br><span> TBF(TFI=0 TLLI=0xffeeddcc DIR=UL STATE=ASSIGN) starting timer T0 [reject (PACCH)] with 0 sec. 2000 microsec, cur_fn=2654167</span><br><span> PDCH(bts=0,trx=0,ts=7) FN=2654218 Scheduling control message at RTS for TBF(TFI=0 TLLI=0xffeeddcc DIR=UL STATE=ASSIGN)</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-pcu/+/23907">change 23907</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/+/23907"/><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: I61df2e4f0f0df1f8db941741a2d35a2319252c5e </div>
<div style="display:none"> Gerrit-Change-Number: 23907 </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>