Commit Graph

207 Commits

Author SHA1 Message Date
Pau Espin b53230acec ms: Get rid of ms->delay field
Simply apply the content of the configured timer when the MS goes idle.
Having that field is convenient to do tricky stuff in unit tests, but
makes the main osmo-pcu app more complex for no good enough reason.

Change-Id: I8d44318b37b6605afd84db8ccec0d75e6db293b9
2023-04-21 14:36:28 +02:00
Pau Espin fe4d2f7dca Add new log category 'ms'
This is useful to track the lifecycle of MS objects. The RLCMAC cateogry
used so far in those log messages is too broad.

Change-Id: Ib4ce88d0f7309ac77c064a94bb0d667e8dbc33dd
2023-04-21 14:36:28 +02:00
Pau Espin eae9147424 ms: Hold a reference during ms_alloc
Make the caller hold a reference to the MS object just allocated, so
that it hs to explicitly unref it and, in turn, if no new references
were added during its use, trigger release of the MS object.
This is useful to avoid leaking MS object if it was allocated and then
no TBF is attached to it because allocation of TBF failed.

Related: OS#6002
Change-Id: I2088a7ddd76fe9157b6626ef96ae4315e88779ea
2023-04-21 14:36:28 +02:00
Pau Espin 403e048ac0 ms: Use osmo_use_count to track references
Change-Id: Ib65629224e6bd5683bb9192ba4354e965e8d39ec
2023-04-20 16:17:39 +02:00
Pau Espin 9da0686371 Merge bts_alloc_ms() and ms_alloc()
gprs_default_cb_ms_idle() is changed to have the same implementation as
previous bts_ms_idle_cb(), since that's the only one being used in
osmo-pcu code. It makes no sense to use different callback logic in unit
tests.

This is another step towards simplifying the code and getting rid of the
idle/active_cb().

Change-Id: I2a06d17588572a21dc5a14ddbde83766076b446d
2023-04-20 16:17:39 +02:00
Pau Espin eb0a0527e0 ms: Merge ms_storage into bts.cpp
That class is mostly a C++ class holding a llist plus some callbacks.
Having that in a separate class makes code more complex for no good
reason. This patch moves the llist into bts and allocates stuff directly
from within bts.
This will allow further cleanup of MS lieficyle in future patches.

Change-Id: I627f5db5073189c23ddf2b7f09c90abb24846f62
2023-04-20 16:17:38 +02:00
Pau Espin cde18c5632 bts: Rename bts_ms_by_{tlli,imsi} -> bts_get_ms_by_{tlli,imsi}
While at it, put them together and mark bts param as const.
This is a preparation for next patch.

Change-Id: Iad8aec4424f1f23cd4d02a14c4f9ec1b9fdb1f75
2023-04-20 16:17:38 +02:00
Pau Espin bfc9756c2b ms: Drop setting (egprs_)ms_class during bts_alloc_ms()
That information is not required during allocation of the object, and
most times it is not known.
Defer setting it only to meaningul values in paths obtaining the
information from peers.

Change-Id: I36f07dc389f7abe205fc4bcddbde93735f5d5cfc
2023-04-20 16:17:38 +02:00
Pau Espin b5a8a67dfe Update libosmogsm deprecated include
The previous gprs/protocol/gsm_04_60.h header from libosmogsm was
misleading because it was placed in the subdirectory used by libosmogb,
and hence was recently deprecated in favour of gsm/protocol/gsm_04_60.h.
Let's follow the pragma message advising to move to the new header file.

Depends: libosmocore 0158b05337a825352d9fd7f074170b686e9fd1e5
Change-Id: I027abbf3ed4c71331000565af1ef4f08f10cfafc
2023-03-14 11:19:53 +01:00
Philipp Maier 72ed33303a pcu_l1_if_phy: add new PHY API function to disconnect PDCH
There is a function l1if_connect_pdch, but no complementary function
like we have it with l1if_open_pdch and l1if_close_pdch. The reason for
this is that the PHY implementations that rely on a femtocell DSP do not
need to disconnect the pdch explcitly. However, the planned support for
the E1 based Ercisson RBS CCU will require an explicit disconnect. So
lets add a function call for this.

Change-Id: Ied88f3289bda87c48f5f9255c4591470633cc805
Related: OS#5198
2023-02-27 16:46:01 +01:00
Pau Espin d1058b9445 Avoid moving DL-TBF from old_msg to new_ms during ms_merge
The DL-TBF assigned to another MS object may have a totally different
set of reserved resources (TS set, TRX, etc.), so one cannot simply move
those to the new MS. To start with, if the 2 MS are on different TRX it
is clear that one of them will not be really in operation. That's most
probably the DL-TBF being in ASSIGN state on CCCH waiting for PCUIF_CNF
and later X2002 to trigger to start sending DL blocks, but without
confirmation whether the MS is really there. Since the other new MS
object probably has a UL-TBF, that's the one probably operative, and
hence a new DL-TBF can be created at that same time and assigned through
UL-TBF's PACCH.

