Review at  https://gerrit.osmocom.org/3155

Encapsulate handling of UL ACK timeout

Use helper methods instead checking and manipulating flag directly.

Change-Id: Ia3f009c52118db95b38a077e08eecda844e7f8d1
Related: OS#1539
---
M src/bts.cpp
M src/tbf.cpp
M src/tbf.h
M src/tbf_ul.cpp
4 files changed, 39 insertions(+), 21 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/55/3155/1

diff --git a/src/bts.cpp b/src/bts.cpp
index 9847e9b..799188b 100644
--- a/src/bts.cpp
+++ b/src/bts.cpp
@@ -978,6 +978,7 @@
        struct gprs_rlcmac_tbf *tbf, *new_tbf;
        uint32_t tlli = packet->TLLI;
        GprsMs *ms = bts()->ms_by_tlli(tlli);
+       gprs_rlcmac_ul_tbf *ul_tbf;
 
        tbf = bts()->ul_tbf_by_poll_fn(fn, trx_no(), ts_no);
        if (!tbf)
@@ -1004,16 +1005,12 @@
        tbf->poll_state = GPRS_RLCMAC_POLL_NONE;
 
        /* check if this control ack belongs to packet uplink ack */
-       if (tbf->ul_ack_state == GPRS_RLCMAC_UL_ACK_WAIT_ACK) {
+       ul_tbf = as_ul_tbf(tbf);
+       if (ul_tbf && ul_tbf->handle_ctrl_ack()) {
                LOGP(DRLCMAC, LOGL_DEBUG, "TBF: [UPLINK] END %s\n", 
tbf_name(tbf));
-               tbf->ul_ack_state = GPRS_RLCMAC_UL_ACK_NONE;
-               if ((tbf->state_flags &
-                       (1 << GPRS_RLCMAC_FLAG_TO_UL_ACK))) {
-                       tbf->state_flags &=
-                               ~(1 << GPRS_RLCMAC_FLAG_TO_UL_ACK);
-                               LOGP(DRLCMAC, LOGL_NOTICE, "Recovered uplink "
-                                       "ack for UL %s\n", tbf_name(tbf));
-               }
+               if (ul_tbf->ctrl_ack_to_toggle())
+                       LOGP(DRLCMAC, LOGL_NOTICE, "Recovered uplink ack for UL 
%s\n", tbf_name(tbf));
+
                tbf_free(tbf);
                return;
        }
diff --git a/src/tbf.cpp b/src/tbf.cpp
index 2e7c65b..fa67e41 100644
--- a/src/tbf.cpp
+++ b/src/tbf.cpp
@@ -607,27 +607,25 @@
 
 void gprs_rlcmac_tbf::poll_timeout()
 {
+       gprs_rlcmac_ul_tbf *ul_tbf = as_ul_tbf(this);
+
        LOGP(DRLCMAC, LOGL_NOTICE, "%s poll timeout for FN=%d, TS=%d (curr FN 
%d)\n",
                tbf_name(this), poll_fn, poll_ts, bts->current_frame_number());
 
        poll_state = GPRS_RLCMAC_POLL_NONE;
 
-       if (ul_ack_state == GPRS_RLCMAC_UL_ACK_WAIT_ACK) {
-               if (!(state_flags & (1 << GPRS_RLCMAC_FLAG_TO_UL_ACK))) {
-                       LOGP(DRLCMAC, LOGL_NOTICE, "- Timeout for polling "
-                               "PACKET CONTROL ACK for PACKET UPLINK ACK\n");
+       if (ul_tbf && ul_tbf->handle_ctrl_ack()) {
+               if (!ul_tbf->ctrl_ack_to_toggle()) {
+                       LOGP(DRLCMAC, LOGL_NOTICE, "- Timeout for polling 
PACKET CONTROL ACK for PACKET UPLINK ACK\n");
                        rlcmac_diag();
-                       state_flags |= (1 << GPRS_RLCMAC_FLAG_TO_UL_ACK);
                }
-               ul_ack_state = GPRS_RLCMAC_UL_ACK_NONE;
                bts->rlc_ack_timedout();
                bts->pkt_ul_ack_nack_poll_timedout();
                if (state_is(GPRS_RLCMAC_FINISHED)) {
-                       gprs_rlcmac_ul_tbf *ul_tbf = as_ul_tbf(this);
                        ul_tbf->m_n3103++;
                        if (ul_tbf->m_n3103 == ul_tbf->bts->bts_data()->n3103) {
                                LOGP(DRLCMAC, LOGL_NOTICE,
-                                       "- N3103 exceeded\n");
+                                    "- N3103 exceeded\n");
                                bts->pkt_ul_ack_nack_poll_failed();
                                ul_tbf->set_state(GPRS_RLCMAC_RELEASING);
                                tbf_timer_start(ul_tbf, 3169, 
ul_tbf->bts->bts_data()->t3169, 0);
@@ -640,10 +638,10 @@
        } else if (ul_ass_state == GPRS_RLCMAC_UL_ASS_WAIT_ACK) {
                if (!(state_flags & (1 << GPRS_RLCMAC_FLAG_TO_UL_ASS))) {
                        LOGP(DRLCMAC, LOGL_NOTICE, "- Timeout for polling "
-                               "PACKET CONTROL ACK for PACKET UPLINK "
-                               "ASSIGNMENT.\n");
-                       rlcmac_diag();
-                       state_flags |= (1 << GPRS_RLCMAC_FLAG_TO_UL_ASS);
+                            "PACKET CONTROL ACK for PACKET UPLINK "
+                            "ASSIGNMENT.\n");
+                               rlcmac_diag();
+                               state_flags |= (1 << 
GPRS_RLCMAC_FLAG_TO_UL_ASS);
                }
                ul_ass_state = GPRS_RLCMAC_UL_ASS_NONE;
                n3105++;
diff --git a/src/tbf.h b/src/tbf.h
index 64cfa91..dba6dbd 100644
--- a/src/tbf.h
+++ b/src/tbf.h
@@ -502,6 +502,8 @@
        gprs_rlcmac_ul_tbf(BTS *bts);
 
        struct msgb *create_ul_ack(uint32_t fn, uint8_t ts);
+       bool ctrl_ack_to_toggle();
+       bool handle_ctrl_ack();
 
        /* blocks were acked */
        int rcv_data_block_acknowledged(
diff --git a/src/tbf_ul.cpp b/src/tbf_ul.cpp
index 1eee41a..1e0898a 100644
--- a/src/tbf_ul.cpp
+++ b/src/tbf_ul.cpp
@@ -95,6 +95,27 @@
        return 0;
 }
 
+bool gprs_rlcmac_ul_tbf::ctrl_ack_to_toggle()
+{
+       if ((state_flags & (1 << GPRS_RLCMAC_FLAG_TO_UL_ACK))) {
+               state_flags &= ~(1 << GPRS_RLCMAC_FLAG_TO_UL_ACK);
+               return true; /* GPRS_RLCMAC_FLAG_TO_UL_ACK was set, now cleared 
*/
+       }
+
+       state_flags |= (1 << GPRS_RLCMAC_FLAG_TO_UL_ACK);
+       return false; /* GPRS_RLCMAC_FLAG_TO_UL_ACK was unset, now set */
+}
+
+bool gprs_rlcmac_ul_tbf::handle_ctrl_ack()
+{
+       /* check if this control ack belongs to packet uplink ack */
+       if (ul_ack_state == GPRS_RLCMAC_UL_ACK_WAIT_ACK) {
+               ul_ack_state = GPRS_RLCMAC_UL_ACK_NONE;
+               return true;
+       }
+
+       return false;
+}
 
 struct msgb *gprs_rlcmac_ul_tbf::create_ul_ack(uint32_t fn, uint8_t ts)
 {

-- 
To view, visit https://gerrit.osmocom.org/3155
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia3f009c52118db95b38a077e08eecda844e7f8d1
Gerrit-PatchSet: 1
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Owner: Max <msur...@sysmocom.de>

Reply via email to