January 2023 - gerrit-log - lists.osmocom.org

Change in osmo-pcu[master]: bts: refuse to set invalid frame numbers

by fixeria

Attention is currently required from: pespin, dexter. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/30863 ) Change subject: bts: refuse to set invalid frame numbers ...................................................................... Patch Set 1: Code-Review-1 (2 comments) Patchset: PS1: > I meant osmo_panic() actually. IMO, OSMO_ASSERT() would be enough as it gives enough context. File src/bts.cpp: https://gerrit.osmocom.org/c/osmo-pcu/+/30863/comment/c867a6de_0be6cd00 PS1, Line 342: fn > GSM_TDMA_HYPERFRAME According to 3GPP TS 45.002, section 4.3.3: "The frame number shall be cyclic and shall have a range of 0 to FN_MAX where FN_MAX = (26 x 51 x 2048) -1 = 2715647 as defined in 3GPP TS 45.010". GSM_TDMA_HYPERFRAME = (26 x 51 x 2048) = 2715648, without the -1, so it's not a valid TDMA Fn value. Your condition is not entirely correct, because it permits this value. Simply do OSMO_ASSERT(fn < GSM_TDMA_HYPERFRAME). -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/30863 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: Iaae31b370fababba975d419b0d20ac15618c296e Gerrit-Change-Number: 30863 Gerrit-PatchSet: 1 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Thu, 05 Jan 2023 13:01:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

2 years, 5 months

1
0
0 0

Change in osmo-pcu[master]: bts: refuse to set invalid frame numbers

by pespin

Attention is currently required from: fixeria, dexter. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/30863 ) Change subject: bts: refuse to set invalid frame numbers ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > Maybe simple OSMO_ASSERT(). […] I meant osmo_panic() actually. -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/30863 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: Iaae31b370fababba975d419b0d20ac15618c296e Gerrit-Change-Number: 30863 Gerrit-PatchSet: 1 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Thu, 05 Jan 2023 12:45:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

2 years, 5 months

1
0
0 0

Change in osmo-pcu[master]: bts: refuse to set invalid frame numbers

by pespin

Attention is currently required from: fixeria, dexter. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/30863 ) Change subject: bts: refuse to set invalid frame numbers ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: Maybe simple OSMO_ASSERT(). if that situation is hit it seems something is terribly wrong? -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/30863 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: Iaae31b370fababba975d419b0d20ac15618c296e Gerrit-Change-Number: 30863 Gerrit-PatchSet: 1 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Thu, 05 Jan 2023 12:44:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

2 years, 5 months

1
0
0 0

Change in osmo-pcu[master]: bts: refuse to set invalid frame numbers

by pespin

Attention is currently required from: fixeria, dexter. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/30863 ) Change subject: bts: refuse to set invalid frame numbers ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/30863 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: Iaae31b370fababba975d419b0d20ac15618c296e Gerrit-Change-Number: 30863 Gerrit-PatchSet: 1 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Thu, 05 Jan 2023 12:44:22 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 5 months

1
0
0 0

Change in osmo-pcu[master]: bts: log FN jump delta in bts_set_current_frame_number()

by pespin

Attention is currently required from: dexter. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/30865 ) Change subject: bts: log FN jump delta in bts_set_current_frame_number() ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/30865 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: Ie361df30852570fe8a47347a42e962db869ccf82 Gerrit-Change-Number: 30865 Gerrit-PatchSet: 2 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Thu, 05 Jan 2023 12:42:43 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 5 months

1
0
0 0

Change in osmo-bsc[master]: HACK: enable PDCH after OML is done

by dexter

dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/30748 ) Change subject: HACK: enable PDCH after OML is done ...................................................................... Patch Set 3: This change is ready for review. -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/30748 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: If5251b102c8aa45dfc8cc4ee4e0223d7dc438938 Gerrit-Change-Number: 30748 Gerrit-PatchSet: 3 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Comment-Date: Thu, 05 Jan 2023 11:35:07 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

2 years, 5 months

1
0
0 0

