The wrap around case was not properly considered in the condition
setting the Marker bit. Let's fix it.
Related: SYS#5987
Change-Id: I6e01f29a6239f930c9be2bcb2efe447e5de8fedf
This test shows that there's a bug where the first RTP packet extracted
from the received osmux batch with seqnum=0 has the M bit marked for no
good reason.
Related: SYS#5987
Change-Id: Ida658c681e84878f209ab4965d8aa821a570a580
There's no real use in having a global seqnum. The seqnum, as specified
by the osmux protocol specification, relates to that of the RTP
seq+timestamp, hence it is per circuit.
Having it per circuit allows detecting gaps and lost batches on the
receiver side, applying the Marker bit on the re-assembled RTP packets.
As a resut of the fix, tests/osmux/osmux_test part validating Marker bit
started failing. It failed because it was wrongly written to start with,
since it used only one osmux_out_handle for the 4 CIDs in use, which was
wrong, since each CID must have its own osmux_out_handle as state is
kept there per circuit.
Related: SYS#5987
Change-Id: I562de6a871d8a35475c314ca107c2a12b55d6683
The value of the first RTP packet in the batch was being encoded,
instead of the last one (as documented in the Osmux protocol
specification).
Related: SYS#5987
Change-Id: I06f3d07a05181d938c22bbd0da7172b5dad6659a
That file focuses on testing the osmux_output_* API, ie receiving Osmux
frames and decoding it into batches and later on into rtp packets.
Change-Id: I7e6eda1e2e676673771bc6a7c3682d07ac1d344e
Let's not make the tests more difficult to maintain and extend by
allowing them to run in real clock time, there's no real need for it.
Change-Id: I549a4722d63d700b54b146260131a68e656b843e
According to API doc and implementation, it never returns >1.
Do as done in all other places where this API is used, that this check
for >0.
Change-Id: If23dfecb566f590b7a898356469df6e322f57653
Until now, the osmux_out_handle was allocated by the client, and passed
to the API to initialize it. This makes it really hard to improve the
implementation without breaking the ABI.
Let's break the ABI now one last time (hopefully) by allocating the
struct through an API. With only this change, the already built users
(osmo-mgw, openbsc) can still work fine, since there's no change on the
struct osmux_out_handle. However, they will somehow break next time the
struct is changed until they are ported to the same API (easy to do).
Related: OS#5987
Change-Id: Ie8df581f375c9a183a7af60b431561bda82f6e34
The functions that convert between octet-aligned and bandwith-efficient
AMR format have good coverage on the octet-aligned side, but the
bandwith-efficient side has a very little number of sample packets. Lets
add some more sample packets to increase coverage.
Related: SYS#5834
Change-Id: I53bd574e1ce7349419553e3957fff19e81567b93
The check was wrong for format types containing extra bits not aligned
to byte boundaries, such as FT7 (AMR Code 12.20, 244 bits, 31 bytes).
if the source has 1-6 extra bits, they can be fit with one less byte
when shifting 10 bits to the left.
Change-Id: I0552d727585886d25f613e64ca815fb6dcd53f25
The proper order is CMR(4)+F(1)+FT(4)+Q(1).
Hence, FT is 3 least significant bits of first byte and 1 most
significant bit of secont byte.
Change-Id: I66f39d3b9a608f07c202e7a5084a8537e9978a94
These APIs allow for easy re-formatting of received AMR to forward
between regular RTP and IuUP.
Related: OS#1937
Change-Id: Id2bd32d5f2060abe581730996dc4251381bf7d4f
Let's disable category here since we don't care about its formatting here.
In any case, every test relying on logging output validation should
always explicitly state the config to avoid issues in the future if
default values change.
Change-Id: Ia4bcf8dc441ad26cffc3aec5b374fbdca4a03841
Related: OS#5034
The for loop in osmo_amr_bwe_to_oa, that converts the body part of the
AMR payload runs one byte too far. This may cause that some of the
padding bits in the end are not set to zero. The loop is designed to
convert n-1 bytes and the nth byte is done separately at the end.
Change-Id: I91e755b83aaac722079879c026d913cc446812d1
Size of a single AMR frame doesn't always shrink by a byte when
converted from octet-aligned to bandwidth-efficient mode. It does
shrink for AMR modes 2, 3, 4, 6, and 7 but doesn't shrink for
AMR modes 0, 1, 5, and SID frames because we only remove 6 bits.
So old code generated truncated AMR packets for those AMR modes.
This patch fixes the length calculation by properly counting bits.
Proper bit counting is also bringing us one small step closer to
properly handlig multi-frame AMR packets.
Change-Id: I0462e054a0adc9080456f3eeea9cab7c229cdb70
This reverts commit 002a51e218.
Reason for revert: amr_test fails with sanitizer build:
Sample No.: 6
bw-efficient: a7bfc03fc03fc03fc03fc03fc03fc03fc03fc03fc03fc03fc03fc03fc03fc03f
1010011110111111110000000011111111000000001111111100000000111111110000000011111111000000001111111100000000111111110000000011111111000000001111111100000000111111110000000011111111000000001111111100000000111111110000000011111111000000001111111100000000111111
../../../src/libosmo-netif/src/amr.c:63:24: runtime error: index 15 out of bounds for type 'size_t [9]'
../../../src/libosmo-netif/src/amr.c:63:24: runtime error: load of address 0x7f69498e56b8 with insufficient space for an object of type 'size_t'
0x7f69498e56b8: note: pointer points here
00 00 00 00 00 00 00 00 00 00 00 00 5f 00 00 00 00 00 00 00 67 00 00 00 00 00 00 00 76 00 00 00
^
=================================================================
==489935==ERROR: AddressSanitizer: global-buffer-overflow on address 0x7f69498e56b8 at pc 0x7f69498abec7 bp 0x7ffeafb35330 sp 0x7ffeafb35328
READ of size 8 at 0x7f69498e56b8 thread T0
#0 0x7f69498abec6 in osmo_amr_bytes ../../../src/libosmo-netif/src/amr.c:63
#1 0x7f69498ac661 in osmo_amr_bwe_to_oa ../../../src/libosmo-netif/src/amr.c:193
#2 0x5648b11afb96 in osmo_amr_bwe_to_oa_test ../../../src/libosmo-netif/tests/amr/amr_test.c:134
#3 0x5648b11af31d in main ../../../src/libosmo-netif/tests/amr/amr_test.c:235
#4 0x7f6948d5de0a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x26e0a)
#5 0x5648b11af3d9 in _start (/n/s/dev/make/libosmo-netif/tests/amr/amr_test+0x43d9)
0x7f69498e56b8 is located 8 bytes to the left of global variable 'amr_ft_to_bits' defined in '../../../src/libosmo-netif/src/amr.c:32:15' (0x7f69498e56c0) of size 72
0x7f69498e56b8 is located 48 bytes to the right of global variable 'amr_ft_to_bytes' defined in '../../../src/libosmo-netif/src/amr.c:44:15' (0x7f69498e5640) of size 72
SUMMARY: AddressSanitizer: global-buffer-overflow ../../../src/libosmo-netif/src/amr.c:63 in osmo_amr_bytes
Change-Id: I8232521c513722435e71dc90bdbfee10f8f83496
Size of a single AMR frame doesn't always shrink by a byte when
converted from octet-aligned to bandwidth-efficient mode. It does
shrink for AMR modes 2, 3, 4, 6, and 7 but doesn't shrink for
AMR modes 0, 1, 5, and SID frames because we only remove 6 bits.
So old code generated truncated AMR packets for those AMR modes.
This patch fixes the length calculation by properly counting bits.
Proper bit counting is also bringing us one small step closer to
properly handlig multi-frame AMR packets.
Change-Id: I9fc5fb92e9bada22a47a82fcfb0925e892e50ced
It makes a lot more sense calling it this way since it matches the state
of the stream at that point.
Change-Id: Ic02aec3f7f095e0e0e1f940425f577be5048e98f
It's not really needed right now from logic point of view, since we
reused NONE for that. But it makes logging and logic clearer, and will
make it easier if we decide to move it to FSMs at a later point in time.
Other state value_string names are also modified with its whitespace
removed since anyway we'd need to change them to match WAIT_RECONNECT
length. Let's drop the space because imho it's not that useful and
anyway if we move to FSMs at some point then we won't have them anyway.
Change-Id: I7b9a6da87081c418b0d14bab5f34369c5eca6fe8
osmux implementation randomizes those values. It seems build in OBS
sometimes provide different values than the ones expected in the test
result. Let's hardcode them to make sure we always have the same values
regarless of the random() implementation.
Values chosen are the one matching the current expected test output so
it doesn't need any change.
Change-Id: Icc553c83ddff41900ae3d5990a655c29c9073e01
Currently osmux related code uses both gettimeofday on some parts and
clock_gettime(CLOCK_MONOTONIC) on others (for different purposes).
Let's fake both clocks and not only the one used by gettimeofday API.
Change-Id: I4e1a0eb4f8c4ea1bc0f963afa18b116d3af9978c
It allows easy verification that timing is correct and makes it easier
to debug time related race conditions.
Change-Id: I86eb1d7a8096011fd273f067255eb8d6484be65c
By using fake own-controlled time we get two benefits:
* Test doesn't take 9 seconds to run anymore
* More fine-grade control of different events happening (and associated
race conditions).
Change-Id: I16b2884b289bfe40dfb8d743dce01bb4c208d117
Previously payload_type was always hardcoded to 98 for generated rtp
packets from incoming osmux frame.
Change-Id: I5cbeb494a8932953d9fd2dc24dacf8cd97fd84e4
This supposed to be variant of osmo_stream_cli_open() with explicit
control over reconnection logic but it's plain broken: doxygen docs
contradict the code, actual reconnection logic is affected by timeout
parameter directly which is set in different function.
It seems like we haven't been affected by this so far because we always
use it in auto-reconnection mode which is triggered by default due to
positive reconnection timeout value (5 sec) automatically used in the
absense of explicitly set timeout.
Looking at commit history, this function already been source of
confusion in the past. Instead of trying to fix this mess, let's just
deprecate it entirely and properly document use of
osmo_stream_cli_set_reconnect_timeout() to control reconnection logic.
The only known user is libosmo-sccp which won't use it as of
0a93a683f3cb8e5977eb4a666ab207db6e7d7af9 commit.
Change-Id: Id988ed0274b363db049f59cbf6a193727c8c3c8a
Fix a symbol lookup error when building a new test on systems where
a previous libosmonetif.so is installed. Symptoms described here in
detail: https://osmocom.org/issues/3812#note-10
-no-install causes libtool to generate output files that link against
libraries in the build tree, instead of linking against the future
installation paths and generating a wrapper script. The wrapper script
should override the library paths, but at least on Debian, it does not
work as it should. Test binaries won't be installed anyway, so we can
safely use -no-install and work around the problem.
See also:
https://autotools.io/libtool/wrappers.htmlhttps://www.gnu.org/software/libtool/manual/html_node/Link-mode.html
Related: OS#3812
Change-Id: I94ccff42dfba71aaf59bb30ca312db0bac58c27d
RFC 3267 describes two different AMR frame formats. Octet Aligned and
Bandwidth efficient mode. In Bandwith efficient mode the padding bits,
which are used to align CMR, TOC and payload on octet boundaries are
saved and the fielda are packed directly one after another.
- Add functions to convert from one mode to the other and vice versa.
- Add function to detect in which mode an AMR frame is encoded.
Change-Id: I5b5a0fa644d8dbb1f04f9d7e35312683c7b3d196
Related: SYS#4470
Previously stream client and server code were only used in examples
which means regressions could be easily introduced unnoticed until they
trigger bugs in external code which relies on osmo_stream_*()
Fix this by adding basic client-server interaction tests with and
without reconnection.
Change-Id: I336f79970982ed8e1d73b73d54fa4c27ba8bce8e
In libosmocore Change-ID I1834d90fbcdbfcb05f5b8cfe39bfe9543737ef8f
we have introduced ipa_ccm_id_resp_parse() as a bugfixed replacement
of ipa_ccm_idtag_parse().
The main difference is that the returned "value" parts now have
a correct reported "length", whereas before this commit they all
reported a one-byte too-long "length" for each IE.
Let's use this opportunity to remove the copy+pasted
osmo_ipa_idtag_parse() function from the libosmo-netif codebase.
Change-Id: I4626d247626543e032593bf226b6c233f6678562
Until this patch, we didn't notify in any way to the RTP reader when an
Osmux frame was lost. Instead, we updated the seq×tamp as if there
was no lost, and as a result the RTP reader would only see a steady
increase of delay every time an osmux frame was lost.
As the batch_factor for the lost packet is unknown, we cannot assume any
number of amr payloads lost, and thus we cannot simply increment seq and
timestamp for a specific amount. Instead, the only viable solution seems
to set the M marker bit in the first rtp packet generated after a
non-consecutive osmux frame is received.
The implementation may act differently with the first generated RTP
packet based on the first osmux seq number used for the stream. In case
0 it's used as first osmux seq number, M will be set depending on
request from original RTP packet having the M bit set. If it's not 0,
the first RTP packer will unconditionally have the M bit. That's not an
issue because it's anyway expect for receiver to sync on the first
packet.
Related: OS#3185
Change-Id: I2efed6d726a1b8e77e686c7a5fe1940d3f4901a7
This test is aimed at testing several specific scenarios related to how
osmux manages in/out of osmux/rtp packets over time.
Change-Id: I3bf59276424ea87c4e66a6ff46de1e3e9a06a904
Pau Espin
Vadim Yanitskiy
Philipp Maier
Alexander Couzens
Neels Hofmeyr
Oliver Smith
Harald Welte