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
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
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
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
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
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
The logic checking whether the UL TBF had already been sent all the data
(and hence was marked as finished and requesting UL ACK to be sent) was
not taking into account the case where there was still no valid block
stored, ie. when the first received UL data block was discarded for some
reason (ex: because TLLI was not set during content resolution).
Related: OS#1940
Change-Id: I739e67ae1bb40555a362170f26fb98ac69caabb2
Let's avoid different code paths in the loop based on is_tlli_invalid.
Instead, always do the proper storing of the block, and if later on the
corner case is found (no TLLI received while in Content Resolution
process) when checking tlli related stuff, then simply invalidate the
block.
Related: OS#1940
Change-Id: I77afaa617d7ce045c0f6d994fc0d8e03fe69de53
Since a while ago, tbf should always have an MS attached since its
creation, so there's no sense to check for it here.
Change-Id: If056a3fb83b43a48c2a6382fc30c6c81fe2b2651
This allows more easily finding when this specific scenario happens, and
can easily be compared against the PACCH one.
Change-Id: I609792a40fda2a798ca71a0e9f5639d0a0f011d7
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
Now that we finally handle N3101 and N3103 correctly, we can fix abuse
of T3169 we were doing to make sure TBFs were freed.
According to 3GPP TS 44.060, T3169 should be armed:
* N3101_MAX reached
* N3103_MAX reached
Furthermore, when T3169 is enabled, the tbf should be in state
RELEASING so that its USF is not used.
See full description: https://osmocom.org/issues/5033#note-2
Related: OS#5033
Change-Id: I2cec531e2633281b88f69ba065c0105580c81076
This API is not really needed anymore, since anyway it works under the
assumption there can only be 1 POLL in transit per TBF, which isn't
necessarily true.
Change-Id: I875f51cade95faeb2d79dcebfead4c83e23a731b
This allows easily checking the initial reason to trigger the poll when
either it is received or times out.
Later on this reason can be transformed into an FSM event and sent to
the related FSM.
Related: OS#5020
Change-Id: Ie8fefd1f47ad674ce597a8065b15284088956bde
When the scheduler detects it's time to receive a UL block due to a
scheduled poll, if that polling is done on a UL TBF, then use its USF if
available instead of using USF_UNUSED (=7) when sending a DL block on
that same FN.
This is not really needed for correct work, since MS take care
themselves of scheduling a UL block when they receive the poll (RRBP)
some time before, and don't check the USF at the time of transmitting.
In any case, it helps understand better when looking at pcap traces that
indeed it a UL block from that MS was requested, instead of setting USF
to 7.
Related: OS#5033
Change-Id: I2ad9d8ea6afc8f83192033470bd27010a7474430
Previous work on BTS class started to get stuff out of the C++ struct
into a C struct (BTS -> struct gprs_glcmac_bts) so that some parts of
it were accessible from C code. Doing so, however, ended up being messy
too, since all code needs to be switching from one object to another,
which actually refer to the same logical component.
Let's instead rejoin the structures and make sure the struct is
accessible and usable from both C and C++ code by rewriting all methods
to be C compatible and converting 3 allocated suboject as pointers.
This way BTS can internally still use those C++ objects while providing
a clean APi to both C and C++ code.
Change-Id: I7d12c896c5ded659ca9d3bff4cf3a3fc857db9dd
As we integrate osmo-pcu more and more with libosmocore features, it
becomes really hard to use them since libosmocore relies heavily on C
specific compilation features, which are not available in old C++
compilers (such as designated initializers for complex types in FSMs).
GprsMs is right now a quite simple object since initial design of
osmo-pcu made it optional and most of the logic was placed and stored
duplicated in TBF objects. However, that's changing as we introduce more
features, with the GprsMS class getting more weight. Hence, let's move
it now to be a C struct in order to be able to easily use libosmocore
features there, such as FSMs.
Some helper classes which GprsMs uses are also mostly move to C since
they are mostly structs with methods, so there's no point in having
duplicated APIs for C++ and C for such simple cases.
For some more complex classes, like (ul_,dl_)tbf, C API bindings are
added where needed so that GprsMs can use functionalitites from that
class. Most of those APIs can be kept afterwards and drop the C++ ones
since they provide no benefit in general.
Change-Id: I0b50e3367aaad9dcada76da97b438e452c8b230c
The assumption that TLLI 0x00000000 is invalid and can be used
as the initializer is wrong. Similar to TMSI, 0x00000000 is a
perfectly valid value, while 0xffffffff is reserved - use it.
According to 3GPP TS 23.003, section 2.4, a TMSI/P-TMSI with
all 32 bits equal to 1 is special and shall not be allocated by
the network. The reason is that it must be stored on the SIM,
where 'ff'O represents the erased state. According to section
2.6 of the same document, a local/foreign TLLI is derived from
P-TMSI, so the same rule applies to TLLI.
I manually checked and corrected all occurances of 'tlli' in the
code. The test expectations have been adjusted with this command:
$ find tests/ -name "*.err" | xargs sed -i "s/0x00000000/0xffffffff/g"
so there should be no behavior change. The only exception is
the 'TypesTest', where TLLI 0xffffffff is being encoded and
expected in the hexdump, so I regenerated the test output.
Change-Id: Ie89fab75ecc1d8b5e238d3ff214ea7ac830b68b5
Related: OS#4844
This is another step forward towards a more clear data model where a TBF
always has a MS object (which may be lacking some information, and at a
later point when more information is found, it may actually be a
duplicated MS object and hence one duplicate removed and the TBF moved
to the object being kept).
This helps for instance in removing duplicated information stored in
the TBF which is really per MS, like ms_class, ta, etc. Since there's
always a MS object there's no need to keep a duplicate in both classes
in case there's no MS object.
It can already be seen looking at unit test logging that this kind of
data model already provides better information.
Some unit test parts were needed to adapt to the new model too.
Change-Id: I3cdf4d53e222777d5a2bf4c5aad3a7414105f14c
Return an interface to the window base class so that the tbf base class
can access the common window methods, such as set_ws(). It will be used
in next commit to get rid of duplicated function enable_egprs in both
dl_tbf and ul_tbf subclasses.
The user of the function can then decide to access more specific
functionaltiites of the window class by static casting it to the
specific direction (which is known by the caller since it operates on a
ul_tbf or a dl_tbf).
Change-Id: Ia2e1decf91be1184668e28297c2126affb9c7ae4
Since commit 322456ed47 (and previous
one), it is expected that a tbf object ALWAYS has a MS object referend
to it, even if it's a temporary copy which will later be merged when
TLLI/IMSI is retrieved and it is found that several MS objects relate to
the same MS.
The purpose of set_tlli_from_ul was mainly to update TBF's ms() to
old_ms before going through usual tbf->update_ms() path. That's not
really needed since ms() is already always set and TBFs for old_ms are
already freed in update_ms() and children function.
Change-Id: Ie8795e7a02032336e53febb65c11f9150c36d2a0
We have same kind of object splitted into two layers, in coding_scheme
and gprs_coding_scheme. Let's merge them together and get rid of the
class, which is not really useful because it's only a set of functions
operating on one enum value.
This change also fixes gcc 10.1.0 error about memseting a complex type
in rlc.h init().
Change-Id: Ie9ce2144ba9e8dbba9704d4e0000a2929e3e41df
In normal conditions ACKing of UL blocks is only sent every
SEND_ACK_AFTER_FRAMES (20) frames. Which means if CV=0 is received (and
hence no more packets are received) less than 20 frames before a lost,
the PCU won't ask for a retransmission and wait there until some timer
destroys the TBF.
This issue is shown by TTCN3 test PCU_Tests.ttcn
TC_ul_intermediate_retrans.
Unit tests triggering this condition are adapted. Some similar tests are
not triggering it because BSN/CV relation being used is totally wrong
(like CV=0 being sent on a BSN with previous value than others).
Change-Id: I9b4ef7b7277efa645bdb5becf2e9f6b32c99a9b1
It's super annoying seeing lots of functions being called everywhere
only to find out they are only incrementing a counter. Let's drop all
those functions and increment the counter so people looking at code
doesn't see dozens of code paths evyerwhere.
Most of the commit was generated by following sh snippet:
"""
#!/bin/bash
define_pattern="^CREATE_COUNT_ADD_INLINE"
generic_func="do_rate_ctr_add"
grep -r -l "${define_pattern}" . | xargs cat | grep "${define_pattern}("| tr -d ",;" | tr "()" " " | awk '{ print $2 " " $3 }' >/tmp/hello
while read -r func_name ctr_name
do
#echo "$func_name -> $ctr_name";
files="$(grep -r -l "${func_name}(" .)"
for f in $files; do
echo "$f: $func_name -> $ctr_name";
sed -i "s#${func_name}(#${generic_func}(${ctr_name}, #g" $f
done;
done < /tmp/hello
grep -r -l "void ${generic_func}" | xargs sed -i "/void ${generic_func}(CTR/d"
grep -r -l "$define_pattern" | xargs sed -i "/$define_pattern/d"
"""
Change-Id: I966221d6f9fb9bb4f6068bf45ca2978008a0efed
It's super annoying seeing lots of functions being called everywhere
only to find out they are only incrementing a counter. Let's drop all
those functions and increment the counter so people looking at code
doesn't see dozens of code paths evyerwhere.
Most of the commit was generated by following sh snippet:
"""
#!/bin/bash
grep -r -l ^CREATE_COUNT_INLINE . | xargs cat | grep "^CREATE_COUNT_INLINE("| tr -d ",;" | tr "()" " " | awk '{ print $2 " " $3 }' >/tmp/hello
while read -r func_name ctr_name
do
#echo "$func_name -> $ctr_name"
files="$(grep -r -l "${func_name}()" .)"
for f in $files; do
echo "$f: $func_name -> $ctr_name";
sed -i "s#${func_name}()#do_rate_ctr_inc(${ctr_name})#g" $f
done;
done < /tmp/hello
grep -r -l "void do_rate_ctr_inc" | xargs sed -i "/void do_rate_ctr_inc(CTR/d"
grep -r -l "CREATE_COUNT_INLINE" | xargs sed -i "/^CREATE_COUNT_INLINE/d"
"""
Change-Id: I360e322a30edf639aefb3c0f0e4354d98c9035a3
Pau Espin
Neels Hofmeyr
Oliver Smith
Vadim Yanitskiy