Commit Graph

1672 Commits

Author SHA1 Message Date
Pau Espin 0f88bcdebf bts: Use public getter instead of class member
Change-Id: Ia7b37a4c721d7d02c516d8d3a5417d166f1d3bec
2021-08-31 14:56:17 +02:00
Pau Espin 767144f7b7 cosmetic: sysmo: Drop unneded comment line
Change-Id: I66685fa69af5ad18e45d97a25e7150815beef805
2021-08-31 14:53:10 +02:00
Pau Espin e376fd57cf Use LOGPDCH macro to standarize log line
Change-Id: If815779b2e2e56707f36c72dbdbfd4c5b07165ed
2021-08-31 14:38:21 +02:00
Pau Espin 3bbb3cc1f2 Fix crash with dyn TS when using direct pcu
It seems there may be a race conditon where lower layers (direct PCU)
send UL blocks to us while the PDCH was already disabled (due to a call
entering on a dynamic TS).
As the PDCH is disabled, the ULC is NULL and shouldn't be used before
being enabled again.

Related: OS#5222
Change-Id: I4b8931f0cc7cfc787a1cc35196295402524b15c3
2021-08-31 14:36:02 +02:00
Pau Espin a89a23881f sched: energy saving: Avoid Tx dummy blocks on empty PDCH TS
Related: SYS#4919
Related: OS#4772
Change-Id: I8d66dd5e838748611e7b77b504fc86295f02c019
2021-08-26 12:51:15 +02:00
Pau Espin c4fe1f97b4 cosmetic: Fix typo in comment
Change-Id: I423410416e572141fc2b44c81215b8f41c13f2c6
2021-08-25 14:35:27 +02:00
Pau Espin d06ec27856 fix typo 's/dowlink/downlink/g'
Change-Id: Iae66aff9eed3856f09e58116ee26ec061733b076
2021-08-23 17:14:23 +02:00
Pau Espin f48de627f4 tbf: Move T3193 to tbf_state FSM
Related: OS#2709
Change-Id: Icf8249651e34132eb7ba99188a23662dec6f8653
2021-08-23 17:14:23 +02:00
Pau Espin a161bf48bd Simplify tbf::set_polling()
When setting a POLL, it will always happen on PACCH, so all the CCCH
part makes no sense there. Let's drop it and move the logging of each
case to the caller, where logging file+line is more useful.

Change-Id: I242f97fd6f927131ac64c1a7c9c3812b6389de04
2021-08-23 17:14:23 +02:00
Pau Espin ea8dbddab1 Move tbf ul_ack_state to osmocom FSM
Related: OS#2709
Change-Id: Icf23bf5a4b85fbcbf1542cebceb76b9ba7185d30
2021-08-23 17:14:23 +02:00
Pau Espin 3e48cfd9f3 tbf.h: Improve documentation on several flags
Change-Id: Ice2c164ced039fb4ab621d8f7c2fb85f8348788a
2021-08-23 17:14:23 +02:00
Pau Espin 405d2d10ec tbf_dl: Clarify requirements for DL ACK/NACK
Method is renamed since it clearly relates to getting DL ACK/NACK, no
CTRL ACK.

use same methods in both scheduler and internal use since they are
expectd to be run in the same code path by the scheduler. This way we
make sure the same conditions apply and it's clearer when looking at
the code.

Change-Id: Ib0e9b9547f5292b95064bab2dc182fdf659f0518
2021-08-23 17:14:23 +02:00
Pau Espin 3225290d77 Move timer X2002 to tbf_fsm
Related: OS#2709
Change-Id: I94b71c60ed49d51ebdf6d6b428056b4b94354676
2021-08-23 17:14:23 +02:00
Pau Espin 907f037339 tbf: Use type bool for upgrade_to_multislot
Change-Id: I644d91b6230a90cc72e83443c11d24b8d0a2dcac
2021-08-23 17:14:23 +02:00
Pau Espin 628d881247 Fix typos in comments documenting fsm st chg macro
Change-Id: I8f1cef5810c84441f7d6d2fbe5b3106e0ae71b69
2021-08-23 17:14:23 +02:00
Pau Espin fbc1baa139 tbf: Merge handle_ack_nack() into rcvd_dl_ack()
There's no real use in having those 2 methods separately, and only adds
complexity. Let's merge it to have 1 TBF code path handling DL ACK/NACK.

