Commit Graph

33 Commits

Author SHA1 Message Date
Pau Espin 76ef66698c Mark several fsm related internal symbols as static
After some earlier refactoring, those fields are only used internally in
the module, and hence can be marked as static.

Change-Id: Ibf2a6ee5636ae7102ffd13b7497769652bcc3202
2023-01-30 10:42:44 +01:00
Pau Espin 343c0ee8d9 Split tbf_fsm as tbf_{ul,dl}_fsm
The 2 types of TBF share some parts of the implementation, but actually
half of the code is specific to one direction or another.
Since FSM are becoming (and will become even) more complex, split the
FSM implementation into 2 separate FSMs, one for each direction.
The FSM interface is kept compatible (events and states), hence code can
still operate on the tbfs on events and states which are shared.

Test output changes are mainly due to:
* FSM instance now created in subclass constructor, so order of log
  lines during constructor of parent class and subclass slightly
  changes.
* osmo_fsm doesn't allow having 2 FSM types with same name registered,
  hence the "TBF" name has to be changed to "DL_TBF" and "UL_TBF" for
  each FSM class.
* FSM classes now use DTBFUL and DTBFDL instead of DTBF for logging. Some
  tests don't have those categories enabled, hence some log lines
  dissappear (it's actually fine we don't care about those in the test
  cases).

Change-Id: I879af3f4b3e330687d498cd59f5f4933d6ca56f0
2022-11-18 16:54:20 +01:00
Pau Espin f197f15b2d tbf_fsm: Move osmo_fsm_inst fi out of struct tbf_fsm_ctx
This is a preparatory step towards splitting tbf_fsm.c into tbf_ul_fsm.c
and tbf_dl_fsm.c.
In order to accomplish it, the struct tbf_fsm_ctx will also be
duplicated (and each one will contain a explicit ul_tbf/dl_tbf pointer).
Hence, a DL_TBF will have a struct tbf_dl_fsm_ctx and a UL_TBF will have
a struct tbf_ul_fsm_ctx, since those hold implementation specific
state. However, the FSM interface will be partly shared (events,
states), and hence we want to keep the "fi" pointer into the "tbf"
parent class so that it can be used regardless of the tbf direction
type.

Change-Id: I03e691ccf6a94431caa55653349158f5b85db017
2022-11-18 14:16:55 +01:00
Pau Espin 06abd3ea19 tbf_fsm: Move tbf_fsm_state_chg macro to .c file
It is nowadays only used internally, hence it can be moved to the .c file
to better describe its scope.

Change-Id: I23cfa5b7efbeb6a58855099780749741c9c947e9
2022-11-18 14:16:55 +01:00
Pau Espin f38a47ee01 tbf_fsm: Introduce new event to act upon contention resolution success
This is a preparation towards fixing MS not recreating a DL-TBF (being
assigned on CCCH) when MS starts an UL-TBF and finishes contention
resolution.

A counter is removed which was counting contention resolution (MS) on
the wrong place.

Change-Id: I8b9555864d3615ce0a024b641c67921f82273a8d
2022-10-31 22:07:31 +01:00
Pau Espin 677bebbe5c Avoid losing DL-TBF during MS merge
It is desired to free pending DL-TBF under some scenarios, which somehow
are related to those where we call the ms_merge() procedure since it's at
time an MS can be identified when coming from packet-idle state.

Until now, the freeing of those DL-TBFs happen because the ms_merge()
procedure doesn't migrate DL-TBF from "old_ms" to "ms". This was done
manually under the cases where it was deemed necessary before calling
the ms_merge() procedure 8because old_ms and its tbfs are gone after
returning from it).
This logic, though convinient for the specific cases at hand, is quite
confusing for readers and program execution since one would expect the
ms merge to, well, merge everything.

Therefore, this patch changes the ms_merge() logic to always merge the
DL-TBF, and only under the specific cases where it makes sense to free
it, do so explicitly after MS merge, where all the info has been updated
and united.
2 code paths are now explicitly freeing the existing DL-TBF when needed:
- TBF_EV_FIRST_UL_DATA_RECVD: 1st data (containing TLLI) is
  received from MS, hence identifyng the MS and potentially having been
  merged with some old MS which used to have a DL-TBF, most probably in
  process of being assigned through CCCH (PCH). This event is triggered
  during MS using 1phase-access, and we should drop the exsitng DL-TBF
  because MS just came from packet-idle mode (CCCH).
- rcv_resource_request(): PktResourceRequest is received at an scheduled
  SBA, meaning the MS is doing 2phase-access, meaning MS also came from
  packet-idle mode (CCCH), so previous DL-TBF can be dropped.

