As found by Asan on a osmo-mgw running in production:
"""
==2238035==ERROR: AddressSanitizer: heap-use-after-free on address 0x62100043bdca at pc 0x7f9bcebaa070 bp 0x7ffcb08f2150 sp 0x7ffcb08f2148
READ of size 2 at 0x62100043bdca thread T0
#0 0x7f9bcebaa06f in msgb_length src/core/msgb.c:287
#1 0x55869457a8ff in conn_osmux_send_rtp src/libosmo-mgcp/mgcp_osmux.c:245
#2 0x558694563a86 in mgcp_dispatch_rtp_bridge_cb src/libosmo-mgcp/mgcp_network.c:1347
#3 0x5586945570a9 in rx_rtp src/libosmo-mgcp/mgcp_network.c:1550
#4 0x5586945570a9 in rtp_recvfrom_cb src/libosmo-mgcp/mgcp_network.c:1505
#5 0x7f9bcebc96cc in iofd_poll_ofd_cb_recvmsg_sendmsg src/core/osmo_io_poll.c:84
#6 0x7f9bcebcb699 in iofd_poll_ofd_cb_dispatch src/core/osmo_io_poll.c:136
#7 0x7f9bcebd7df5 in poll_disp_fds src/core/select.c:419
#8 0x7f9bcebd7df5 in _osmo_select_main src/core/select.c:457
#9 0x7f9bcebd8298 in osmo_select_main src/core/select.c:496
#10 0x558694534f2e in main src/osmo-mgw/mgw_main.c:428
"""
Related: SYS#7450
Change-Id: Id90c77aaf44422c3ed70ffb06560537e920a468c
E1 endpoints still have usually 1 RTP connection on it (they do
E1-slot<->RTP), so we still need to update the RTP addr+port binding
upon CRCX & MDCX on those endpoints.
This fixes a recent regression with setups with E1 BTS failing to
establish calls.
Fixes: b317b0056a
Change-Id: I52e91cd2d9d4b64961b76bb0fecc82494ca0d7cb
Prior to this patch, osmo-mgw would wait until first IuUP Data frame
would be received on RAN-side (from HNB) to trigger active
initialization (Tx IuUP-Init) on CN-side conn towards CN.
With this patch, this happens ASAP, whenever the CN-side conn is created
by HNBGW when it receives the RAB-ASS-RESP from HNB.
Related: OS#6685
Change-Id: Iab0af88852994e73bfe6c3b27fe196fd655cce03
Move code logic doing actions based on endpoint state (and its conns)
out of the CRCX/MDCX MGCP handlers.
Most of that logic present in CRCX and MDCX is actually the same and can
be merged.
This way we can also make the updating logic more complex without
extending the CRCX/MDCX handlers for and more adding spaghetti code.
Change-Id: I24dd15ec76bd6e949a178a3b998b76a44ddbab50
Make code in CRCX and MDCX look closer, since it is really doing the
same.
Move the dummy ping code after everything has been done correctly, since
the ping is just a side effect of the conn being created.
Change-Id: I42acda16cb2d59a9b7aae06b7584d4dfc1e91f9e
This has never been used afaik, and it's only clogging the code and
adding extra indirect function pointer calls to hot paths for no good
reason.
Drop all of it, and re-ad whatever makes sense once someone really wants
to implement transcoding.
Change-Id: Id16af528d94612e771f94333ad5404e5cd1c3318
Same order as done during MDCX. This way we can also eg. check
configured codec is AMR before successfully entering configuring osmux.
Change-Id: If291db0c048196e6e0eee2c38e648e5a25438078
The SDP parsing is moved to a step beforehand, so that validation of
full message can be done in a subsequent step. That validation step is
moved into a function to give some air to handle_modify_con() and easily
spot the logic.
Change-Id: I469413dbe89cc4795deb50d76b434655cba9b776
We could not do it before because we had no easy access to the command
verb being parsed. Now that we have access to it, bettter increase it
where we have exact path where stuff happens.
Change-Id: I3ea45fc1d25284b253ac9b7f0c0a925c10c994ca
The SDP parsing is moved to a step beforehand, so that validation of
full message can be done in a subsequent step. That validation step is
moved into a function to give some air to handle_create_con() and easily
spot the logic.
The logic based on force_realloc is split so that code modifying the
object is moved to the later update step.
Change-Id: I639ad0a25a0af4a4637045ca8bf61e436a789426
Split current code into 2 steps, so first read-only parsing can be done,
then when all parsing and checks have been done, finally can be applied
o the object.
Change-Id: If75731fb92329dca6d092ffc7ff17cb354d28c0d
In the absence of a proper TFO transform (TS 28.062 section
C3.2.1.1), OsmoMGW-E1 inserts constant fill frames when there is
no RTP input, or when RTP input fails conversion to TRAU-DL and
thus does not enqueue anything to the I.460 mux.
This dummy fill was previously implemented for FR and EFR codecs,
but not for HRv1. Fix this omission.
With this change and with a fix to OsmoBSC to select 8k subslot
endpoints for TCH/H instead of 16k, OsmoMGW-E1 now supports HR
codec to the same degree to which it supports FR and EFR: far from
perfect, but mostly usable, especially if DTXu is disabled
and there is no TrFO interworking with a far end that does DTXu.
Change-Id: I188bc0a0468b19126281016f45e36f1de617e9ee
In the absence of a proper TFO transform (TS 28.062 section
C3.2.1.1), OsmoMGW-E1 inserts constant fill frames when there is
no RTP input, or when RTP input fails conversion to TRAU-DL and
thus does not enqueue anything to the I.460 mux. (We accept
TW-TS-001 and TW-TS-002 from RTP side, but bad frames cannot be
converted to TRAU-DL without a TFO transform.)
Prior to the present change, this idle fill of the DL was done
using fully hard-coded frames, i.e., the operation of converting
the desired fill to TRAU-DL bit format was done once externally
and the raw bit frames were hard-coded. Change this approach:
use fill frames in RTP format provided by libosmocodec, and
execute osmo_rtp2trau() followed by osmo_trau_frame_encode() in
the fill frame output path just like we do for regular RTP to DL.
Depends: libosmocore.git I2c510ac62a0786c137115c45eee7a48b9736265f
Change-Id: I123f77295c56d61ff9aac5f5d64b25eca01e0d63
TW-TS-001 and TW-TS-002 are enhanced RTP transport formats for
GSM FR/HR/EFR codecs that restore the lost semantics of GSM 08.60
and 08.61 TRAU-UL frames.
TW-TS-003 BSSMAP extension provides a way for a core network to
ask the BSS to use these TRAU-like enhanced RTP formats instead of
those specified in 3GPP TS 48.103; OsmoBSC already supports this
mechanism when the BSS is comprised of IP-native OsmoBTS.
However, in order to achieve the same effect when using E1-based
legacy BTS hardware, the task of generating TW-TS-001/002 RTP
packets for UL and accepting them for DL moves from OsmoBTS to
the E1-Abis-interfacing MGW. osmo_trau2rtp() is already capable
of generating these extended RTP formats on request, but until now
there was no mechanism to signal from OsmoBSC to its associated
E1 Abis MGW whether and when these extensions should be used.
Considering that MGCP as it is used in Osmocom is essentially a
private interface between OsmoBSC or OsmoMSC masters and OsmoMGW
slaves, while the externally defined and generally interoperable
interface is 3GPP TS 48.008, possibly extended with TW-TS-003,
the sensible solution is to make a private extension to the way
FR, EFR and HR codecs are described in SDP in the context of
Osmocom-MGCP.
The SDP extension birthed in the present patch consists of an fmtp
parameter for GSM, GSM-EFR and GSM-HR-08 codecs that is structured
just like the already implemented octet-align parameter for AMR.
TW-TS-001 for FR and EFR shall be described as follows:
m=audio 1234 RTP/AVP 3 110
a=rtpmap:3 GSM/8000/1
a=fmtp:3 tw-ts-001=1
a=rtpmap:110 GSM-EFR/8000/1
a=fmtp:110 tw-ts-001=1
TW-TS-002 for HR codec shall be described as follows:
m=audio 1234 RTP/AVP 111
a=rtpmap:111 GSM-HR-08/8000/1
a=fmtp:111 tw-ts-002=1
The present patch affects two areas:
* Experimental support for the newly defined extension is added to
OsmoMGW-E1. This support is deemed experimental (not for
production use) because even with this extension added, OsmoMGW-E1
is still unable to satisfy ThemWi requirements: neither ThemWi
RTP endpoint library nor the TFO transform of TS 28.062 section
C.3.2.1.1 are currently available in the repertoire of libraries
whose use is allowed in mainline OsmoCNI components, yet both are
required.
* Support is added to libosmo-mgcp-client whereby OsmoBSC will be
able to issue CRCX and MDCX commands to E1 Abis MGW endpoints
with TW-TS-001 and/or TW-TS-002 enabled. Adding the necessary
support to OsmoBSC will allow a complete working system to be
deployed using OsmoBSC plus tw-e1abis-mgw, a replacement for
OsmoMGW-E1 that works by using both Osmocom and ThemWi libraries.
Related: OS#6614
Change-Id: I0d58e6d84418f50670c8ab7cf8490af3bc2f5c26
This will allow allocating parsed objects as child of the request and
make sure they are freed after handling the request finishes.
Change-Id: I2b971d0c8268f4c0a30b84b54a2e5f16ada4ecdb
This will also allow eg. parsing/validating differently based on the
command being parsed, or incrementing rate counters per cmd group.
Change-Id: I464258ca1a8817d58ae5c5426dfc3b7cee6763d3
The externa object context is held in rq object, not in pdata object,
which holds parsed msg state only.
Change-Id: Ie64fb6250cc84f1e7737b5c1cf5cca314c7020ed
All the related handling structs are placed in the header file, let's
move them there. This will also allow accessing the rq object
information in other files.
Change-Id: I1b2831874c1aaad054ad19bf87646062ba9d4da4
This simplifies code further, and will simpify more code since we'll
have some easy way to check for the command we are in during a request.
Change-Id: I2d4b5ddb93376b59413b34c9668c41157ab05497
This allows already starting to mark a clear line in CRCX handling
function between *read-only parsing + validation* and *read-write
allocation and update* of objects.
This in turn will allow further clean up on the second mentioned step.
Change-Id: Ia41f7383171bb24f48297456935c633536aa7b05
The build process copies include/osmocom/mgcp/mgcp_common.h to
include/osmocom/mgcp_client/mgcp_common.h; the former is a true
source while the latter is a build-generated file. The copied
file needs to be included in .gitignore, otherwise it adds
noise to git status output.
Change-Id: Icf0e3f724564d87eef9351b22de7a3d3aa7e4264
This new behavior rejects the CRCX before creating a conn, simplifying
the code and also allows getting rid of the remote_osmux_cid local variable.
Change-Id: I0245b6c02bf7a3452532e8bf0d7c33479999ce9f
The writer of this commit wrongly interpreted conn->end to be an
mgcp_endpoint, but it's actually an struct mgcp_rtp_end, hence codecs
are set per connection as expected.
Change-Id: I773065972342b8cedc900e3d194c48775c3adb5a
This callback was drepecated and is not ever called since
libosmocore.git 70ce871532ab21955e0955d7e230eae65438f047 (release 1.3.0).
See also libosmocore.git d31de237582f6fe3315d61bb9a488d4cda92654e.
Change-Id: Ifdecb35f329c243222bc5d77f0a6d394c9413684
The only truly correct way to pass FR/HR/EFR traffic from an
incoming RTP stream to TRAU-DL frames is to apply the transform
of TS 28.062 section C.3.2.1.1, originally specified for TFO
but also necessary in the non-3GPP-specified case of GSM TrFO
as it happens here. Unlike the situation with EFR, a FOSS
implementation of TFO transform for FRv1 does exist in Themyscira
libgsmfr2 - however, making these Themyscira codec libraries
usable from mainline Osmocom programs by way of a proposed
libosmocore DSO plugin mechanism, followed by a major redesign
of OsmoMGW-E1 to pass all DL traffic through this TFO transform,
would be a major project, difficult to justify prior to development
of a proper TFO transform for EFR to complement the one for FRv1.
Hence no change is being made currently to the arguably flawed
general architecture of OsmoMGW-E1, which consists of passing
through unaltered all valid codec frames that are received in RTP,
and inserting fixed dummy TRAU-DL frames when no RTP-derived ones
are available.
The original dummy TRAU-DL frame for FRv1 exhibited the following
defects:
* The payload frame transmitted to the MS consisted of all zero
bits. Standard type-approved GSM MS would interpret this bit
pattern as a SID frame, even though the BTS is told via C16 bit
"this frame is not a SID". Passing a SID is not inherently bad
in itself in this context, but a SID with all LARc parameters
set to 0 is a strange/poor choice.
* The setting of C-bits (defined in TS 48.060 section 5.5.1.1.1)
was a mish-mash between TRAU-UL and TRAU-DL frame formats,
thereby not matching the standard TRAU-DL C-bits fill produced
by osmo_rtp2trau().
The replacement fill frame introduced here differs as follows:
* The payload bit content is the silence frame of GSM 46.011
Table 1: certainly better than a SID with all LARc parameters
set to 0, and arguably better than any SID at all.
* The full TRAU-DL frame (C-bits pattern, peculiar ordering of
D-bits) was generated by passing said silence frame through
osmo_rtp2trau(), using this ad hoc tool:
https://gitea.osmocom.org/themwi/dummy-dl-frames
Change-Id: I995824586058e4e4ed77e900d4b57e5113f9eff6
The only truly correct way to pass FR/HR/EFR traffic from an
incoming RTP stream to TRAU-DL frames is to apply the transform
of TS 28.062 section C.3.2.1.1, originally specified for TFO
but also necessary in the non-3GPP-specified case of GSM TrFO
as it happens here. However, no FOSS or even published-source
implementation of TFO transform for EFR exists at the present time
(no implementations at all are known outside of TRAU DSP firmware),
hence the general architectural approach of OsmoMGW-E1, flawed
as it is, has to remain for now. This flawed architecture consists
of passing through unaltered all valid codec frames that are
received in RTP, and inserting fixed dummy TRAU-DL frames when no
RTP-derived ones are available.
The original dummy TRAU-DL frame for EFR exhibited the following
defects:
* D1 bit was set to 0, even though TS 48.060 section 5.5.1.1.2
states it shall be 1;
* All 5 EFR frame CRCs were invalid: each 3-bit CRC was 000, even
though the correct TS 48.060 section 5.5.1.1.2 CRC from an
all-zeros payload would be 111;
* Even if the 5 CRCs were fixed, transmitting an EFR frame of all
zero bits to the MS won't produce good results - it is a bad
choice of dummy filler;
* The setting of C-bits (defined in TS 48.060 section 5.5.1.1.1)
was a mish-mash between TRAU-UL and TRAU-DL frame formats,
thereby not matching the standard TRAU-DL C-bits fill produced
by osmo_rtp2trau().
The replacement fill frame introduced here differs as follows:
* The payload bit content is the decoder homing frame (DHF) of
TS 46.060 section 8.2 Table 7 - the best we can do in the absence
of a proper TFO transform for EFR;
* The full TRAU-DL frame (C-bits, CRC etc) was generated by passing
said EFR DHF through osmo_rtp2trau(), using this ad hoc tool:
https://gitea.osmocom.org/themwi/dummy-dl-frames
Change-Id: I5a33a1c9ddf1372f91870d61b5eafac4729ee458
By changing the white space structure of these hard-coded frames
to only have one leading tab on each line of 8 frame bits, we make
it easier to replace these hard-coded frames with different versions
that are programmatically generated, as will be done in the following
patches.
Change-Id: I3d0c931d4dc3d916ba4c5eb081bea22d9c7144de
The TODO was written because the author had in mind the ptr where the
codec was stored was a MGCP endpoint object, but it is actually an
rtp_end which is an object under conn_rtp, so that's fine already with
current code.
Change-Id: I99d2211e81443883c45cc3fdda10e39a8c152063
Do most of the initial parsing and verification in a prior step, filling
in a "parsed" struct which simplifies logic coming after for different
message types.
Change-Id: I557a3a257ddefedc479a4aff974a74c4e4e2c85d
Do most of the initial parsing and verification in a prior step, filling
in a "parsed" struct which simplifies logic coming after for different
message types.
This commit only modifies stuff enough to work for MDCX.
Further work (commits) will follow for DLCX.
Change-Id: I6ecb41fc2cc737c3a161b6bc98bd08ae01909655
Do most of the initial parsing and verification in a prior step, filling
in a "parsed" struct which simplifies logic coming after for different
message types.
This commit only modifies stuff enough to work for CRCX.
Further work (commits) will follow for MDCX and DLCX.
Change-Id: I3ee5158c254213203830fe9c38de11c15b4b19c1
Clean up pointers passed. Rename function since it's clearly operating
on trunk related fields through mutexes.
Change-Id: Ib894afcb61609c247883d5ccdd7b8fbf29b2cbf8
Clean up pointers, join 2 functions only making stuff more confusing.
Rename function to make it clear it operates on a conn_rtp object.
Change-Id: I50a251b66e85ceeeccb30dcd1813863d7c754961
The previous logic to configure SSRC patching was over complex and
confusing, with even some broken logic like "patch only once", which is
not really needed. Hence, simplify the internal logic to either patch
SSRC always or don't do it, based on VTY configuration.
The "force SSRC" feature was added in
db2d431697 and further changed in
e2292f3aa1, due to the need for nanoBTS to
always keep the same SSRC on received packets. However, it makes no
sense that is actually needed only once: if remote end changed several
times, then we should keep patching in that case.
This change allows getting rid of confusing applying of settings at a
later time through mgcp_rtp_end_config(), which is no longer needed and
can be dropped, simplifying steps during CRCX/MDCX/DLCX.
Change-Id: Ib7216a775f4ad126e62b9e99aff381fd45015819
Apply the forced value automatically during init, and then only
override it based on received input from MGCP when it has not been
forced by VTY config.
Change-Id: I02a92a090887caa8e05917a44c8b2351fa7b9b50
We do tons of operations on 3 fields (array of codecs, len of array,
selected code) which can be isolated.
Right now, we are using APIs which somehow require structs 2-3 levels of
encapsulation above the ones really required, which makes all code
totally entangled, difficult to understand and prone to errors when
changing stuff in deeply nested structs.
A prove of this is how this patch affects a lot of lines in lots of
places.
Change-Id: Id7db7ab01d56b7fa2415123b604375e48c82ab25
Preparation path to add an init function and start untangling related
code.
While at it, move definition of struct mgcp_rtp_codec to the mgcp_codec.h where it belongs.
Change-Id: Id1b6cab57e44ad4859bde8212d0ac9f7146d7198
While at it, moving the only clearly existing API for the same object to
the same place to have them together
(mgcp_rtp_end_remote_addr_available()).
Change-Id: Ifabd1cf69273a6d22feb65c08de590d083987d09