Change-Id: I546d2e46bda96a2f551b28673464e57831c71828
2021-08-23 17:14:23 +02:00
Pau Espin afe189e802 Get rid of lots of code only used by tests
There are 2 methods "rcvd_dl_ack()" in osmo-pcu code. One is used by
osmo-pcu itself, and the other is only used in tests.
Changing the tests to use the same method as osmo-pcu allows removing
the second one, and with it, a lot of code and complexity out of
osmo-pcu.

Change-Id: I14d9312cb61534dc97fca83141b9c0cd933c9206
2021-08-23 17:14:23 +02:00
Pau Espin 9d67e72e85 Move timer X2001 to tbf_fsm
The side effect is that the timer is enabled for other scenarios where a
PACCH assignment happens, like an Assignment Reject or Ul Assignment
(that's why there's more lines showing up now in TbfTest.err).

Change-Id: Ib8ab2f7397ad05c6fcd5dd74af55a1e2c56e1463
2021-08-23 17:14:22 +02:00
Pau Espin c65c9e56e1 tbf: Drop unuseful flag GPRS_RLCMAC_FLAG_UL_DATA
Same information is available under ul_tbf->m_rx_counter.

Change-Id: I1d993117c7daa2609b132c2d0fd748e0338ef559
2021-08-23 17:14:22 +02:00
Pau Espin 5bc6560efc tbf: Drop unuseful flag GPRS_RLCMAC_FLAG_TO_DL_ASS
The flag is only used to print some non interesting stuff, let's drop it
in order to simplify code. We can add later whatever we want in the new
shiny FSM.

Change-Id: I13f92f058c219f230d57b3c00b8ae1d187603813
2021-08-23 17:14:22 +02:00
Pau Espin 49a2f404e8 replace dl_ass_state with osmocom FSM
Related: OS#2709
Change-Id: Ia33418478e17986a316ffda48b091030f53fa371
2021-08-23 17:14:22 +02:00
Pau Espin 432d4f3b89 tbf: Drop unuseful flag GPRS_RLCMAC_FLAG_TO_UL_ASS
The flag is only used to print some uninteresting stuff, let's drop it
in order to simplify code. We can add later whatever we want in the new
shiny FSM.

Change-Id: I20aa7f83cc4f32de129e64c74a91745b983a7b16
2021-08-23 17:14:22 +02:00
Pau Espin ab8fba3a20 tbf: Reimplement rlcmac_diag() and make it available from C
We never use the std:string anyway, we always call .c_str() to log using
osmocom logging system.
Furthermore, we'll need to use it from C code soon (next commit).

Change-Id: I3ad66f9f3f4d55d11da3a3b8b38656ae2dd50603
2021-08-23 17:14:22 +02:00
Pau Espin 6ad11a6990 Replace ul_ass_state with osmocom FSM
Related: OS#2709
Change-Id: Id414eafe9c04a9a8759c6fb1a483bf2ee093a4d2
2021-08-23 17:14:22 +02:00
Pau Espin b0ead922a1 tbf_free: Get rid of uneeded tbf_state transition
We are freeing the object immediately afterwards anyway, so no need to
pretend it went through the normal state release.
Leaving current state as it is actually provides more information on
what was the status/state at the time the TBF had to be freed.

Change-Id: I3016caaccc2c43e1e300f3c6042d69f8adcd9d69
2021-08-23 17:14:22 +02:00
Pau Espin 284711d627 Get rid of tbf_dl:abort()
Having that code in a separate function is confusing and adds code
complexity since it looks like an entry point to start feeing a TBF, but
it simply some (not yet really useful) set of instructions to be called
one 1 code path in tbf_free.
Let's move it there, this way it becomes clear tbf_free() is THE place
to be (if you want to get rid of a TBF).