Related: OS#5700
Change-Id: I29f4ffa904d79d58275c6596cc5ef6790b6e68c6
2022-10-31 22:07:31 +01:00
Pau Espin 338a5ae770 tbf_fsm: Introduce new event TBF_EV_FIRST_UL_DATA_RECVD
This allows easier tracking of this event. It will also extended later
on with more logic which is better placed in tbf_fsm.

Change-Id: I5c935914e13db3928c32621ec04eb2760367615d
2022-10-31 22:07:31 +01:00
Pau Espin 9438613792 Move UL allocation&assign functions to gprs_ms.c
Similar structure as what we have with DL-TBF.

Change-Id: I256aeede2a2678c9738539fb6ec4db9766fa85e4
2022-10-31 22:07:31 +01:00
Pau Espin 3f33929119 tbf_fsm: Identify several events as Dl-TBF only
This will help in the future when splitting tbf_fsm into different FSMs
for UL-TBF and DL-TBF, since only some of the events and states are
shared.
That means we can keep a single state and event enum, but the FSMs can
be implemented separately in different files, easing a lot extending and
understanding the logic.

Change-Id: Id97f0d532d2d7ab2791a35c1f836d7c8da050124
2022-10-31 22:07:18 +01:00
Pau Espin 091642a8f5 Replace tbf->establish_dl_tbf_on_pacch() refactoring GprsMs functions
ms_new_dl_tbf_assignment() is split into 2 functions, one to
allocate+assign on PACCH and another one for PCH.
This makes a lot clearer the aim of each caller of the function.
Once this is done, it becomes obvious tbf->establish_dl_tbf_on_pacch()
is basically doing the same as ms_new_dl_tbf_assigned_on_pacch() so drop
it.

Change-Id: I610210e3120c962d91ce8ff94c66161e086761ba
2022-10-28 18:13:41 +02:00
Pau Espin ee35008037 Refactor and clarify tbf->triger_ass() code
Split the function into 2 functions, one for assignment on PACCH and one
for assignment on PCH. This makes code calling this API far more clearer
on what is the exact aim when assigning the TBF.

Change-Id: Ic92867e55337b0bd6b5bfc97f13b7982eedb1cb7
2022-10-28 17:35:06 +02:00
Pau Espin 22b26d8a1c Delay ImmAss(PCH, PktDlAss) if waiting for PKT_CTRL_ACK answering UL_ACK_NACK (FinACK=1)
In that state (ul_tbf=TBF_ST_FINISHED), we are unable to reach the MS to
assign a new DL TBF.
* MS Is not available in CCCH because it's attached the PDCH.
* MS won't be able to PKT_CTRL_ACK a PktDlAss on PACCH, because next
  thing it will do is to PKT_CTRL_ACK the UL_ACK_NACK(FINACK=1) we
  already polled it for, and immediatelly after that it will release the
  UL TBF on its side and go back to packet idle mode.

Hence, we must wait for MS to send the PKT_CTRL_ACK to us in order to be
able to assign the DL TBF in PCH (CCCH).

Related: OS#5700
Change-Id: I7a30db9cc7dae70e04054f1a4dba004bd1780d4a
2022-10-28 16:33:45 +02:00
Pau Espin cc30b052db Rename as_{dl,ul}_tbf -> tbf_as_{dl,ul}_tbf
Prefix the function accordingly to specify the input object.

Change-Id: I225568fe2cb5236817cdd9609354c24c66723606
2022-10-28 15:14:44 +02:00
Oliver Smith 8d03c50024 treewide: remove FSF address
Remove the paragraph about writing to the Free Software Foundation's
mailing address. The FSF has changed addresses in the past, and may do
so again. In 2021 this is not useful, let's rather have a bit less
boilerplate at the start of source files.

Change-Id: I4a49dbeeec89b22624c968152118aecf8886dac6
2021-12-14 12:19:13 +01:00
Pau Espin 5deac1404d Fix MS ending up with assigned imsi 000
The whole paging path and data structre is cleaned up.
New MS helpers ms_imsi_is_valid() and ms_paging_group() are introduced
to help in the process and keep implementation details inside GprsMs
class.

Related: OS#5303
Change-Id: I4c0838b26ede58e4b711410eee2a8e4f71e9414b
2021-11-12 18:38:43 +01:00
Pau Espin 89a995a439 tbf_fsm: Add assert verifying X2002 only triggers for DL TBF
Code above setting the timer in same tbf_fsm already has this kind of
assert, but it helps understanding the code having this assert here.