Unit test test_ms_merge_dl_tbf_different_trx showcases the above
scenario.

Related: SYS#6231
Related: OS#5700
Related: 677bebbe5c
Change-Id: Ie8cb49d5492cfc4cbf8340f3f376b0e6105e8c82
2022-12-19 12:32:44 +01:00
Pau Espin 0aacd21658 tests/TbfTest: reproduce buggy corner case: MS with TBFs on 2 TRXs
Add a test which showcases a scenario where the PCU ends up with 1
GprsMs object holding a DL-TBF in a weird condition half in one TRX and
half in other due to ms_merge().

This test (slightly adapted) used to cause a crash in osmo-pcu.git
586ddda9bc (a few versions behind current
master).

Related: SYS#6231
Change-Id: Ic16b5e96debf91e72684833cd64253687857f3aa
2022-12-19 12:32:33 +01:00
Pau Espin d29a1435ad Pass gprs_rlcmac_pdch to create_dl_acked_block()
This allows having full TS information, not only ts_no, which will be
needed later on followup patches.

Change-Id: I1efccb32c5afa4fe62233bf114ea95b6fbbe1a06
2022-12-16 12:11:31 +01:00
Pau Espin ef1b9847f1 tests/tbf: test_tbf_dl_llc_loss(): Fix wrong param passed and wrong expectancies
The DL-TBF in the test gets its TS allocated on TS4 (only that one is
enabled through setup_bts(bts, ts_no=4)), and hence it makes no sense to
ask it to send data on TS7.

Change-Id: Ibe6fc073b7438b8024c4d3ddb0e60c6112752351
2022-12-16 12:11:31 +01:00
Pau Espin e2ed40d02b Convert tbf->control_ts to be a gprs_rlcmac_pdch*
This allows having full information on the control TS easily reachable
(like TRX ofthe PDCH), and makes it easy to compare TS by simply
matching the pointer address.

Change-Id: I6a97b6528b2f9d78dfbca8fb97ab7c621f777fc7
2022-12-16 12:11:31 +01:00
Pau Espin ff7c581011 Rename gprs_rlcmac_ts_alloc.cpp -> alloc_algo.cpp & create own .h file
First commit towards trying to have alloc algorithm as isolated as
possible from others parts of the code trying to avoid state changes on
data structures.
Change name also because the alloc_algo not only allocated TS, but TFIs
and USFs.

Change-Id: I33a6c178c64a769f05d3880a69c38acb154afa62
2022-12-16 12:08:07 +01:00
Pau Espin a621d59ea8 Refactor code rejecting UL-TBF upon rx of PktResourceReq
* Make it similar to the already existing TBF allocation procedures
* Pass pdch pointer instead of trx and ts numbers

Change-Id: I04b3b65942732cc652adeaa507529b849292ff61
2022-12-16 11:05:46 +00:00
Pau Espin b7a2b59c68 Refactor code related to DL-TBF upgrade to multislot
* Make clear the code relates to DL-TBF and not UL-TBF.
* Change wording to "upgrade" to match the existing field and API
  "tbf_can_upgrade_to_multislot()".
* Free the TBF if we cannot allocate new resources.

Change-Id: I0e4f8d7e46235a471b2124b280c81ff07b6967a4
2022-12-16 11:05:46 +00:00
Pau Espin 5ba3ef9a2b sched: Pass pdch to *_create_rlcmac_msg() functions
The pdch pointer contains more info than just timeslot number. For
instance, it contains information about the TRX owning the TS.

Change-Id: Ic31a7360a29e61f70bb1338ddab6f5f31aa8b26e
2022-12-13 12:20:02 +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 8abe13cf75 Rework tbf::update_ms()
That function was pretty confusing since it used a "enum
gprs_rlcmac_tbf_direction dir" param (whose type is expected to describe
the data direction of a TBF) to describe the direction of the the packet
which triggered its call.
The parameter was actually always called with "GPRS_RLCMAC_UL_TBF" which
in this case meant "uplink direction" which meant "TLLI was updated from
the MS, not the SGSN".
The DL direction was only used in unit tests, which can hence be simply
replaced by ms_confirm_tlli(), which this commit does.
So this update_ms() function was actually used in practice in osmo-pcu
to trigger update of TLLI and find duplicates only when an RLCMAC block
(control or data) was received from the MS. Therefore, this function is
renamed in this patch and moved to the gprs_ms class, since it really
does nothing with the TBF.

Related: OS#5700
Change-Id: I1b7c0fde15b9bb8a973068994dbe972285ad0aff
2022-10-31 22:07:31 +01:00
Pau Espin bda7bb7667 Rename tbf_alloc_ul_tbf -> ul_tbf_alloc
Use proper prefix according to the type being allocated.