Change-Id: I30febf4d21a0bfab37524c07598bbb0dd32f7f65
2021-08-23 17:14:22 +02:00
Pau Espin 131deb059f Move rate_ctr free to tbf subclass destructor
This way we clean up tbf_free entry point, and leave memory freeing for
later on at the end when talloc_free is called.

Change-Id: I1c45e3296e565725bcbbca391d9518772fffa89d
2021-08-23 17:14:22 +02:00
Pau Espin 9dacf0b35b Remove duplicate call to gprs_rlcmac_lost_rep
Function is already called by gprs_rlcmac_received_lost(), so next call
following it will be sum=0 and return EINVAL.

Change-Id: I015ba16d18fdd6e2441ec3c256b5ac88771d7a8b
2021-08-23 17:14:22 +02:00
Pau Espin 8c4f978483 Drop logging last mas report before freeing TBF
There's no much use in logging it since anyway we are immediately
getting rid of it.

Change-Id: I9b712f720b5874886cc19d998fb8fcd0e618d590
2021-08-23 17:14:21 +02:00
Pau Espin 62e06f92e9 Put dl_tbf::cleanup into destructor
It's fine to always attemt dropping the timer since it's set up in the
constructor.
This also drps the double function call abort()+cleanup() which is
confusing.

Change-Id: Ia2aaa43bd8faacf09fe4b36b11b38022bea7a59c
2021-08-23 17:14:21 +02:00
Pau Espin f45ede640b Drop duplicate log line
Same line (or similar if run_diag) is logged immediately below, showing
up twice in log which is confusing:

"""
20210726171543005 DTBF tbf.cpp:455 TBF(TFI=2 TLLI=0xfe563576 DIR=DL STATE=WAIT_RELEASE EGPRS) T3193 timeout expired, freeing TBF
20210726171543005 DTBF tbf.cpp:462 TBF(TFI=2 TLLI=0xfe563576 DIR=DL STATE=WAIT_RELEASE EGPRS) T3193 timeout expired, freeing TBF
"""

Change-Id: Ie171c458e670f8471ac93f78520a05926114c974
2021-08-23 17:14:21 +02:00
Pau Espin cfb61d9536 Move T3169 and T3195 to tbf_fsm
Change-Id: I599f4e7e82b0a8c0f5cf633c2d8b1975435f0b60
2021-08-23 17:14:21 +02:00
Pau Espin 55f600b702 Move RELEASING tbf_state transition to tbf_fsm
PdchUlcTest output changes because the original state NULL is not
expected when transactioning to RELEASING upon MAX N310* being hit. In
any case, none of those events should happen in NULL state, but we
don't really care about TBF states there so we are fine with whatever
the state is.

Related: OS#2709
Change-Id: I516b8d989a0d705e5664f8aeaf7d108e0105aa16
2021-08-23 17:14:21 +02:00
Pau Espin efcb046ce1 Move WAIT_RELEASE tbf_state transition to tbf_fsm
While at it, method maybe_start_new_window is renamed to
rcvd_dl_final_ack to make more sense out of the code.

Related: OS#2709
Change-Id: Iebd650c1036ef2d5132789778be7117ce3391c01
2021-08-23 17:14:21 +02:00
Pau Espin c32c4a3648 Move FINISHED tbf_state transition to tbf_fsm
Related: OS#2709
Change-Id: I81f507e3a2821254f03364a58ead02333e63099f
2021-08-23 17:14:21 +02:00
Pau Espin 720e19e7f3 Move FLOW tbf_state transition to tbf_fsm.
Related: OS#2709
Change-Id: Ia8c7de759c195d09263fb1f083fbf6cfa3087f8d
2021-08-23 17:14:21 +02:00
Pau Espin 33e8007100 Move NULL and ASSIGN tbf_state transition to tbf_fsm
At some point later in time the state_flags will most probably be split
into different variables, one ending up in a different FSM. It is moved
so far to the exsiting FSM from the C++ class since it's easier to
access it from C and C++ code, and anyway that kind of information
belongs to the FSM.

Related: OS#2709
Change-Id: I3c62e9e83965cb28065338733f182863e54d7474
2021-08-23 17:14:21 +02:00
Pau Espin 88f34812df Revert "Revert "Stop abusing T3169""
This reverts commit 112c63e9b4.