Change in osmo-pcu[master]: bts: log FN jump delta in bts_set_current_frame_number()

by dexter

Attention is currently required from: dexter. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-pcu/+/30865 to look at the new patch set (#2). Change subject: bts: log FN jump delta in bts_set_current_frame_number() ...................................................................... bts: log FN jump delta in bts_set_current_frame_number() In case of an FN jump the expected value is logged. Lets also log the delta between the expected and the current FN as it may give a better clue what goes wrong Change-Id: Ie361df30852570fe8a47347a42e962db869ccf82 --- M src/bts.cpp M tests/tbf/TbfTest.err 2 files changed, 64 insertions(+), 64 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/65/30865/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/30865 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: Ie361df30852570fe8a47347a42e962db869ccf82 Gerrit-Change-Number: 30865 Gerrit-PatchSet: 2 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: newpatchset

2 years, 5 months

1
0
0 0

Change in osmo-pcu[master]: bts: log FN jump delta in bts_set_current_frame_number()

by dexter

dexter has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-pcu/+/30865 ) Change subject: bts: log FN jump delta in bts_set_current_frame_number() ...................................................................... bts: log FN jump delta in bts_set_current_frame_number() In case of an FN jump the expected value is logged. Lets also log the delta between the expected and the current FN as it may give a better clue what goes wrong Change-Id: Ie361df30852570fe8a47347a42e962db869ccf82 --- M src/bts.cpp 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/65/30865/1 diff --git a/src/bts.cpp b/src/bts.cpp index f2fb10e..c5a45f1 100644 --- a/src/bts.cpp +++ b/src/bts.cpp @@ -352,8 +352,8 @@ * and start of another frame (every 3 blocks). */ if (fn != bts->cur_fn && bts->cur_fn != FN_UNSET && fn != fn_next_block(bts->cur_fn)) { LOGP(DRLCMAC, LOGL_NOTICE, - "Detected FN jump! %u -> %u (expected %u)\n", - bts->cur_fn, fn, fn_next_block(bts->cur_fn)); + "Detected FN jump! %u -> %u (expected %u, delta %u)\n", + bts->cur_fn, fn, fn_next_block(bts->cur_fn), GSM_TDMA_FN_DIFF(bts->cur_fn, fn)); } bts->cur_fn = fn; } -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/30865 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: Ie361df30852570fe8a47347a42e962db869ccf82 Gerrit-Change-Number: 30865 Gerrit-PatchSet: 1 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 5 months

1
0
0 0

Change in osmo-pcu[master]: bts: use GSM_TDMA_FN_ macros and uint32_t in bts_rfn_to_fn

by dexter

