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.
The ctr field of the osmux header is 3 bits long, make sure we
don't run over that boundary. This should not happen in practise
unless we have to deal with network congestion or broken RTP
stacks, but osmux should not crash in that case.
With this patch, osmux_xfrm_input() returns 0 (means "message has been
processed") instead of 1 (means "retry") if the RTP message is too big
to fit into one osmux batch. This fixes a likely infinite loop in the
caller, which will retry forever for a message does not fit into the
batch.
Unlikely to happen in normal scenario, as RTP+AMR messages are way
smaller than the interface MTU.
The AMR FT field is used to infer the length of the payload, if
a value higher than 8 (SID) is received, skip it.
This fixes a possible crash in osmux_snprintf() in case we receive
a malformed osmux header.
This is also addresses the crash described in c733ae5b6e.
valgrind reports the following crash backtrace:
!<001c> osmux.c:687 No room for OSMUX payload: only 49 bytes
==12800==
==12800== Process terminating with default action of signal 11 (SIGSEGV)
==12800== Access not within mapped region at address 0xDFA8E473
==12800== at 0x4073FD2: osmux_snprintf (osmux.c:628)
==12800== by 0x80524F1: osmux_deliver (osmux.c:50)
==12800== by 0x407371C: osmux_xfrm_input_deliver (osmux.c:302)
==12800== by 0x4073792: osmux_batch_timer_expired (osmux.c:312)
==12800== by 0x405A4A0: osmo_timers_update (timer.c:243)
==12800== by 0x405A79A: osmo_select_main (select.c:133)
==12800== by 0x8049A53: main (mgcp_main.c:307)
The problem is that osmux_snprintf() is not handling multi-batch
messages (ie. messages that contain several osmux batches). More
specifically, the offset to print the osmux batches was reset
when parsing every osmux batch.
The problem also manifested with wrong outputs.
Reported by Mattias Lundstrom.
Valgrind complains about a possible use after free:
==12800== Invalid read of size 4
==12800== at 0x4073DF6: osmux_tx_sched (linuxlist.h:119)
==12800== by 0x8052B0F: osmux_read_from_bsc_nat_cb (osmux.c:261)
==12800== by 0x453F967: ???
==12800== Address 0x453f710 is 48 bytes inside a block of size 145
+free'd
==12800== at 0x402750C: free (vg_replace_malloc.c:427)
==12800== by 0x4064ADE: talloc_free (talloc.c:609)
==12800== by 0x405AAAA: msgb_free (msgb.c:72)
==12800== by 0x8052492: scheduled_tx_bts_cb (osmux.c:196)
==12800== by 0x4072CF8: osmux_tx_cb (osmux.c:554)
==12800== by 0x4073F03: osmux_tx_sched (osmux.c:582)
==12800== by 0x8052B0F: osmux_read_from_bsc_nat_cb (osmux.c:261)
==12800== by 0x453F967: ???
The problem is that osmux_tx_sched may immediately call osmux_tx_cb for
the first extracted RTP message from the osmux batch, which releases the
message after that.
Remove the message from our list of messages to be transmitted before
the message is passed to the tx callback.
Reported by Mattias Lundstrom.
According to RFC3267, AMR FT upper 9 should be discarded. This patch
adds extra validation to make sure that input RTP traffic encapsulating
AMR payload and OSMUX amr_ft field are OK with regards to that
restriction.
rtp.c:154:26: warning: The left operand to '/' is always 0
frame_diff = (usec_diff / 20000);
~~~~~~~~~ ^
rtp.c:157:43: warning: The left operand to '-' is always 0
long int frame_diff_excess = frame_diff - 1;
~~~~~~~~~~ ^
rtp.c:153:39: warning: The right operand to '+' is always 0
usec_diff = tv_diff.tv_sec * 1000000 + tv_diff.tv_usec;
^ ~~~~~~~~~~~~~~~
rtp.c:153:29: warning: The left operand to '*' is always 0
usec_diff = tv_diff.tv_sec * 1000000 + tv_diff.tv_usec;
~~~~~~~~~~~~~~ ^
4 warnings generated.
Reported by Holger Hans Peter Freyther.
osmux.c:622:20: warning: Value stored to 'osmuxh' during its
initialization is never read
struct osmux_hdr *osmuxh = (struct osmux_hdr *)msg->data;
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Reported by Holger Hans Peter Freyther.
If osmux notices a gap between two RTP packets, fill it with
the last RTP packet seen. Without this patch, 10% of packet loss
is enough to get garbage, with it we get glitches in the conversation
with 30%, and pretty much broken conversation with 40% of it.
Use talloc_size not talloc. Should fix:
0xb779401a in rb_erase (node=0x200200, root=0xb779c908) at rbtree.c:230
0xb779401a in rb_erase (node=0x200200, root=0xb779c908) at rbtree.c:230
0xb778ee48 in osmo_timer_del (timer=0x94aacd0) at timer.c:110
0xb778ef65 in osmo_timer_add (timer=0x94aacd0) at timer.c:72
0xb778f03c in osmo_timer_schedule (timer=0x94aacd0, seconds=0, microseconds=64000)
0xb77360ff in osmux_xfrm_input (h=0x94a4280, msg=0x94b8a50, ccid=18) at osmux.c:390
Due to uninitialization batch structures.
Remove these functions:
- osmux_xfrm_input_get_ccid
- osmux_xfrm_input_register_ccid
The ccid will be managed by the BSC and it will be stored in the
mgcp_endpoint structure.
Also adjust all tests and examples using the API.
Rework batching routine to reduce its complexity, updates:
* Now it uses a list of lists to store the messages that will be
batched.
batch list
|
`-> node SSRC=a ---> ... ---> node SSRC=b
| |
msg seq=x1 msg seq=y1
| |
msg seq=x2 msg seq=y2
| |
msg seq=x3 msg seq=y3
| |
msg seq=x4 msg seq y4
This keeps easier the creation of the final batch that is sent from that
data structure.
* We also detect duplicate messages in the batch, ie. messages with the
same sequence are skipped.
Still pending to resolve reordering, corruption and omissions (reliability
is desired).
Between two RTP messages that were extracted from a batch, the
timestamp difference should be 160.
Signed-off-by: Pablo Neira Ayuso <pablo@gnumonks.org>
Instead of internally released. This is required if we use the
osmo_dgram infrastructure, to avoid a double release.
Signed-off-by: Pablo Neira Ayuso <pablo@gnumonks.org>
This patch removes osmo_ipa_recv_msg, it performs two syscall invocations
and it's stream generic. Now we use the specific receival function
we want to use (no matter if stream or datagram based) and then we
call osmo_ipa_process_msg to check that the IPA message correct.
This adds the possibility to specify the variant of the channel.
This was discussed during the osmocom workshop. Harald wanted a way
to say if the channel is using TCP, UDP, DADHDI and so on.
This patch fixes the ordering in RTP sequence number. Before this patch,
the list was inverted.
This also fixes the calculation of the room that still remains in the
batch.
osmux only lags ~0.15 ms at maximum to transmit one scheduled RTP message
according to my tests with PCAP traces.
Yes, only ~0.15 milliseconds, this is not wrong :-).
This is good news, our timer infrastructure seems to be quite precise.
This fixes the algorithms to include the messages in order in the
batch based on the TRP SSRC (from lower to higher).
This is very important to reduce the amount of bytes that we
spend on the osmux header.
This patch cleans up the transmission path for osmux, this involves
the functions that extract the messages from the batch and the one
that reconstruct the timing.
They now take a list that contains the reconstructed RTP messages:
osmux_xfrm_output(osmuxh, &h_output, &list);
osmux_tx_sched(&list, &tv, tx_cb, NULL);
This patch adds the counter field to the osmux header, so we can
reduce the size of the batch even further, eg.
osmuxhdr (ctr=3)
speech
speech
speech
osmuxhdr (ctr=2)
speech
speech
...
The new header is the following:
0 1 2 3
0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
| FT | CTR |F|Q| SeqNR | Circuit ID |AMR-FT |AMR-CMR|
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
The counter field is 3 bits long, thus, we can batch up to 8
RTP speech frames into one single batch per circuit ID.
I have also removed the RTP marker, since it can be reconstructed
from the AMR information.
Moreover, the entire workflow has been also reworked. Whenever a
packet arrives, we introduce it into the batch list. This batch
list contains a list of RTP messages ordered by RTP SSRC. Then,
once the batch timer expires or the it gets full, we build the
batch from the list of RTP messages.
Note that this allows us to put several speech frame into one
single osmux header without actually worrying about the amount
of messages that we'll receive.
The functions that reconstruct the RTP messages has been also
adjusted. Now, it returns a list of RTP messages per RTP SSRC
that has been extracted from the batch.
This function schedules the transmission of a RTP message that was
obtained from one osmux batch. It takes the time (in microseconds)
after which the message should be transmitted.
I was using AMR CMR 2 (15 bytes) as the initial tests were done
with the codec variant.
This patch fixes this by using the new generic osmo_amr_bytes
extracted from 3GPP TS 26.101.
This patch splits osmo_rtp_parse in two functions:
osmo_rtp_get_hdr
osmo_rtp_get_payload
So we can validate corrent RTP header to access its fields. Then,
obtain the payload.
This patch adds the initial RTP support for libosmo-netif, it's based
on Harald's RTP support available in openBSC.
I have also added a couple of example to show how our new channel
infrastructure interacts with the RTP layer.
Signed-off-by: Pablo Neira Ayuso <pablo@gnumonks.org>
This patch adds the generic channel infrastructure that allows to
create channel of different types. Each channel has their own
configuration functions.
struct osmo_chan *chan;
chan = osmo_chan_create(tall_example, CHAN_ABIS_IPA_SERVER);
...
/* specific configuration functions per supported channel. */
osmo_chan_abis_ipa_server_set_cb_signalmsg(chan, signal_msg_cb);
osmo_chan_abis_ipa_unit_add(chan, 1801, 0);
/* open channel. */
osmo_chan_open(chan);
The input path requires a callback to be registered. The output path
is handled through:
int osmo_chan_enqueue(struct osmo_chan *c, struct msgb *msg);
The msg->dst must be set (it can be taken from the original message
to route one reply).
This patch also adds A-bis IPA server support. It has been tested with
e1inp_ipa_bsc_test available in libosmo-abis.
This patch adds IPA helper function that can be use on top of stream
sockets.
The current API is just a copy and paste from libosmo-abis, it will
change in follow up patches to improve it.
We provide osmo_dgram_conn_recv(...) which allows you to take control
on the message allocation and receival process. Instead of hiding this
details inside the datagram infrastructure.
Providing more control to clients of this code means more flexibility.
This patch changes the prototype of osmo_dgram_conn_set_read_cb.
Now it takes one callback to a generic struct osmo_dgram_conn
instead of osmo_dgram_server_conn. This is useful in case that
you want to reply to one message using the datagram socket (without
this change is not possible).
Add new functions:
osmo_stream_server_link_get_data
osmo_stream_server_conn_get_data
osmo_stream_client_conn_get_data
To obtain private data from osmo_stream_* handlers.
This patch also introduces missing osmo_stream_server_conn_set_data.
Pau Espin
Harald Welte
Neels Hofmeyr
Daniel Willmann
Max
Martin Hauke