Currently in handle_create_con(), mgcp_conn_alloc() is called with NULl
ctx. As soon as this ctx is changed to be part of the trunk's endpoint
array (tcfg->endpoints), test will segfault because some fds from
previous tcfg are still registered after the whole tcfg object was freed
with talloc_free() by previous test. That's because
mgcp_endpoint_release() must be called on all endpoints to make sure all
registered components are correctly unplugged.
Related: OS#3950
Change-Id: I813d52b518ed0bb8db4e42dff83e040b0891fee2
In codecs_same(), do not compare the complete audio_name. The parts of it are
already checked individually:
- subtype_name ("AMR"),
- rate ("8000"; defaults to 8000 if omitted) and
- channels ("1"; defaults to 1 if omitted)
So by also checking the complete audio_name, we brushed over the match of
implicit "/8000" and "/8000/1", which otherwise works out fine.
As a result, translating payload type numbers in RTP headers now also works if
one conn of an endpoint set an rtpmap with "AMR/8000" and the other conn set
"AMR/8000/1".
It seems to me that most PBX out there generate ptmaps omitting the "/1", so
fixing this should make us more interoperable with third party SDP.
See IETF RFC4566 section 6. SDP Attributes:
For audio streams, <encoding parameters> indicates the number
of audio channels. This parameter is OPTIONAL and may be
omitted if the number of channels is one, provided that no
additional parameters are needed.
Also allowing to omit the "/8000" is a mere side effect of this patch.
Omitting the rate does not seem to be specified in an RFC, but is logical for
audio codecs defined to require exactly 8000 set as rate (most GSM codecs).
Add tests in mgcp_test.c.
Change-Id: Iab00bf9a55b1847f85999077114b37e70fb677c2
Instead of manually entering codec values, use mgcp_codec_add() to populate
test conns with codecs. The idea is to better test what actually happens when
parsing SDP codec strings.
Rewrite current test_mgcp_codec_pt_translate() from procedural to a data model
with human readable stdout logging.
This prepares to enable interpreting codec strings like "FOO/8000/1" as
equivalent with "FOO/8000": the SDP standard defines the final "/1", indicating
the nr of channels, as optional for a single channel, but osmo-mgw currently is
unable to match these two formats as identical. So prepare the
test_mgcp_codec_pt_translate() so that upcoming patches can incorporate strings
with and without the final "/1" by extending the struct arrays.
Change-Id: I888000d77512cfecb0f199b86ef6003e7fc0e6cb
MGCP RFC3435 (https://tools.ietf.org/html/rfc3435) states almost all
text has to be handled in a case-insensitive way, except SDP parts.
Related: OS#4001
Change-Id: I51dc1cdcbe2a5587769335fbecb5039ef22cae5d
MGCP RFC3435 (https://tools.ietf.org/html/rfc3435) states almost all
text has to be handled in a case-insensitive way, except SDP parts.
Related: OS#4001
Change-Id: I4da93dfc69b5585a197a7e201a1afb72c2f97030
MGCP RFC3435 (https://tools.ietf.org/html/rfc3435) states almost all
text has to be handled in a case-insensitive way, except SDP parts.
Related: OS#4001
Change-Id: I48252415f9d0cd985ad097f334aa4c1665f52511
MGCP RFC3435 (https://tools.ietf.org/html/rfc3435) states almost all
text has to be handled in a case-insensitive way, except SDP parts.
Related: OS#4001
Change-Id: Ic28a5eacc4c441d68e8a20d2743956ab2e01125d
* Cleanup naming to make it far more clear
* Drop 2 variables holding CID values (allocated_cid, cid), in favour of
1 value holdinf the value and one bool stating whether the value is
used.
* Change conn_osmux_allocate_cid to allow allocating either next
available CID or a specific one, in preparation for forthcoming patches.
This commit can be merged straight away because anyway osmux cannot be
enabled in current status (blocked by vty config) and
(conn_)osmux_allocate_cid was/is not called anywhere. However, it helps
improving code base for future re-introduction of Osmux as it is
envisioned.
Change-Id: I737a248ac6c74add8e917fe2e2f36779d0f1d685
RFC3267 specifies two framing modes for AMR packets. An octet aligned
mode is specified where all fields of the AMR packets are aligned to
octet boundaries. The second framing mode is the bandwith efficient mode
where the fields are directly packed one after another.
- add paring/generation functions for related SDP fmtp parameters
- add conversion function to convert AMR payload
Depends: libosmo-netif I5b5a0fa644d8dbb1f04f9d7e35312683c7b3d196
Change-Id: I622c01874b25f5049d4f59eb8157e0ea3cbe16ba
Related: OS#3807
Add a watchdog timer to connections, and close these connections when
the watchdog timer expires. Kick the watchdog whenever RTP messages or
the relevant MGCP messages arrive. Add the currently remaining timeout
to "show mgcp stats" in the VTY.
This feature is disabled by default, as it is incompatible with LCLS
(connections in LCLS state appear to be inactive). Enable it with the
new "conn-timeout" VTY setting. In general, this feature can be used to
work around interoperability problems causing connections to stay open
forever, and slowly exhausting all available ports. This happened for
various reasons already.
MDCX is the only relevant MGCP message:
- CRCX creates the conn and timer
- DLCX deletes the conn and timer
- MDCX is the only remaining supported MGCP message that indicates a CI
- Can't easily generically parse a CI for all MGCP messages, parsing is
done in handle_modify_con().
Related: OS#3429
Change-Id: I18886052e090466f73829133c24f011806cc1fe0
The Connection Identifier is defined as a hex string, so clients may send the
ID back with or without leading zeros. Ignore all leading zeros when comparing.
A specific SCCPlite MSC is observed to DLCX with Connection Identifier with
leading zeros removed, which would mismatch pefore this patch.
Extend test_conn_id_matching() in mgcp_test.c to include leading zero tests.
Now, mgcp_conn_get() would match a valid id with *any* amount of leading zeros,
even if that far surpasses the permitted conn id length. Valid lengths of
incoming conn ids should be and is checked elsewhere.
Related: OS#3509
Change-Id: If55a64a2da47b6eff035711c08e4114d70dbec91
The Connection Identifier is defined as a hex string, so clients may send the
ID back in lower case. Convert to upper case before comparing.
A specific SCCPlite MSC is observed to DLCX with Connection Identifier in lower
case, which would mismatch pefore this patch.
Add test_conn_id_matching() in mgcp_test.c to verify case insensitivity.
Cosmetic: use strcmp(), not strncmp(). In the presence of a terminating nul as
we can assume here, this makes no functional difference, but it clarifies the
code.
Related: OS#3508
Depends: Ib0ee1206b9f31d7ba25c31f8008119ac55440797 (libosmocore)
Change-Id: I8e52278c3abe9e9c8c848c2b1538bce443f68a43
MGCP_CONN_ID_MAXLEN actually includes a terminating nul, so we need to compare
strlen() against MGCP_CONN_ID_MAXLEN-1.
Log the length if it is too long.
Add MDCX_TOO_LONG_CI test to mgcp_test.c, testing a conn id of 33 characters.
Before this patch, the test returns error code 515 meaning "not found", while
now it returns 510 meaning "invalid", showing the off-by-one. Same is
illustrated by the error log ("not found" before, "too long" now), but the
error log is not verified by mgcp_test.c.
Change-Id: I8d6cc96be252bb486e94f343a8c7cae641ff9429
This function is set on conn ID length of 32 characters. Make it detect a
shorter length also when parsing 'o=-' headers. Before, this failed to
recognize a space as the end of the conn ID, now sees any non-hex char as end.
Related: OS#3507
Change-Id: I762c273bac172acb6d0aae6ea6267603ab654cbf
Flip logic to accurately log whether an 'I:' is included, instead of logging
the opposite.
Note that it isn't possible to log the actual conn ID, because they are random
and differ in every test run, which would collide with the fixed expected
output file mgcp_test.ok.
Change-Id: Idcd731b9daf618b97d8f7e6a776266071cd29e08
Adjust the X-Osmo-IGN parsing to use string tokens instead of parsing single
characters.
Reconsidering the first implementation as a poor choice, rather specify the
format of X-Osmo-IGN as any-length string tokens separated by spaces, which is
more flexible and more future proof.
See also osmo-gsm-manuals If15a88c3b5b40fd1d24ad0f94f3231f678669ab1 which
defines the X-Osmo-IGN format as string tokens, matching this patch.
In mgcp_test, add an unknown X-Osmo-IGN item. Though the output is not checked
by the testsuite.at, running manually shows the error log about the unkown
X-Osmo-IGN item.
Change-Id: Ia6fe5ead4b601931c1bf41b29fc1b237aac37d2c
The format is
CRCX ...
C: ...
M: ...
X-Osmo-IGN: C
So far the only ignorable element is C, i.e. the CallID. Any other items may be
added in the future.
(I initially intended to also add '@' to ignore the endpoint name's domain
part, but in the osmo-mgw code base the domain part is verified long before any
additional headers are even parsed, so sparing that refactoring for now.)
The intention is that osmo-bsc will issue "X-Osmo-IGN: C" for all SCCPlite
calls, because we are unable to retrieve the CallID that the MSC sends to
osmo-mgw for the network side of the endpoint.
Testing with a specific SCCPlite MSC, I actually observe that all CallIDs are
1, even for concurrent calls. So, an alternative hacky solution would have been
to always pass CallID == 1 for SCCPlite connections from osmo-bsc.
Related: I257ad574d8060fef19afce9798bd8a5a7f8c99fe (osmo-bsc)
Change-Id: Id7ae275ffde8ea9389270cfe3db087ee8db00b51
The test function test_multilple_codec() in mgcp_test.c creates a
lot of connections, but it never releases them. Just freeing the
cfg object is not enough since the UDP ports stay open and this
may interfere with other tests that also create connections
(port numbers).
- Make sure all endpoints are released when test_multilple_codec()
is done.
Change-Id: Ic13b4368162149ec36c93f4188fa4c71166e08d5
Since no transcoding is in place osmo-mgw forwards the incoming rtp
packets as they are (there may be minor modifications of the header) from
an ingress connection to an egress connection.
This works without problems as long as both connections use the same
payload type. For IANA defined fixed payload type numbers this is
usually the case, but for dynemic payload type numbers both ends may set
up the same codecs but with different payload type numbers.
When different payload type numbers are set up, and the packet is passed
through without modification, it will have the wrong payload type when
it is sent. The receiving end may then toss the packet since it expects
packets with the payload type it has configured.
The machanism, which is introduced with this patch looks up actual codec
inside the struct data of the ingress connection and then looks for the
matching codec in the struct data of the egress connection. When it
finds the codec there it looks up the payload type of this codec. The
header of the RTP packet is then patched with the correct payoad type.
- Add function mgcp_codec_pt_translate() to look up the payload type
- Add unit-test for function mgcp_codec_pt_translate()
- Add payload type translation to mgcp_network.c
Change-Id: I3a874e59fa07bcc2a67c376cafa197360036f539
Related: OS#2728
Related: OS#3384
In struct mgcp_rtp_end one finds unsigned int counters. Those should
be replaced with libosmocore rate counters
- replace packets_rx, octets_rx, packets_tx, octets_tx and
dropped_packets with libosmocore rate counters.
Change-Id: I47c5c9006df5044e59ddebb895e62adb849d72d5
Related: OS#2517
The codec negotiation via SDP is currently in a neglected state. Also
osmo-mgw does some kind of codec decision wile the SDP is parsed, the
result is information for one codec, even when there are multiple codecs
negotiated. This is problematic because we loose all information about
alternate codecs while we parse. This should be untangled and the
information should be presevered. Also we are not really capable
picking a default. Wehen we do not supply any codec information (not
even LCO), then we should pick a sane default codec.
- separate the codec decision from the sdp parser and concentrate
codec related code in a separate c file
- add support for multiple codecs in one SDP negotiation
- do not initalize "magic" codec defaults during conn allocation
- do not allow invalid payload types, especially not 255. When
someone tries to select an invalid payload type, do not fail
hard, just pick a sane default.
- handle the codec decision in protocol.c, pick a sane default
codec when no (valid) codec has been negotiated (no LCO, no SDP)
Change-Id: If730d022ba6bdb217ad4e20b3fbbd1114dbb4b8f
Closes: OS#2658
Related: OS#3114
Related: OS#2728
At the moment osmo-mgw will accept multiple lco options. (e.g.
p:10, a:PCMU, p:10) If LCO appear multiple times, than the first
appearance of will be parsed and used, all following appearances
will be ignored. However, having multiple appearances of LCO is
illegal and affected requests should be rejected. Also osmo-mgw
should reject illegal formatted LCO strings
- make sure that multiple appearances of LCOs will be rejected
- make sure that illegal formated LCOs are rejected
- add testcases with garbeled LCO and valid LCO examples
Change-Id: Iae2fddfa5f2bcfc952f8ab217b3056694e5f7812
Closes: OS#3119
The alt_codec field is not used anywhere in the code
- remove unused alt_codec field
Change-Id: I5ff2899e3e61f33eb86f284b50ad8a94a949ed16
Related: OS#3114
The two counters: in_stream.err_ts_counter and out_stream.err_ts_counter
are still handcoded. To make them better accessible they should
be replaced with libosmocore rate counters.
- replace state.in_stream.err_ts_counter with libosmocore rate counter
- replace state.out_stream.err_ts_counter with libosmocore rate counter
Change-Id: I9fbd65bf2f4d1e015a05996db4c1f7ff20be2c95
Related: OS#2517
The two counters: in_stream.err_ts_counter and out_stream.err_ts_counter
are still handcoded. To make them better accessible they should
be replaced with libosmocore rate counters.
- replace state.in_stream.err_ts_counter with libosmocore rate counter
- replace state.out_stream.err_ts_counter with libosmocore rate counter
Change-Id: I67aa7a8602f60366ef3ba2c5b1319b1b85719f64
Related: OS#2517
This is an internal library simmilar to 'libmsc' in osmo-msc, which
we don't expect to be used by other programs except osmo-mgw. Hence,
there's no need to install it as a shared library, which introduces
requirements about ABI/API stability and the like.
osmo-bsc_nat uses libosmo-legacy-mgcp, and once we should rewrite
osmo-bsc_nat, we might need some of the libosmo-mgcp related functions,
but at this point it's unclear what exactly would be needed and if
current libosmo-mgcp can provide that. As needed, we can introduce
a related shared library at that point.
Change-Id: Iba0a2c9c694e360356ac2ca584e97795281c6198
The short term of endpoint has always been "endp" througout the whole
project and not "ep".
- rename mcgp_ep.c to mgcp_endp.c
- rename mgcp_ep.h to mgcp_endp.h
Change-Id: Id52047bb2d0407655ac272c858ed3412b8ae9e6d
In order to allow clean prefixes for future endpoint related
functions the "rlease" should be moved to the end of the
function name.
- rename mgcp_release_endp to mgcp_endp_release
Change-Id: I22e938e702f57ad76d38c9f4a1370b110ac1ba11
Some of the line breaks lack the \r character, which leads to an
inconsistancy. While our software and even wireshark does ignore
the problem, other third party implementations might reject those
messages.
- Add the missing \r characters to make the message format
consistant.
Change-Id: I0cd80afae65accd3b4ddc5d82e5d30385879141c
The mgcp protocol in general allows wildcarded endpoints on CRCX.
osmo-mgw does not support this feature yet.
- when the endpoint name contains a wildcard character, search
a free endpoint automatically
- return the resulting endpoint name in the parameter section of
the mgcp response
- add parsing support for the returned endpoint names
- Be more concious about the parameters that are returned with
each response. Do not unnecessarily attach known parameters.
Return the connection ID only on CRCX commands. Only return
the endpoint ID on CRCX commands that are wildcarded.
Change-Id: Iebc95043569191b6f5fbc8fe266b13fcfcab2e48
related: OS#2631
At the moment the MGW has a fixed domain name string that is not even
checked properly.
- Make domain name configurable, use the current "mgw" string as
defualt to maintain compatibility
- Check the domain name with each request. If the endpoint contains
an unexpected domain name, the request must be rejected.
Change-Id: Ia91ac428ba83ac1f9b52a0ec8dbf00ef7876da9e
Previously we
* did not distinguish between the cause of errors in mgcp_header_parse
* common errors were not handled in mgcp_handle_message() but in
individual per-verb functions
Let's centralize the handling of generating error responses and remove
that burden from the individual per-verb handler functions.
Change-Id: I463b27306e10ae3b021583ed102977e7299e5e66
The returncode of get_conn_id_from_response() is unchecked, which
is intentional since some of the test messages will intentionally cause
this function to fail (the response does not inclde a connection
identifier). This means it makes no sense to assert this function
to a fixed value.
In order to spot regressions better print a log message depending
on the return code.
Change-Id: I9bce9ca39b7751b557374b7ab57c6c9005bcdb7d
Fixes: Coverity CID#180534
The CRCX string parameter lacks the quotes and the result of
the function call is not checked against zero. Also the
return code of get_conn_id_from_response() is not asserted.
Fixes: Coverity CID#180534
Change-Id: If4f3ed2c3572da196160569a9705b7a302b700a9
The MGCP connection identifier is allocated by the MGW while processing
the CRCX, see RFC3435 2.1.3.2:. Including/Accepting a connection
identifier in CRCX is "forbidden" as per RFC3435 Section 3.2.2.
So the MGW side must *reject* a CRCX message with 'I' parameter, and
allocate a connection identifier which is subsequently returned in the
response.
Closes: OS#2648
Change-Id: Iab6a6038e7610c62f34e642cd49c93d11151252c
The MGCP spec in RFC3435 is quite clear: Connection Identifiers are
hexadecimal strings of up to 32 characters. We should not print and
parse them as integers on either client or server.
Change the internal uint32_t representation of connection identifiers
to a string representation in the client and also in the server.
Closes: OS#2649
Change-Id: I0531a1b670d00cec50078423a2868207135b2436
The variable number_endpoints is used as a length indicator
for the array that contains the trunk endpoints at all times.
When osmo-mgw is startet up, the variable is set and osmo-mgw
will allocate the memory for the endpoints. However, it is
still possible to manipulate the variable via the telnet
interface. When the value is increased osmo-mgw might start
using unallocated memory at some point.
Store subsequent changes of number_enspoints in a separate
variable in order to write them to the config file. The
changes will then take effect after a restart.
Closes: OS#2632
Change-Id: I3994af016fb96427263edbba05f560743f85fdd4
Ensure that all msgb were cleaned up, then free the overall msgb_ctx, in order
to not leave any memory leaks the sanitizer build complains about.
Change-Id: I53373023a6c3f490d6d6cb1c283db5dfb915882c
Eric Wild
Pau Espin
Neels Hofmeyr
Philipp Maier
Oliver Smith
Harald Welte