<p>pespin has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/c/osmo-pcu/+/23532">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">tbf: Get rid of attribute poll_ts<br><br>That field is not needed anymore, and it works only under the assumption<br>that only 1 poll request can be active at a time per TBF, which is not<br>true.<br><br>Change-Id: I9b8bed7741d385bab4cd8c64b841a78a02a05fe1<br>---<br>M src/pdch_ul_controller.c<br>M src/tbf.cpp<br>M src/tbf.h<br>M src/tbf_dl.cpp<br>M tests/tbf/TbfTest.cpp<br>5 files changed, 22 insertions(+), 29 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/32/23532/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/pdch_ul_controller.c b/src/pdch_ul_controller.c</span><br><span>index 5911080..f8089ff 100644</span><br><span>--- a/src/pdch_ul_controller.c</span><br><span>+++ b/src/pdch_ul_controller.c</span><br><span>@@ -318,7 +318,7 @@</span><br><span>                        LOGPDCH(ulc->pdch, DRLCMAC, LOGL_NOTICE,</span><br><span>                          "Timeout for registered POLL (FN=%u): %s\n",</span><br><span>                               item->fn, tbf_name(item->tbf_poll.poll_tbf));</span><br><span style="color: hsl(0, 100%, 40%);">-                     tbf_poll_timeout(item->tbf_poll.poll_tbf, item->fn, item->tbf_poll.reason);</span><br><span style="color: hsl(120, 100%, 40%);">+                  tbf_poll_timeout(item->tbf_poll.poll_tbf, ulc->pdch, item->fn, item->tbf_poll.reason);</span><br><span>                   break;</span><br><span>               case PDCH_ULC_NODE_SBA:</span><br><span>                      sba = item->sba.sba;</span><br><span>diff --git a/src/tbf.cpp b/src/tbf.cpp</span><br><span>index 92dede5..69c35f8 100644</span><br><span>--- a/src/tbf.cpp</span><br><span>+++ b/src/tbf.cpp</span><br><span>@@ -125,7 +125,6 @@</span><br><span>       first_ts(0),</span><br><span>         first_common_ts(0),</span><br><span>  control_ts(0xff),</span><br><span style="color: hsl(0, 100%, 40%);">-       poll_ts(0),</span><br><span>  fT(0),</span><br><span>       num_fT_exp(0),</span><br><span>       was_releasing(0),</span><br><span>@@ -581,45 +580,44 @@</span><br><span>                      chan, new_poll_fn, ts);</span><br><span>            return;</span><br><span>      }</span><br><span style="color: hsl(0, 100%, 40%);">-       poll_ts = ts;</span><br><span> </span><br><span>    switch (reason) {</span><br><span>    case PDCH_ULC_POLL_UL_ASS:</span><br><span>           ul_ass_state = GPRS_RLCMAC_UL_ASS_WAIT_ACK;</span><br><span> </span><br><span>              LOGPTBFDL(this, LOGL_INFO, "Scheduled UL Assignment polling on %s (FN=%d, TS=%d)\n",</span><br><span style="color: hsl(0, 100%, 40%);">-                    chan, new_poll_fn, poll_ts);</span><br><span style="color: hsl(120, 100%, 40%);">+                          chan, new_poll_fn, ts);</span><br><span>            break;</span><br><span>       case PDCH_ULC_POLL_DL_ASS:</span><br><span>           dl_ass_state = GPRS_RLCMAC_DL_ASS_WAIT_ACK;</span><br><span> </span><br><span>              LOGPTBFDL(this, LOGL_INFO, "Scheduled DL Assignment polling on %s (FN=%d, TS=%d)\n",</span><br><span style="color: hsl(0, 100%, 40%);">-                    chan, new_poll_fn, poll_ts);</span><br><span style="color: hsl(120, 100%, 40%);">+                          chan, new_poll_fn, ts);</span><br><span>            break;</span><br><span>       case PDCH_ULC_POLL_UL_ACK:</span><br><span>           ul_ack_state = GPRS_RLCMAC_UL_ACK_WAIT_ACK;</span><br><span> </span><br><span>              LOGPTBFUL(this, LOGL_DEBUG, "Scheduled UL Acknowledgement polling on %s (FN=%d, TS=%d)\n",</span><br><span style="color: hsl(0, 100%, 40%);">-                      chan, new_poll_fn, poll_ts);</span><br><span style="color: hsl(120, 100%, 40%);">+                          chan, new_poll_fn, ts);</span><br><span>            break;</span><br><span>       case PDCH_ULC_POLL_DL_ACK:</span><br><span>           LOGPTBFDL(this, LOGL_DEBUG, "Scheduled DL Acknowledgement polling on %s (FN=%d, TS=%d)\n",</span><br><span style="color: hsl(0, 100%, 40%);">-                      chan, new_poll_fn, poll_ts);</span><br><span style="color: hsl(120, 100%, 40%);">+                          chan, new_poll_fn, ts);</span><br><span>            break;</span><br><span>       case PDCH_ULC_POLL_CELL_CHG_CONTINUE:</span><br><span>                LOGPTBFDL(this, LOGL_DEBUG, "Scheduled 'Packet Cell Change Continue' polling on %s (FN=%d, TS=%d)\n",</span><br><span style="color: hsl(0, 100%, 40%);">-                   chan, new_poll_fn, poll_ts);</span><br><span style="color: hsl(120, 100%, 40%);">+                          chan, new_poll_fn, ts);</span><br><span>            break;</span><br><span>       }</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-void gprs_rlcmac_tbf::poll_timeout(uint32_t poll_fn, enum pdch_ulc_tbf_poll_reason reason)</span><br><span style="color: hsl(120, 100%, 40%);">+void gprs_rlcmac_tbf::poll_timeout(struct gprs_rlcmac_pdch *pdch, uint32_t poll_fn, enum pdch_ulc_tbf_poll_reason reason)</span><br><span> {</span><br><span>         uint16_t pgroup;</span><br><span>     gprs_rlcmac_ul_tbf *ul_tbf = as_ul_tbf(this);</span><br><span> </span><br><span>    LOGPTBF(this, LOGL_NOTICE, "poll timeout for FN=%d, TS=%d (curr FN %d)\n",</span><br><span style="color: hsl(0, 100%, 40%);">-            poll_fn, poll_ts, bts_current_frame_number(bts));</span><br><span style="color: hsl(120, 100%, 40%);">+             poll_fn, pdch->ts_no, bts_current_frame_number(bts));</span><br><span> </span><br><span>         if (ul_tbf && ul_tbf->handle_ctrl_ack(reason)) {</span><br><span>          if (!ul_tbf->ctrl_ack_to_toggle()) {</span><br><span>@@ -679,7 +677,7 @@</span><br><span>                /* reschedule DL assignment */</span><br><span>               dl_ass_state = GPRS_RLCMAC_DL_ASS_SEND_ASS;</span><br><span>  } else if (m_ms->nacc && m_ms->nacc->fi->state == NACC_ST_WAIT_CELL_CHG_CONTINUE_ACK &&</span><br><span style="color: hsl(0, 100%, 40%);">-                m_ms->nacc->continue_poll_fn == poll_fn && m_ms->nacc->continue_poll_ts == poll_ts) {</span><br><span style="color: hsl(120, 100%, 40%);">+             m_ms->nacc->continue_poll_fn == poll_fn && m_ms->nacc->continue_poll_ts == pdch->ts_no) {</span><br><span>          /* Timeout waiting for CTRL ACK acking Pkt Cell Change Continue */</span><br><span>           osmo_fsm_inst_dispatch(m_ms->nacc->fi, NACC_EV_TIMEOUT_CELL_CHG_CONTINUE, NULL);</span><br><span>               return;</span><br><span>@@ -1209,7 +1207,7 @@</span><br><span>      return tbf->set_polling(new_poll_fn, ts, t);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-void tbf_poll_timeout(struct gprs_rlcmac_tbf *tbf, uint32_t poll_fn, enum pdch_ulc_tbf_poll_reason reason)</span><br><span style="color: hsl(120, 100%, 40%);">+void tbf_poll_timeout(struct gprs_rlcmac_tbf *tbf, struct gprs_rlcmac_pdch *pdch, uint32_t poll_fn, enum pdch_ulc_tbf_poll_reason reason)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">-      tbf->poll_timeout(poll_fn, reason);</span><br><span style="color: hsl(120, 100%, 40%);">+        tbf->poll_timeout(pdch, poll_fn, reason);</span><br><span> }</span><br><span>diff --git a/src/tbf.h b/src/tbf.h</span><br><span>index b0bec31..ca6de0f 100644</span><br><span>--- a/src/tbf.h</span><br><span>+++ b/src/tbf.h</span><br><span>@@ -193,7 +193,7 @@</span><br><span> int tbf_assign_control_ts(struct gprs_rlcmac_tbf *tbf);</span><br><span> int tbf_check_polling(const struct gprs_rlcmac_tbf *tbf, uint32_t fn, uint8_t ts, uint32_t *poll_fn, unsigned int *rrbp);</span><br><span> void tbf_set_polling(struct gprs_rlcmac_tbf *tbf, uint32_t new_poll_fn, uint8_t ts, enum pdch_ulc_tbf_poll_reason t);</span><br><span style="color: hsl(0, 100%, 40%);">-void tbf_poll_timeout(struct gprs_rlcmac_tbf *tbf, uint32_t poll_fn, enum pdch_ulc_tbf_poll_reason reason);</span><br><span style="color: hsl(120, 100%, 40%);">+void tbf_poll_timeout(struct gprs_rlcmac_tbf *tbf, struct gprs_rlcmac_pdch *pdch, uint32_t poll_fn, enum pdch_ulc_tbf_poll_reason reason);</span><br><span> #ifdef __cplusplus</span><br><span> }</span><br><span> #endif</span><br><span>@@ -251,7 +251,7 @@</span><br><span>  int check_polling(uint32_t fn, uint8_t ts,</span><br><span>           uint32_t *poll_fn, unsigned int *rrbp) const;</span><br><span>        void set_polling(uint32_t poll_fn, uint8_t ts, enum pdch_ulc_tbf_poll_reason reason);</span><br><span style="color: hsl(0, 100%, 40%);">-   void poll_timeout(uint32_t poll_fn, enum pdch_ulc_tbf_poll_reason reason);</span><br><span style="color: hsl(120, 100%, 40%);">+    void poll_timeout(struct gprs_rlcmac_pdch *pdch, uint32_t poll_fn, enum pdch_ulc_tbf_poll_reason reason);</span><br><span> </span><br><span>        /** tlli handling */</span><br><span>         uint32_t tlli() const;</span><br><span>@@ -291,9 +291,6 @@</span><br><span>         struct gprs_rlcmac_pdch *pdch[8]; /* list of PDCHs allocated to TBF */</span><br><span> </span><br><span>   gprs_llc m_llc;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- uint8_t poll_ts; /* TS to poll */</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span>    gprs_rlc m_rlc;</span><br><span> </span><br><span>  unsigned int fT; /* fTxxxx number */</span><br><span>diff --git a/src/tbf_dl.cpp b/src/tbf_dl.cpp</span><br><span>index c122bba..dd5d0f2 100644</span><br><span>--- a/src/tbf_dl.cpp</span><br><span>+++ b/src/tbf_dl.cpp</span><br><span>@@ -994,7 +994,7 @@</span><br><span> </span><br><span>                  LOGPTBFDL(this, LOGL_INFO,</span><br><span>                             "Scheduled Ack/Nack polling on FN=%d, TS=%d\n",</span><br><span style="color: hsl(0, 100%, 40%);">-                               new_poll_fn, poll_ts);</span><br><span style="color: hsl(120, 100%, 40%);">+                                new_poll_fn, ts);</span><br><span>          }</span><br><span>    }</span><br><span> </span><br><span>diff --git a/tests/tbf/TbfTest.cpp b/tests/tbf/TbfTest.cpp</span><br><span>index 8fbdde6..5ce2611 100644</span><br><span>--- a/tests/tbf/TbfTest.cpp</span><br><span>+++ b/tests/tbf/TbfTest.cpp</span><br><span>@@ -671,16 +671,14 @@</span><br><span> {</span><br><span>  RlcMacUplink_t ulreq = {0};</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- OSMO_ASSERT(tbf->is_control_ts(tbf->poll_ts));</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span>         ulreq.u.MESSAGE_TYPE = MT_PACKET_CONTROL_ACK;</span><br><span>        Packet_Control_Acknowledgement_t *ctrl_ack =</span><br><span>                 &ulreq.u.Packet_Control_Acknowledgement;</span><br><span> </span><br><span>     ctrl_ack->PayloadType = GPRS_RLCMAC_CONTROL_BLOCK;</span><br><span>        ctrl_ack->TLLI = tbf->tlli();</span><br><span style="color: hsl(0, 100%, 40%);">-     send_ul_mac_block(tbf->bts, tbf->trx->trx_no, tbf->poll_ts,</span><br><span style="color: hsl(0, 100%, 40%);">-         &ulreq, get_poll_fn(tbf, tbf->poll_ts));</span><br><span style="color: hsl(120, 100%, 40%);">+       send_ul_mac_block(tbf->bts, tbf->trx->trx_no, tbf->control_ts,</span><br><span style="color: hsl(120, 100%, 40%);">+            &ulreq, get_poll_fn(tbf, tbf->control_ts));</span><br><span> }</span><br><span> </span><br><span> static void send_empty_block(gprs_rlcmac_tbf *tbf, unsigned ts_no, unsigned fn)</span><br><span>@@ -1939,8 +1937,8 @@</span><br><span> </span><br><span>   dl_tbf = ms_dl_tbf(ms1);</span><br><span>     OSMO_ASSERT(dl_tbf);</span><br><span style="color: hsl(0, 100%, 40%);">-    fn = get_poll_fn(dl_tbf, dl_tbf->poll_ts);</span><br><span style="color: hsl(0, 100%, 40%);">-   send_empty_block(dl_tbf, dl_tbf->poll_ts, fn);</span><br><span style="color: hsl(120, 100%, 40%);">+     fn = get_poll_fn(dl_tbf, dl_tbf->control_ts);</span><br><span style="color: hsl(120, 100%, 40%);">+      send_empty_block(dl_tbf, dl_tbf->control_ts, fn);</span><br><span>         fn = fn_add_blocks(fn, 1);</span><br><span> </span><br><span>       /* Now establish a new TBF for the RA UPDATE COMPLETE (new TLLI) */</span><br><span>@@ -2164,7 +2162,7 @@</span><br><span>  ack->DOWNLINK_TFI = dl_tbf1->tfi();</span><br><span>    ack->Ack_Nack_Description.FINAL_ACK_INDICATION = 1;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-      send_ul_mac_block(bts, 0, dl_tbf1->poll_ts, &ulreq, get_poll_fn(dl_tbf1, dl_tbf1->poll_ts));</span><br><span style="color: hsl(120, 100%, 40%);">+        send_ul_mac_block(bts, 0, dl_tbf1->control_ts, &ulreq, get_poll_fn(dl_tbf1, dl_tbf1->control_ts));</span><br><span> </span><br><span>     OSMO_ASSERT(dl_tbf1->state_is(GPRS_RLCMAC_WAIT_RELEASE));</span><br><span> </span><br><span>@@ -2713,7 +2711,7 @@</span><br><span>             /* Request to send one RLC/MAC block */</span><br><span>              request_dl_rlc_block(dl_tbf, &fn);</span><br><span>       }</span><br><span style="color: hsl(0, 100%, 40%);">-       send_empty_block(dl_tbf, dl_tbf->poll_ts, fn);</span><br><span style="color: hsl(120, 100%, 40%);">+     send_empty_block(dl_tbf, dl_tbf->control_ts, fn);</span><br><span> </span><br><span>     /* Schedule a large LLC frame */</span><br><span>     dl_tbf->append_data(1000, test_data, sizeof(test_data));</span><br><span>@@ -2725,7 +2723,7 @@</span><br><span>          /* Request to send one RLC/MAC block */</span><br><span>              request_dl_rlc_block(dl_tbf, &fn);</span><br><span>       }</span><br><span style="color: hsl(0, 100%, 40%);">-       send_empty_block(dl_tbf, dl_tbf->poll_ts, fn);</span><br><span style="color: hsl(120, 100%, 40%);">+     send_empty_block(dl_tbf, dl_tbf->control_ts, fn);</span><br><span> </span><br><span>     OSMO_ASSERT(dl_tbf->state_is(GPRS_RLCMAC_FLOW));</span><br><span> </span><br><span>@@ -3069,7 +3067,7 @@</span><br><span>              MAKE_ACKED(msg, dl_tbf, fn, mcs, true);</span><br><span>      }</span><br><span>    /* Clean up pending items in UL controller: */</span><br><span style="color: hsl(0, 100%, 40%);">-  send_empty_block(dl_tbf, dl_tbf->poll_ts, fn+50);</span><br><span style="color: hsl(120, 100%, 40%);">+  send_empty_block(dl_tbf, dl_tbf->control_ts, fn+50);</span><br><span>      tbf_cleanup(dl_tbf);</span><br><span> }</span><br><span> </span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-pcu/+/23532">change 23532</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/+/23532"/><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: I9b8bed7741d385bab4cd8c64b841a78a02a05fe1 </div>
<div style="display:none"> Gerrit-Change-Number: 23532 </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>