Change-Id: Ic98c3c852c96c3f52f1c8f531b8d0fd28ce5aef5
2022-10-31 22:07:31 +01: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 8757384aec Rename tbf_alloc_dl_tbf() -> dl_tbf_alloc()
Rename it so that it follows the usual prefix for the type it allocates.

Change-Id: I7d30a85fefaa61d100fbd51af4601a3cf7de2159
2022-10-27 13:53:03 +02:00
Pau Espin 14beef6cfe Move LLC enqueuing and retransmit timer to MS object
The LLC queue is already in the MS object. The LLC timer and most
of the logic to enqueue its data is independent from the TBF.

Change-Id: I56b89fcac838d8eb732b629734d5e458e9c806d1
2022-10-27 13:52:59 +02:00
Pau Espin 7f360e74b7 Call ms_store->get_ms() with GSM_RESERVED_TMSI instead of 0
That's the special value checked in the implementation of get_ms() to
skip lookups based on TLLI.
This should save some cicles trying to match TLLI 0.

Change-Id: I364d238ff8a82abb14281140fe18b273c0e8f541
2022-10-21 14:31:05 +02:00
Pau Espin 52e2c08f66 TbfTest: Reset MS timeout to 0 in test_tbf_dl_llc_loss()
The timeout is set to a high value to avoid freeing the MS during main
loop iteration. Let's set it back to 0 to have a common free path in
test infrastructure.

Change-Id: I6dc4765163fde1a46574b49f3185aea65391e0d0
2022-05-09 17:47:28 +02:00
Pau Espin bf5e3cb3a4 bts: Call gprs_bssgp_destroy() in destructor
Change-Id: I7ed7f489f36f88277e2d5e393edcb339bf0cbba0
2022-05-09 16:40:43 +02:00
Pau Espin 9184e855a1 tests/tbf: Set up pcu timers in prepare_pcu()
Change-Id: I30a491f82cced558ad72108c3c5ed7015d1d4d3b
2022-05-09 16:32:03 +02:00
Pau Espin 1463383505 llc: Convert to C: s/m_//g
Change-Id: Ia5272841392110b87725e87a7e6824c17a70d6d1
2022-03-31 19:09:08 +02: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 422636d752 tests: TbfTest: Fix wrong behavior in test_tbf_dl_reuse()
The test uses get_poll_fn() to submit a UL ctrl block on the next
expected poll TS+FN.

During initial transmit_dl_data(), 2 POLLs for DL_ACK are set on
different TS, but the test only updates the clock for one of them in
send_ul_mac_block(). As a result, one POLL item is left in the rb_tree,
and later on, when send_control_ack() is called, get_poll_fn() will pick
that 2nd DL_ACK poll FN+TS which was left untouched (due to sending no
events to the PCU clock) instead of the FN+TS which was expected to be
allocated by PCU for DL_ASS.

Until now this was not an issue since rcv_control_ack() was not properly
veirfying what the poll scheduler was expecting and accepted it. This is
no longer the case after the follow up patch refactoring
rcv_control_ack() and improving its robustness.

Change-Id: I3a4b089fe66a99e73e07bd1c690cd4d67752fad9
2021-09-28 16:05:03 +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 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 3225290d77 Move timer X2002 to tbf_fsm
Related: OS#2709
Change-Id: I94b71c60ed49d51ebdf6d6b428056b4b94354676
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 49a2f404e8 replace dl_ass_state with osmocom FSM
Related: OS#2709
Change-Id: Ia33418478e17986a316ffda48b091030f53fa371
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 65bba93afa tests: tbf: Fix dl_tbf polled for data without being in FLOW state
Prior code was wrong, as in it did stuff diferent to what is expected
and actually done in osmo-pcu. In osmo-pcu code, the function is guarded
and only called in FLOW or FINISHED state by the scheduler.

Change-Id: If8029bee90adceee128ebb20c033756efd50e90e
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 945be91032 tests/tbf: Fix null pointer access if slowly stepping with gdb
When slowly debugging test_tbf_dl_llc_loss, bssgp_tx_llc_discarded() may
trigger, submitting events to the libosmogb code. Since it didn't
properly set up the callback, it would end up in a null pointer
dereference when lib code tried to use backward-compatible API (which
was neither set up properly).

"""
TBF(TFI=0 TLLI=0xc0123456 DIR=DL STATE=ASSIGN) Discarding LLC PDU because lifetime limit reached, count=3 new_queue_size=0
BSSGP (BVCI=2234) Tx LLC-DISCARDED TLLI=0xc0123456, FRAMES=3, OCTETS=57
/git/libosmocore/src/gb/gprs_ns.c:271:2: runtime error: member access within null pointer of type 'struct gprs_ns_inst'
"""