Change-Id: Ic18674ccd38f81ddd46e1ec733159df350991899
2021-08-23 17:14:19 +02:00
Pau Espin b5fece959f Revert "fix: handle NULL return of as_dl_tbf() and as_ul_tbf()"
This reverts commit d8e8ea9c8f.

Change-Id: I8000e78515b25b9be5c28a249bde330dac915dcb
2021-08-23 17:14:17 +02:00
Pau Espin bc139a4af4 Revert "coverity: fix null deref from recent UL TBF leak fix"
This reverts commit 3bd6488889.

Change-Id: I59c4ae726286216850ad9b53fee34ab4bda5630f
2021-08-23 17:14:15 +02:00
Oliver Smith 3f79470453 bts: delete pch_timer list in destructor
Run bts_pch_timer_remove() on each entry of the BTS specific pch_timer
list, so we don't have a memory leak and so the timer doesn't
potentially fire for a deallocated BTS.

Fixes: d3c7591 ("Add counters: pcu.bts.N.pch.requests.timeout")
Change-Id: Ia5e33d1894408e93a51c452002ef2f5758808269
2021-08-23 14:51:18 +02:00
Neels Hofmeyr 3bd6488889 coverity: fix null deref from recent UL TBF leak fix
Fix a possible NULL deref, introduced in recent patch
I8ce21be6836549b47a606c00b793d6f005964c5c /
d8e8ea9c8f

Related: OS#5205 SYS#5561 CID#239246
Change-Id: I603d4a5bc0fe5bd2e9f0dba171604c459e38aeaf
2021-08-18 18:23:44 +02:00
Neels Hofmeyr d8e8ea9c8f fix: handle NULL return of as_dl_tbf() and as_ul_tbf()
Go through all callers of as_dl_tbf() and as_ul_tbf(), and make sure
they can handle the possible NULL return value.

OS#5205 reports a NULL deref crash of osmo-pcu at pdch.cpp:525. The
immediate cause is that as_dl_tbf() may well return NULL, which this
caller does not handle and instead dereferences immediately.
This is a code path that apparently assumes that a DL-TBF should always
be present. The higher level cause for the NULL DL-TBF has not been
identified.

Related: OS#5205 SYS#5561
Change-Id: I8ce21be6836549b47a606c00b793d6f005964c5c
2021-08-17 12:17:13 +00:00
Neels Hofmeyr 112c63e9b4 Revert "Stop abusing T3169"
This reverts commit 846fd248dc.

The commit introduced a leak of UL-TBF, which do not time out and
accumulate indefinitely, leading to out-of-memory for the running
osmo-pcu process.

A proper fix for the leak is pending on a development branch pespin/fsm,
but that branch is not yet ready for merging. Hence let's re-introduce
timer T3169 to avoid the OOM due to lingering UL-TBF.

Related: OS#5209
Change-Id: I99a7d2ddf68a76739ce2db1d6a44967dd97667b0
2021-08-15 17:46:53 +00:00
Neels Hofmeyr 4163361906 T_defs_bts: remove unit from doc strings
The main reason to change this is that the unit for T3172 is wrong. It
is defined as ms but the doc string says "(s)".

The tdef implementation already includes the unit as defined for each T
in the doc string implicitly, so instead of fixing that string, just
remove the unit strings from all the doc strings.

Now it will show:

 OsmoPCU# show bts-timer
 BTS0:
  T3142 = 20 s	Wait Indication used in Imm Ass Reject during TBF Establishment (CCCH) (default: 20 s, range: [0 .. 255])
  T3169 = 5 s	Reuse of USF and TFI(s) after the MS uplink TBF assignment is invalid (default: 5 s)
  T3172 = 5000 ms	Wait Indication used in Imm Ass Reject during TBF Establishment (PACCH) (default: 5000 ms, range: [0 .. 255000])
  T3191 = 5 s	Reuse of TFI(s) after sending (1) last RLC Data Block on TBF(s), or (2) PACKET TBF RELEASE for an MBMS radio bearer (default: 5 s)
  T3193 = 1600 ms	Reuse of TFI(s) after reception of final PACKET DOWNLINK ACK/NACK from MS for TBF (default: 100 ms)
  T3195 = 5 s	Reuse of TFI(s) upon no response from the MS (radio failure or cell change) for TBF/MBMS radio bearer (default: 5 s)