dexter has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-pcu/+/30864 ) Change subject: bts: use GSM_TDMA_FN_ macros and uint32_t in bts_rfn_to_fn ...................................................................... bts: use GSM_TDMA_FN_ macros and uint32_t in bts_rfn_to_fn The function bts_rfn_to_fn() uses int32_t for its internal variables and the input parameter rfn while the callers and everything outside uses uint32_t to store frame numbers. Lets convert this to uint32_t and use GSM_TDMA_FN_ macros wherever possible. Change-Id: Iedd493bb30dd1c342dec031883060c545432e740 Related: OS#5198 --- M src/bts.cpp M src/bts.h 2 files changed, 12 insertions(+), 14 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/64/30864/1 diff --git a/src/bts.cpp b/src/bts.cpp index 069758b..f2fb10e 100644 --- a/src/bts.cpp +++ b/src/bts.cpp @@ -33,6 +33,7 @@ #include <gprs_ms_storage.h> #include <sba.h> #include <bts_pch_timer.h> +#include <osmocom/gsm/gsm0502.h> extern "C" { #include <osmocom/core/talloc.h> @@ -720,15 +721,14 @@ } /* Determine the full frame number from a relative frame number */ -uint32_t bts_rfn_to_fn(const struct gprs_rlcmac_bts *bts, int32_t rfn) +uint32_t bts_rfn_to_fn(const struct gprs_rlcmac_bts *bts, uint32_t rfn) { - int32_t m_cur_rfn; - int32_t fn; - int32_t fn_rounded; + uint32_t m_cur_rfn; + uint32_t fn_rounded; - /* double-check that relative FN is not negative and fits into int32_t */ + /* make sure RFN does not exceed the maximum possible value of a valid + * GSM frame number. */ OSMO_ASSERT(rfn < GSM_MAX_FN); - OSMO_ASSERT(rfn >= 0); /* Note: If a BTS is sending in a rach request it will be fully aware * of the frame number. If the PCU is used in a BSC-co-located setup. @@ -748,13 +748,13 @@ /* Compute a "rounded" version of the internal frame number, which * exactly fits in the RFN_MODULUS raster */ - fn_rounded = bts->cur_fn - m_cur_rfn; + fn_rounded = GSM_TDMA_FN_SUB(bts->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 * rach request belongs to a the previous rfn period. To correct this, * we roll back the rounded frame number by one RFN_MODULUS */ - if (abs(rfn - m_cur_rfn) > RFN_THRESHOLD) { + 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); @@ -762,17 +762,15 @@ LOGP(DRLCMAC, LOGL_DEBUG, "Cornercase detected: wrapping crosses %u border\n", GSM_MAX_FN); - fn_rounded = GSM_MAX_FN - (RFN_MODULUS - fn_rounded); + fn_rounded = GSM_TDMA_FN_SUB(GSM_MAX_FN, (GSM_TDMA_FN_SUB(RFN_MODULUS, fn_rounded))); } else - fn_rounded -= RFN_MODULUS; + fn_rounded = GSM_TDMA_FN_SUB(fn_rounded, RFN_MODULUS); } /* The real frame number is the sum of the rounded frame number and the * relative framenumber computed via RACH */ - fn = fn_rounded + rfn; - - return fn; + return GSM_TDMA_FN_SUM(fn_rounded, rfn); } /* 3GPP TS 44.060: diff --git a/src/bts.h b/src/bts.h index 085b448..61c5e43 100644 --- a/src/bts.h +++ b/src/bts.h @@ -296,7 +296,7 @@ struct GprsMs *bts_alloc_ms(struct gprs_rlcmac_bts *bts, uint8_t ms_class, uint8_t egprs_ms_class); int bts_add_paging(struct gprs_rlcmac_bts *bts, const struct paging_req_cs *req, struct GprsMs *ms); -uint32_t bts_rfn_to_fn(const struct gprs_rlcmac_bts *bts, int32_t rfn); +uint32_t bts_rfn_to_fn(const struct gprs_rlcmac_bts *bts, uint32_t rfn); struct gprs_rlcmac_dl_tbf *bts_dl_tbf_by_tfi(struct gprs_rlcmac_bts *bts, uint8_t tfi, uint8_t trx, uint8_t ts); struct gprs_rlcmac_ul_tbf *bts_ul_tbf_by_tfi(struct gprs_rlcmac_bts *bts, uint8_t tfi, uint8_t trx, uint8_t ts); -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/30864 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: Iedd493bb30dd1c342dec031883060c545432e740 Gerrit-Change-Number: 30864 Gerrit-PatchSet: 1 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 5 months

1
0
0 0

Change in osmo-pcu[master]: bts: refuse to set invalid frame numbers

by dexter

