Attention is currently required from: pespin.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/33157 )
Change subject: Use OSMO_UNLIKELY() in bts_rfn_to_fn()
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/33157
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I4c71f3481764b501a4441bc735a87725884a3e75
Gerrit-Change-Number: 33157
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 02 Jun 2023 14:34:43 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: pespin.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/33154 )
Change subject: Move call to bts_set_current_frame_number() earlier in the code path
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/33154
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: If36f22a1067c904fa7fda87bed5062b6738f0dd1
Gerrit-Change-Number: 33154
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 02 Jun 2023 14:34:35 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: pespin.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/33138 )
Change subject: Use fn_valid() helper in pcu_rx_time_ind()
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/33138
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I5b1f1d4cd621d81fb99b87761a878af242227a10
Gerrit-Change-Number: 33138
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 02 Jun 2023 13:47:59 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: fixeria.
Hello Jenkins Builder, fixeria,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-pcu/+/32339
to look at the new patch set (#3).
Change subject: Fix DL_TBF PACCH ass done on UL_TBF already scheduled to tx last PKT CTRL ACK
......................................................................
Fix DL_TBF PACCH ass done on UL_TBF already scheduled to tx last PKT CTRL ACK
Dispatching TBF_EV_CONTENTION_RESOLUTION_MS_SUCCESS means the UL TBF is
able to be used to assign DL TBFs over PACCH.
However, there's an extra implicit restriction: if the PCU already sent
the final UL ACK/NACK to that UL TBF, then whatever message sent
after it cannot be reliably answered, since the MS will go back to idle
state after issues the PKT CTRL ACK for that final UL ACK/NACK.
This condition is already being checked in
contention_resolution_success()():
"""
if (ms_need_dl_tbf(ms()) && !tbf_ul_ack_waiting_cnf_final_ack(this))
ms_new_dl_tbf_assigned_on_pacch(ms(), this);
"""
Since we are considering the UL TBF to have done contention resolution
when we transmit the *first* UL ACK/NACK, it mans we are doing both things
on the same code path iif the *first* UL ACK/NACK is at the same time
the *final* UL ACK for that UL TBF. This can happen if the UL TBF is
only sending 1 block, which will have CV=0. This can usually happen
during GMM Attach Request.
In this scenario, with current code goes into a situation where first
the TBF_EV_CONTENTION_RESOLUTION_MS_SUCCESS is triggered and *afterwards*
the UL_ACK/NACK state is updated. As a result, the code snippet check
above (!tbf_ul_ack_waiting_cnf_final_ack()) will be true and the DL TBF
be assigned, but afterwards in the same code path it figures out the
final ack happens.
In order to fix this, first update the ACK/NACK state and only
afterwards trigger he Contention Resolution Success event.
Change-Id: I62ae91b494e4fd0ade3f4a3ba3817bcaedbdebf5
---
M src/tbf_ul_ack_fsm.c
M tests/tbf/TbfTest.err
2 files changed, 64 insertions(+), 18 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/39/32339/3
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/32339
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I62ae91b494e4fd0ade3f4a3ba3817bcaedbdebf5
Gerrit-Change-Number: 32339
Gerrit-PatchSet: 3
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: newpatchset
pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/33120 )
Change subject: Error trying to obtain FN from RFN if curr_fn not known
......................................................................
Error trying to obtain FN from RFN if curr_fn not known
This may happen if a RACH.ind is received before any DATA.ind has been
received.
With usual osmo-bts-trx or osmo-bts-sysmo, this shouldn't happen
nowadays, but it is still a problem with osmo-bts-virtual, where lower
layers don't submit NOPE.ind in the absence of data, and hence it won't
sent DATA.ind to osmo-pcu all the time.
This change helps in showcasing confusing scenarios where the RFN
generated in the Imm Ass was wrong.
Change-Id: I29b7ba828fe890f90e35686bbb04d4abfe56b955
---
M src/bts.cpp
1 file changed, 30 insertions(+), 4 deletions(-)
Approvals:
fixeria: Looks good to me, but someone else must approve
Jenkins Builder: Verified
laforge: Looks good to me, but someone else must approve
pespin: Looks good to me, approved
diff --git a/src/bts.cpp b/src/bts.cpp
index 8d6f156..6cac93f 100644
--- a/src/bts.cpp
+++ b/src/bts.cpp
@@ -740,20 +740,27 @@
/* Determine the full frame number from a relative frame number */
uint32_t bts_rfn_to_fn(const struct gprs_rlcmac_bts *bts, uint32_t rfn)
{
- uint32_t m_cur_rfn;
+ uint32_t m_cur_fn, m_cur_rfn;
uint32_t fn_rounded;
/* Ensure that all following calculations are performed with the
* relative frame number */
OSMO_ASSERT(rfn < RFN_MODULUS);
+ m_cur_fn = bts_current_frame_number(bts);
+ if (m_cur_fn == FN_UNSET) {
+ LOGP(DRLCMAC, LOGL_ERROR, "Unable to calculate full FN from RFN %u: Current FN not known!\n",
+ rfn);
+ return rfn;
+ }
+
/* Compute an internal relative frame number from the full internal
frame number */
- m_cur_rfn = fn2rfn(bts->cur_fn);
+ m_cur_rfn = fn2rfn(m_cur_fn);
/* Compute a "rounded" version of the internal frame number, which
* exactly fits in the RFN_MODULUS raster */
- fn_rounded = GSM_TDMA_FN_SUB(bts->cur_fn, m_cur_rfn);
+ fn_rounded = GSM_TDMA_FN_SUB(m_cur_fn, m_cur_rfn);
/* If the delta between the internal and the external relative frame
* number exceeds a certain limit, we need to assume that the incoming
@@ -762,7 +769,7 @@
if (GSM_TDMA_FN_DIFF(rfn, m_cur_rfn) > RFN_THRESHOLD) {
LOGP(DRLCMAC, LOGL_DEBUG,
"Race condition between rfn (%u) and m_cur_fn (%u) detected: rfn belongs to the previous modulus %u cycle, wrapping...\n",
- rfn, bts->cur_fn, RFN_MODULUS);
+ rfn, m_cur_fn, RFN_MODULUS);
if (fn_rounded < RFN_MODULUS) {
LOGP(DRLCMAC, LOGL_DEBUG,
"Cornercase detected: wrapping crosses %u border\n",
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/33120
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I29b7ba828fe890f90e35686bbb04d4abfe56b955
Gerrit-Change-Number: 33120
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: merged