Change-Id: I7588deef5073694eb5fecdb516c241a04594e2b0
2021-10-18 15:36:58 +02:00
Pau Espin b3291bc0e3 Abort scheduling of pending Pkt Ul Ass if tbf goes into RELEASE step
Change-Id: I20bab79070274b1d8f6b4e1867b30de61983ab54
2021-10-14 19:31:18 +02:00
Pau Espin ffe998ce9d tbf: Document temporary change of control_ts and move code assigning it back to FSM
Change-Id: I1b7eb7802060778487e5729ee789b2323b6636f8
2021-10-12 19:32:28 +02:00
Pau Espin 85aa87b61f tbf_fsm: Handle MAX_N3105 in state ASSIGN
Seen on a runnig osmo-pcu against real MS:
"""
pdch_ul_controller.c:329 PDCH(bts=0,trx=1,ts=7) Timeout for registered POLL (FN=751140): TBF(TFI=0 TLLI=0xe8c12143 DIR=UL STATE=ASSIGN EGPRS)
tbf.cpp:542 TBF(TFI=0 TLLI=0xe8c12143 DIR=UL STATE=ASSIGN EGPRS) poll timeout for FN=751140, TS=7 (curr FN 751140)
tbf.cpp:384 TBF(TFI=0 TLLI=0xe8c12143 DIR=UL STATE=ASSIGN EGPRS) N3105 exceeded MAX (8)
tbf.cpp:594 TBF(UL-TFI_0)[9bc050]{ASSIGN}: Received Event MAX_N3105
tbf.cpp:594 TBF(UL-TFI_0)[9bc050]{ASSIGN}: Event MAX_N3105 not permitted
"""

It was first though when FSMs where introduced that an FSM in ASSIGN
state could not receive this kind of event because it was believed to be
sending no CTRL blocks at all until flow state. That's because the
believe was that Assignment over PACCH was done by another existing TBF.
It turns out this is usually the case, but not in all cases. In at least
one case, the tbf object (and tbf_fsm/tbf_{ul,dl}_ass_fsm) itself is
handling its own assignment (hence eg. sending the UL assignment and waiting
response through tbf_ul_ass_fsm. This happens if a UL TBF sends a Pkt
Resource Req as a response to RRBP of final UL ACK/NACK in order to
request a new TBF, where it temporarily uses the control_ts of the
previous TBF to get a new Pkt UL Assignment over PACCH.

If Pkt Ul Assignment doesn't receive a CTRL ACK, tbf_ul_ass_fsm will
retrnamist it, until MAX_N3015 is reached (the event we failed to
handle until now). At this point, we really want to transition to
RELEASING in order to avoid keeping the TBF allocating resources (until
X2001 times out).

Related: SYS#5647
Change-Id: I86d5c1bbccd06673d08451b812d149e727404733
2021-10-12 18:57:44 +02:00
Pau Espin 0043005afb tbf_fsm: rename state NULL -> NEW
This helps distinguishing the case where a TBF is in the initial state
and the unexpected case where osmo_fsm_inst_state_name reports "NULL"
due to fi pointer being NULL.

Change-Id: Ieaabfc9fa0dedb299bcf4541783cf80e366a88c3
2021-10-12 11:01:07 +00:00
Pau Espin 7a2b65ed2c Handle Final UL ACK/NACK Confirmation in tbf_fsm
Pass the event over the FSM for better understading of the entire
lifecycle of the TBF.

Change-Id: If30d881037209d33b2b41ecf8bb8419caf36e367
2021-10-12 12:41:53 +02:00
Pau Espin cee6b122c2 tbf_fsm: Ignore event DL_ACKNACK_MISS in state RELEASING
Fixes following error log line:
"{RELEASING}: Event DL_ACKNACK_MISS not permitted"

Rationale: We may move to RELEASING state at some point, for instance
due to MAX_N3101/MAX_N3105 while still having some active poll
registered in some PDCH ulc. Upon that poll (most probably) timing out,
it will send a DL_ACKNACK_MISS event to us. Since we are already
determined to release the TBF (waiting for T3195 or T3169 to trigger),
simply ignore the event and avoid logging an error.

Fixes: OS#5240
Change-Id: Ibfb49356d2b3b5fccb6d59db8593b2256e5c51fb
2021-09-28 16:53:00 +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 3225290d77 Move timer X2002 to tbf_fsm
Related: OS#2709
Change-Id: I94b71c60ed49d51ebdf6d6b428056b4b94354676
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 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 dc2aaac29f tbf: Move existing tbf_state implementation to osmo_fsm
This is only an initial implementation, where all state changes are
still done outside the FSM itself.
The idea is to do the move in several commits so that they can be
digested better in logical steps and avoid major break up.

Related: OS#2709
Change-Id: I6bb4baea2dee191ba5bbcbec2ea9dcf681aa1237
2021-05-19 12:50:25 +02:00