<p>pespin <strong>submitted</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/osmo-pcu/+/25111">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Jenkins Builder: Verified
pespin: Looks good to me, approved
osmith: Looks good to me, but someone else must approve
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">Simplify tbf::set_polling()<br><br>When setting a POLL, it will always happen on PACCH, so all the CCCH<br>part makes no sense there. Let's drop it and move the logging of each<br>case to the caller, where logging file+line is more useful.<br><br>Change-Id: I242f97fd6f927131ac64c1a7c9c3812b6389de04<br>---<br>M src/nacc_fsm.c<br>M src/tbf.cpp<br>M src/tbf_dl.cpp<br>M src/tbf_dl.h<br>M src/tbf_dl_ass_fsm.c<br>M src/tbf_ul_ack_fsm.c<br>M src/tbf_ul_ass_fsm.c<br>7 files changed, 20 insertions(+), 46 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/nacc_fsm.c b/src/nacc_fsm.c</span><br><span>index 9a5f5b5..738b2c5 100644</span><br><span>--- a/src/nacc_fsm.c</span><br><span>+++ b/src/nacc_fsm.c</span><br><span>@@ -207,6 +207,8 @@</span><br><span> rate_ctr_inc(rate_ctr_group_get_ctr(bts_rate_counters(ms->bts), CTR_PKT_CELL_CHG_CONTINUE));</span><br><span> talloc_free(mac_control_block);</span><br><span> tbf_set_polling(tbf, *new_poll_fn, data->ts, PDCH_ULC_POLL_CELL_CHG_CONTINUE);</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPTBFDL(tbf, LOGL_DEBUG, "Scheduled 'Packet Cell Change Continue' polling on PACCH (FN=%d, TS=%d)\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ *new_poll_fn, data->ts);</span><br><span> return msg;</span><br><span> </span><br><span> free_ret:</span><br><span>diff --git a/src/tbf.cpp b/src/tbf.cpp</span><br><span>index b43ffde..acd191b 100644</span><br><span>--- a/src/tbf.cpp</span><br><span>+++ b/src/tbf.cpp</span><br><span>@@ -529,49 +529,10 @@</span><br><span> </span><br><span> void gprs_rlcmac_tbf::set_polling(uint32_t new_poll_fn, uint8_t ts, enum pdch_ulc_tbf_poll_reason reason)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- const char *chan = "UNKNOWN";</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- if (state_fsm.state_flags & (1 << (GPRS_RLCMAC_FLAG_CCCH)))</span><br><span style="color: hsl(0, 100%, 40%);">- chan = "CCCH";</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- if (state_fsm.state_flags & (1 << (GPRS_RLCMAC_FLAG_PACCH)))</span><br><span style="color: hsl(0, 100%, 40%);">- chan = "PACCH";</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- if ((state_fsm.state_flags & (1 << (GPRS_RLCMAC_FLAG_PACCH))) &&</span><br><span style="color: hsl(0, 100%, 40%);">- (state_fsm.state_flags & (1 << (GPRS_RLCMAC_FLAG_CCCH))))</span><br><span style="color: hsl(0, 100%, 40%);">- LOGPTBFDL(this, LOGL_ERROR,</span><br><span style="color: hsl(0, 100%, 40%);">- "Attempt to schedule polling on %s (FN=%d, TS=%d) with both CCCH and PACCH flags set - FIXME!\n",</span><br><span style="color: hsl(0, 100%, 40%);">- chan, new_poll_fn, ts);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> /* schedule polling */</span><br><span style="color: hsl(0, 100%, 40%);">- if (pdch_ulc_reserve_tbf_poll(trx->pdch[ts].ulc, new_poll_fn, this, reason) < 0) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGPTBFDL(this, LOGL_ERROR, "Failed scheduling poll on %s (FN=%d, TS=%d)\n",</span><br><span style="color: hsl(0, 100%, 40%);">- chan, new_poll_fn, ts);</span><br><span style="color: hsl(0, 100%, 40%);">- return;</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%);">- switch (reason) {</span><br><span style="color: hsl(0, 100%, 40%);">- case PDCH_ULC_POLL_UL_ASS:</span><br><span style="color: hsl(0, 100%, 40%);">- 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, ts);</span><br><span style="color: hsl(0, 100%, 40%);">- break;</span><br><span style="color: hsl(0, 100%, 40%);">- case PDCH_ULC_POLL_DL_ASS:</span><br><span style="color: hsl(0, 100%, 40%);">- 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, ts);</span><br><span style="color: hsl(0, 100%, 40%);">- break;</span><br><span style="color: hsl(0, 100%, 40%);">- case PDCH_ULC_POLL_UL_ACK:</span><br><span style="color: hsl(0, 100%, 40%);">- 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, ts);</span><br><span style="color: hsl(0, 100%, 40%);">- break;</span><br><span style="color: hsl(0, 100%, 40%);">- case PDCH_ULC_POLL_DL_ACK:</span><br><span style="color: hsl(0, 100%, 40%);">- 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, ts);</span><br><span style="color: hsl(0, 100%, 40%);">- break;</span><br><span style="color: hsl(0, 100%, 40%);">- case PDCH_ULC_POLL_CELL_CHG_CONTINUE:</span><br><span style="color: hsl(0, 100%, 40%);">- 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, ts);</span><br><span style="color: hsl(0, 100%, 40%);">- break;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(120, 100%, 40%);">+ if (pdch_ulc_reserve_tbf_poll(trx->pdch[ts].ulc, new_poll_fn, this, reason) < 0)</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPTBFDL(this, LOGL_ERROR, "Failed scheduling poll on PACCH (FN=%d, TS=%d)\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ new_poll_fn, ts);</span><br><span> }</span><br><span> </span><br><span> 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>diff --git a/src/tbf_dl.cpp b/src/tbf_dl.cpp</span><br><span>index 905bf05..65785f2 100644</span><br><span>--- a/src/tbf_dl.cpp</span><br><span>+++ b/src/tbf_dl.cpp</span><br><span>@@ -958,6 +958,9 @@</span><br><span> rc = check_polling(fn, ts, &new_poll_fn, &rrbp);</span><br><span> if (rc >= 0) {</span><br><span> set_polling(new_poll_fn, ts, PDCH_ULC_POLL_DL_ACK);</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPTBFDL(this, LOGL_DEBUG,</span><br><span style="color: hsl(120, 100%, 40%);">+ "Scheduled DL Acknowledgement polling on PACCH (FN=%d, TS=%d)\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ new_poll_fn, ts);</span><br><span> </span><br><span> m_tx_counter = 0;</span><br><span> /* start timer whenever we send the final block */</span><br><span>diff --git a/src/tbf_dl.h b/src/tbf_dl.h</span><br><span>index 27b6a2c..d20ad75 100644</span><br><span>--- a/src/tbf_dl.h</span><br><span>+++ b/src/tbf_dl.h</span><br><span>@@ -38,8 +38,6 @@</span><br><span> DL_PRIO_CONTROL, /* a control block needs to be sent */</span><br><span> };</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-#define LOGPTBFDL(tbf, level, fmt, args...) LOGP(DTBFDL, level, "%s " fmt, tbf_name(tbf), ## args)</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> struct gprs_rlcmac_dl_tbf : public gprs_rlcmac_tbf {</span><br><span> gprs_rlcmac_dl_tbf(struct gprs_rlcmac_bts *bts, GprsMs *ms);</span><br><span> ~gprs_rlcmac_dl_tbf();</span><br><span>@@ -153,6 +151,8 @@</span><br><span> const uint8_t *data, const uint16_t len);</span><br><span> </span><br><span> void tbf_dl_trigger_ass(struct gprs_rlcmac_dl_tbf *tbf, struct gprs_rlcmac_tbf *old_tbf);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+#define LOGPTBFDL(tbf, level, fmt, args...) LOGP(DTBFDL, level, "%s " fmt, tbf_name(tbf), ## args)</span><br><span> #ifdef __cplusplus</span><br><span> }</span><br><span> #endif</span><br><span>diff --git a/src/tbf_dl_ass_fsm.c b/src/tbf_dl_ass_fsm.c</span><br><span>index cf09588..5ac1c1a 100644</span><br><span>--- a/src/tbf_dl_ass_fsm.c</span><br><span>+++ b/src/tbf_dl_ass_fsm.c</span><br><span>@@ -31,7 +31,7 @@</span><br><span> #include <encoding.h></span><br><span> #include <bts.h></span><br><span> #include <tbf.h></span><br><span style="color: hsl(0, 100%, 40%);">-#include <tbf_ul.h></span><br><span style="color: hsl(120, 100%, 40%);">+#include <tbf_dl.h></span><br><span> </span><br><span> #define X(s) (1 << (s))</span><br><span> </span><br><span>@@ -137,6 +137,8 @@</span><br><span> bts_do_rate_ctr_inc(ms->bts, CTR_PKT_DL_ASSIGNMENT);</span><br><span> </span><br><span> tbf_set_polling(ctx->tbf, new_poll_fn, d->ts, PDCH_ULC_POLL_DL_ASS);</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPTBFDL(ctx->tbf, LOGL_INFO, "Scheduled DL Assignment polling on PACCH (FN=%d, TS=%d)\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ new_poll_fn, d->ts);</span><br><span> </span><br><span> talloc_free(mac_control_block);</span><br><span> return msg;</span><br><span>diff --git a/src/tbf_ul_ack_fsm.c b/src/tbf_ul_ack_fsm.c</span><br><span>index 16ca22b..32e3533 100644</span><br><span>--- a/src/tbf_ul_ack_fsm.c</span><br><span>+++ b/src/tbf_ul_ack_fsm.c</span><br><span>@@ -91,8 +91,12 @@</span><br><span> if (ms_tlli(ms) != GSM_RESERVED_TMSI)</span><br><span> ul_tbf_contention_resolution_success(ctx->tbf);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (final)</span><br><span style="color: hsl(120, 100%, 40%);">+ if (final) {</span><br><span> tbf_set_polling(tbf, new_poll_fn, d->ts, PDCH_ULC_POLL_UL_ACK);</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPTBFUL(tbf, LOGL_DEBUG,</span><br><span style="color: hsl(120, 100%, 40%);">+ "Scheduled UL Acknowledgement polling on PACCH (FN=%d, TS=%d)\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ new_poll_fn, d->ts);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> </span><br><span> return msg;</span><br><span> }</span><br><span>diff --git a/src/tbf_ul_ass_fsm.c b/src/tbf_ul_ass_fsm.c</span><br><span>index 00f4bfd..eab34ee 100644</span><br><span>--- a/src/tbf_ul_ass_fsm.c</span><br><span>+++ b/src/tbf_ul_ass_fsm.c</span><br><span>@@ -127,6 +127,8 @@</span><br><span> bts_do_rate_ctr_inc(ms->bts, CTR_PKT_UL_ASSIGNMENT);</span><br><span> </span><br><span> tbf_set_polling(ctx->tbf, new_poll_fn, d->ts, PDCH_ULC_POLL_UL_ASS);</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPTBFDL(ctx->tbf, LOGL_INFO, "Scheduled UL Assignment polling on PACCH (FN=%d, TS=%d)\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ new_poll_fn, d->ts);</span><br><span> </span><br><span> talloc_free(mac_control_block);</span><br><span> return msg;</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-pcu/+/25111">change 25111</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/+/25111"/><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: I242f97fd6f927131ac64c1a7c9c3812b6389de04 </div>
<div style="display:none"> Gerrit-Change-Number: 25111 </div>
<div style="display:none"> Gerrit-PatchSet: 4 </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: osmith <osmith@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>