This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
Max gerrit-no-reply at lists.osmocom.orgReview at https://gerrit.osmocom.org/5340 TBF: move window parameters to UL/DL level The UL and DL TBF use different classes implementing window management. Hence it's better to use it explicitly instead of using the common window management superclass inside common TBF superclass. While at it, also remove the direct access to window class - use accessor functions instead. Related: OS#1759 Change-Id: I0b55aa8947db65f7206adcf53ea32b74a831d9e6 --- M src/bts.cpp M src/encoding.cpp M src/pcu_vty_functions.cpp M src/tbf.cpp M src/tbf.h M src/tbf_dl.cpp M tests/tbf/TbfTest.cpp 7 files changed, 50 insertions(+), 40 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/40/5340/1 diff --git a/src/bts.cpp b/src/bts.cpp index 341c9d4..feb6bcd 100644 --- a/src/bts.cpp +++ b/src/bts.cpp @@ -1197,7 +1197,7 @@ num_blocks = Decoding::decode_gprs_acknack_bits( &ack_nack->Ack_Nack_Description, &bits, - &bsn_begin, &bsn_end, &tbf->m_window); + &bsn_begin, &bsn_end, tbf->window()); LOGP(DRLCMAC, LOGL_DEBUG, "Got GPRS DL ACK bitmap: SSN: %d, BSN %d to %d - 1 (%d blocks), " @@ -1271,8 +1271,8 @@ (void *)&ack_nack->EGPRS_AckNack.Desc, (int)offsetof(EGPRS_AckNack_t, Desc), (int)offsetof(EGPRS_AckNack_w_len_t, Desc), - tbf->m_window.v_a(), - tbf->m_window.v_s(), + tbf->window()->v_a(), + tbf->window()->v_s(), osmo_hexdump((const uint8_t *)&ack_nack->EGPRS_AckNack.Desc.URBB, sizeof(ack_nack->EGPRS_AckNack.Desc.URBB))); @@ -1282,7 +1282,7 @@ num_blocks = Decoding::decode_egprs_acknack_bits( &ack_nack->EGPRS_AckNack.Desc, &bits, - &bsn_begin, &bsn_end, &tbf->m_window); + &bsn_begin, &bsn_end, tbf->window()); LOGP(DRLCMAC, LOGL_DEBUG, "Got EGPRS DL ACK bitmap: SSN: %d, BSN %d to %d - 1 (%d blocks), " diff --git a/src/encoding.cpp b/src/encoding.cpp index 3e86222..4d9946d 100644 --- a/src/encoding.cpp +++ b/src/encoding.cpp @@ -695,7 +695,7 @@ { bitvec_write_field(dest, &wp, tbf->current_cs().to_num() - 1, 2); // CHANNEL_CODING_COMMAND - write_packet_ack_nack_desc_gprs(bts, dest, wp, &tbf->m_window, is_final); + write_packet_ack_nack_desc_gprs(bts, dest, wp, tbf->window(), is_final); bitvec_write_field(dest, &wp, 1, 1); // 1: have CONTENTION_RESOLUTION_TLLI bitvec_write_field(dest, &wp, tbf->tlli(), 32); // CONTENTION_RESOLUTION_TLLI @@ -892,7 +892,7 @@ /* -2 for last bit 0 mandatory and REL5 not supported */ unsigned bits_ack_nack = dest->data_len * 8 - wp - 2; - write_packet_ack_nack_desc_egprs(bts, dest, wp, &tbf->m_window, is_final, bits_ack_nack); + write_packet_ack_nack_desc_egprs(bts, dest, wp, tbf->window(), is_final, bits_ack_nack); bitvec_write_field(dest, &wp, 0, 1); // fixed 0 bitvec_write_field(dest, &wp, 0, 1); // 0: don't have REL 5 diff --git a/src/pcu_vty_functions.cpp b/src/pcu_vty_functions.cpp index f1dd25c..6362281 100644 --- a/src/pcu_vty_functions.cpp +++ b/src/pcu_vty_functions.cpp @@ -64,13 +64,13 @@ } if (tbf->trx != NULL) vty_out(vty, " TRX_ID=%d", tbf->trx->trx_no); - vty_out(vty, " CS=%s WS=%d", - tbf->current_cs().name(), tbf->window()->ws()); + vty_out(vty, " CS=%s", + tbf->current_cs().name()); if (ul_tbf) { - gprs_rlc_ul_window *win = &ul_tbf->m_window; - vty_out(vty, " V(Q)=%d V(R)=%d", - win->v_q(), win->v_r()); + gprs_rlc_ul_window *win = ul_tbf->window(); + vty_out(vty, " WS=%d V(Q)=%d V(R)=%d", + ul_tbf->ws(), win->v_q(), win->v_r()); vty_out(vty, "%s", VTY_NEWLINE); vty_out(vty, " TBF Statistics:%s", VTY_NEWLINE); if(GprsCodingScheme::GPRS == tbf->ms()->mode()) { @@ -80,9 +80,9 @@ } } if (dl_tbf) { - gprs_rlc_dl_window *win = &dl_tbf->m_window; - vty_out(vty, " V(A)=%d V(S)=%d nBSN=%d%s", - win->v_a(), win->v_s(), win->resend_needed(), + gprs_rlc_dl_window *win = dl_tbf->window(); + vty_out(vty, " WS=%d V(A)=%d V(S)=%d nBSN=%d%s", + dl_tbf->ws(), win->v_a(), win->v_s(), win->resend_needed(), win->window_stalled() ? " STALLED" : ""); vty_out(vty, "%s", VTY_NEWLINE); vty_out_rate_ctr_group(vty, " ", tbf->m_ctrs); diff --git a/src/tbf.cpp b/src/tbf.cpp index 98005dc..87101d0 100644 --- a/src/tbf.cpp +++ b/src/tbf.cpp @@ -1449,6 +1449,11 @@ return ts == control_ts; } +gprs_rlc_ul_window *gprs_rlcmac_ul_tbf::window() +{ + return &m_window; +} + struct gprs_rlcmac_ul_tbf *handle_tbf_reject(struct gprs_rlcmac_bts *bts, GprsMs *ms, uint32_t tlli, uint8_t trx_no, uint8_t ts) { diff --git a/src/tbf.h b/src/tbf.h index f324cba..26d6598 100644 --- a/src/tbf.h +++ b/src/tbf.h @@ -166,8 +166,6 @@ GprsMs *ms() const; void set_ms(GprsMs *ms); - gprs_rlc_window *window(); - uint8_t tsc() const; int rlcmac_diag(); @@ -413,7 +411,7 @@ struct gprs_rlcmac_dl_tbf : public gprs_rlcmac_tbf { gprs_rlcmac_dl_tbf(BTS *bts); - + gprs_rlc_dl_window *window(); void cleanup(); void enable_egprs(); /* dispatch Unitdata.DL messages */ @@ -453,7 +451,6 @@ * All states that need reset must be in this struct, so this is why * variables are in both (dl and ul) structs and not outside union. */ - gprs_rlc_dl_window m_window; int32_t m_tx_counter; /* count all transmitted blocks */ uint8_t m_wait_confirm; /* wait for CCCH IMM.ASS cnf */ bool m_dl_ack_requested; @@ -504,11 +501,18 @@ enum egprs_rlcmac_dl_spb get_egprs_dl_spb(int bsn); struct osmo_timer_list m_llc_timer; + + /* Please note that all variables below will be reset when changing + * from WAIT RELEASE back to FLOW state (re-use of TBF). + * All states that need reset must be in this struct, so this is why + * variables are in both (dl and ul) structs and not outside union. + */ + gprs_rlc_dl_window m_window; }; struct gprs_rlcmac_ul_tbf : public gprs_rlcmac_tbf { gprs_rlcmac_ul_tbf(BTS *bts); - + gprs_rlc_ul_window *window(); struct msgb *create_ul_ack(uint32_t fn, uint8_t ts); bool ctrl_ack_to_toggle(); bool handle_ctrl_ack(); @@ -547,7 +551,6 @@ * All states that need reset must be in this struct, so this is why * variables are in both (dl and ul) structs and not outside union. */ - gprs_rlc_ul_window m_window; int32_t m_rx_counter; /* count all received blocks */ uint8_t m_n3103; /* N3103 counter */ uint8_t m_usf[8]; /* list USFs per PDCH (timeslot) */ @@ -559,6 +562,13 @@ protected: void maybe_schedule_uplink_acknack(const gprs_rlc_data_info *rlc); + + /* Please note that all variables below will be reset when changing + * from WAIT RELEASE back to FLOW state (re-use of TBF). + * All states that need reset must be in this struct, so this is why + * variables are in both (dl and ul) structs and not outside union. + */ + gprs_rlc_ul_window m_window; }; #ifdef __cplusplus @@ -612,16 +622,6 @@ return static_cast<gprs_rlcmac_dl_tbf *>(tbf); else return NULL; -} - -inline gprs_rlc_window *gprs_rlcmac_tbf::window() -{ - switch (direction) - { - case GPRS_RLCMAC_UL_TBF: return &as_ul_tbf(this)->m_window; - case GPRS_RLCMAC_DL_TBF: return &as_dl_tbf(this)->m_window; - } - return NULL; } #endif diff --git a/src/tbf_dl.cpp b/src/tbf_dl.cpp index ad2f17f..c3d2c89 100644 --- a/src/tbf_dl.cpp +++ b/src/tbf_dl.cpp @@ -985,6 +985,11 @@ return lost * 100 / (lost + received); } +gprs_rlc_dl_window *gprs_rlcmac_dl_tbf::window() +{ + return &m_window; +} + int gprs_rlcmac_dl_tbf::update_window(unsigned first_bsn, const struct bitvec *rbb) { diff --git a/tests/tbf/TbfTest.cpp b/tests/tbf/TbfTest.cpp index 6ae0207..60a1ac3 100644 --- a/tests/tbf/TbfTest.cpp +++ b/tests/tbf/TbfTest.cpp @@ -297,9 +297,9 @@ /* Receive an ACK */ #define RCV_ACK(fin, tbf, rbb) do { \ - tbf->rcvd_dl_ack(fin, tbf->m_window.v_s(), rbb); \ + tbf->rcvd_dl_ack(fin, tbf->window()->v_s(), rbb); \ if (!fin) \ - OSMO_ASSERT(tbf->m_window.window_empty()); \ + OSMO_ASSERT(tbf->window()->window_empty()); \ } while(0) static void test_tbf_delayed_release() @@ -1725,7 +1725,7 @@ "Got MS: TLLI = 0x%08x, TA = %d\n", ms->tlli(), ms->ta()); send_dl_data(&the_bts, tlli, imsi, test_data, sizeof(test_data)); - ul_tbf->m_window.reset_v_x(); + ul_tbf->window()->reset_v_x(); /* Function to generate URBB with length */ ul_tbf = establish_ul_tbf_two_phase_puan_URBB_with_length(&the_bts, ts_no, tlli, &fn, qta, ms_class, egprs_ms_class, ul_tbf); @@ -1737,7 +1737,7 @@ send_dl_data(&the_bts, tlli, imsi, test_data, sizeof(test_data)); - ul_tbf->m_window.reset_v_x(); + ul_tbf->window()->reset_v_x(); /* Function to generate CRBB */ bts->ws_base = 128; bts->ws_pdch = 64; @@ -2529,7 +2529,7 @@ dl_tbf = create_dl_tbf(&the_bts, ms_class, egprs_ms_class, &trx_no); dl_tbf->update_ms(tlli, GPRS_RLCMAC_DL_TBF); - prlcdlwindow = &dl_tbf->m_window; + prlcdlwindow = dl_tbf->window(); prlcmvb = &prlcdlwindow->m_v_b; prlcdlwindow->m_v_s = 1288; prlcdlwindow->m_v_a = 1176; @@ -2561,7 +2561,7 @@ Decoding::decode_egprs_acknack_bits( &ack_nack->EGPRS_AckNack.Desc, &bits, - &bsn_begin, &bsn_end, &dl_tbf->m_window); + &bsn_begin, &bsn_end, dl_tbf->window()); dl_tbf->rcvd_dl_ack( ack_nack->EGPRS_AckNack.Desc.FINAL_ACK_INDICATION, @@ -2755,17 +2755,17 @@ } #define NACK(tbf, x) do { \ - tbf->m_window.m_v_b.mark_nacked(x); \ - OSMO_ASSERT(tbf->m_window.m_v_b.is_nacked(x)); \ + tbf->window()->m_v_b.mark_nacked(x); \ + OSMO_ASSERT(tbf->window()->m_v_b.is_nacked(x)); \ } while(0) #define CHECK_UNACKED(tbf, cs, bsn) do { \ - OSMO_ASSERT(tbf->m_window.m_v_b.is_unacked(bsn)); \ + OSMO_ASSERT(tbf->window()->m_v_b.is_unacked(bsn)); \ OSMO_ASSERT(tbf->m_rlc.block(bsn)->cs_current_trans.to_num() == cs); \ } while(0) #define CHECK_NACKED(tbf, cs, bsn) do { \ - OSMO_ASSERT(tbf->m_window.m_v_b.is_nacked(bsn)); \ + OSMO_ASSERT(tbf->window()->m_v_b.is_nacked(bsn)); \ OSMO_ASSERT(tbf->m_rlc.block(bsn)->cs_current_trans.to_num() == cs); \ } while(0) -- To view, visit https://gerrit.osmocom.org/5340 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I0b55aa8947db65f7206adcf53ea32b74a831d9e6 Gerrit-PatchSet: 1 Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Owner: Max <msuraev at sysmocom.de>