"""
(gdb) bt
 #0  0x00007ffff729cac0 in gprs_active_nsvc_by_nsei (nsi=nsi@entry=0x0, nsei=2234, bvci=bvci@entry=0)
    at /git/libosmocore/src/gb/gprs_ns.c:271
 #1  0x00007ffff72b1fec in gprs_ns_sendmsg (nsi=0x0, msg=0x621000000160) at /git/libosmocore/src/gb/gprs_ns.c:1087
 #2  0x00007ffff72d1803 in _gprs_ns_sendmsg (ctx=<optimized out>, msg=<optimized out>) at /git/libosmocore/src/gb/gprs_bssgp.c:80
 #3  0x00007ffff730226f in bssgp_tx_llc_discarded (bctx=<optimized out>, tlli=<optimized out>, num_frames=<optimized out>, num_octets=<optimized out>)
    at /git/libosmocore/src/gb/gprs_bssgp_bss.c:249
 #4  0x000055555588243e in gprs_rlcmac_dl_tbf::llc_dequeue (this=0x7ffff1622860, bctx=<optimized out>)
    at /git/osmo-pcu/src/tbf_dl.cpp:413
"""

Change-Id: Iee5bcf21afc8980a14f90f5b1ead6d2460a244ea
2021-07-26 16:05:55 +02:00
Pau Espin 4f67a9bf46 pdch: Fix heap-use-after-free in pdch->ulc
In existing previous code, pdch->ulc would be freed in
gprs_rlcmac_pdch::free_resources() when  it became disabled as per PCUIF
info_ind (for instance, when a DYN TS is switched PDCH->SDCCH8).
However, pdch->ulc was so far only allocated during pdch_init, which is
only called during bts_alloc() time.
Hence, after first info_ind disabling it, if it became again enabled
(again by info_ind re-enabling it after SDCCH8 was not longer in use),
the pdch->ulc would be used again but it would point to freed memory.

Let's rearrange how/when resources are freed to make it more logical.
With this patch, pdch internal resources are freed upon ->disable(), and
re-allocated upon ->enable().

Change-Id: Id51f5f6a54ac9f24b784c17bc360ac38f5726fc7
2021-07-01 13:09:10 +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
Pau Espin c432e062ea encoding: Encode TA in UL ACK/NACK if available
Change-Id: I3b060ee16aeac5f5d9b314b6bc46383f5e9c44c3
2021-05-11 13:24:13 +02:00
Pau Espin 2ab840a1fa Make WaitIndication T3172 configurable
Tbftest expectatins need to change because 5000/20 = 250 < 255, hence
the message is now sent as units of 20ms instead of seconds.

Related: OS#3928
Change-Id: I48b34b94b1a5dfb046a3a6cf8a0d944a7c9b6754
2021-04-26 17:53:09 +02:00
Pau Espin 54742f287c ul_tbf: Clean up handle_tbf_reject()
Document the function, make it look similar to usual TBF creation path
tbf_alloc_ul()->tbf_alloc_ul_tbf->tbf::setup(), which it mimics with
some differences.
Get rid of unneeded stuff like creating MS and settings its TLLI (that's
already done in only caller of the function). There's no need for
calling update_ms() either.

Change-Id: I61df2e4f0f0df1f8db941741a2d35a2319252c5e
2021-04-26 17:19:07 +02:00
Pau Espin 434799720c pdch: rcv_resource_request: Improve robustness
Use recently added PDCH UL Controller to verify expectancies.

Test test_packet_access_rej_prr is rewritten since it didn't make sense
as it was before, since it relied on osmo-pcu not checking stuff
properly to trigger the reject. The RACH requests are changed to
allocate 8 SBAs (maximum of 7 concurrent USFs). Allocating the SBA
doesn't reserve a USF, that happens at PKT RESOURCE REQUEST, hence we
end up exhausting resources there and triggering the REJECT at that
point.
Previous version of the patch allocated TBFs directly through RACH req,
and then submitted an extra PKT RESOURCE REQUEST which PCU didn't expect
to trigger the reject.

Change-Id: I157e72160317340ee7742c78c62a25d3d98fc01e
2021-04-22 20:28:40 +02:00
Pau Espin 16e1678bfc tbf: Get rid of attribute poll_ts
That field is not needed anymore, and it works only under the assumption
that only 1 poll request can be active at a time per TBF, which is not
true.

Change-Id: I9b8bed7741d385bab4cd8c64b841a78a02a05fe1
2021-03-31 17:39:50 +02:00
Pau Espin 58046e45d1 tbf: Get rid of attribute poll_fn
That field is not needed anymore, and it works only under the assumption
that only 1 poll request can be active at a time per TBF, which is not
true.

Change-Id: I63a34a702f028b871530fb7caeb13e8ea1cc78ac
2021-03-31 17:39:50 +02:00