<p>pespin has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/c/osmo-pcu/+/25629">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">pdch: refactor rcv_control_ack() with a switch statement<br><br>This clarifies the different paths and uniforms them. Makes code far<br>easier to read and debug.<br><br>New improved verification already found some misehavior in some tests.<br><br>Change-Id: I7e4a88d6e004bbb7974595320ed73742162c7ad7<br>---<br>M src/pdch.cpp<br>1 file changed, 56 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/29/25629/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 76acbca..de77075 100644</span><br><span>--- a/src/pdch.cpp</span><br><span>+++ b/src/pdch.cpp</span><br><span>@@ -344,37 +344,28 @@</span><br><span> fn, get_value_string(pdch_ulc_tbf_poll_reason_names, reason));</span><br><span> pdch_ulc_release_fn(ulc, fn);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /* check if this control ack belongs to packet uplink ack */</span><br><span style="color: hsl(0, 100%, 40%);">- ul_tbf = as_ul_tbf(tbf);</span><br><span style="color: hsl(0, 100%, 40%);">- if (ul_tbf && reason == PDCH_ULC_POLL_UL_ACK && tbf_ul_ack_exp_ctrl_ack(ul_tbf, fn, ts_no)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ switch (reason) {</span><br><span style="color: hsl(120, 100%, 40%);">+ case PDCH_ULC_POLL_UL_ACK:</span><br><span style="color: hsl(120, 100%, 40%);">+ ul_tbf = as_ul_tbf(tbf);</span><br><span style="color: hsl(120, 100%, 40%);">+ OSMO_ASSERT(ul_tbf);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!tbf_ul_ack_exp_ctrl_ack(ul_tbf, fn, ts_no)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPTBF(tbf, LOGL_NOTICE, "FN=%d, TS=%d (curr FN %d): POLL_UL_ACK not expected!\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ fn, ts_no, bts_current_frame_number(tbf->bts));</span><br><span style="color: hsl(120, 100%, 40%);">+ return;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> osmo_fsm_inst_dispatch(ul_tbf->ul_ack_fsm.fi, TBF_UL_ACK_EV_RX_CTRL_ACK, NULL);</span><br><span> /* We can free since we only set polling on final UL ACK/NACK */</span><br><span> LOGPTBF(tbf, LOGL_DEBUG, "[UPLINK] END\n");</span><br><span> tbf_free(tbf);</span><br><span> return;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">- if (tbf->dl_ass_state_is(TBF_DL_ASS_WAIT_ACK)) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGPTBF(tbf, LOGL_DEBUG, "[UPLINK] DOWNLINK ASSIGNED\n");</span><br><span style="color: hsl(0, 100%, 40%);">- /* reset N3105 */</span><br><span style="color: hsl(0, 100%, 40%);">- tbf->n_reset(N3105);</span><br><span style="color: hsl(0, 100%, 40%);">- osmo_fsm_inst_dispatch(tbf->dl_ass_fsm.fi, TBF_DL_ASS_EV_RX_ASS_CTRL_ACK, NULL);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- new_tbf = ms_dl_tbf(ms);</span><br><span style="color: hsl(0, 100%, 40%);">- if (!new_tbf) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGPDCH(this, DRLCMAC, LOGL_ERROR, "Got ACK, but DL "</span><br><span style="color: hsl(0, 100%, 40%);">- "TBF is gone TLLI=0x%08x\n", tlli);</span><br><span style="color: hsl(120, 100%, 40%);">+ case PDCH_ULC_POLL_UL_ASS:</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!tbf->ul_ass_state_is(TBF_UL_ASS_WAIT_ACK)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPTBF(tbf, LOGL_NOTICE, "FN=%d, TS=%d (curr FN %d): POLL_UL_ASS not expected! state is %s\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ fn, ts_no, bts_current_frame_number(tbf->bts),</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_fsm_inst_state_name(tbf_ul_ass_fi(tbf)));</span><br><span> return;</span><br><span> }</span><br><span style="color: hsl(0, 100%, 40%);">- if (tbf->state_is(TBF_ST_WAIT_RELEASE) &&</span><br><span style="color: hsl(0, 100%, 40%);">- tbf->direction == new_tbf->direction)</span><br><span style="color: hsl(0, 100%, 40%);">- tbf_free(tbf);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- osmo_fsm_inst_dispatch(new_tbf->state_fsm.fi, TBF_EV_ASSIGN_ACK_PACCH, NULL);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- tbf_assign_control_ts(new_tbf);</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%);">- if (tbf->ul_ass_state_is(TBF_UL_ASS_WAIT_ACK)) {</span><br><span> LOGPTBF(tbf, LOGL_DEBUG, "[DOWNLINK] UPLINK ASSIGNED\n");</span><br><span> /* reset N3105 */</span><br><span> tbf->n_reset(N3105);</span><br><span>@@ -398,10 +389,41 @@</span><br><span> * established */</span><br><span> if (ms_need_dl_tbf(new_tbf->ms()))</span><br><span> new_tbf->establish_dl_tbf_on_pacch();</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> return;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">- if (ms->nacc && nacc_fsm_exp_ctrl_ack(ms->nacc, fn, ts_no)) {</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ case PDCH_ULC_POLL_DL_ASS:</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!tbf->dl_ass_state_is(TBF_DL_ASS_WAIT_ACK)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPTBF(tbf, LOGL_NOTICE, "FN=%d, TS=%d (curr FN %d): POLL_DL_ASS not expected! state is %s\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ fn, ts_no, bts_current_frame_number(tbf->bts),</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_fsm_inst_state_name(tbf_dl_ass_fi(tbf)));</span><br><span style="color: hsl(120, 100%, 40%);">+ return;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPTBF(tbf, LOGL_DEBUG, "[UPLINK] DOWNLINK ASSIGNED\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ /* reset N3105 */</span><br><span style="color: hsl(120, 100%, 40%);">+ tbf->n_reset(N3105);</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_fsm_inst_dispatch(tbf->dl_ass_fsm.fi, TBF_DL_ASS_EV_RX_ASS_CTRL_ACK, NULL);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ new_tbf = ms_dl_tbf(ms);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!new_tbf) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPDCH(this, DRLCMAC, LOGL_ERROR, "Got ACK, but DL "</span><br><span style="color: hsl(120, 100%, 40%);">+ "TBF is gone TLLI=0x%08x\n", tlli);</span><br><span style="color: hsl(120, 100%, 40%);">+ return;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ if (tbf->state_is(TBF_ST_WAIT_RELEASE) &&</span><br><span style="color: hsl(120, 100%, 40%);">+ tbf->direction == new_tbf->direction)</span><br><span style="color: hsl(120, 100%, 40%);">+ tbf_free(tbf);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_fsm_inst_dispatch(new_tbf->state_fsm.fi, TBF_EV_ASSIGN_ACK_PACCH, NULL);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ tbf_assign_control_ts(new_tbf);</span><br><span style="color: hsl(120, 100%, 40%);">+ return;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ case PDCH_ULC_POLL_CELL_CHG_CONTINUE:</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!ms->nacc || !nacc_fsm_exp_ctrl_ack(ms->nacc, fn, ts_no)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPTBF(tbf, LOGL_NOTICE, "FN=%d, TS=%d (curr FN %d): POLL_CELL_CHG_CONTINUE not expected!\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ fn, ts_no, bts_current_frame_number(tbf->bts));</span><br><span style="color: hsl(120, 100%, 40%);">+ return;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> osmo_fsm_inst_dispatch(ms->nacc->fi, NACC_EV_RX_CELL_CHG_CONTINUE_ACK, NULL);</span><br><span> /* Don't assume MS is no longer reachable (hence don't free) after this: TS 44.060</span><br><span> * "When the mobile station receives the PACKET CELL CHANGE ORDER</span><br><span>@@ -412,10 +434,15 @@</span><br><span> * switch to a new cell."</span><br><span> */</span><br><span> return;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ case PDCH_ULC_POLL_DL_ACK:</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Handled in rcv_control_dl_ack_nack() upon receival of DL ACK/NACK as a response to our POLL. */</span><br><span style="color: hsl(120, 100%, 40%);">+ return;</span><br><span style="color: hsl(120, 100%, 40%);">+ default:</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPDCH(this, DRLCMAC, LOGL_ERROR, "FN=%" PRIu32 " "</span><br><span style="color: hsl(120, 100%, 40%);">+ "Error: received PACKET CONTROL ACK at no request (reason=%s)\n", fn,</span><br><span style="color: hsl(120, 100%, 40%);">+ get_value_string(pdch_ulc_tbf_poll_reason_names, reason));</span><br><span> }</span><br><span style="color: hsl(0, 100%, 40%);">- LOGPDCH(this, DRLCMAC, LOGL_ERROR, "FN=%" PRIu32 " "</span><br><span style="color: hsl(0, 100%, 40%);">- "Error: received PACKET CONTROL ACK at no request (reason=%s)\n", fn,</span><br><span style="color: hsl(0, 100%, 40%);">- get_value_string(pdch_ulc_tbf_poll_reason_names, reason));</span><br><span> }</span><br><span> </span><br><span> void gprs_rlcmac_pdch::rcv_control_dl_ack_nack(Packet_Downlink_Ack_Nack_t *ack_nack, uint32_t fn, struct pcu_l1_meas *meas)</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-pcu/+/25629">change 25629</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/+/25629"/><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: I7e4a88d6e004bbb7974595320ed73742162c7ad7 </div>
<div style="display:none"> Gerrit-Change-Number: 25629 </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>