OsmocomBB

Seems the fix should be done somehow releasing the BSN=0 when previous BSN=127 is received, around lines:

345481    16:57:52.613529009    127.0.0.1    38705    127.0.0.1    4729    GSMTAP    162    - Raising V(R) to 0 
345483    16:57:52.613566161    127.0.0.1    38705    127.0.0.1    4729    GSMTAP    184    - Taking block 127 out, raising V(Q) to 0 

I tried the following patch, and then the previous step seems to be overcome, but then it gets stuck again after reaching START_SEQUENCE=64 with all RRBPs to 1 this time:

diff --git a/src/rlcmac/rlc_window_dl.c b/src/rlcmac/rlc_window_dl.c
index a2bef5f..ef2d689 100644
--- a/src/rlcmac/rlc_window_dl.c
+++ b/src/rlcmac/rlc_window_dl.c
@@ -225,6 +225,7 @@ void gprs_rlcmac_rlc_dl_window_raise_v_r(struct gprs_rlcmac_rlc_dl_window *dlw,
                        gprs_rlcmac_rlc_dl_window_raise_v_r_to(dlw, 1);
                }
                LOGRLCMAC(LOGL_DEBUG, "- Raising V(R) to %d\n", gprs_rlcmac_rlc_dl_window_v_r(dlw));
+               gprs_rlcmac_rlc_v_n_mark_missing(&dlw->v_n, gprs_rlcmac_rlc_dl_window_v_r(dlw));
        }
 }

I think osmo-pcu also may have a similar bug on the UL, but it's harder to spot there since the UL TBF cannot be kept open forever as easily as DL TBF and hence it may close in general before submitting 128 BSNs?

I started writting a TTCN3 PCU_Tests test to validate whether wraparound was going correct or there was a bug, and it seems the wraparound is wroking fine.

Further investigation found out that in the condition which was reoslving the BSN in libosmo-gprs-rlcmac ""BSN 0 already received" is slightly different in original osmo-pcu code and in the ported code (probably an overlook on my side when porting).

Original osmo-pcu code is using in gprs_rlcmac_ul_tbf::rcv_data_block_acknowledged():

else if (m_window.is_received(rdbi->bsn)) {
            LOGPTBFUL(this, LOGL_DEBUG,
                  "BSN %d already received\n", rdbi->bsn);
            continue;
}

While libosmo-gprs-rlcmac code is using in gprs_rlcmac_dl_tbf_rcv_data_block():

else if (gprs_rlcmac_rlc_v_n_is_received(&dl_tbf->dlw->v_n, rdbi->bsn)) {
            LOGPTBFDL(dl_tbf, LOGL_DEBUG, "BSN %d already received\n", rdbi->bsn);
            continue;
}

But it should actually be calling gprs_rlcmac_rlc_dl_window_is_received() instead, which applies several more restrictions before returning null:

bool gprs_rlcmac_rlc_dl_window_is_received(const struct gprs_rlcmac_rlc_dl_window *dlw, uint16_t bsn)
{
    const struct gprs_rlcmac_rlc_window *w = rlc_dlw_as_w_const(dlw);
    uint16_t offset_v_r;

    /* Offset to the end of the received window */
    offset_v_r = (dlw->v_r - 1 - bsn) & gprs_rlcmac_rlc_window_mod_sns(w);
    return gprs_rlcmac_rlc_dl_window_is_in_window(dlw, bsn) &&
           gprs_rlcmac_rlc_v_n_is_received(&dlw->v_n, bsn) &&
           offset_v_r < gprs_rlcmac_rlc_window_ws(w);
}

The first condition "gprs_rlcmac_rlc_dl_window_is_in_window(dlw, bsn)" we don't really case because it's the same condition driving us to this code path, so we know gprs_rlcmac_rlc_dl_window_is_in_window() is always true, so no need to call it. The second condition "gprs_rlcmac_rlc_v_n_is_received(&dlw->v_n, bsn)" is the one we are already using in libosmo-gprs-rlcmac. The third condition "offset_v_r < gprs_rlcmac_rlc_window_ws(w)" was the one taking care of not resolving the BSN as received in this wraparound case, since v_r=0, bsn=0: "(0 - 1 - 0) & 127" ----> 0xffff & 0x007f ---> 0x007f > 64.

However, I'm so far becoming to like more the fix I implemented for libosmo-gprs-rlcmac in https://gerrit.osmocom.org/c/libosmo-gprs/+/33886, since it keeps the good state in the V(N) array instead of doing heuristics to find out if a BSN is received after having gone through the first BSN wraparound. So I may actually apply the same fix in osmo-pcu, and change the following:

diff --git a/src/rlc.cpp b/src/rlc.cpp
index a0825976..8b6b027d 100644
--- a/src/rlc.cpp
+++ b/src/rlc.cpp
@@ -268,8 +268,14 @@ void gprs_rlc_ul_window::raise_v_r(const uint16_t bsn)
        /* Positive offset, so raise. */
        if (offset_v_r < (sns() >> 1)) {
                while (offset_v_r--) {
-                       if (offset_v_r) /* all except the received block */
-                               m_v_n.mark_missing(v_r());
+                       const uint16_t _v_r = v_r();
+                       const uint16_t bsn_no_longer_in_ws = mod_sns(_v_r - ws());
+                       //LOGP(DRLCMACUL, LOGL_DEBUG, "- Mark BSN %u as INVALID\n", bsn_no_longer_in_ws);
+                       m_v_n.mark_invalid(bsn_no_longer_in_ws);
+                       if (offset_v_r) {/* all except the received block */
+                               //LOGP(DRLCMACUL, LOGL_DEBUG, "- Mark BSN %u as MISSING\n", _v_r);
+                               m_v_n.mark_missing(_v_r);
+                       }
                        raise_v_r_to(1);
                }
                LOGP(DRLCMACUL, LOGL_DEBUG, "- Raising V(R) to %d\n", v_r());
diff --git a/src/rlc.h b/src/rlc.h
index d4a7a5e1..0a297336 100644
--- a/src/rlc.h
+++ b/src/rlc.h
@@ -329,6 +329,7 @@ struct gprs_rlc_v_n {

        void mark_received(int bsn);
        void mark_missing(int bsn);
+       void mark_invalid(int bsn);

        bool is_received(int bsn) const;

@@ -649,6 +650,11 @@ inline void gprs_rlc_v_n::mark_missing(int bsn)
        return mark(bsn, GPRS_RLC_UL_BSN_MISSING);
 }

+inline void gprs_rlc_v_n::mark_invalid(int bsn)
+{
+       return mark(bsn, GPRS_RLC_UL_BSN_INVALID);
+}
+
 inline bool gprs_rlc_v_n::is_received(int bsn) const
 {
        return is_state(bsn, GPRS_RLC_UL_BSN_RECEIVED);
diff --git a/src/tbf_ul.cpp b/src/tbf_ul.cpp
index 5ae0d495..3746130f 100644
--- a/src/tbf_ul.cpp
+++ b/src/tbf_ul.cpp
@@ -303,7 +303,7 @@ int gprs_rlcmac_ul_tbf::rcv_data_block_acknowledged(
                                  rdbi->bsn,
                                  m_window.v_q(), m_window.mod_sns(m_window.v_q() + ws - 1));
                        continue;
-               } else if (m_window.is_received(rdbi->bsn)) {
+               } else if (m_window.m_v_n.is_received(rdbi->bsn)) {
                        LOGPTBFUL(this, LOGL_DEBUG,
                                  "BSN %d already received\n", rdbi->bsn);
                        continue;