The osmux header goes before in the packet, so let's move the line
checking is size before the content.
Change-Id: I33feac834700d22ed06472d8971abd0567ce623b
The RTP msg will be dropped, so it makes no sense to signal the caller
to deliver the batchbeing built, since it may still have space for next
non-duplicated message.
The exception is the case where the new packet has the M marker bit set,
since the sequence numbers can be reset or jump in those scenarios.
Change-Id: Idc457bc3b26bed68796d714bc3f37a2d016ba5c3
This way the caller can log or make statistics based on the return code.
All known implementations simply check the return code to be >0, so we
are fine here.
Change-Id: I981cc7e560cd9c792a8a2a219b3612f9834296ce
The user may want to check the flags in order to know if the content
pointed at by msgb is an sctp_notification structure, and not in-band
data.
This is useful for the user in order to gain connection state such as
SCTP RESET notification, which means the client reconnected reusing the
same socket, loosing all higher layers state.
The MSG_NOTIFICATION from netinet/sctp.h is not reused, and instead an
osmocom specific flag (and bitmask) is used. This is done in order to
fit the flags in one byte, since for instance MSG_NOTIFICATION requires
2 bytes in my system (0x8000).
Related: SYS#6113
Change-Id: I0ee94846a15a23950b9d70eaaef1251267296bdd
The fd is not valid anymore after close() call, so let's set it to a
clearly invalid value, to avoid confusion on closed_cb() users
attempting to use the fd get info about the socket at that time.
Change-Id: I82d9bdfb38cf5e9f689eca0d9a4c19ddd5541af7
This is useful for users of the API which need to keep forwarding the
msgb to lower layers which may need prepending a new header to the msgb,
like osmo-bts with l1sap.
Related: SYS#5987
Change-Id: I632654221826340423e1e97b0f8ed9a2baf6c6c3
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
Those APIs where deprecated 4 years ago (end of Aug 2018), and they have
not been used since around that time. Hence it can be considered safe to
drop them, since they only make the whole code more complex to
understand.
API osmux_xfrm_output_init() is left since openbsc.git is still using
it.
Related: OS#5987
Change-Id: Icbdd364a8161a8113dbf1406716946f684d0a853
Found out by following gcc warning:
"""
libosmo-netif/src/stream.c:147:11: warning: argument to variable-length array may be too large due to conversion from ‘int’ to ‘unsigned int’ [-Wvla-larger-than=]
147 | uint8_t buf[sctp_sockopt_event_subscribe_size];
| ^~~
"""
Change-Id: I181ae5c0480788fc96abd2bb1edf003244884c8f
Make coverity happy, since this seems to spread as a tainted error
further below.
Related: Coverity CID#273000
Change-Id: I5d457183043d4c902f473b828815b9c62a01d47d
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
This commits adds a generic mechanism for applications to validate
support for SAPs and specific versions of them.
A new special SAPI is introduced to manage inbound control messages for
the protocol.
In that SAPI a new primitive HELLO.req/.cnf is added to negotiate
support for versions of any given SAP.
The idea is that the client upon connecting submits a HELLO.req(SAP,
VER) for each SAP it plans to use, including the version of the special
CTL SAPI itself (preferrably to be checked first).
Upon receiving such a message, osmo_prim_srv handles it using a special
path, which ends up calling the user provided rx_sapi_version_cb(SAP,
VER). In there, the user can either:
* Accept the version (return same VER value)
* Reject the requested version but propose another candidate version
(return some positive VER value other than received VER). In this
case, the client can decide whether to request another VER or close the
connection.
* Reject the proposed version and close the connection.
Change-Id: I0c2d92cfdb5433e3caab51d712fd947d51eeef23
This new module allows easy exchange of osmo_prim based data types over
IPC communication (UD socket supported only so far), by replacing the
osmo_prim_hdr struct with a serialized header when submitting/receiving
it from the IPC socket.
This patch introduces the server side of the UD socket, but the client
side can easily be introduced in the same file whenever needed.
Related: SYS#5516
Change-Id: I7cab15ac092e45a256c4f0bab11b3962df861044
This makes it easier to follow the general path selection based on
protocol type. It will also make it easier when we add new paths based
on socket domain.
Change-Id: Ia3e0f4407e00a2dac9ee885fe1cc1cb4b463898a
These value_strings are taken from existing private ones in
libosmo-sccp.git osmo_ss7.c, and are moved here in order to be
reused by other projects, such as osmo-hnodeb.
Change-Id: Ica6f01851fb94e31f4ef097494bb1b8a95597ba6
In the case of SCTP, an association supports now containing both IPv4
and v6 addresses.
Depends: libosmocore.git Change-Id I36d8ab85d92bba4d6adb83bc1875eb61094ed2ef
Change-Id: Ie6bb17a9af6ca21d5e350f9c9d2d74c97c5a00af
Some previous commit added some code to workaround some kernel ABI
unstabilities but forgot to add the code inside a conditional
preprocessor block.
Fixes: 157ad6d707
Change-Id: Ibee01222d8d11cf2520cdbd4d0d5bed6b72d2107
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
New define is available since libosmocore 1.1.0, and we already require
1.3.0, so no need to update dependenices.
Let's change it to avoid people re-using old BSC_FD_* symbols when
copy-pasting somewhere else.
Change-Id: I269690c1c9e4d19b5b69eef206b95e71f7931188
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
With previous state, osmo_stream_cli_close() could be called from
osmo_stream_cli_open()(), and in that case state was kept as NONE while
ending up with an associated fd being registered in the select loop.
As a result, osmo_stream_cli_fd_cb() could be called while in state
NONE, which was not expected and would simply return without modifying
fd state flags, causing it to be called again and again.
Related: OS#4378
Change-Id: Ie3342f882893a71ad1538c17ad9ee9fa4433eaa4
It should not happen anyway because no fd should be active if state is
NONE, but still it's an extra check.
Change-Id: I6d58762b7d10078eb8d0981c13d35cb6f85cfe86
Similar to what we do in libosmocore already, we want to
deterministically enable or disable support for the feature without
having into account if the system has a libsctp. If libsctp is missing
and support is enabled, then fail.
Extra checks are also added:
* Check netinet/sctp.h header
* Check libosmocore was built with libsctp support (API
osmo_sock_init2_multiaddr() we require).
* In stream.c make sure it can be built without HAVE_LIBSCTP, and that
set_addrs() fails for more than 1 address (since that feature is only
supported through osmo_sock_init2_multiaddrs()).
Change-Id: I4b3e1f1894f13ac1175a71a5139c02a2633be26d
This API will be later used to set multiple addresses for SCTP sockets.
Depends: libosmocore.git Ic8681d9e093216c99c6bca4be81c31ef83688ed1
Related: OS#3608
Change-Id: I09f0d989f2309abdeb304fe570355abed2cd107b
This API will be later used to set multiple addresses for SCTP sockets.
Depends: libosmocore.git Ic8681d9e093216c99c6bca4be81c31ef83688ed1
Related: OS#3608
Change-Id: I0fe62f518e195db4e34f3b0ad1762bb57ba9d92a
It seems different compiler versions (jenkins and my workstation) are
generating different line number for that message, and it makes osmux_test
fail when improving logging on next commit.
Change-Id: Ie2bb0ecf4cc165b9a1080e2558b33ba37014278c
If messages are sent using osmo_stream_cli_send() while the stream
is still (re)connecting, they won't have a chance to be sent until the
stream is connected, and hence they are queued until
CONNECTING->CONNECTED is done. However, at that time
(osmo_stream_cli_fd_cb), the WRITE flag was dropped unconditionally,
which meant already queued packets didn't have the opportunity to be
sent by the same callback until first message is enqueued and WRITE flag
is set (again by osmo_stream_cli_send()).
Let's make them be sent as soon as possible once the connection is
available.
Related: OS#4188
Change-Id: I289495f9aad6389c5f2623fb072d676235b7d24c
Previously payload_type was always hardcoded to 98 for generated rtp
packets from incoming osmux frame.
Change-Id: I5cbeb494a8932953d9fd2dc24dacf8cd97fd84e4
The header of an AMR header payload is 2 bytes long. At the moment we
use just a constant of 2 when we refer to the header length, but we have a
struct amr_hdr defined. Lets use sizeof(struct amr_hdr) to make it more
clear that we are refering to the header length.
Change-Id: Ic7ca04b99a97d7d3b91717b0c3e6c55ef3001a3e
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
osmo_amr_bwe_to_oa() uses an internal buffer with static size to store
intermediate results. The buffer is large enough for any real world
situation, but the check that tests if the result would fit into the
internal buffer is incorrect. It checks if there is enough room for the
existing payload, but does not include the expected growth of the
payload. Eventually the buffer could be overrun by one byte if one would
put a 256 byte long AMR payload.
Fixes: CID#195926
Change-Id: I4d7ac570a0b48368a82183673c46bca5f235f228
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
AMR uses different payload sizes, those sizes are well defined in RFC
3267. Lets add define constants and replace the magic values with the
define constants.
Also correct the value for AMR_FT_SID in amr_ft_to_bytes from 6 to 5
(39bits / 8 = 4.875 bytes ==> 5 byte, see also RFC 3267, chapter 3.6)
Change-Id: I65b5da920d58015b875d6dcf17aacdc04b58955e
Previously closing the client did not alter its state, so we might
end-up with a client without any file descriptors, but being in state
STREAM_CLI_STATE_CONNECTED. Fix this inconsistency by setting
appropriate state.
Related issue is that reconnect function, which is always (at least in
the library and examples) called when some problem with the connection
is detected, closed the connection only after checking whether
reconnection is enabled. This might result in another inconsistency
fixed in this patch by moving the check below connection cleanup.
While at it, also move connection close logging to appropriate place:
it's confusing to see logs about connection being closed while in
reality it wasn't even established.
Change-Id: If41ed60bd625488c283d1e8a2b078e640f04c78e
Add functions to get the description of a server link or client
connection which examine data on corresponding socket.
Those functions use static buffers and intended for single use in
log/printf statements as illustarted by corresponding example changes.
Change-Id: If9a8e211da85956781479862a63c4fc6e53ed6be
Introduce logging macro wrapper to properly log current client state and
function to aid in debugging.
Change-Id: Ie22a80dcec95998cce0b25053fdf74f23eab6e53
While we are processing a read event, the connection's
callback might free the connection. Check for this and don't
attempt to process further events on an already freed connection.
Change-Id: I0a9c7d8e3263c73440f7084dbb1792a4ca5038f0
Related: OS#3685
Depends: g#11704 (for libosmo-sccp)
The recently-introduced dependency to libosmogsm symbols needed
some explicit addition of linker flags to avoid user applications
to fail linking with
/usr/bin/ld: /usr/local/lib/libosmonetif.so: undefined reference to `ipa_ccm_id_resp_parse'
Change-Id: I07a28f8970b90f82736e2de783bafc9d2c5ea0e5
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
When establishing a client-side stream connection via libosmo-netif,
we must using non-blocking connect if we want to avoid blocking/stalling
the entire process. The libosmocore socket API provides the
OSMO_SOCK_F_NONBLOCK flag for this. Make use of it!
Change-Id: I9bfcb39b5801a36ef32ca0d1f3eb8236687d7ed6
Related: OS#3383
The "channel" layer on top of IPA client + server was introduced in
2011 but never used in any osmocom program/project so far. Contrary
to the several other IPA multiplex related implementations in libosmo*,
it did not deal properly with segmented IPA messages, i.e. where a
single TCP segment (and hence recv/read call) does not contain a full
IPA message.
So rather than fixing it up and having yet another IPA related API in
our libraries, let's remove it.
Change-Id: I97c378750acb1637ee032fa88a968edf68d8979f
This message is expected as all code filling batches call
osmux_batch_enqueue() and checks for error to know if it must tell the
user of the lib to call osmux_xfrm_input_deliver.
Change-Id: I3d8227f2281f6ca92fd2502d3e328765dc7ecfe9
In Change-Id: I2efed6d726a1b8e77e686c7a5fe1940d3f4901a7 we're adding a
new member to 'struct osmux_out_handle' which is not initialized.
Rather than initializing this single new member, let's do a memset()
over the entire osmux_out_handle at the beginnign of
osmux_xfrm_output_init().
Change-Id: I751e9414c6de2413a9f977e5ae5655ebfd114f45
Closes: OS#3219
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
With old implementation, in conditions with jitter we could end up
scheduling RTP generated packets from two consecutive osmux frames in an
interleaved way (from seq field point of view).
This new implementation should make it easier for any RTP
reader/playback to have better results in those conditions.
Old APIs osmux_xfm_output and osmux_tx_sched are marked as deprecated in
favour of the new one, which has a better control of generated RTP
packets. However, they are still usable despite the implementation changes
done to support the new API.
Related: OS#3180
Change-Id: I4e05ff141eb4041128ae77812bbcfe84ed4c02de
Documentation of osmo_sock_init2 doesn't provide information of any
specific value of errno set/expected after running the function. It is
incorrect to expect a specific value of errno and looking at the
implementation it is actually not a good idea to check it.
If reconnect flag is set, let's reconnect always instead of looking at
errno to decide.
Change-Id: I25b33f4cdc496ae31ff240d445b9b2805091845c
Introduce osmo_stream_srv_set_flush_and_destroy() which marks a
stream to be 'flushed and destroyed'. No new messages will be
received on this stream, and no new messages can be queued.
Once the Tx queue has been drained, the connection is destroyed.
The API user is given a chance to perform cleanup operations
in the closed_cb() callback for the connection.
The same mechanism will be added for client-side connections
in a follow-up patch.
Change-Id: I8ed78fe39c463e9018756700d13ee5ebe003b57f
Related: OS#2789
Suggested-by: Harald Welte
On destroying a client or server stream, deallocate any msgbs that are still
pending in the queue.
In libosmo-sccp, the ss7_test.c in test_as(), messages are queued and were,
before this, left floating after the stream was destroyed, causing a sanitizer
memory leak. This patch fixes the leak.
Depends: Ia291832ca445d4071f0ed9a01730d945ff691cf7 (libosmocore)
Change-Id: Iaad35f03e3bdfabf3ba82b16e563c0a5d1f03639
In previous implementation, if no reconfiguring is needed, a new socket
would be created without closing the old one, leaking the previous
socket. Instead, if we don't need reconfiguring, we return 0 as no
operation is required.
Change-Id: I6c1a7fff63e44840fb5e2bc7ace5e9a61e304987
Short changelog:
* Add Doxygen documentation
* SCTP support in stream.c
* new udp-test-client and udp-test-server programs
* better / more verbose error handling in examples
* new osmo_dgram_tx_set_local_{addr,port}() functions
* use IPA definitions from libosmogsm, rather than repeating them
* encode RTP header M field of RFC3550/4867 in OSMUX header
* new osmo_stream_srv_link_set_nodelay()
* new osmo_stream_srv_link_set_proto()
* new osmo_stream_cli_set_nodelay()
* new osmo_stream_cli_set_proto()
* new osmo_stream_cli_set_local_addr()
* new osmo_stream_cli_set_local_port()
* new osmo_stream_cli_reconnect()
* new osmo_stream_cli_open2() with reconnect argument
* more vrebose osmux_snprintf()
* remove mistaken reference to AGPL in rs232.c
* fix memory leak in osmo_stream_srv_link_set_addr()
* add osmo-pcap-test for SLL and Ethernet
* extend osmux-test
Change-Id: Ibf75fcd6643351ce3946faa155ae1db8c33a5e35
Previous implementation handled all types as if they were Osmux AMR
frames. For Dummy frames, we account the padding but we don't care about
the padding content. For Signalling ones, as they are not in the
specification yet, it is better avoid using unespecified fields and
return an error because it's still not known how extra data will be
handled in the input msgb.
Change-Id: I48565472b47c2a0e5db50881fbb005537af8c70d
The current code still expects to parse only AMR osmux frames, but that
will be fixed in following patches.
Change-Id: Ic2f4d1d3cc88af912bb43c8ecd90eacc6ff7190f
Previously recv() returning 0 for UDP socket was considered as error
although it's legitimate return value for empty UDP packets. Relax the
error check to avoid confusing error messages.
The function behavior is the same:
* msg is not altered while receiving 0-length UDP packet
* return value is 0
The only result of the relaxed error check is the absense of error log
message for 0-length UDP packet.
Change-Id: I32e5fcbf5ed92cc923660ac59e6a37fd3f0703a7
Fixes: OS#2219
There's no need for the rs232 code to include a files from
libosmoabis. The only users of libosmoabis left now are the LAPD
examples:
examples/lapd-over-datagram-network.c
examples/lapd-over-datagram-user.c
Change-Id: Ie1bc0dd811362cec546486edc41d632740ed19cd
This patch inconditionally initializes the buffer we get to
nul-terminate it, whenever possible. It's a very simple solution to
catch three overly corner cases:
1) snprintf() returns -1, very much unlikely in practise.
2) msg->len == 0: In such case, I would expect this function is never
called with an empty message, but let's be safe in this case too.
3) If your buffer is empty, it doesn't nul-terminate the buffer.
Change-Id: I97e517f2d98e83894ea707c63489559302ff6bd2
SNPRINTF_BUFFER_SIZE() looks too complex, previous version maintains two
different variables to account for the remaining space in the buffer,
one of them is always decremented based on what snprintf() returns,
which may result in underflow. These variables are swapped - not used
consistently - all over this code.
Replace this macro by a simplified version, with one single parameter to
account for remaining space. This macro also deals with two corner
cases:
1) snprintf() fails, actually never happens in practise, but
documentation indicates it may return -1, so let's catch this case
from here to stick to specs.
2) There is not enough space in the buffer, in that case, keep
increasing offset, so we know how much would have been printed, just
like snprintf() does.
Thanks to Pau Espin for reporting, and Holger for clues on this.
I have run osmux_test and, at quick glance, it looks good.
Change-Id: I5b5d6ec57a02f57c23b1ae86dbd894bad28ea797
The buffer for osmux_test is increased as the former doesn't seem to be
able to cope with the whole output.
Change-Id: Ic838dd9d7ad89b4510ccfa58c0390c69a075b616
We didn't check for cases where setsockopt_nodelay() fails. Let's check
for that and bail out + close the socket.
Change-Id: I0adbc4cec35be7c36bdf01d4d8fefd6097e9be5d
Fixes: coverity CID#166970
We couldn't figure out what "crx" as supposed to mean and assume
it is a typo. Fix the code and call it ctx, this is fixing the
API documentation as well.
Change-Id: I27ed1178fdbbcf3fc0e1070dc19b4ecf9a327a04
According to RFC4867 (RTP payload format for AMR):
"The RTP header marker bit (M) SHALL be set to 1 if the first frameblock
carried in the packet contains a speech frame which is the first in a
talkspurt. For all other packets the marker bit SHALL be set to zero (M=0)."
This information bit provides a way for the receiver to better
synchronize the delay with ther sender.
This is specially useful if AMR DTX features are supported and
enabled on the sender.
Change-Id: I0315658159429603f1d80a168718b026015060e9
This commit should fix a bug present if for instance batch_factor < 8
and osmux_batch_enqueue is called from osmux_replay_lost_packets and
enough packets were lost from last received packet.
Change-Id: I5d643810949aeca4762f0cad05eed534d35087f7
Enabling sender_dry_event in SCTP_FLAGS breaks reliable delivery of DATA
chunks to the scoket user on Linux. Let's avoid enabling that, while
still enabling various other interesting events.
See https://bugzilla.redhat.com/show_bug.cgi?id=1442784 for all related
details.
Change-Id: Ib616cd07a6044ca2ee7e05093b22df3369c62b56
We were using the wrong variable when setting the SCTP_NODELAY,
resulting in the setsoctkopt() fialing.
Change-Id: Ic04cb8bb5ff41f177f7f5b7f7e2a2ecc686dd4c0
So far we seem to assume that the accept_cb does all handling of socket
fd cleanup. However, there are cases where there is no accept_cb set,
the accept_cb returns error, or an earlier sctp_sock_activate_events()
or the activation of non-blocking mode fails.
For those cases, close the socket and return an error code.
Fixes: CID#57915
Change-Id: I3a3ce9194ab7ca5c1921fc79c2a1c9e10a552cf0
In a659590e29 we fixed endianness issues
with the Stream ID field, but at the same time mistook the PPID field
for 16bits. In reality it is 32bits, and hence our 'htons' is rendering
wrong PPID values.
Change-Id: I1b60523044835ee630dba9a43d26af4f1ebd1ced
Using this function, the user can configure if sockets related to the
respective stream client or server should have the NODELAY socket
option set in order to avoid Nagle algorithm or related algorithms
that may introduce packet delay on the transmitter side.
Change-Id: Ibeb9ba227bab18f7f4f16518c0022c4f003cc8e9
In their infinite wisdom, the inventors of SCTP designed an API (the
sockets API described in RFC6458), where some members are in host byte
order (like the stream identifier), while other members are in network
byte order (like the PPID).
Let's handle this properly (we assumed both are network byte order), and
also use 16-bit htons/ntohs fo the PPID, rather than htonl/ntohl.
Change-Id: I777174ca2915c6de0063db41a745c71b4a09bbec
* when using osmo_*_destroy(), always call *_close() internally to
make sure we don't free memory holding references to sockets that are
still open
* when closing the socket, always make sure to set the fd to -1 in all
cases, to avoid attempts to avoid later close() on a new file using
the same fd number as the socket closed previously.
Change-Id: I29c37da6e8f5be8ab030e68952a8f92add146821
during osmo_*_set_addr(), we must make sure to talloc_free() any old
address before copying in the new address. Not all functions did this,
and those that did implemented it manually. Let's use
osmo_talloc_replace_string() which is exactly intended for this case.
Change-Id: Ie1b140a160c66e8b62c745174865d5ba525cb2c2
This uses the new osmo_sock_init2() features introduced in libosmocore
Change-Id Idab124bcca47872f55311a82d6818aed590965e6 to bind *and*
connect a given socket during creation.
Change-Id: I013f4cc10b26d332d52d231f252bb0f03df8c54b
We should have doxygen documentation for all libosmo-* APIs.
libosmo-netif is currently devoid of any API docs. Let's start with the
stream and datagram socket related functions.
Change-Id: I589a5e60d9df2b8a65fbaf68f80e3ae0039d8c2a
without setting the BSC_FD_* flags prior to reconnection, the re-connect
would happen silently and the client program would not be notified via the
connect_cb().
Change-Id: Iaf8ec8662cf83476eee1b76fa41dc57f063f0ad3
In case the application is using the read-callback and a read returns 0,
then the application itself would want to trigger the reconnect. This
is different from the osmo_stream_cli_recv() case where the reconnect
can be handled internally to the library.
Change-Id: I41314bad4a9f44e8a61b9d2ba33d1a76b25bd145
if osmo_stream_cli_destroy() is called while the reconnect timer is
running, we would end up in a crash.
Change-Id: If6597130f472f1e2b8d9682002250ecd54675bb0
We use the magic value '-1' in case the file descriptor is not yet
initialized. If somebody calls osmo_stream_*_close() before this
changes, we used to crash. Let's check for this and avoid a crash.
Also, after close let's change the fd to -1 again to mark the fd
invalidity.
Change-Id: I3aa04999ab01cb7971ee2dad45dfc31ab4142868
The reconnect behavior was likely broken in commit
de3f57a8293a5b39435d6f283da23e0172bad8bb
If the user requests a re-connect, we should start it. Not only in case
the connection drops later, but also if the initial connection itself
fails.
Change-Id: I817e026404cbd9145cae2ce90bc57a1db1d2e12b
osmo_sock_init() never returns -1 + errno EINPROGRESS. It will return a
positive fd in case the connect operation is in progress. Therefore,
the related code in osmo_stream_cli_open2() was impossible to execute.
Change-Id: Id3483d1d1d4d2eabba94729ea29e5c93b33abff0
Fixes: Coverity CID 57861
When the setsockopt() in sctp_sock_activate_events() indicates an error,
let's print an error message in the log about this.
Change-Id: I5920154e23debe6d01eaa156005db0842f1a18cc
Fixes: Coverity CID 57634
Use this function in osmux_batch_del_circuit() since msgs are stored in a list
per circuit. After the circuit is free()d the msgs are lost.
Before this patch any messages enqueued inside a batch when the circiut is
deleted were leaking.
Change-Id: Ib0311652183332d0475bf7347023d518d38487ef
Ticket: OS#1733
Reviewed-on: https://gerrit.osmocom.org/120
Tested-by: Jenkins Builder
Reviewed-by: Holger Freyther <holger@freyther.de>
caught by valgrind:
==3150== Syscall param sendmsg(msg.msg_control) points to uninitialised byte(s)
==3150== at 0x60DC0D0: __sendmsg_nocancel (syscall-template.S:81)
==3150== by 0x5DF3413: sctp_send (in /usr/lib/x86_64-linux-gnu/libsctp.so.1.0.16)
==3150== by 0x65AA066: osmo_stream_srv_write (stream.c:540)
So far, when the first connection attempt failed in
osmo_stream_cli_open(), we returned a terminal errro without any
re-connection attempts. While this may be useful in some cases, our
general idea of the stream client logic is to handle the reconnection
attempts insid the library. We introduce a new osmo_stream_cli_open2()
function while keping the old behavior for backwards compatibility.
Wih this change, osmo_stream_ client and server API can be used not only
for TCP but also for SCTP. The latter is needed in SIGTRAN ad Iuh
applications, for example.
I have added two new interfaces to add and to delete circuits from osmux input
handles. The client application needs to be adapted to use them otherwise it
will break. That's why I'm making this library release incompatible with the
previous generation.
This new function allows you to create a circuit on an existing input handle.
We don't create the circuit anymore from the first packet seen, instead the
client application is in full control of opening and closing the circuit.
This change includes a new feature to pad a circuit until we see the first
packet that contains voice data. This is useful to preallocate bandwidth on
satellite links such as Iridium/OpenPort.
Add this new function to explicitly remove an existing circuit. Thus, the
client application (openbsc) is in full control to release circuits.
Before this patch, the circuit object was added when the first RTP messages was
seen, and it was removed when transforming the list of pending RTP messages to
the Osmux message (once the timer expired).
Moreover, check circuit->nmsgs to account bytes that are consumed by the osmux
header, given that !circuit doesn't mean "this is the first packet" anymore.
Add a new field to count the number of messages in the batch that are pending
to be transformed to osmux. Use this new field to check when to enable the
osmux timer.
The follow up patch keeps circuit objects in the batch until they are closed,
so we won't be able to rely on this to know when to enable the timer anymore.
I think this is a better name for this object. Basically, an input handle
represents a batch that is composed of one or more circuit objects.
Each circuit object contains a list of RTP messages that are pending to be
converted to the osmux format in one single batch.
This new structure serves as container of the internal osmux state during
transformation from RTP AMR to the compressed osmux format.
This reduces the footprint of several functions and it makes them easier to
extend if we need to pass new information between functions.
When libosmo-abis is installed in a distinct prefix, the build failed
with non-found headers, for example:
../../src/rs232.c:38:35: fatal error: osmocom/abis/e1_input.h: No such file or directory
#include <osmocom/abis/e1_input.h>
Use the new macros to deal with little/big endian. Im a bit
worried to make this change due the little test coverage in
this module but in case of a typo the elements would not be
defined.
Make sure we don't release a osmux handle with an armed batch timer.
==9753== Invalid read of size 4
==9753== at 0x4043CA2: rb_first (rbtree.c:293)
==9753== by 0x403E172: osmo_timers_check (timer.c:256)
==9753== by 0x403E69E: osmo_select_main (select.c:124)
==9753== by 0x804EBD3: main (bsc_nat.c:1613)
==9753== Address 0x4302670 is 56 bytes inside a block of size 108 free'd
==9753== at 0x4023B6A: free (vg_replace_malloc.c:366)
==9753== by 0x40494CF: talloc_free (talloc.c:609)
==9753== by 0x40AB279: osmux_xfrm_input_fini (osmux.c:620)
==9753== by 0x80651FC: osmux_disable_endpoint (mgcp_osmux.c:96)
==9753== by 0x805CAFF: mgcp_release_endp (mgcp_protocol.c:1540)
==9753== by 0x805DD35: handle_delete_con (mgcp_protocol.c:1274)
==9753== by 0x805E998: mgcp_handle_message (mgcp_protocol.c:415)
==9753== by 0x804CFF1: mgcp_do_read (bsc_mgcp_utils.c:972)
==9753== by 0x403F96D: osmo_wqueue_bfd_cb (write_queue.c:48)
==9753== by 0x403E724: osmo_select_main (select.c:158)
==9753== by 0x804EBD3: main (bsc_nat.c:1613)
If it is not possible to put the RTP message into the batch in case that:
1) The message is malformed.
2) The message is duplicated.
3) OOM.
4) The batch is already full.
Osmux releases the messages and gets back to the upper layer with an OK
to give another chance to follow up RTP messages.
This patch also fixes a use-after-free that was possible when the batch
was full. The message was released and osmux_batch_add() was returning 0
osmux_xfrm_input(), which resulted in accessing the released message
when updating the statistics.
This is ugly, with many ifdefs, but they don't want any debug message
that can be spamming. I don't want to remove these because there is
no dissector for osmux, and this can be useful for troubleshooting.
Should be enabled only in case you consider that Osmux should
is lagging when reconstructing RTP messages. I haven't seen
any issue regarding this so far. Let's disable it by default.
This patch adds a new field to the struct osmux_in_handle that allows
you to specify the osmux frame size. If not specified, the default
size assumes your nic uses a mtu of 1500 bytes.
Remove message that is printed twice and reword the messages to
make it more clear the traffic flow when compressing and
decompressing the RTP AMR traffic.
(dc898ab osmux: don't trust AMR FT field) was not correctly
validating the AMR FT field as it was comparing the same
value twice calculated in different ways.
Use osmo_amr_bytes(amrh->ft) to obtain the expected length
and check if it is what we got. Use the output of
osmo_rtp_get_payload() as it also includes the RTP payload
stripping possible extensions.
Pau Espin
Max
Philipp Maier
Harald Welte
Vadim Yanitskiy
Neels Hofmeyr
Sylvain Munaut
Daniel Willmann
Martin Hauke