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
This allows distinguishing when a TBF didn't set the TFI. Useful to
identify dummy reject TBFs, etc, and make sure a non-dummy TBF set its
TFI properly.
Change-Id: Iecf54a24041bd14f4ef5b86e57c3732e1b69d463
This way it's easier to distinguish when this value was not properly
filled when debugging or looking at logs.
Change-Id: I0c9c9fdcfca9eb15125ea49efcbb76711850052e
This way the timestamp is also set for dummy reject TBFs. For other
TBFs, setup() is called immediatelly after calling the constructor, so
we are fine too.
Change-Id: I2966ec7f3f9161d528a173d94797b72d1398c747
This function is expected to be used only on DL TBF so far, so let's
really assert to avoid going through if something is wrong and ending up
later with other issues.
Change-Id: If398ee48364fce5b5e38830b2b278b3bad9a48a2
In that function, previous PDCHs are unlinked and then alloc_algorithm
is expected to assign new TFIs.
Change-Id: I7bcbb223ca32400bede7ab638695ba3c015c9946
TSC is not really a property of a TBF, so let's drop it in order to avoid
confusing and possible misuse of that accessor.
Change-Id: I105eb65d507e45631faddb23420c42bc9560e580
The situation holds true as long as the assignment is resolved. Hence,
it can also happen that the TBF is in RELEASE state, because it was
unable to do the assignment (and after retrying, MAX_N3105 moved it into
RELEASING).
Let's not explicitly check states, the other conditions should be
enough.
Related: SYS#5647
Change-Id: I05fb0ea44aeb3fbda9e8e1c449e9366efaa2c511
When MS sends us the Packet Resource Request as RRBP from final UL ACK/NACK, we create a new TBF
with a different set of allocated TS. However, we must send the Pkt UL Assignment with information
of the new TBF using that same TS where we receive the Packet Resource Request, which happens to
be the control TS of the previous/old TBF. The original control TS of the new TBF is kept in
tbf->first_common_ts.
Hence the code does gprs_rlcmac_pdch::rcv_resource_request():
"""
ul_tbf->control_ts = ts_no;
"""
And later, when we receive a CTRL ACK answering the Pkt UL Assigment, we change the control TS of
the new TBF back to the new one, by calling tbf_assign_control_ts(), which basically does:
"""
tbf->control_ts = tbf->first_common_ts;
"""
So, for instance we have a TBF which was allocated with tbf->control_ts=4 and hence is only attached
to PDCH 4 (tbf->pdch[]), but for which is temporarily applied tbf->control_ts=7. Hence, when a poll
is requested, it is done in control_ts, aka 7, which is not in the array of attached PDCH.
The problem is of course if we never reach the point where the final control_ts is set, due to never
receiving the CTRL ACK. If the TBF is freed (due to timer X2001) before receiving the CTRL ACK and
hence tbf_assign_control_ts() is called, a crash may occur, because potentially a poll for the TBF is
left in TS 7 because it's not a PDCH attached to the TBF and hence poll
entries on that TS are not released, hence keeping a pointer to the
freed TBF.
Related: SYS#5647
Change-Id: I0c49f2695e0d932d956c01976c9458eebee63cd4
Move FSM internal state checks to its own file. Re-use the helper
function in the 2 places where same stuff is checked.
Change-Id: I9ded6e1c80e6cd7bcf6883bc2e853b6dafb33f7c
Add stats needed for performance measurements in
3GPP TS 52.402 § B.2.1.54-55.
Split m_num_tbfs to count GPRS and EGPRS TBFs separately. Move the code
that updates m_num_tbfs and sets the PDCH_OCCUPIED stats to a separate
function, as it's mostly the same in the TBF attach and detach.
Related: SYS#4878
Change-Id: I0c0a1121b4ae5f031782e7e63a0c28eb0b6c8b42
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
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
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
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
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
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
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
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
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
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
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
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
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
Oliver Smith
Pau Espin
Neels Hofmeyr