Related: OS#5209
Change-Id: I140122bb10f750bf996272cc7f9c5b541c9bd364
2021-08-12 15:08:16 +02:00
Oliver Smith d3c7591304 Add counters: pcu.bts.N.pch.requests.timeout
Implement T3113 for paging over PCH with default value of 7s (same as
T3113 in OsmoBSC). Increase the new counter on timeout.

Related: SYS#4878
Change-Id: I97475c3dbe2cf00b9cbfec39e93a3c65cb7f749f
2021-08-11 13:42:30 +02:00
Oliver Smith 4df959d305 Add counters: pcu.bts.N.pch.requests
Count attempted paging requests over PCH.

Related: SYS#4878
Change-Id: I1026780ef8542f40060b961df2f37213e15c29d7
2021-08-10 10:35:18 +00:00
Oliver Smith 978396732b Add counters: pcu.sgsn.N.rx_paging_{cs,ps}
Related: SYS#4878
Change-Id: Iefba6f3d29c69fd4865c084bd9cf1a3a78f5c202
2021-08-10 10:35:18 +00:00
Pau Espin 81db7334da tbf: Drop impossible paths in create_dl_ass()
create_dl_ass() is only called in gprs_rlcmac_sched.cpp on
tbf_cand->dl_ass pointer, which is always assigned under the guard
"!tbf->is_control_ts(pdch->ts_no)", since we only send CTRL messages for
a TBF on its control TS.
Hence, condition "!is_control_ts(ts)" in create_dl_ass will always be
false, and as a result poll_ass_dl will always be 1.
So we can drop different code paths.

Change-Id: Ibea4100a5dc8bd49303cb6a3d02417038c3d3887
2021-07-22 20:06:18 +02:00
Pau Espin 890de986ce Make gcc 11.1.0 false positivies happy
After my system's gcc was upgraded, I get false positivies like the one
below:
"""
/git/osmo-pcu/src/gprs_bssgp_pcu.c: In function ‘ns_configure_nse’:
/git/osmo-pcu/src/gprs_bssgp_pcu.c:1103:58: error: ‘%d’ directive output may be truncated writing between 1 and 11 bytes into a region of size 2 [-Werror=format-truncation=]
 1103 |                         snprintf(name, sizeof(name), "pcu%d", i);
      |                                                          ^~
/git/osmo-pcu/src/gprs_bssgp_pcu.c:1103:54: note: directive argument in the range [-2147483648, 1]
 1103 |                         snprintf(name, sizeof(name), "pcu%d", i);
      |                                                      ^~~~~~~
/git/osmo-pcu/src/gprs_bssgp_pcu.c:1103:25: note: ‘snprintf’ output between 5 and 15 bytes into a destination of size 5
 1103 |                         snprintf(name, sizeof(name), "pcu%d", i);
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
"""
In this case, i can't never take a value with more than 1 digit, but gcc
seems to be unable to see that.

Let's increase the buffer size a few bytes to make gcc happy, and make
the variable unsigned since it never will get negative values.

Next change is also a false positive, since variables are always
initialized beforehand in the cod epaths where they are used:
"""
/git/osmo-pcu/src/bts.cpp: In function ‘int bts_rcv_rach(gprs_rlcmac_bts*, const rach_ind_params*)’:
/git/osmo-pcu/src/bts.cpp:859:25: error: ‘ts_no’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  859 |         uint8_t trx_no, ts_no;
      |                         ^~~~~
/git/osmo-pcu/src/bts.cpp:859:17: error: ‘trx_no’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  859 |         uint8_t trx_no, ts_no;
      |                 ^~~~~~
"""

Change-Id: I1362a335a0c761bde367dbc779de4afa88f13584
2021-07-15 14:39:39 +02:00