Age | Commit message (Collapse) | Author | Files | Lines |
|
Under some circumstances, it could happen that a DL TBF is created as a
GPRS TBF due to not yet having enough information of the MS, and only
after the TBF is created the PCU gains that information and upgrades the
MS mode to "EGPRS". Hence, there's the possibility to run into a
situation where a GPRS TBF is attached to a EGPRS MS.
It may also happen sometimes that despite the TBF and the MS be EGPRS,
there's need to further limit the DL MCS to use, eg. MCS1-4 (GMSK).
As a result, when asking for the current DL (M)CS to use, we must tell
the MS which kind of limitations we want to apply. The later reasoning
was already implemented when GPRS+EGPRS multiplexing was added, but the
former was not being checked. Hence, by further spreading through the
call stack the "req_kind_mode" we match both cases.
Related: OS#4973
Change-Id: Ic0276ce045660713129f0c72f1158a3321c5977f
|
|
The TBF can sometimes be detached from an MS, for eg. when switching
from one MS object to another due to them being merged after we found
duplicate objects upon receiving new information from it, but that
change is instantaneous so it shouldn't be a problem. The only other way
where an MS can be detached from an MS is during the end of its (or the
MS) life, where it is not sending data anymore.
Hence, it is safe to drop those checks for MS not being null. Those
being trigger, it should be considered a bug.
Change-Id: If292a53a09a64664031e756bff4735b9c6ee8651
|
|
Move each method to the object on which they operate, be it a trx or a
pdch ts.
Change-Id: Ida715cbf384431d37b2b192fbd7882957c93a4d1
|
|
Those files are not really being used other than for calling
get_current_fn() which is just a placeholder to call
bts_current_frame_number on the global bts object.
Change-Id: I6d50a8c15c1de5e2a308a24b313a7776f94ae54f
|
|
Before, we used tho have a BTs object split into 2 parts, a C
gprs_rlcmac_bts struct and a C++ BTS struct, and "bts_data" naming was
used to distinguish them in variable names. Nowadays the struct is
finally combined into one, so there's no point in using this "bts_data"
terminology, we use always "bts".
Change-Id: I9852bf439292d1abc70711bea65698b21bde0ee8
|
|
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
|
|
Change-Id: I997bc52f0d924c8f2a0b1d6cf23af98828ad4258
|
|
Change-Id: I2fdd9c8a7393157183fff64084bb10e2a3b1dc63
|
|
Change-Id: I0cac5c12dff2e90b52d00383a00b4b94a9603a0a
|
|
Currently the BTS object (and gprs_rlcmac_bts struct) are used to hold
both PCU global fields and BTS specific fields, all mangled together.
The BTS is even accessed in lots of places by means of a singleton.
This patch introduces a new struct gprs_pcu object aimed at holding all
global state, and several fields are already moved from BTS to it. The
new object can be accessed as global variable "the_pcu", reusing and
including an already exisitng "the_pcu" global variable only used for
bssgp related purposes so far.
This is only a first step towards having a complete split global pcu and
BTS, some fields are still kept in BTS and will be moved over follow-up
smaller patches in the future (since this patch is already quite big).
So far, the code still only supports one BTS, which can be accessed
using the_pcu->bts. In the future that field will be replaced with a
list, and the BTS singletons will be removed.
The cur_fn output changes in TbfTest are actually a side effect fix,
since the singleton main_bts() now points internally to the_pcu->bts,
hence the same we allocate and assign in the test. Beforehand, "the_bts"
was allocated in the stack while main_bts() still returned an unrelated
singleton BTS object instance.
Related: OS#4935
Change-Id: I88e3c6471b80245ce3798223f1a61190f14aa840
|
|
It is expected that the tbf object is freed at any moment in time, for
instance if osmo-pcu drops PCUIF connection with osmo-bts. I couldn't
find any reason why it would e dangerous to free the tbf, so let's
remove this message.
related: OS#4779
Change-Id: I4ab5ccaa5bf6257b18d8fd5ba06baab083821817
|
|
Change-Id: Id9f8df9a5c0e0f88a811c5d7f06821cb4f30ab93
|
|
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
|
|
Change-Id: I5f37f3512dde60e4eb1ccebbb2d96de24604d241
|
|
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 patch avoids enabling EGPRS on MS objects if BTS/VTY assigned no
MCS supported/available for use.
As a result, if NO MCS is enabled/supported EGPRS won't be used despite
the MS announcing through EGPRS MS class that it supports EGPRS.
Change-Id: Ib19e9e006d851c2147de15f4aec36ab65250bdd3
|
|
Some tests were wrong (TypesTest) and required modification, since they
were setting a EGPRS MS but then expecting a GPRS assignment.
Change-Id: I9d3ee21c765054a36bd22352e48bde5ffca9225a
|
|
Avoid passing tons of params to internal helper function
tbf_nel_dl_assignment() in order to either fetch again the ms object or
create a new one. Let's instead create the ms function earlier if needed
and fill it with all the discovered information prior to calling the
helper function. This provides cleaner code and also better log output.
This way we also avoid trying to fill the MS twice and unneeded
getter+setter for TA.
tbf::imsi(): There' always an ms, so simply forward call to
ms()->imsi().
We can also get rid of assign_imsi, since the modified code is the only
place where it's used and there's already some code in place there to
update the MS. We instead merge it with set_imsi and keep the
duplication code to catch possible bugs from callers.
Move merge_and_clear_ms from tbf class to GprsMS, where it really
belongs.
Change-Id: Id18098bac3cff26fc4a8d2f419e21641a1f4c83b
|
|
Change-Id: I50d41b1c6f244edcfb78646d0fac4e47c2e3e561
|
|
Change-Id: Ifd98abbcce49e4605c764267965903fbf9f35867
|
|
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
|
|
Change-Id: I7587fd2ee97424020a099a8513f95544d6635f3d
|
|
It wasn't move when all the UL specific code was moved to a separate
file.
Change-Id: I6f8c2d568ffdea3826ec1e11358d24eea6c9335b
|
|
Output of all diag in different lines is really confusing, since the
user reads a timeout ocurred and then later in another line something
like "Downlink ACK was received" while no GSMTAP message shows any ACK.
Change-Id: I6a7d79c16c930f0712bc73b308409ececb1946ba
|
|
It's quite odd to see that in write_packet_downlink_assignment()
we initialize an 'RlcMacDownlink_t', so then the caller can use
the power of CSN.1 codec to generate the final sequence of bytes
to be transmitted, while in write_packet_uplink_assignment() we
already compose the final RLC/MAC message straight away using
the low-level bitvec API (like bitvec_write_field()).
I guess the reason is that at the time of writing this code, the
CSN.1 codec was not stable enough, so it was safer to generate
the message 'by hand'. This would also explain why we *decode*
the final RLC/MAC message in create_ul_ass() right after encoding.
Rewrite write_packet_uplink_assignment(), so now it initializes
a caller-provided 'RlcMacDownlink_t' structure. Given that it's
allocated on heap using talloc_zero(), do not initialize presence
indicators of fields that are not present in the message.
This would facilitate handling of frequency hopping parameters
in the upcoming changes, in particular we can now introduce a
function that would compose Frequency Parameters IE for both
write_packet_{downlink,uplink}_assignment().
Tested manually by running a GPRS-enabled network, as well as by
running test cases from ttcn3-pcu-test => no regressions observed.
Change-Id: I2850b91e0043cdca8ae7498a5fc727eeedd029b6
Related: SYS#4868, OS#4547
|
|
Initialize the bit vector to use already allocated memory,
so we would not need to allocate additional 23 bytes and
copy them from the bit vector to a msgb.
Change-Id: I4190707d7fa5b1c4c3db745635f88d5afb9e21ca
|
|
Change-Id: Ib42770cb009e8d559f733ebedd058e2f0a18820a
|
|
Since commit 322456ed474a733094c9f3e240295b469023ad14 (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
|
|
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
|
|
The function is simply setting the ta on the ms, so simply make sure ta
is set on callers before passing the ms object.
Change-Id: Iebb9c57f458690e045ddc45c800209ad8cf621e0
|
|
Same as previous commit, this time for the DL counterpart.
Change-Id: I87f6cdf8288a688466020bda0874e68b57aa71c4
|
|
It's really non-sense from architectural point of view to pass an
optional pointer to the MS holding the TBF and creating it otherwise.
TBFs shouldn't be creating MS they belong too.
This simple change requiring so many code line changes really exhibits
how badly entangled the object relationship is.
Another commit will follow doing the same for dl tbf.
Change-Id: I010aa5877902816ae246e09ad5ad87946f96855c
|
|
Should fix assertion triggered due to the tbf not set up properly
beforehand.
Fixes: OS#4524
Change-Id: I267b147520ef5a50f40ad4bc19e7b5fb3e708127
|
|
Original file from wireshark.git (packet-gsm_csn1.c) is being built and
maintained as a C file. There's no real need for us to maintain it as a
C++, and doing so will make both files derive over time (as already
happened). Let's keep it as a C compiler (which btw seems to be more
strict) to make it easier to port patches back and forth wireshark.git.
Take the chance to move some declarations we added to csn1.h to be able
to build it out of wireshark. Let's keep those in a separate header file
to ease looking for differences.
Change-Id: I818a8ae947f002d35142f9f5473454cfd80e1830
|
|
Including the <new> header is required as explained by the c++ specs [1]
osmo-pcu/src/tbf.cpp: In function ‘gprs_rlcmac_ul_tbf* tbf_alloc_ul_tbf(gprs_rlcmac_bts*, GprsMs*, int8_t, uint8_t, uint8_t, bool)’:
osmo-pcu/src/tbf.cpp:1002:39: error: no matching function for call to ‘operator new(sizetype, gprs_rlcmac_ul_tbf*&)’
1002 | new (tbf) gprs_rlcmac_ul_tbf(bts->bts);
| ^
Most of the times this issue is not detected because other STL headers
are already including <new>.
[1] http://www.cplusplus.com/reference/new/operator%20new/
Change-Id: Ie5fb536ae29dcf40e2a0dbe67432bebd61b8c7aa
|
|
Change-Id: I3091d917a13e45b3aef8e52a53dcafa308581652
|
|
The problem is that bitvec_free() is not NULL-safe. Ideally we
need to fix it in libosmocore [1], but let's also fix it here,
so OsmoPCU can be safely used with older libosmocore versions.
[1] https://gerrit.osmocom.org/c/libosmocore/+/17114
Change-Id: I7647d17b3d03f8e193ef6e793a2d3c1967744eef
Fixes: CID#208181, CID#208179
|
|
Change-Id: Ice4c4db20551753fa4219e7a216309229f7a2ab5
|
|
Change-Id: Iabcb768bd714680aa768b35c786dea2015d1e451
|
|
Change-Id: Id0663a81f439f2d0b893b0d34f85a6db1927ef8e
|
|
Fixes: 9d1cdb1f697057033394590a9b2815efe6c08cd9
Change-Id: Id258589d46de42ad4e27889bc396f930b7f94b79
|
|
It's a good start towards clearing current mess between parent and the 2
children classes.
Change-Id: Ibc22ea2e02609af7ee058b8bc15df2115d4c6f60
|
|
This will allow for configuration of some of the timers by the user,
and allow him to inspect current values being used.
It will be also useful for TTCN3 tests which may want to test some of
the timers without having to wait for lots of time.
Timers are splitted into 2 groups: BTS controlled ones and PCU controlled
ones. The BTS controlled ones are read-only by the user (hence no
"timer" VTY command is provided to change them).
TbfTest.err output changes due to timers being set up correctly as a
consequence of changes. Other application such as pcu_emu.cpp and
pcu_main.cpp had to previosuly set the initial values by hand (and did
so), but apparently TbfTest.c was missing that part, which is now fixed
for free.
Depends: libosmocore.git Id56a1226d724a374f04231df85fe5b49ffd2c43c
Change-Id: I5cfb9ef01706124be262d4536617b9edb4601dd5
|
|
Those namings my collide with usual osmocom "T" variable name associated
to a timer number, which will be added in following patches.
Change-Id: Ic2b5068a4882e4a043bf81496be30a378fdb9a09
|
|
Change-Id: I381618eb55cc513cfa9c2e384c27cead0935c8bf
|
|
The bts is not used at all.
Change-Id: Ia07755e825913a16352ab13f6cf55f2918de8681
|
|
In preparation for Channel Coding Command encoder in follow-up patches
let's add necessary helpers. Those are similar to previously used
helpers from GprsCodingScheme class but without CamelCase and with less
typo chances between Gprs and Egprs cases.
Change-Id: I6699cbc8d7ae766fa4d2b3d37e5f9ff1cf158b7e
|
|
Move Mode (EDGE/GPRS) definition and related functions outside of
GprsCodingScheme class. This allows us to use standard libosmocore
value_string functions.
Change-Id: I3baaac7f1ca3f5b88917a23c1679d63847455f47
|
|
* remove unused variable
* use bool for boolean types
* add clarification comments
Change-Id: I363445063e2d873d9194b2a5924b9e59b8b7ea53
|