Until now, the osmux_in_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, openbsci, osmo-bts) can still work fine, since there's no
change on the struct osmux_in_handle. However, they will somehow break
next time thestruct is changed until they are ported to the same API
(easy to do).
Change-Id: I752ab031f935f04731bb1a354333f1682a1aa5bd
Related: SYS#5987
Replacing this one with the newer API was missed a few commits ago when
this API was marked as deprectated. Do it now.
Change-Id: Ia0958dfae951d82feafe427eff2112d327d3b0a4
The output and input paths are totally independent, they share no code
or structures holding state. They can be operated totally independently.
Hence, let's split the code into separate file since when someone looks
at the osmux code one is really looking specifically at Tx or Rx side,
but not at both sides.
Moreover, the "input" and "output" concepts have always been a bit
difficult to understand (would have been better calling them "src" and
"sink" instead), which adds more confusion when trying to find the
relevant part of code.
Change-Id: Ia72995092a36ca50147611e617cb88c4dcf231d5
It was not being used anywhere, yet older applications used to set it
(always to 0, which was the default value applied internally).
Let's make use of it and apply it as first seqnum to be used on a
circuit.
This value is applied upon call to osmux_xfrm_input_open_circuit(),
hence it can be set independently for every new circuit.
Change-Id: Ia26fcba5d7364a5744b2d64d0542a2b3880eee34
There's no real use in having it split, it just makes it more difficult
to extend since extra prams must be passed over 2 functions.
In fact, the duplicity of "struct osmux_in_handle" and "struct
osmux_batch" as its ->internal_data should go away in the future.
Change-Id: Ie4c6b55827ac27bcdfbe1b14fb238f5873243000
It was seen on a real pcap trace (sctp & gsmtap_log) that the Linux
kernel stack may decide to kill the connection (sending an ABORT) if
it fails to transmit some data after a while:
ABORT Cause code: "Protocol violation (0x000d)",
Cause Information: "Association exceeded its max_retrans count".
When this occurs, the kernel sends the
MSG_NOTIFICATION,SCTP_ASSOC_CHANGE,SCTP_COMM_LOST notification when
reading from the socket with sctp_recvmsg(). This basically signals that
the socket conn is dead, and subsequent writes to it will result in
send() failures (and receive SCTP_SEND_FAILED notification upon follow
up reads).
It's important to notice that after those events, there's no other sort
of different event like SHUTDOWN coming in, so that's the time at which
we must tell the user to close the socket.
Hence, let's signal the caller that the socket is dead by returning 0,
to comply with usual recv() API.
Related: SYS#6113
Change-Id: If94d44f25b76a96a5ea402fec9fc14c4e6296ba3
It could be that the user reuses the msgb passed to
osmo_stream_srv_recv() all the time, hence we need to reset the flags,
otherwise it may end up being set forever.
Change-Id: Id358140db82b018e3973111e530834589c0b7224
This allows the user to properly check the size of the content in case
the struct ABI changes.
-EAGAIN is still returned to avoid older users of the API reading SCTP
notifications as user data.
Change-Id: I95e2457498fd8e0d790d221cb97695ace0dd673e
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
The AMR FT (and hence payload content and size) may well change over the
lifetime of an RTP stream. Since all AMR payloads in an osmux batch
share the same FT (its joined payload is calculated based on
osmuxh->ctr*amr_bytes((osmuxh->amr_ft)), if a new RTP/AMR with a
different FT arrives we cannot simply append it to the current batch.
Instead, the current batch must be considered done and a new batch with
anew osmuxh header must be prepared for the resulting osmux frame to
send over UDP.
Before this patch, when a packet with a different AMR FT was submitted
for batching, it would be added in the same batch and decoding would
fail since the sizes of the batches would be wrong.
Related: SYS#5987
Change-Id: I25eb6ee54c898f575cc71ccfd6d190fe51d56dbe
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
If there's an error while replaying lost packets, return the error to
the caller.
If the batch is found full, early return indicating so, there's no need
to continue flow calling osmux_batch_enqueue() in osmux_batch_add()
again.
Change-Id: I62f494d2b2e7903a6683f6dea00781bb721a3d41
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
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
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
This has been the port being used historically in most osmux setups,
and projects using osmux.h have it defined locally. Let's define it here
so that there's no need to define it on each client.
Change-Id: Ibfd058bceeeaa1384a00d8fcd6d6268b445e19bd
Pau Espin
Max