<p>pespin <strong>submitted</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/osmo-pcu/+/20945">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Jenkins Builder: Verified
fixeria: Looks good to me, approved
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">tbf_dl: Update (egprs_)ms_class for already known MS<br><br>If SGSN provides us with MS class information upon DL data, let's use it<br>and set it in an already existing MS object if not yet known.<br><br>Also remove all unneeded code passing ms_class to append_data() which<br>would simply try to (again) set the ms_class.<br><br>Change-Id: I4979c9344bffd3ba7657bbab94981d233eab801f<br>---<br>M src/tbf_dl.cpp<br>M src/tbf_dl.h<br>M tests/tbf/TbfTest.cpp<br>3 files changed, 20 insertions(+), 33 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/tbf_dl.cpp b/src/tbf_dl.cpp</span><br><span>index 7d1f85c..c25f2af 100644</span><br><span>--- a/src/tbf_dl.cpp</span><br><span>+++ b/src/tbf_dl.cpp</span><br><span>@@ -94,13 +94,6 @@</span><br><span> tbf_dl_egprs_ctr_description,</span><br><span> };</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-static inline void tbf_update_ms_class(struct gprs_rlcmac_tbf *tbf,</span><br><span style="color: hsl(0, 100%, 40%);">- const uint8_t ms_class)</span><br><span style="color: hsl(0, 100%, 40%);">-{</span><br><span style="color: hsl(0, 100%, 40%);">- if (!tbf->ms_class() && ms_class)</span><br><span style="color: hsl(0, 100%, 40%);">- tbf->ms()->set_ms_class(ms_class);</span><br><span style="color: hsl(0, 100%, 40%);">-}</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> static void llc_timer_cb(void *_tbf)</span><br><span> {</span><br><span> struct gprs_rlcmac_dl_tbf *tbf = (struct gprs_rlcmac_dl_tbf *)_tbf;</span><br><span>@@ -225,9 +218,8 @@</span><br><span> }</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-int gprs_rlcmac_dl_tbf::append_data(const uint8_t ms_class,</span><br><span style="color: hsl(0, 100%, 40%);">- const uint16_t pdu_delay_csec,</span><br><span style="color: hsl(0, 100%, 40%);">- const uint8_t *data, const uint16_t len)</span><br><span style="color: hsl(120, 100%, 40%);">+int gprs_rlcmac_dl_tbf::append_data(uint16_t pdu_delay_csec,</span><br><span style="color: hsl(120, 100%, 40%);">+ const uint8_t *data, uint16_t len)</span><br><span> {</span><br><span> struct timespec expire_time;</span><br><span> </span><br><span>@@ -240,7 +232,6 @@</span><br><span> gprs_llc_queue::calc_pdu_lifetime(bts, pdu_delay_csec, &expire_time);</span><br><span> memcpy(msgb_put(llc_msg, len), data, len);</span><br><span> llc_queue()->enqueue(llc_msg, &expire_time);</span><br><span style="color: hsl(0, 100%, 40%);">- tbf_update_ms_class(this, ms_class);</span><br><span> start_llc_timer();</span><br><span> </span><br><span> if (state_is(GPRS_RLCMAC_WAIT_RELEASE)) {</span><br><span>@@ -308,15 +299,6 @@</span><br><span> </span><br><span> /* check for existing TBF */</span><br><span> ms = bts->bts->ms_store().get_ms(tlli, tlli_old, imsi);</span><br><span style="color: hsl(0, 100%, 40%);">- if (ms) {</span><br><span style="color: hsl(0, 100%, 40%);">- dl_tbf = ms->dl_tbf();</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- /* If we known the GPRS/EGPRS MS class, use it */</span><br><span style="color: hsl(0, 100%, 40%);">- if (ms->ms_class() || ms->egprs_ms_class()) {</span><br><span style="color: hsl(0, 100%, 40%);">- ms_class = ms->ms_class();</span><br><span style="color: hsl(0, 100%, 40%);">- egprs_ms_class = ms->egprs_ms_class();</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span> </span><br><span> if (ms && strlen(ms->imsi()) == 0) {</span><br><span> ms_old = bts->bts->ms_store().get_ms(0, 0, imsi);</span><br><span>@@ -329,7 +311,7 @@</span><br><span> </span><br><span> GprsMs::Guard guard_old(ms_old);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (!dl_tbf && ms_old->dl_tbf()) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!ms->dl_tbf() && ms_old->dl_tbf()) {</span><br><span> LOGP(DTBF, LOGL_NOTICE,</span><br><span> "IMSI %s, old TBF %s: moving DL TBF to new MS object\n",</span><br><span> imsi, ms_old->dl_tbf()->name());</span><br><span>@@ -345,15 +327,21 @@</span><br><span> ms = bts->bts->ms_alloc(ms_class, egprs_ms_class);</span><br><span> ms->set_imsi(imsi);</span><br><span> ms->confirm_tlli(tlli);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!ms->ms_class() && ms_class) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ms->set_ms_class(ms_class);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!ms->egprs_ms_class() && egprs_ms_class) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ms->set_egprs_ms_class(egprs_ms_class);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ dl_tbf = ms->dl_tbf();</span><br><span> if (!dl_tbf) {</span><br><span> rc = tbf_new_dl_assignment(bts, ms, &dl_tbf);</span><br><span> if (rc < 0)</span><br><span> return rc;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /* TODO: ms_class vs. egprs_ms_class is not handled here */</span><br><span style="color: hsl(0, 100%, 40%);">- rc = dl_tbf->append_data(ms_class, delay_csec, data, len);</span><br><span style="color: hsl(120, 100%, 40%);">+ rc = dl_tbf->append_data(delay_csec, data, len);</span><br><span> </span><br><span> return rc;</span><br><span> }</span><br><span>diff --git a/src/tbf_dl.h b/src/tbf_dl.h</span><br><span>index caa439a..685e855 100644</span><br><span>--- a/src/tbf_dl.h</span><br><span>+++ b/src/tbf_dl.h</span><br><span>@@ -49,9 +49,8 @@</span><br><span> const uint8_t egprs_ms_class, const uint16_t delay_csec,</span><br><span> const uint8_t *data, const uint16_t len);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- int append_data(const uint8_t ms_class,</span><br><span style="color: hsl(0, 100%, 40%);">- const uint16_t pdu_delay_csec,</span><br><span style="color: hsl(0, 100%, 40%);">- const uint8_t *data, const uint16_t len);</span><br><span style="color: hsl(120, 100%, 40%);">+ int append_data(uint16_t pdu_delay_csec,</span><br><span style="color: hsl(120, 100%, 40%);">+ const uint8_t *data, uint16_t len);</span><br><span> </span><br><span> int rcvd_dl_ack(bool final, uint8_t ssn, uint8_t *rbb);</span><br><span> int rcvd_dl_ack(bool final_ack, unsigned first_bsn, struct bitvec *rbb);</span><br><span>diff --git a/tests/tbf/TbfTest.cpp b/tests/tbf/TbfTest.cpp</span><br><span>index 04e711c..096b071 100644</span><br><span>--- a/tests/tbf/TbfTest.cpp</span><br><span>+++ b/tests/tbf/TbfTest.cpp</span><br><span>@@ -277,8 +277,8 @@</span><br><span> llc_data[i] = i%256;</span><br><span> </span><br><span> /* Schedule two LLC frames */</span><br><span style="color: hsl(0, 100%, 40%);">- dl_tbf->append_data(ms_class, 1000, llc_data, sizeof(llc_data));</span><br><span style="color: hsl(0, 100%, 40%);">- dl_tbf->append_data(ms_class, 1000, llc_data, sizeof(llc_data));</span><br><span style="color: hsl(120, 100%, 40%);">+ dl_tbf->append_data(1000, llc_data, sizeof(llc_data));</span><br><span style="color: hsl(120, 100%, 40%);">+ dl_tbf->append_data(1000, llc_data, sizeof(llc_data));</span><br><span> </span><br><span> </span><br><span> /* Send only a few RLC/MAC blocks */</span><br><span>@@ -362,8 +362,8 @@</span><br><span> OSMO_ASSERT(dl_tbf->state_is(GPRS_RLCMAC_FLOW));</span><br><span> </span><br><span> /* Schedule two LLC frames */</span><br><span style="color: hsl(0, 100%, 40%);">- dl_tbf->append_data(ms_class, 1000, llc_data, sizeof(llc_data));</span><br><span style="color: hsl(0, 100%, 40%);">- dl_tbf->append_data(ms_class, 1000, llc_data, sizeof(llc_data));</span><br><span style="color: hsl(120, 100%, 40%);">+ dl_tbf->append_data(1000, llc_data, sizeof(llc_data));</span><br><span style="color: hsl(120, 100%, 40%);">+ dl_tbf->append_data(1000, llc_data, sizeof(llc_data));</span><br><span> </span><br><span> OSMO_ASSERT(dl_tbf->state_is(GPRS_RLCMAC_FLOW));</span><br><span> </span><br><span>@@ -2668,7 +2668,7 @@</span><br><span> OSMO_ASSERT(dl_tbf->state_is(GPRS_RLCMAC_FLOW));</span><br><span> </span><br><span> /* Schedule a small LLC frame */</span><br><span style="color: hsl(0, 100%, 40%);">- dl_tbf->append_data(ms_class, 1000, test_data, 10);</span><br><span style="color: hsl(120, 100%, 40%);">+ dl_tbf->append_data(1000, test_data, 10);</span><br><span> </span><br><span> OSMO_ASSERT(dl_tbf->state_is(GPRS_RLCMAC_FLOW));</span><br><span> </span><br><span>@@ -2678,7 +2678,7 @@</span><br><span> request_dl_rlc_block(dl_tbf, &fn);</span><br><span> </span><br><span> /* Schedule a large LLC frame */</span><br><span style="color: hsl(0, 100%, 40%);">- dl_tbf->append_data(ms_class, 1000, test_data, sizeof(test_data));</span><br><span style="color: hsl(120, 100%, 40%);">+ dl_tbf->append_data(1000, test_data, sizeof(test_data));</span><br><span> </span><br><span> OSMO_ASSERT(dl_tbf->state_is(GPRS_RLCMAC_FLOW));</span><br><span> </span><br><span>@@ -2726,7 +2726,7 @@</span><br><span> * 2 RLC data blocks. Which are enough to test Header Type 1</span><br><span> * cases</span><br><span> */</span><br><span style="color: hsl(0, 100%, 40%);">- dl_tbf->append_data(ms_class, 1000, test_data, 100);</span><br><span style="color: hsl(120, 100%, 40%);">+ dl_tbf->append_data(1000, test_data, 100);</span><br><span> </span><br><span> OSMO_ASSERT(dl_tbf->state_is(GPRS_RLCMAC_FLOW));</span><br><span> </span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-pcu/+/20945">change 20945</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/+/20945"/><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: I4979c9344bffd3ba7657bbab94981d233eab801f </div>
<div style="display:none"> Gerrit-Change-Number: 20945 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: fixeria <vyanitskiy@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>