- Always print the state after a state transition.
This shows that actually state transitions are missing for states that have
no timer defined. This is a bug in tdef_test.c, to be fixed subsequently.
- Instead of total time passed since start, print the individual fake time
intervals. Omit initial useless zero fake time advance.
- Add two more state transitions, back out from and into a state that has no
timeout set.
Change-Id: Icb31af96d37741e256ff07868f3d4f5c48cdda74
fi->T values are int, i.e. can be negative. Do not log them as unsigned, but
define a distinct timer class "Xnnnn" for negative T values: i.e. for T == -1,
print "Timeout of X1" instead of "Timeout of T4294967295".
The negative T timer number space is useful to distinguish freely invented
timers from proper 3GPP defined T numbers. So far I was using numbers like
T993210 or T9999 for invented T, but X1, X2 etc. is a better solution. This way
we can make sure to not accidentally define an invented timer number that
actually collides with a proper 3GPP specified timer number that the author was
not aware of at the time of writing.
Add OSMO_T_FMT and OSMO_T_FMT_ARGS() macros as standardized timer number print
format. Use that in fsm.c, tdef_vty.c, and adjust vty tests accordingly.
Mention the two timer classes in various API docs and VTY online-docs.
Change-Id: I3a59457623da9309fbbda235fe18fadd1636bff6
Check return value of RACH encode/decode functions and fail test on
unexpected results.
Change-Id: I41bfa808e3c064a11152e7ce8ee77a01d38a0744
Related: OS#1854
Add a flag that adds timeout info to osmo_fsm_inst state change logging.
To not affect unit testing, make this an opt-in feature that is disabled by
default -- mostly because osmo_fsm_inst_state_chg_keep_timer() will produce
non-deterministic logging depending on timing (logs remaining time).
Unit tests that don't verify log output and those that use fake time may also
enable this feature. Do so in fsm_test.c.
The idea is that in due course we will add osmo_fsm_log_timeouts(true) calls to
all of our production applications' main() initialization.
Change-Id: I089b81021a1a4ada1205261470da032b82d57872
This function performs sending a NS-ALIVE PDU and starting Tns-Test,
let's use it in all places where we used to do that.
As part of this, also fix a bug where the sendto() return value (number
of bytes sent) would actually propagate up all the way to
gprs_ns_rx_reset() return value, which in turn affects the test results
on stdout.
Change-Id: I4d303117f77fabb74bbb91887b9914a81c2a084a
Add functions to dump LCLS (without GCR) and GCR. Dumping entire struct
results in inconveniently long string hence the separate functions. Both
use talloc functions so they expect caller to take care of providing
proper allocation context and freeing memory.
Change-Id: Ic3609224c8f3282d667e75f68bc20327e36eb9e6
We are using macros like this or different workarounds in libmsc. In the course
of implementing inter-MSC handover, I am encountering yet another such
situation of appending multiple strings to a limited char buffer. Standardize.
Add a unit test to utils_test.c.
Change-Id: I2497514e26c5e7a5d88985fc7e58343be1a027b2
It's defined in logging.h for quite some time but is not actually
enabled alongside with other internal logging categories.
Change-Id: I0e7a2add6293a072752900608c8ba34cc3850f31
Run INT_MAX and ULONG_MAX related tests only manually, remove from automatic
testing. This will hopefully fix recent build failures on various platforms.
Add a 64 bit output example for expected results when invoking
`./tdef_test range'. This is not checked automatically and merely serves for
manual reference.
For vty tests, use 32bit max values instead of INT_MAX and ULONG_MAX.
Change-Id: I6242243bde1d7ddebb858512a1f0b07f4ec3e5c2
This is similar to msgb_tailroom(): It returns the amount of space
left at the end of the bit vector (compared to the current cursor).
The function returns the number of bits left in the bitvec.
Change-Id: I8980a6b6d1973b67a2d9ad411c878d956fb428d1
Move T_def from osmo-bsc to libosmocore as osmo_tdef. Adjust naming to be more
consistent. Upgrade to first class API:
- add timer grouping
- add generic vty support
- add mising API doc
- add C test
- add VTY transcript tests, also as examples for using the API
From osmo_fsm_inst_state_chg() API doc, cross reference to osmo_tdef API.
The root reason for moving to libosmocore is that I want to use the
mgw_endpoint_fsm in osmo-msc for inter-MSC handover, and hence want to move the
FSM to libosmo-mgcp-client. This FSM uses the T_def from osmo-bsc. Though the
mgw_endpoint_fsm's use of T_def is minimal, I intend to use the osmo_tdef API
in osmo-msc (and probably elsewhere) as well. libosmocore is the most sensible
place for this.
osmo_tdef provides:
- a list of Tnnnn (GSM) timers with description, unit and default value.
- vty UI to allow users to configure non-default timeouts.
- API to tie T timers to osmo_fsm states and set them on state transitions.
- a few standard units (minute, second, millisecond) as well as a custom unit
(which relies on the timer's human readable description to indicate the
meaning of the value).
- conversion for standard units: for example, some GSM timers are defined in
minutes, while our FSM definitions need timeouts in seconds. Conversion is
for convenience only and can be easily avoided via the custom unit.
By keeping separate osmo_tdef arrays, several groups of timers can be kept
separately. The VTY tests in tests/tdef/ showcase different schemes:
- tests/vty/tdef_vty_test_config_root.c:
Keep several timer definitions in separately named groups: showcase the
osmo_tdef_vty_groups*() API. Each timer group exists exactly once.
- tests/vty/tdef_vty_test_config_subnode.c:
Keep a single list of timers without separate grouping.
Put this list on a specific subnode below the CONFIG_NODE.
There could be several separate subnodes with timers like this, i.e.
continuing from this example, sets timers could be separated by placing
timers in specific config subnodes instead of using the global group name.
- tests/vty/tdef_vty_test_dynamic.c:
Dynamically allocate timer definitions per each new created object.
Thus there can be an arbitrary number of independent timer definitions, one
per allocated object.
T_def was introduced during the recent osmo-bsc refactoring for inter-BSC
handover, and has proven useful:
- without osmo_tdef, each invocation of osmo_fsm_inst_state_chg() needs to be
programmed with the right timeout value, for all code paths that invoke this
state change. It is a likely source of errors to get one of them wrong. By
defining a T timer exactly for an FSM state, the caller can merely invoke the
state change and trust on the original state definition to apply the correct
timeout.
- it is helpful to have a standardized config file UI to provide user
configurable timeouts, instead of inventing new VTY commands for each
separate application of T timer numbers.
Change-Id: Ibd6b1ed7f1bd6e1f2e0fde53352055a4468f23e5
Since very recently we sensibly handle commands like
cmd ([one]|[two]|[three])
as optional multi-choice arguments. In addition, support the more obvious
syntax of
cmd [(one|two|three)]
Internally, the tokens are mangled to [one] [two] and [three], which is how the
rest of the code detects optional args, and makes sense in terms of UI:
> cmd ?
[one]
[two]
[three]
(i.e. optional arguments are always shown in braces in '?' listings)
Before this patch, commands defined with a syntax like [(one|two)], would lead
to an assertion (shows as "multiple") during program startup.
Change-Id: I952b3c00f97e2447f2308b0ec6f5f1714692b5b2
Add basic optional multi-choice argument support.
The VTY detects optional arguments by square braces.
> cmd ?
[optional-arg]
> cmd optional-arg
ok
> cmd
ok
However, within multi-choice args, these braces were so far not treated as
optional:
> list
cmd2 ([one]|[two]|[three])
> cmd2
% Command incomplete
In preparation for I952b3c00f97e2447f2308b0ec6f5f1714692b5b2 which will enable
the more obvious syntax of
cmd [(one|two)]
for reasons of internal implementation, first support a syntax of
cmd ([one]|[two])
The internal vty implementation always needs square braces around each option.
There is currently no good way to prevent developers from defining braces
inside multi-arguments, so it is easiest to allow and handle them:
> list
cmd2 ([one]|[two]|[three])
> cmd2
ok
The VTY doesn't guard against a mix like
cmd (one|[two])
With this patch, a multi-choice command is treated as optional iff the first
element is in square brackets. The remaining elements' square brackets have no
effect besides confusing the user. This is not explicitly checked against.
In general, I would prefer to check all of these details, but the current VTY
code with its endless code duplication and obscure string mangling just doesn't
provide that luxury. There are numerous worse errors hidden in there.
Change-Id: I9a8474bd89ddc2155c58bfca7bd038d586aaa60a
I want to tweak general VTY features and need to cover with a transcript test
to show the differences. Start by showing the current situation of optional
and multi-choice arguments.
Change-Id: I5a79c83fabd02aba6406b6e0d620969c4bd0cc1d
Before this patch, if timeout_secs == 0 was passed to
osmo_fsm_inst_state_chg(), the previous T value remained set in the
osmo_fsm_inst->T.
For example:
osmo_fsm_inst_state_chg(fi, ST_X, 23, 42);
// timer == 23 seconds; fi->T == 42
osmo_fsm_inst_state_chg(fi, ST_Y, 0, 0);
// no timer; fi->T == 42!
Instead, always set to the T value passed to osmo_fsm_inst_state_chg().
Adjust osmo_fsm_inst_state_chg() API doc; need to rephrase to accurately
describe the otherwise unchanged behaviour independently from T.
Verify in fsm_test.c.
Rationale: it is confusing to have a T number remaining from some past state,
especially since the user explicitly passed a T number to
osmo_fsm_inst_state_chg(). (Usually we are passing timeout_secs=0, T=0).
I first thought this behavior was introduced with
osmo_fsm_inst_state_chg_keep_timer(), but in fact osmo_fsm_inst_state_chg()
behaved this way from the start.
This shows up in the C test for the upcoming tdef API, where the test result
printout was showing some past T value sticking around after FSM state
transitions. After this patch, there will be no such confusion.
Change-Id: I65c7c262674a1bc5f37faeca6aa0320ab0174f3c
It was introduced and forgotten to add to EXTRA_DIST in:
"logging vty: add VTY transcript test"
commit 3a9ff11e57
change-Id I948e832a33131f8eab98651d6010ceb0ccbc9a9c
Change-Id: I1bcedf3097f02b2adc679560d1cbceb27dbc345e
Add osmo_hexdump_buf() as an all-purpose hexdump function, which all other
osmo_hexdump_*() implementations now call. It absorbs the static
_osmo_hexdump(). Add tests for osmo_hexdump_buf().
Rationale: recently during patch review, a situation came up where two hexdumps
in a single printf would have been useful. Now I've faced a similar situation
again, in ongoing development. So I decided it is time to provide this API.
The traditional osmo_hexdump() API returns a non-const char*, which should
probably have been a const instead. Particularly this new function may return a
string constant "" if the buf is NULL or empty, so return const char*. That is
why the older implementations calling osmo_hexdump_buf() separately return the
buffer instead of the const return value directly.
Change-Id: I590595567b218b24e53c9eb1fd8736c0324d371d
Add
* osmo_lai_cmp() (to use in gsm0808_cell_id_u_matches())
* osmo_cgi_cmp() (to use in gsm0808_cell_id_u_matches())
* gsm0808_cell_id_u_match() (to re-use for single IDs and lists)
* gsm0808_cell_ids_match()
* gsm0808_cell_id_matches_list()
* Unit tests in gsm0808_test.c
Rationale:
For inter-BSC handover, it is interesting to find matches between *differing*
Cell Identity kinds. For example, if a cell as CGI 23-42-3-5, and a HO for
LAC-CI 3-5 should be handled, we need to see the match.
This is most interesting for osmo-msc, i.e. to direct the BSSMAP Handover
Request towards the correct BSC or MSC.
It is also interesting for osmo-bsc's VTY interface, to be able to manage
cells' neighbors and to trigger manual handovers by various Cell Identity
handles, as the user would expect them.
Change-Id: I5535f0d149c2173294538df75764dd181b023312
The IMEI-SV is speified as a 16 digit number: 14 digits of IMEI plus
two digits of software version. Let's not try to feed 18 digit long
numbers into our functions, as the resulting behavior is unspecified.
Change-Id: I6fb85a0516dc387902ad9de4fe8c1ba82d68cae6
Most of the time we'll have GCR filled anyway so it make sense to have
it as static parameter instead of a pointer to separately allocated
structure. Update tests to cover both static and dynamic osmo_lcls
allocation variants.
Change-Id: I905c36d8455911c68c30bc429379b7313dd46aea
* add gsm0808_create_ass_compl2() with additional gsm0808_lcls_status
parameter and make gsm0808_create_ass_compl() into trivial wrapper
around it
* update tests accordingly
Change-Id: I547c6b8707123aa8c1ef636db88908df112d90a4
Related: OS#2487
Verify 14 digit and 15 digit IMEI strings. OsmoHLR will use the 14
digit version to check IMEIs before writing them to the DB.
Place the Luhn checksum code in a dedicated osmo_luhn() function, so
it can be used elsewhere.
Related: OS#2541
Change-Id: Id2d2a3a93b033bafc74c62e15297034bf4aafe61
As part of fixing issue OS#3075, we want to migrate support
for encoding system information from osmo-bsc to libosmocore.
This change ports one of the prerequisites for doing so:
osmo-bsc code for range-encoding ARFCNs, including tests.
An osmo_gsm48_ prefix has been prepended to public symbols in
order to avoid clashes with existing symbols in osmo-bsc code.
Change-Id: Ia220764fba451be5e975ae7c5eefb1a25ac2bf2c
Related: OS#3075
We have gsm48_mi_to_string() and osmo_bcd2str(), but still lack a function that
conveniently prints both MI type and value in one function call.
Related: http://people.osmocom.org/neels/mi_mi_mi.jpg
Change-Id: I7798c3ef983c2e333b2b9cbffef6f366f370bd81
Both session state and session ID IEs were left from the initial
version of Ibe325c64ae2d6c626b232533bb4cbc65fc2b5d71. There is
no need to use them (as we use SM-RP-MR), so let's clean up.
Change-Id: I0d910b87f15ffbc0aeeca9cb4fcbef32bdf3ef88
In 3GPP TS 48.008 the Global Call Reference IE is only used in HANDOVER
REQUEST (§3.2.1.8) and ASSIGNMENT REQUEST (§3.2.1.1) messages which
also include LCLS Config and CSC parameters. Hence, there's no point in
using GCR encode/decode functions alone.
Introduce gsm0808_dec_lcls() and gsm0808_enc_lcls() as trivial wrappers
on top of GCR enc/dec routines which are made static. Adjust tests
accordingly. Test output intentionally left unchanged.
Change-Id: Icfbb2404e1a1d500243e2071173299b557369335
Since I3bc95f2f5ab6e3f4b502647fb3e0aaaf1f7c4cf5, we have some
helpers to compare certain msgb layer to a given buffer. Let's
change 'VERIFY' macro to use msgb_eq_l3_data_print().
Change-Id: Ib6be778236eff8f2153f3113f9379ecfbec9052b
Implement necessary messages for Procedure Check_IMEI_VLR (TS 23.018
Chapter 7.1.2.9). This lets the VLR ask the EIR to check if an IMEI
is valid. In the Osmocom stack, we don't have an EIR and this request
will be handled by the HLR. We will be able to store the IMEI in the
HLR as side-effect (OS#2541).
This is roughly based on TS 29.002 8.7.1 MAP_CHECK_IMEI service, but
only implements the bare minimum required IEs (imei and imei_result).
Related: OS#3733
Change-Id: I085819df0ea7f3bfeb0cabebb5fd1942a23c6155
Simplify gsup_test.c by defining an end marker in gsup.h. No need to
manually update the last element every time anymore.
The C standard guarantees, that the end marker will have the last value
plus one: "Each subsequent enumerator with no = defines its enumeration
constant as the value of the constant expression obtained by adding 1 to
the value of the previous enumeration constant." (From C99: 6.7.2.2
Enumeration specifiers)
Change-Id: I2aab7245e209f0ebd2f33a83d4d181dd3339cb17
This is 1:1 the result of doing
cd libosmocore
./contrib/struct_endianess.py
git commit -a
Running struct_endianess.py again should result in no changes.
That means we could include such a check in the gerrit verification job now.
Change-Id: Ia0b99d76932aeb03e93bd0c62d3bf025dec5f9d2
According to 3GPP TS 29.002, section 12.4, MAP-READY-FOR-SM is
used between the MSC and VLR as well as between the VLR and the
HLR to indicate that a subscriber has memory available for SMS.
This change replicates this service in GSUP as READY_FOR_SM_*.
The only mandatory IE for this service (excluding Invoke ID) is
'Alert Reason' that is replicated by OSMO_GSUP_SM_ALERT_RSN_IE.
Change-Id: Ic37f3b2114b8095cfce22977e67133b9103942e3
Related Change-Id: (docs) I549b6c8840a1e86caac09e77fb8bc5042d939e62
Related Change-Id: (TTCN) If2256607527ecfcb10285583332fb8b0515d7c78
Related: OS#3587
According to 3GPP TS 29.002, there are two services:
- MAP-MO-FORWARD-SHORT-MESSAGE (see 12.2),
- MAP-MT-FORWARD-SHORT-MESSAGE (see 12.9),
which are used to forward MO/MT short messages.
This change replicates both services as GSUP messages:
- OSMO_GSUP_MSGT_MO_FORWARD_SM_*,
- OSMO_GSUP_MSGT_MT_FORWARD_SM_*.
Please note, that only the 'must-have' IEs are introduced
by this change, in particular the following:
- OSMO_GSUP_SM_RP_MR_IE (see note below),
- OSMO_GSUP_SM_RP_DA_IE (see 7.6.8.1),
- OSMO_GSUP_SM_RP_OA_IE (see 7.6.8.2),
- OSMO_GSUP_SM_RP_UI_IE (see 7.6.8.4),
- OSMO_GSUP_SM_RP_MMS_IE (see 7.6.8.7),
- OSMO_GSUP_SM_RP_CAUSE_IE (see GSM TS 04.11, 8.2.5.4),
where both SM_RP_DA and SM_RP_OA IEs basically contain
a single nested TV of the following format:
- T: identity type (see 'osmo_gsup_sms_sm_rp_oda_t'),
- V: encoded identity itself (optional).
According to GSM TS 04.11, every single message on the SM-RL has
an unique message reference (see 8.2.3), that is used to link
an RP-ACK or RP-ERROR message to the associated (preceding)
RP-DATA or RP-SMMA message transfer attempt.
In case of TCAP/MAP, this message reference is being mapped to the
Invoke ID. But since GSUP has no 'Invoke ID' IE, and it is not
required for other applications (other than SMS), this change
introduces a special 'SM_RP_MR' IE that doesn't exist in MAP.
Change-Id: Ibe325c64ae2d6c626b232533bb4cbc65fc2b5d71
Related Change-Id: (docs) Ie0150756c33c1352bc4eb49421824542c711175c
Related Change-Id: (TTCN) Ibf49474a81235096c032ea21f217170f523bd94e
Related: OS#3587
* add functions to encode Global Call. Ref. from TS 29.205 as 3GPP TS
48.008 §3.2.2.115 information element
* add corresponding tests
Change-Id: I82ce0207dc8de50689a8806c6471ad7fbae6219d
Add functions to encode and decode Global Call Reference as per
3GPP TS 29.205 Table B 2.1.9.1 add corresponding tests.
Change-Id: Iee95aa4e5c056645b6cb5667e4a067097d52dfbf
Related: OS#2487
All successful cases already return from the switch(), so simply handle all
errors below it by returning an empty string (if there is enough string
buffer).
Change-Id: I709ac3b9efb7b4258d8660715b10312e11b9b571
For MI encoding, see 3GPP TS 24.008, 10.5.1.4 Mobile Identity. The 'odd' flag
indicates whether the last BCD nibble is used. Of course that flag should be
made sure to reflect the actual length.
Change-Id: Id6e695ebf9f86b295eaa7e2c6228989256f37e68
By using osmo_bcd2str(), ensure that the resulting string is always nul
terminated, and always return strlen()+1 whether truncated or not.
Still keep up the previous return value style, even if that isn't consistent at
all.
The difference between IMSI/IMEI and TMSI return values remains and is not part
of this patch.
Change-Id: I1b51b72a721e1cc9d69796b804ebda741ff0f36b
Add a standalone bcd-to-string conversion function with generic parameters.
Add a regression test in utils_test.c.
So far there is no single universal implementation that converts a BCD to a
string. I could only find gsm48_mi_to_string(), which also interprets
surrounding bytes, MI type and TMSI as non-BCD value.
The idea is to use this function from gsm48_mi_to_string() and similar
implementations in subsequent commits.
Root cause: in osmo-msc, I want to have an alternative MI-to-string function
for composing an FSM name, which needs the BCD part of gsm48_mi_to_string() but
not the TMSI part.
Change-Id: I86b09d37ceef33331c1a56046a5443127d6c6be0
One would think by now we would solidly encode and decode Mobile Identities.
Well, guess again.
- rc is sometimes the amount of bytes written, sometimes actual strlen().
- on string truncation, rc is sometimes strlen() (assuming nul terminated), and
sometimes snprintf()-style would-be strlen().
- returned string, when truncated by not enough buffer size, is sometimes nul
terminated, sometimes not.
- gsm48_mi_to_string() happily reads a byte from zero-length input buffer.
- gsm48_mi_to_string() happily writes to zero length output buffer.
- gsm48_mi_to_string() returns nonempty string for empty input.
- encoding a MI type that still has the GSM_MI_ODD flag set results in encoding
an even-length MI as odd-length (hence appending a stray 'F').
I am going to tweak the implementation of gsm48 mobile identity encoding /
decoding, so first pinpoint the current behavior in a unit test, and show how
perforated even such a seemingly trivial API can be.
Change-Id: Iaae3af87f82f1a8f2e6273984c011b2813038cf7
At the moment the all gsm0808 cause codes are encoded directly using the
tlv API directly to put a one byte TLV field. This works ok for most
situations where the cause code consists of a single byte. However,
gsm0808 specifies a two byte cause code model where cause codes may be
extended up to two bytes. Instead of implementing the encoding over and
over and again, let's rather have an encoder function we can call.
- Add an encoder function that can generate single byte and extended
cause codeds and makes the length decision automatically.
- Use only this function to append cause codes
Change-Id: I71d58fad89502a43532f60717ca022c15c73f8bb
In gprs_ns_process_msg(), we were dispatching the S_NS_UNBLOCK
signal before sending out the NS_POUT_UNBLOCK_ACK message.
Signal handlers might send messages to the other side, assuming
that NS is now unblocked. However, since such messages will arrive
before the UNBLOCK_ACK message the receiver might discard them.
This problem has been observed with our TTCN3 BSSGP_Emulation
as a peer to osmo-pcu.
This patch makes TTCN3 PCU TC_paging() test pass regardless of
whether the test or osmo-pcu is started first. Before this patch,
this test would only pass if the test was started before osmo-pcu.
A remaining problem is that the test does not yet keep passing
reliably unless osmo-pcu is restarted between test runs.
Change-Id: I3af54a14bb6bcfa167c9a9d9f67835e7f5b9f1bb
Related: OS#2890
Related: OS#2388
* add function to generate cipher mode reject with extended (2-byte)
Cause IE
* add function to get (extended) Cause value
* add corresponding (extended cause) test
* update existing (non-extended cause) test
* use enum as a parameter for existing non-extended version to make
interface more unified
Change-Id: Id5509b94a18180a44f45300caaa02b843c166fa3
Related: OS#3187
* add note about (yet) unsupported standard feature
* use enum constant instead of hex value
Change-Id: I5cc5fc1ae8d5474dbc8d3385a493adf6420a3c98
Related: OS#3187
3GPP_TS_48.008, 3.2.2.103 Speech Codec List states the following:
"The length indicator (octet 2) is a binary number indicating the
absolute length of the contents after the length indicator.
The length depends on the number and type of Speech Codec Elements
to be included. The minimum length of one Speech Codec Element is 1
octet and the maximum length is 3 octets. The maximum number of
Speech Codec Elements within the Speech Codec List is not defined."
This clearly refers only to the length of a single speech codec
element but not on the overall list. So speech codec lists with
length 0 are indeed permitted by the spec.
- Remove the assertion that checks on zero length speech codec
lists.
Change-Id: I1eb1f4466b98bdd26d765b0e4cc690b5e89e9dd6
Related: OS#3657
We must explicitly check the return value of osmo_log_vty2tgt(), or may run
into a segfault. I wasn't aware of this and introduced numerous such cases in
I36f17c131cc70ce5a1aef62fd9693097de230cd4.
Reproduce: on the VTY, do not issue 'logging enable', invoke 'logging level
force-all LEVEL' first.
Show in logging_vty_test.vty that this situation is now again amended by
telling the user that logging was not enabled.
Related: OS#3611
Change-Id: Id11702d1801d6654ca5e5a51b597a0d802e2e8dd
We want to have well-formed config files that print exactly one space
per VTY/config node level, and not two.
Change-Id: Ia75c7067284ea225cffe13ca71bad05a7747ae66
Add a function to convert S0-S15 bits to struct gsm48_multi_rate_conf,
which hold the codec settings for AMR.
Change-Id: I973736273c236eee84decf15868190e339c3fed4
Related: OS#3548
Add a function to convert struct gsm48_multi_rate_conf, which holds the
codec settings for AMR, to S0-S15 bit representation as defined in
3GPP TS 48.008 3.2.2.49
This resurrects change-id I4e656731b16621736c7a2f4e64d9ce63b1064e98
which was reverted in I9e0d405e303ed86d23703ca6362e958dddb2f861
due to gsm0808_test failing.
The test failure is fixed by properly clearing the struct
gsm48_multirate_cfg prior to running tests (add memset(0)).
Change-Id: Ia782e21c206c15e840226d79b4209d13658ee916
Related: OS#3548
Add new command to once-off set each and every category to the given log level,
as discussed at length on the openbsc@ mailing list.
Show that it works in logging_vty_test.vty.
Change-Id: I4c3e4f786476cb813fdc0a7c64f30ee04758309d
Add a function to convert struct gsm48_multi_rate_conf, which holds the
codec settings for AMR, to S0-S15 bit representation as defined in
3GPP TS 48.008 3.2.2.49
Change-Id: I4e656731b16621736c7a2f4e64d9ce63b1064e98
Related: OS#3548
Add 'logging level force-all <level>' and 'no logging level force-all' as new
names for 'logging level all <level>' and 'logging level all everything'.
Resurrect the functionality of 'logging level all everything' -- even if it is
still deprecated because the name is confusing, it is now just an alias for
'no logging level force-all'.
Show in logging_vty_test.vty that we can now again lift the global logging
clamp, both with the new commands as well as with the deprecated ones.
Also show that 'force-all' is written back properly, if set.
Change-Id: I36f17c131cc70ce5a1aef62fd9693097de230cd4
The 'logging level all everything' has not had an effect for some time now. The
plan is to bring back its old functionality, but to keep it deprecated and
rather define a less confusing name.
* Deprecate 'everything'.
* Do not write 'everything' during 'write file' or 'show running-config', which
we curiously still do until now.
BTW, the reason why we need to compose a complete list of categories for the
deprecated 'everything' command is explained in detail in the commit log for
I3b083f27e3d751ccec258880ae7676e9af959a63
Change-Id: Ib75fedb0572570a61bb34ee729a2af86cf5f16da
I am setting out to refactor various details about logging. To show the effect,
I am first adding this new test to illustrate the exact effects on the various
osmo programs.
Add logging_vty_test.c as a standalone program that simply defines a few
logging categories and opens a telnet vty to play with.
Add logging_vty_test.vty, as an osmo_verify_transcript_vty.py test script.
Add --enable-external-tests to configure.ac, to enable running
logging_vty_test.vty during 'make check'.
Also allow running 'make vty-test' without the need to first configure with
--enable-external-tests (a flexibility I've missed many times over in the other
osmo source trees).
Add a Makefile.am stub for external CTRL tests, basically a copy-paste from
osmo-msc.git. I doubt that libosmocore will get python driven CTRL interface
testing any time soon, but if so we will know to not run it concurrently.
Change-Id: I948e832a33131f8eab98651d6010ceb0ccbc9a9c
Do not link against the system-wide installed libosmo* libs when building the
regression test programs. Always use the locally built ones.
Linking some libosmo libraries causes libtool to pull in other libosmo libs
even though they were not explicitly named. For example, ctrl_test explicitly
links libosmoctrl, but this also has dependencies to libosmovty and libosmogsm:
ldd src/ctrl/.libs/libosmoctrl.so | grep osmo
libosmocore.so.11 => /usr/local/lib/libosmocore.so.11 (0x00007f26c26d4000)
libosmogsm.so.10 => /usr/local/lib/libosmogsm.so.10 (0x00007f26c22bb000)
libosmovty.so.4 => /usr/local/lib/libosmovty.so.4 (0x00007f26c2171000)
If we omit explicit LDADD of these dependencies in the Makefile.am, libtool
will take the first canonical place to find them, which may just be the already
installed older versions of the same libs, which may or may not be compatible
with the current build. In any case, it is never intended to link installed
libs.
All library dependencies are listed by this quick script:
cd libosmocore
for l in $(find . -name "*.so") ; do echo; echo "$l"; ldd $l | grep libosmo; done
./.libs/libosmocore.so
./coding/.libs/libosmocoding.so
libosmocore.so.11 => /usr/local/lib/libosmocore.so.11 (0x00007f25fc3c2000)
libosmogsm.so.10 => /usr/local/lib/libosmogsm.so.10 (0x00007f25fbfa9000)
libosmocodec.so.0 => /usr/local/lib/libosmocodec.so.0 (0x00007f25fbf9b000)
./codec/.libs/libosmocodec.so
libosmocore.so.11 => /usr/local/lib/libosmocore.so.11 (0x00007fb4c900d000)
./ctrl/.libs/libosmoctrl.so
libosmocore.so.11 => /usr/local/lib/libosmocore.so.11 (0x00007f5df5129000)
libosmogsm.so.10 => /usr/local/lib/libosmogsm.so.10 (0x00007f5df4d10000)
libosmovty.so.4 => /usr/local/lib/libosmovty.so.4 (0x00007f5df4bc6000)
./gb/.libs/libosmogb.so
libosmocore.so.11 => /usr/local/lib/libosmocore.so.11 (0x00007f788e536000)
libosmovty.so.4 => /usr/local/lib/libosmovty.so.4 (0x00007f788e3ec000)
libosmogsm.so.10 => /usr/local/lib/libosmogsm.so.10 (0x00007f788dfd3000)
./vty/.libs/libosmovty.so
libosmocore.so.11 => /usr/local/lib/libosmocore.so.11 (0x00007f3b7ed21000)
./gsm/.libs/libosmogsm.so
libosmocore.so.11 => /usr/local/lib/libosmocore.so.11 (0x00007fc69472e000)
./sim/.libs/libosmosim.so
libosmocore.so.11 => /usr/local/lib/libosmocore.so.11 (0x00007f2f6412d000)
libosmogsm.so.10 => /usr/local/lib/libosmogsm.so.10 (0x00007f2f63d14000)
Add all explicit linking of all required library dependencies in all regression
test programs, as shown by above listing.
Example for reproducing a problem:
In libosmocore.a, introduce a new function, and call that from libosmovty code.
For example, I made loglevel_strs non-static in logging.c, and used that in
logging_vty.c. Build and install this in a place where libtool can find it.
Then go back to before this change and rebuild. You will see that linking
ctrl_test (before this patch) then complains about libosmovty requiring the
loglevel_strs symbol which it cannot find in libosmocore.so.
Change-Id: Id084e6e6efd25cd62b1bd7a4fc7c5985c39130c6
We already have osmo_str2lower() and osmo_str2upper(), but these lack:
* proper destination buffer bounds checking,
* ability to call directly as printf() argument.
Deprecate osmo_str2upper() and osmo_str2lower() because of missing bounds
checking.
Introduce osmo_str_tolower_buf(), osmo_str_toupper_buf() to provide
bounds-safe conversion, also able to safely convert a buffer in-place.
Introduce osmo_str_tolower(), osmo_str_toupper() that call the above _buf()
equivalents using a static buffer[128] and returning the resulting string
directly, convenient for direct printing. Possibly truncated but always safe.
Add unit tests to utils_test.c.
Replace all libosmocore uses of now deprecated osmo_str2lower().
Naming: the ctype.h API is called tolower() and toupper(), so just prepend
'osmo_str_' and don't separate 'to_lower'.
Change-Id: Ib0ee1206b9f31d7ba25c31f8008119ac55440797
In the past, the function ipa_ccm_idtag_parse() was used to parse
the payload of IPA CCM ID RESP packets. However, the function was
based on a possible misunderstanding of the message encoding, and
callers actually counted the first (upper) length nibble as part
of the header and passed a pointer to the second
(lower) length nibble of the first TLV into this function. As such,
it was unfixable, and had to be replaced with a new function called
ipa_ccm_id_resp_parse(). At the same time, we also add
ipa_ccm_id_get_parse() to parse the slightly different format of
the IPA CCM ID GET payload.
We can never be 100% sure what is "correct", as our understanding
of the protocol is entirely based on protocol analysis, without any
official documentation available.
This patch also introduces unit test coverage for both of the new
functions.
Revert "ipa: Add libosmogsm.map entry for ipa_ccm_idtag_parse_off"
This reverts commit 7f31c90b80.
Revert "ipa: Properly parse LV stream of a ID_GET request"
This reverts commit f558ed4bb9.
It introduced a function/behavior that was not originally intended:
The parse of IPA CCM ID GET (8bit length followed by 1 byte tag
and variable-length payload) instead of the IPA CCM ID RESP (16bit
length followed by 1 byte tag and variable-length payload).
Change-Id: I1834d90fbcdbfcb05f5b8cfe39bfe9543737ef8f
As oap_client has moved from osmo-sgsn to libosmogsm, it is only fair
that the related unit test shall also be moved here.
Change-Id: I9d64e10b4bacac9b530cf077841bad762fc6d558
This buffer verifies that all XMAXC fields must be zero before the
entire buffer is considered as silent by osmo_ecu_fr_conceal().
Change-Id: I14a192d001b5e167437cedbe76a1a3dd84dde35c
This makes it easy to debug how XMAXC fields are decreased every
iteration in osmo_ecu_fr_conceal().
Change-Id: I678d4be5e0b15b05873b0d3bf5ea5bbee7bef839
Recent OS#3407 shows that we should verify stderr to catch sanitizer failures.
(They might not always be ignorable like that one.)
Change-Id: Ic9e437a1cc96ae081e0fd6a9b6e3156987e14c0c
Multiplying the uint16_t x by itself seems to default to be calculated in
int32_t range, while it obviously needs uint32_t. This causes sporadic
sanitizer barfs:
Testing integer square-root
utils_test.c:445:18: runtime error: signed integer overflow: 60369 * 60369 cannot be represented in type 'int'
The final result is still correct, because it is in fact interpreted as uint32_t.
Cast to uint32_t to make sure the sanitizer doesn't complain.
Related: OS#3407
Change-Id: I83c14e38deaa466d977ee43c9420534ed90f090d
vty_test: add test against ambiguous cmd causing use-after-free and memory
leaks. Add this test along with the fix, because the new test triggers the
memory use-after-free and leaks, causing build failures.
Add cmd_deopt_with_ctx() to allow passing a specific talloc ctx.
is_cmd_ambiguous(): keep all cmd_deopt() allocations until the function exits.
Add a comment explaining why. Before this, if a command matched an optional
"[arg]" with square brackets, we would keep it in local var 'matched', but we
would free the string it points to at the end of that loop iteration; upon
encountering another match, we would attempt to strcmp against the freed
'matched'. Instead of adding hard-to-read and -verify free/alloc dances to keep
the 'matched' accurately freed/non-freed/..., just keep all cmd_deopt() string
allocated until done.
Needless to say that this should have been implemented on a lower level upon
inventing optional args, but at least this is fixing a program crash.
Related: OS#33903390
Change-Id: Ia71ba742108b5ff020997bfb612ad5eb30d04fcd
The conv_gen.py utility was tested against both Python 2 and 3,
so there is no need to enforce Python 2. Also, having:
#!/usr/local/bin/python{2|3}
is a bad idea, because Python may be installed in a different location.
Change-Id: I6007d481047b584db13d6eda70fb99f11f9ddaa1
This function can be used when there is only a part of GSM 04.80
message available - Facility IE, e.g. when a message is carried
over GSUP/MAP. Let's expose it.
Refactoring includes the following:
- adding the 'gsm0480_' prefix;
- correcting inverted return value;
- cosmetic code style changes.
Change-Id: I623c39ffbe6cdee65eade8435a2faa04d0da193e
In some cases, there is no need to parse the whole message,
e.g. during the conversion from DTAP to GSUP/MAP. This
function can be used to extract given IE from a message.
Change-Id: I3989d061903352473305f80712f1a1560d05df3d
In order to be able to transfer SS/USSD messages via GSUP,
this change introduces the following new message types:
- OSMO_GSUP_MSGT_PROC_SS_*,
and the following new IE:
- OSMO_GSUP_SS_INFO_IE
which represents an ASN.1 encoded MAP payload coming to/from
the mobile station 'as is', without any transcoding.
Change-Id: Ie17a78043a35fffbdd59e80fd2b2da39cce5e532
Related: OS#1597
Unlike TCAP/MAP, GSUP is just a transport layer without the
dialogue/context. This prevents us from having session based
communication, required e.g. for USSD. But we can emulate
TCAP dialogue by adding additional IEs, which would allow
to relate each message to a particular session.
This change introduces the following IEs:
- OSMO_GSUP_SESSION_ID_IE,
- OSMO_GSUP_SESSION_STATE_IE,
which optionally can be used to indicate that the message is
related to a session with given ID, and to manage session
state, i.e. initiate, continue, and finish.
Change-Id: I1cee271fed0284a134ffed103c0d4bebbcfde2a8
Related: OS#1597
Fixes following AddressSanitizer report during gea_test run with gcc
8.1.0:
==8899==ERROR: AddressSanitizer: dynamic-stack-buffer-overflow on address 0x7ffc5f1719bb at pc 0x7fe574adc5fe bp 0x7ffc5f171460 sp 0x7ffc5f171450
WRITE of size 1 at 0x7ffc5f1719bb thread T0
#0 0x7fe574adc5fd in osmo_store64be_ext ../../include/osmocom/core/bit64gen.h:75
#1 0x7fe574adc649 in osmo_store64be ../../include/osmocom/core/bit64gen.h:104
#2 0x7fe574ade936 in _kasumi_kgcore libosmocore/src/gsm/kasumi.c:186
#3 0x7fe574ae2532 in gea4 libosmocore/src/gsm/gea.c:44
#4 0x7fe574ae266c in gea3 libosmocore/src/gsm/gea.c:60
#5 0x7fe574a9b616 in gprs_cipher_run libosmocore/src/gsm/gprs_cipher_core.c:95
#6 0x56422d3fb2ee in test_gea libosmocore/tests/gea/gea_test.c:29
#7 0x56422d3fb506 in main libosmocore/tests/gea/gea_test.c:49
#8 0x7fe5730f406a in __libc_start_main (/usr/lib/libc.so.6+0x2306a)
#9 0x56422d3fadf9 in _start (libosmocore/tests/gea/.libs/lt-gea_test+0x1df9)
The kasumi_test is updated to calculate the entire array of bits
according to expected result. Before this commit it worked by writing
the entire last 64bit block, and addressSanitizer cannot catch it
because the allocated buffer is 64bit aligned too.
Change-Id: I7b2a0224a3b5527d5a3ad7e17efc73081b63eac1
Before this patch, osmo_hexdump is called stacked in th esame printf
function. As a result, the first returned buffer is overwriten by the
second, which means the printed buffers will show as the same always.
Change-Id: I364328a59da31537c6c9b969e34edd360b685081
Fixes following AddressSanitizer report:
==1983==ERROR: AddressSanitizer: dynamic-stack-buffer-overflow on address 0x7ffc245f47e6 at pc 0x7f3e2deea68c bp 0x7ffc245f4750 sp 0x7ffc245f4740
WRITE of size 1 at 0x7ffc245f47e6 thread T0
#0 0x7f3e2deea68b in osmo_nibble_shift_right libosmocore/src/bits.c:92
#1 0x55c01902e1ab in sh_chk libosmocore/tests/bits/bitrev_test.c:215
#2 0x55c01902ed8f in main libosmocore/tests/bits/bitrev_test.c:305
#3 0x7f3e2c93006a in __libc_start_main (/usr/lib/libc.so.6+0x2306a)
#4 0x55c01902c059 in _start (libosmocore/tests/bits/.libs/lt-bitrev_test+0x5059)
This patch can be seen as a follow-up of commit
4fd6023b03, which already fixed the
left-shift case in the same way.
Change-Id: I6e86d0164b7e982bf7b7449d5b3abfb3e1e5da46
Sometimes the library probiding dlopen is not the same one providing
dlsym.
This is the case when compiling with AddressSanitizer enabled. In this
case, AC_SEARCH_LIBS([dlopen]...) reports no lib is required, but tests
using dlsym still require to link against -ldl.
Change-Id: Ic619b0885688066b60c97caf1e2c7e5402c1d9f7
In recent Iaa20c59f624fbdc69a018cabd0f7e9c5a1389519 I fixed one missing init
issue and didn't notice the N other similar ones right next to it. Also fix the
remaining missing inits.
Fixes:
../../../../src/libosmocore/src/gsm/gsm0808_utils.c:187:8: runtime error: load of value 13, which is not a valid value for type '_Bool'
../../../../src/libosmocore/src/gsm/gsm0808_utils.c:191:8: runtime error: load of value 119, which is not a valid value for type '_Bool'
Related: OS#3148
Change-Id: Ie8a1a9b3132024135ca70390eae4d21c907b2edc
3GPP TS 48.058 has a very clear definition of which messages are
"transparent" and hence have the T-bit == 1. This is *not* just
all RLL messages, but basically only RLL_DATA.{ind,req} and
RLL_UNITDATA.{ind,req}. All other messages are non-transparent.
Change-Id: I9f83654af189d818563d799bf623325b7fee8e70
Closes: OS#3188
Provide comprehensive API to obtain string representations of Cell Identifiers
and -Lists.
Change gsm0808_test.c to use the new functions (which simplifies the output a
bit), so that we don't duplicate printing code in gsm0808_test.c, and so that
the not-so-trivial printing code is also tested.
In gsm0808_test, also test gsm0808_cell_id_list_name_buf()'s return value and
truncation behavior.
The rationale for gsm0808_cell_id_list_name(), i.e. printing an entire list of
cell identifiers, is that even though the maximum is 127 elements, a list of
more than a few elements is hardly ever expected in practice (even more than
one element isn't actually expected: either "entire BSS" or a single LAC). It
is thus useful to log the entire list when it shows up in Paging and Handover.
Change-Id: I9b2106805422f96c5cc96ebb9178451355582df3
The uninitialized members of enc_sc sporadically hit address sanitizer failure
during gsm0808_test, like:
../../../../src/libosmocore/src/gsm/gsm0808_utils.c:187:8: runtime error: load of value 13, which is not a valid value for type '_Bool'
../../../../src/libosmocore/src/gsm/gsm0808_utils.c:191:8: runtime error: load of value 119, which is not a valid value for type '_Bool'
How the test survived so long is a mystery to me; as soon as some uninitialized
members would by coincidence not be zero, the test should always have failed at
OSMO_ASSERT(memcmp(&enc_sc, &dec_sc, sizeof(enc_sc)) == 0).
Related: OS#3148
Change-Id: Iaa20c59f624fbdc69a018cabd0f7e9c5a1389519
Clarify semantics and micro-optimise for the case of single Cell Identifer IEs.
Test in gsm0808_test.c
So far we have gsm0808_enc_cell_id_list2(), but there also exist instances of
single Cell Identifiers (3GPP TS 48.008 3.2.2.17).
It is possible to decode the same using the cell identifier list API, but this
forces the caller to also keep a full struct gsm0808_cell_id_list2 with all its
127 entries around.
E.g. for handover, there are two Cell Identifiers (Serving and Target); I'd
need two full cell id lists for each, and these would be dynamically allocated
for each handover operation, whether it uses them or not.
Related: OS#2283 (inter-BSC HO, BSC side)
Change-Id: I9f9c528965775698ab62ac386af0516192c4b0cc
Allow passing multiple struct tlv_parsed in an array, to allow parsing as many
repeated IEs as are expected by the caller.
From tlv_parse(), call tlv_parse2() with dec_multiple = 1 to yield the previous
behavior. tlv_parse() remains valid API.
An example of multiple IEs is the BSSMAP Handover Request, containing Cell
Identifier (Serving) and Cell Identifier (Target), both defined by 3GPP TS
48.008 3.2.2.17 with identical IE tags; both are mandatory.
Related: OS#2283 (inter-BSC HO, BSC side)
Change-Id: Id04008eaf0a1cafdbdc11b7efc556e3035b1c84d
This will be used by the upcoming neighbor_ident API in osmo-bsc, where the vty
interface allows composing neihbor BSS cell identifier lists, and we want to
allow adding individual items from individual user commands.
It will also be useful to accumulate cell identifiers in case a subscriber sees
multiple alternative cells from a neighboring BSS, and we want to pass these on
to the MSC in a Handover Required.
Related: OS#2283 (inter-BSC HO, BSC side)
Change-Id: I5781f5fa5339c92ab2e2620489b002829d206925
In the osmo-msc, I would like to set the subscr conn FSM identifier by a string
format, to include the type of Complete Layer 3 that is taking place. I could
each time talloc a string and free it again. This API is more convenient.
From osmo_fsm_inst_update_id(), call osmo_fsm_inst_update_id_f() with "%s" (or
pass NULL).
Put the name updating into separate static update_name() function to clarify.
Adjust the error message for erratic ID: don't say "allocate", it might be from
an update. Adjust test expectation.
Change-Id: I76743a7642f2449fd33350691ac8ebbf4400371d
On erratic id in osmo_fsm_inst_update_id(), don't say "Attempting to allocate
FSM instance".
Escape the invalid id using osmo_quote_str().
Change-Id: I770fc460de21faa42b403f694e853e8da01c4bef
Since alloc relies on osmo_fsm_inst_update_id() to set the name, never skip
that.
In osmo_fsm_inst_alloc(), we allow passing a NULL id, and in
osmo_fsm_inst_update_id(), we set the name without id if id is NULL.
Change-Id: I6d6b09a811b82770818f19b189a57d9fc4a8133b
Place id and name testing in its separate section, test_id_api().
Add a test that actually allocates an FSM instance with a NULL id, which is
allowed, but uncovers a bug of an unset FSM instance name. osmo_fsm_inst_name()
falls back to the fsm struct's name on NULL, but osmo_fsm_inst_find_by_name()
fails to match if the instance's name is NULL (and until recently even
crashed). Show this in fsm_test.c with loud comments.
Add test to clear the id by passing NULL.
Add test for setting an empty id.
Add test for setting an invalid identifier (osmo_identifier_valid() == false).
Change-Id: I646ed918576ce196c395dc5f42a1507c52ace2c5
In fsm_test.c, we have FSM instance cleanup after the select main loop, but we
exit(0) in the timer cb; hence the final code is never called.
Rather clean up the instance and hence also test that, by using a global flag
to exit the main loop upon timeout.
Adjust expected stderr output.
BTW, in a subsequent commit, I want to move the fsm instance id testing to
below the main loop, to more clearly group the tested bits.
Change-Id: Ia47811ffcc1bd68d2630c86be7ab98fc1f338773
Rationale: with osmo_escape_str(), you get the escaped contents of the string,
but not so graceful handling of NULL strings. The caller needs to quote it, and
for NULL strings not quote it.
osmo_quote_str() is like osmo_escape_str() but always quotes a non-NULL string,
and for a NULL string returns a literal NULL, i.e. it should (tm) give the
exact C representation of a string.
That's useful in testing, to show exactly what char* situation we have, without
jumping through hoops like
if (str)
printf("\"%s\"", osmo_escape_str(str, -1));
else
printf("NULL");
Copy the unit test for osmo_escape_str() and adjust. To indicate that the
double quotes are returned by osmo_quote_str(), use single quotes in the test
printf()s.
I considered allowing to pick the quoting characters by further arguments, but
that complicates things: we'd need to escape the quoting characters. Just
hardcode double quotes like C.
Change-Id: I6f1b3709b32c23fc52f70ad9ecc9439c62b02a12
Ironically, when deprecating osmo_init_logging() in
I216837780e9405fdaec8059c63d10699c695b360, I forgot to change the callers
within libosmocore itself, i.e. in the various regression tests.
Change-Id: Ia36c248f99353d5baaa2533f46a2f60a8579bdf8
Add logging to root ctx, add msgb ctx to root ctx, free wqueue to simulate the
msgb being sent, and assert final talloc size.
Change-Id: Ief3d5e7b6c4d781b3854e230e45a67d5281b94cd
The CTRL interface has a ctrl_cmd_def_* API that allows deferring a CTRL
command reply until later. However, the command handling currently fails to
acknowledge this and deallocates the struct ctrl_cmd anyway.
Fix: in struct ctrl_cmd, add a defer pointer to be populated by
ctrl_cmd_def_make(). A cmd thus marked as deferred is not deallocated at the
end of command handling. This fix needs no change in calling code.
(Another idea was to return a different code than CTRL_CMD_HANDLED when the
command is to be deferred, but that would require adjusting each user of
ctrl_cmd_def_make(). The implicit marking is safer and easier.)
Show that handling deferred commands is fixed by adjusting the expectations of
ctrl_test.c's test_deferred_cmd() and removing the now obsolete exit_early
label.
One symptom of the breakage is that osmo-bts-sysmo crashes when asked to report
a trx's clock-info, which is aggravated by the fact that the sysmobts-mgr does
ask osmo-bts-sysmo for a clock-info.
The crash appears since Id583b413f8b8bd16e5cf92a8a9e8663903646381 -- it looked
like just fixing an obvious memory leak, which it did as shown by the unit
test, but deferred ctrl commands actually relied on that leak. Both fixed now.
Related: OS#3120
Change-Id: I24232be7dcf7be79f4def91ddc8b8f8005b56318
Handling a deferred command currently deallocates the struct ctrl_cmd upon
exiting the initial command handling, while it should actually stay around for
the asynchronous/deferred handling of the ctrl command.
Show the current bug by means of a ctrl test. The test will be adjusted to
expect the correct result when the bug is fixed in a subsequent commit
(I24232be7dcf7be79f4def91ddc8b8f8005b56318).
Change-Id: Ibbc847fc583bdd8e5e53a008258805e634ea12b4
Recent commit I77cd4b9142510c6914298b720d9c19ab68f9ebef left an obsolete
assertion around. It is already done in the if-body now.
Change-Id: I1bb2ea363e8a9d86b24338df3584abc93ebc6dd4
Use non-deprecated API to decode encode in gsm0808_enc_cell_id_list2().
Adjust gsm0808_test.c to now expect the correct results instead of previous
failure.
Change-Id: I1ce78883995e0d484368046b69db5afb2b4adc97
The test currently sets the MCC by a hex value, which is a weird choice. The
MCC gets BCD'd and hence we will see the decimal values 1:1 in the encoded
octets as hex digits. Using hex as input obscures that:
Right now it sets mcc = 0x123, which is actually 291 in decimal, and we hence
see "92 .1" in the expected BCD result. Using 0x124 in the test source actually
makes it hard to see where the 0x123 went.
Change the MCC to decimal notation (123, 124, 125) and adjust the expected
encoded output.
Change-Id: I973835c54a90fefe50d2b3581324d12556715f58
I am going to ehance the test to actually include leading-zero MNC, but first I
would like to simplify how the test source struct is initialized, before I edit
around in it.
Also, when the memcmp() fails, print hexdumps of expected and actual result for
comparison. I needed it to figure out a test failure, might as well keep it.
Change-Id: I77cd4b9142510c6914298b720d9c19ab68f9ebef
The speech codec defaults are not correct. The defaults recommended
in 3GPP TS 28.062, Table 7.11.3.1.3-2 are limited by 3GPP TS 48.008,
Section 3.2.2.103. Some defaults are actually reserved for future
use. Also the endianess of the 16 bit values is reversed.
- correct values so that they match the specification
- transmit bytes in the correct endianess
Change-Id: I6c3a34d39a375d71c4128fd38f06629e8b98b100
If the name stays the same the log messages will still log with the old
id. Since we can now change the id we need to update the name as well.
NULL as id was allowed before so we should allow that as well.
Change-Id: I6b01eb10b8a05fee3e4a5cdefdcf3ce9f79545b4
Since commit bf383a1d83 tlv_parse()
will return the first occurrence of a repeated IE. Add a test to
verify this behaviour. This test passes with the current code and
fails if bf383a1d83 is reverted.
While here, fix lies in documentation about the return value of tlv_parse()
and fix a typo in another comment.
Change-Id: I041f38548c5e4236920991d6c681c1c1e04de9ca
Related: OS#2904
Add a regression test which encodes and decodes a cell identifier
list of type CELL_IDENT_WHOLE_GLOBAL.
Change-Id: Ie633d9e55c6a30555c0153d35aaf9f982d6e0088
Depends: If0fafbc7171da2a3044bfa9a167208a1afa1c07b
Related: OS#2847
The implementation was entirely broken, reading data from wrong offsets
and always writing to the first element of the decoded list.
Also, add a new test for this function which found the problems.
Change-Id: If0fafbc7171da2a3044bfa9a167208a1afa1c07b
Related: OS#2847
Depends: Ife4e485e2b86c6f3321c9700611700115ad247b2
Cell ID lists with CI were misparsed because parse_cell_id_ci_list()
failed to report the amount of consumed bytes to its caller.
Also add a regression test which uncovered the bug.
Change-Id: Ife4e485e2b86c6f3321c9700611700115ad247b2
Depends: If6b941720de33dca66b6b1aa2cb95a3275708b7f
Related: OS#2847
This makes gsm0808_dec_cell_id_list() properly decode 3-digit MNCs.
Add a test which encodes/decodes a LAI_AND_LAC list with 3-digit MNCs.
Change-Id: If6b941720de33dca66b6b1aa2cb95a3275708b7f
Related: OS#2847
The cell ID list decoder merged in 11a4d9dd91
has a bug which was introduced part-way through the review process in
gerrit at https://gerrit.osmocom.org/#/c/6509/
When Neels suggested "why not just {...}id_list[MAXLEN] once?" I changed
the cell identifier list from a union of arrays to an array of unions.
After this change, elements smaller than the largest type in the union
were not laid out consecutively in memory anymore. E.g. uint16_t lac
values now occur at offsets of sizeof(id_list[0]) instead of offsets
of sizeof(uint16_t).
The problem is that I forgot to adjust the decoder accordingly, so the
decoder writes to the wrong offsets and returns cell identifier lists
which appear to contain uninitialized values when read back by API
consumers.
I found this problem while adding new regression tests to libosmocore to
test encoding and decoding. This commit adds one such tests for LAC list
decoding, which failed due to the above bug. I plan to write more tests,
however because this first test already uncovered a severe issue I chose
to submit a fix now and work on additional tests in later commits.
Change-Id: Ie1a5a9d858226be578cf11a03cf996d509bd51fb
Related: OS#2847
Introduce gsm0808_dec_cell_id_list2() with supports additional types of
cell identifier lists. The new parsing routines are based on similar
routines used by the paging code in osmo-bsc's osmo_bsc_bssap.c.
Likewise, introduce gsm0808_enc_cell_id_list2() with support for the
same additional types of cell identifier lists.
The old API using struct gsm0808_cell_id_list is deprecated.
The previous definition was insufficient because it assumed that all
decoded cell ID types could be represented with a single uint16_t.
It was declared in a GSM protocol header (gsm/protocol/gsm_08_08.h)
despite being a host-side representation of data in an IE.
The only user I am aware of is in osmo-msc, where this struct is used
for one local variable. osmo-msc releases >= 1.1.0 make use of this API.
While here, fix a small bug in a test:
test_gsm0808_enc_dec_cell_id_list_bss() set the cell ID type to 'LAC'
but obviously wants to use type 'BSS'.
Change-Id: Ib7e754f538df0c83298a3c958b4e15a32fcb8abb
Related: OS#2847
In osmo_mnc_from_str() do not try to return some values even if the validation
fails; hence don't try to decode a NULL pointer. That whole idea was half-baked
and a can of worms to begin with.
Change-Id: Ibaaa128ac60b941a015a31134eb52aef56bc6e22
In certain builds (for me a build with no -O2 flag) the recently added
gsm23003_test test_mnc_from_str() fails, because bcmp() compares all bytes of
sizeof(struct test_mnc_from_str_result), which has valid data for 7 bytes plus
one padding byte that may contain arbitrary values. Instead of bcmp(), rather
compare the actual members one by one.
Change-Id: I28b28457c7b0462c950612fd9b87b5c7181d8bad
osmo_mnc_from_str() preserves leading zeros in the string and is useful for
VTY config parsing (osmo-bsc, osmo-msc, osmo-sgsn, osmo-pcu).
osmo_{plmn,mnc}_cmp() takes care of the slight intricacy of ignoring the 3-digit flag
if the MNC is anyway >99. Will be used by osmo-sgsn.git and osmo-bsc.git. (All
current users just care about identical MNC, but a proper cmp doesn't hurt.)
Change-Id: Ib7176b1d65a03b76f41f94bc9d3293a8a07d24c6
Note that on the input side, the 3-digits flag may be left false when the MNC
is >99 anyway. On the decoded side, the flag is set accurately.
Change-Id: I89765613d8c5bd939a6957f7443ac88475f1b93c
Event names are displayed in VTY commands so all FSM should have them.
Print an error message if an FSM is registered without event names.
We could also return an error code, however at present no caller checks
the return value of osmo_fsm_register() so this would be pointless.
Add event names to the test FSM and update expected output accordingly.
Change-Id: I08b100d62b5c50bf025ef87d31ea39072539cf37
Related: OS#3008
For all other decode operations we report the BER, but not for the
RACH. This results in osmo-bts-trx not being able to report BER
to the higher layers, which is possible on other BTS backends.
Let's close this gap by introducing gsm0503_rach_ext_decode_ber()
and gsm0503_rach_decode_ber() with the usual n_errors / n_bits_total
arguments.
Change-Id: I2b1926a37bde860dcfeb0d613eb55a71271928c5
It seems with default flags in_buf was being memzeroed by the compiler.
When compiling with -O0, that's not the case anymore and printf prints
after first 16 bytes, printing extra garbage which doesn't match the
expected output.
Change-Id: I736c1e4d625f647d3bb794fa717256e9dbf36e87
inline keyword is a hint for the compiler to inline the function, but
it's not mandatory. If no static or extern is specified, the definition
is only visible in the current unit but the identifier still has
external linkage.
When running with -O0 it seems the compiler (gcc 7.2.1) decides to use
the external linkage but at the same time it seems it's not generating
the function symbol. Fix it by explicitly stating that we want to use
static linking for this function.
coding/coding_test.o: In function `test_xcch':
libosmocore/tests/coding/coding_test.c:86: undefined reference to `dump_ubits'
libosmocore/tests/coding/coding_test.c:87: undefined reference to `dump_sbits'
Change-Id: I18018adec05ce1c2ddbca38653311d74c7454ce8
* match return type of osmo_gsup_encode() with osmo_gsup_decode() to allow
propagating error to caller
* check return value of osmo_gsup_encode() in GSUP test
* return errors instead of braking app with aseert
Change-Id: Idaa1deecb6d9e15329bd51867b4f6a03357461f0
Related: OS#2864
Instead of forcing test failure via assert on first error encountered,
let it run until completion and print detailed error log. This
simplifies troubleshooting by letting user to see more errors from
single run and more details on each of the errors. Update test output
with explicit test results.
Change-Id: I016a28fe04f7b194e22c15e936095004c5f079d1
Previously an incorrect length value was passed to both
gsm_7bit_decode_n_ussd() and gsm_7bit_encode_n_ussd()
functions during test_7bit_ussd() execution, due to:
octets_written = strlen(decoded);
The problem is that a 7-bit encoded string takes less memory
than its 8-bit equivalent. So, here strlen() returns one-byte
bigger value, that octets_written is. This then causes the
uninitialized memory access.
Found using Valgrind:
Conditional jump or move depends on uninitialised value(s)
at 0x506DCCC: gsm_7bit_decode_n_ussd (gsm_utils.c:248)
by 0x40134B: test_7bit_ussd (ussd_test.c:104)
by 0x400F5D: main (ussd_test.c:161)
Conditional jump or move depends on uninitialised value(s)
at 0x506DBB7: gsm_7bit_decode_n_hdr (gsm_utils.c:220)
by 0x506DC9E: gsm_7bit_decode_n_ussd (gsm_utils.c:246)
by 0x40134B: test_7bit_ussd (ussd_test.c:104)
by 0x400F5D: main (ussd_test.c:161)
Conditional jump or move depends on uninitialised value(s)
at 0x506DBCB: gsm_septet_lookup (gsm_utils.c:153)
by 0x506DBCB: gsm_7bit_decode_n_hdr (gsm_utils.c:224)
by 0x506DC9E: gsm_7bit_decode_n_ussd (gsm_utils.c:246)
by 0x40134B: test_7bit_ussd (ussd_test.c:104)
by 0x400F5D: main (ussd_test.c:161)
Change-Id: Ic31805b6a5a917dfc6284edba6ffdd21246ac20c
The sercomm functions are unavailable in case of embedded build. Add
stub and link the tests against it.
Change-Id: I9bc5cb2f822b1a3ffdc6ec29f46b6bac8288314e
As of 67bdd80a96 the stats.c is
effectively disable so we should disable the corresponding tests as
well.
Change-Id: I42ff7a6619c0a5926fdc2ec779cf04689c567e15
Previously the same length value was used for both ussd_request
and interrogate_ss payloads, despite they are different.
Change-Id: I90ae7c51b75dcdb9d8ee042af23d127e6db8771d
When a bad GSM voice frame is received, it's being replaced
by a silence frame. This may cause unpleasant audio effects.
This change implements a functionality to craft a replacement
frame from the last known good frame. Currently, only FR is
supported, support for other codecs may be added latter.
Change-Id: I06a21f60db01bfe1c2b838f93866fad1d53fdcd1
Add gsm48_encode_ra() which takes appropriate struct as [out] parameter
instead of generic buffer. Using uint8_t buffer instead of proper struct
type prooved to be error-prone - see Coverity CID57877, CID57876.
Old gsm48_construct_ra() is made into tiny wrapper around new
function. The test output is adjusted because of the change in function
return value which was constant and hence ignored anyway.
Related: OS#1640
Change-Id: I31f9605277f4945f207c2c44ff82e62399f8db74
Validate that incoming CTRL commands...
- have decimal IDs,
- return error on trailing characters,
- have invalid characters in variable identifiers,
- send detailed error messages as reply to the requestor.
Adjust ctrl_test.{c,ok}, which best show the change in behavior.
Message handling causes log messages on stderr; previously, stderr was empty.
Add '[ignore]' in testsuite.at so that the nonempty stderr doesn't cause test
failures.
Change-Id: I96a9b6b6a3a5e0b80513aa9eaa727ae8c9c7d7a1
Recent patch I563764af1d28043e909234ebb048239125ce6ecd introduced returning
NULL from rate_ctr_group_alloc() when the index passed already exists.
Instead of returning NULL, find an unused group index and use that, adjust the
error message.
In stats_test.c, adjust, and also assert allocated counter group indexes
everywhere.
Rationale:
The original patch causes osmo-sgsn to crash as soon as the second subscriber
attempts to establish an MM context. Of course osmo-sgsn is wrong to a) fail to
check a NULL return value and crash and b) to fail to allocate an MM context
just because the rate counter group could not be allocated (it still rejects
the MM context completely if rate_ctr_group_alloc() fails).
Nevertheless, the price we pay for rate counter correctness is, at least in
this instance, way too high: osmo-sgsn becomes completely unusable for more
than one subscriber.
Numerous other places exist where rate_ctr_group_alloc() is called with a
constant index number; from a quick grep magic I found these possible breaking
points:
osmo-sgsn/src/gprs/gb_proxy.c:1431: cfg->ctrg = rate_ctr_group_alloc(tall_bsc_ctx, &global_ctrg_desc, 0);
osmo-sgsn/src/gprs/gprs_sgsn.c:139: sgsn->rate_ctrs = rate_ctr_group_alloc(tall_bsc_ctx, &sgsn_ctrg_desc, 0);
osmo-sgsn/src/gprs/gprs_sgsn.c:270: ctx->ctrg = rate_ctr_group_alloc(ctx, &mmctx_ctrg_desc, 0);
osmo-sgsn/src/gprs/gtphub.c:888: b->counters_io = rate_ctr_group_alloc(osmo_gtphub_ctx,
>phub_ctrg_io_desc, 0);
osmo-bsc/src/libfilter/bsc_msg_acc.c:87: lst->stats = rate_ctr_group_alloc(lst, &bsc_cfg_acc_list_desc, 0);
osmo-pcu/src/bts.cpp:228: m_ratectrs = rate_ctr_group_alloc(tall_pcu_ctx, &bts_ctrg_desc, 0);
osmo-pcu/src/tbf.cpp:793: tbf->m_ctrs = rate_ctr_group_alloc(tbf, &tbf_ctrg_desc, 0);
osmo-pcu/src/tbf.cpp:879: tbf->m_ul_egprs_ctrs = rate_ctr_group_alloc(tbf, &tbf_ul_egprs_ctrg_desc, 0);
osmo-pcu/src/tbf.cpp:880: tbf->m_ul_gprs_ctrs = rate_ctr_group_alloc(tbf, &tbf_ul_gprs_ctrg_desc, 0);
osmo-pcu/src/tbf.cpp:970: tbf->m_dl_egprs_ctrs = rate_ctr_group_alloc(tbf, &tbf_dl_egprs_ctrg_desc, 0);
osmo-pcu/src/tbf.cpp:977: tbf->m_dl_gprs_ctrs = rate_ctr_group_alloc(tbf, &tbf_dl_gprs_ctrg_desc, 0);
osmo-pcu/src/tbf.cpp:1475: ul_tbf->m_ctrs = rate_ctr_group_alloc(ul_tbf, &tbf_ctrg_desc, 0);
osmo-pcu/src/bts.cpp:226: m_ratectrs = rate_ctr_group_alloc(tall_pcu_ctx, &bts_ctrg_desc, 1);
We can fix all of these callers and then reconsider returning NULL, but IMO
even into the future, rate counter group indexes are not something worth
failing to provide service for. For future bugs we should keep the automatic
index picking in case of index collisions. We will get an error message barfed
and can fix the issue in our own time, while the application remains completely
usable, and even the rate counters can still be queried (at wrong indexes, but
life is tough).
Related: I49aa95b610f2faec52dede2e4816da47ca1dfb14 (osmo-sgsn's segfault)
Change-Id: Iba6e41b8eeaea5ff6ed862bab3f34a62ab976914
The recently added ctrl_cmd_parse2() returns non-NULL cmd with error messages
upon parsing errors. In handle_control_read(), use ctrl_cmd_parse2() and send
those back to the CTRL command sender as reply.
Retain the previous "Command parser error" reply only in case ctrl_cmd_parse2()
should return NULL, which shouldn't actually happen at all.
Change-Id: Ie35a02555b76913bb12734a76fc40fde7ffb244d
Neels Hofmeyr
Harald Welte
Oliver Smith
Philipp Maier
Neels Hofmeyr
Pau Espin
Daniel Willmann