dexter has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-pcu/+/30863 ) Change subject: bts: refuse to set invalid frame numbers ...................................................................... bts: refuse to set invalid frame numbers A valid GSM frame ranges from 0 to 2715647. When using set_current_frame_number() to set the current frame number (source usually is the layer 1 and below) we should not allow invalid frame numbers. Note: this also fixes FnTest which uses invalid frame numbers for testsing. Change-Id: Iaae31b370fababba975d419b0d20ac15618c296e Related: OS#5198 --- M src/bts.cpp M tests/fn/FnTest.cpp M tests/fn/FnTest.ok 3 files changed, 21 insertions(+), 14 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/63/30863/1 diff --git a/src/bts.cpp b/src/bts.cpp index cf39aa4..069758b 100644 --- a/src/bts.cpp +++ b/src/bts.cpp @@ -338,6 +338,13 @@ void bts_set_current_frame_number(struct gprs_rlcmac_bts *bts, uint32_t fn) { + /* Refuse to set frame numbers that exceed the valid range */ + if (fn > GSM_TDMA_HYPERFRAME) { + LOGP(DRLCMAC, LOGL_ERROR, + "cannot set frame number (%u) exceeds valid range (0-%u)\n", fn, GSM_TDMA_HYPERFRAME); + return; + } + /* The UL frame numbers lag 3 behind the DL frames and the data * indication is only sent after all 4 frames of the block have been * received. Sometimes there is an idle frame between the end of one diff --git a/tests/fn/FnTest.cpp b/tests/fn/FnTest.cpp index e5ac46d..ae15164 100644 --- a/tests/fn/FnTest.cpp +++ b/tests/fn/FnTest.cpp @@ -92,13 +92,13 @@ fn = calc_fn(bts, RFN_MODULUS - 1); OSMO_ASSERT(fn == 42431); - set_fn(bts, RFN_MODULUS * 123 + 16); + set_fn(bts, RFN_MODULUS * 12 + 16); fn = calc_fn(bts, RFN_MODULUS - 4); - OSMO_ASSERT(fn == 5219132); + OSMO_ASSERT(fn == 509180); - set_fn(bts, RFN_MODULUS * 123 + 451); + set_fn(bts, RFN_MODULUS * 12 + 451); fn = calc_fn(bts, RFN_MODULUS - 175); - OSMO_ASSERT(fn == 5218961); + OSMO_ASSERT(fn == 509009); /* Lets check a special cornercase. We assume that @@ -125,7 +125,7 @@ /* Also check with some corner case * values where Fn and RFn reach its * maximum/minimum valid range */ - set_fn(bts, GSM_MAX_FN); + set_fn(bts, GSM_MAX_FN-1); fn = calc_fn(bts, RFN_MODULUS-1); OSMO_ASSERT(fn == GSM_MAX_FN-1); @@ -133,9 +133,9 @@ fn = calc_fn(bts, RFN_MODULUS-1); OSMO_ASSERT(fn == GSM_MAX_FN-1); - set_fn(bts, GSM_MAX_FN); + set_fn(bts, GSM_MAX_FN-1); fn = calc_fn(bts, 0); - OSMO_ASSERT(fn == GSM_MAX_FN); + OSMO_ASSERT(fn == GSM_MAX_FN-RFN_MODULUS*2); set_fn(bts, 0); fn = calc_fn(bts, 0); diff --git a/tests/fn/FnTest.ok b/tests/fn/FnTest.ok index be6400f..4884ca5 100644 --- a/tests/fn/FnTest.ok +++ b/tests/fn/FnTest.ok @@ -19,11 +19,11 @@ bts: fn=42433 rfn=42431 ==> fn=42431 -bts: fn=5219152 -rfn=42428 ==> fn=5219132 +bts: fn=509200 +rfn=42428 ==> fn=509180 -bts: fn=5219587 -rfn=42257 ==> fn=5218961 +bts: fn=509635 +rfn=42257 ==> fn=509009 bts: fn=0 rfn=42419 ==> fn=2715635 @@ -37,14 +37,14 @@ bts: fn=23 rfn=42390 ==> fn=2715606 -bts: fn=2715648 +bts: fn=2715647 rfn=42431 ==> fn=2715647 bts: fn=0 rfn=42431 ==> fn=2715647 -bts: fn=2715648 -rfn=0 ==> fn=2715648 +bts: fn=2715647 +rfn=0 ==> fn=2630784 bts: fn=0 rfn=0 ==> fn=0 -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/30863 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: Iaae31b370fababba975d419b0d20ac15618c296e Gerrit-Change-Number: 30863 Gerrit-PatchSet: 1 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 5 months

1
0
0 0

2025

2024

2023

2022

gerrit-log January 2023