Removing the duality of codecs[] and ptmap[] in structs mgcp_msg,
mgcp_response and mgcp_conn_peer has removed the need to "map" from
codec type enum to payload type number. They are stored together now.
Remove functions that are no longer used.
None of our osmocom users of libosmo-mgcp-client call these functions.
Change-Id: I84e5285831397c992af59deee12dea8458d16cc6
codecs[] is an array of enum osmo_mgcp_codecs.
ptmap[] is an array of { enum osmo_mgcp_codecs, unsigned int ptmap }.
MGCP lists first a bunch of payload type numbers and then specifies them
again for details, like the numbers 112, 96, 3 in this example:
m=audio <port> RTP/AVP 112 96 3
a=rtpmap:112 AMR/8000
a=rtpmap:96 VND.3GPP.IUFP/16000
a=rtpmap:3 GSM-FR/8000
So far we keep these lists in two separate arrays:
- codecs[], codecs_len stores the 'm=audio' list
- ptmap[], ptmap_len stores the 'a=rtpmap' list (and may omit some
elements present in codecs[])
This applies to both struct mgcp_response and struct mgcp_msg.
These are semantically identical, and the separation leads to checks,
conversions and dear hopes of correct ordering.
So let's keep only one list with both codec and payload type number in
it. The 'm=audio' list establishes the order of the pt numbers, and the
'a=rtpmap' list adds codec information to the established entries.
In the internal API structs mgcp_msg and mgcp_response, just drop the
codecs[] entirely.
In public API struct mgcp_conn_peer, keep the codecs[] array, mark it
deprecated, and provide a backwards compat conversion: if any caller
invokes mgcp_conn_create() or mgcp_conn_modify() with codecs[] still
present in the verb_info arg, then move codecs[] entries over to the
ptmap[] array in a sensible way.
(BTW, even mgcp_conn_create() and mgcp_conn_modify() are never called
from outside of libosmo-mgcp-client in any of our osmo-cni programs;
users call osmo_mgcpc_ep_ci_add() and osmo_mgcpc_ep_ci_request(), which
in turn may pass user-provided codecs[] lists on to mgcp_conn_create() or
mgcp_conn_modify().)
Tests for parsing the MGCP response are mostly missing. They will be
added in upcoming patch I842ce65a9a70f313570857b7df53727cc572b9e6,
because they will be using only the new ptmap API.
Related: OS#6171
Change-Id: I798e02c6663376d3d52f4a74fc4b32411ce95bed
So far, the users of the old non-pooled API were in charge of allocating
the struct mgcp_client_conf by themselves, then init them using
mgcp_client_conf_init(). This causes a major problem, since it makes it
difficult to extend the struct mgcp_client_conf structure to add new
features, which may happen frequently.
The MGW pool API doesn't have this problem, because the struct
mgcp_client_conf is allocated as parts/fields of private structs defined
in internal headers. Only pointers to it are used in public headers.
Since it still has to internally initialize the conf fields, we still
need the API to initialize it internally, and hence why is it marked as
DEPRECTED_OUTSIDE instead of DEPRECATED.
While some programs already moved to the new MGW pool infrastructure,
they still use the old APIs to accomodate for old config files in order
to be back-compatible, hence most users of libosmo-mgcp-client are
affected.
Introduce an API to allocate the conf struct internally, which, while
still breaking the ABI, allows for a more relaxed update path where it's
possible to extend the struct mgcp_client_conf at the end.
Eventually the non pooled API should be gone and the struct
mgcp_client_conf can then be moved to a private header, but for now
let's add this small feature to avoid major ABI breakage.
Change-Id: Iba0853ed099a32cf1dde78c17e1b34343db41cfc
Unfortunately "-std=c99" is not sufficient to make gcc ignore code that
uses constructs of earlier C standards, which were abandoned in C99.
See https://lwn.net/ml/fedora-devel/Y1kvF35WozzGBpc8@redhat.com/ for
some related discussion.
Change-Id: I98a3c0d5cfda2c4b020652efb4f445f8288342b6
Existing mgcp_client_test code required the '.' to trigger the same code
path, since with this commit we do extra checks and without a dot the
address is not accepted as IPv4 by osmo_ip_str_type().
Change-Id: I936bf57d37f5f0607dfe7fc66c37e424c3793f9b
mgcp_client.h offers functions to generate endpoint names for wildcarded
request. This is used in osmo-bsc, lets now also add a function that can
generate e1-endpoint names.
Change-Id: Iec35b5bae8a7b07ddb3559f7114a24dcd10e8f14
Related: OS#2547
SDP is an optional part of MGCP messages. Do not fail when there is no SDP part.
Practically this is useful to compose simpler MGCP responses from TTCN3 tests.
osmo-mgw itself always includes SDP, so there is no real impact on operating
libosmo-mgcp-client with osmo-mgw from osmo-bsc or osmo-msc.
Change-Id: I608001626459ea72415fb142f857550bbb90c683
Remove public API that makes no sense anymore and is dead code.
I see the dropped API as a dead-end initial misconception of the early mgcp
client, and it doesn't really make sense to drag this stuff along. It has not
been used by osmo-msc,-bsc for a long time now, and just confuses the reader.
It is public API, yes, and older versions of osmo-msc / osmo-bsc will not be
able to compile against this, but even if it did, the resulting MGCP client
would not work with the current osmo-mgw: this API is still based on the
premise that the MGCP client dictates the MGW endpoint numbers, a concept that
cannot be used with the current osmo-mgw. Instead, osmo-mgw expects a
wildcarded endpoint upon CRCX and assigns its own endpoint names.
Also, the bts-base configuration is unused and a legacy of when osmo-bsc_mgcp
had explicit BTS and CN sides.
Change-Id: I98a9f1f17a1c4ab20cea3b08c7d21663592134d6
Add a full length (32 characters according to spec) conn ID in a CRCX response,
as well as a too long one.
The too long one is currently silently truncated, a subsequent patch will
improve on that (If2a1aab1f13e771a6705c430e3c75bd42477a23b).
Change-Id: I5f2d52f086ea2d330fcce88a176488ace972bf79
The separator between MGCP and SDP section is typically "\r\n\r\n". For some
reason the test so far used "\n\n" instead, rather use the standard separator.
Change-Id: I41c73722e5fae00663bcf96de0b57b7155809a06
I want to test arbitrary length Conn IDs ('I:'), and hence don't want to pass
the conn_id as int, but rather just include it in the message string. Prepare
for that by eliminating the extra conn_id arg and just pass a params string.
Change-Id: Ib2e718dda3aa1f6e9979dee823d973dd002e2318
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 current implementation does not support any way to influence the
codec that is negotiated via SDP or LCO. The client statically
negotitates AMR on an invalid payload type number. Also we ignore
any codec information in the responses.
- Add struct members to allow setting of user defined codec information.
- Add struct members to retrieve parsed codec info from responses.
- Add code to generate codec information in SDP
- Add code to parse SDP codec info in MGCP responses
Change-Id: I78e72d41b73acfcb40599a0ff4823f17c3642059
Related: OS#2728
Related: OS#3334
After call to mgcp_find_section_end(), actually check the proper variable to
evaluate its return value.
Show in mgcp_client_test output that the parsing errors are fixed, and enable
the assertion that no tests should fail.
Change-Id: I62a2453cd9e2e7d5408423161fa65ec9c9989f98
To show how the current code fails, add test_sdp_section_start() to
mgcp_client_test.c, and temporarily accept failing output. This will be fixed
in change I62a2453cd9e2e7d5408423161fa65ec9c9989f98.
Change-Id: I5c6d3566b8f6dbf04c0cd8b127423f5295c19f8d
The test that tests the cancelation of a pending mgcp message
uses an integer as connection identifier, which leads to a
segfault since connection identifiers are represented as strings.
Use a string as connection identifier.
Change-Id: I395a23c1828cf216031d69d481ad35dd458ee7d4
So far, if an MGCP message is sent, the transaction gets enqueued, but there is
no way to end the transaction other than receiving a valid reply. So, if the
caller decides that the transaction timed out and tears down the priv pointer
passed to mgcp_client_tx, and if then a late reply arrives, the callback will
dereference the invalid priv pointer and cause a segfault. Hence it is possible
to crash an mgcp_client program by sending a late response.
Furthermore, if no reply ever arrives, we would keep the pending response in
the list forever, amounting to a "memory leak".
Add mgcp_client_cancel() to discard a pending transaction. The caller can now
decide to discard a pending response when it sees fit (e.g. the caller's
timeout expired). This needs to be added to OsmoMSC and OsmoBSC.
Add mgcp_msg_trans_id() to provide an obvious way to obtain the transaction id
from a generated MGCP message.
No public API is broken; but refine the negative return code from
mgcp_client_rx(): return -ENOENT if no such transaction ID is found, and still
-1 if decoding failed. This is mainly for mgcp_client_test.
Implement a test for mgcp_client_cancel() in mgcp_client_test.c.
Tweak internal mgcp_client_response_pending_get() to take only the transaction
id as argument instead of the entire mgcp message struct.
Found-by: dexter
Related: OS#2695 OS#2696
Change-Id: I16811e168a46a82a05943252a737b3434143f4bd
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
currently the only way to generate MGCP messages is to use
mgcp_msg_crcx(), mgcp_msg_mdcx() and mgcp_msg_dlcx(). All
three function take a fixed set of parameters via their
parameter list. There is no way to add or leave away optional
parameters.
add function mgcp_msg_gen(), this function takes a unified
message struct. The struct features a presence bitmask which
allows to enable and disable parameters as needed. It is also
possible to add new parameters in the future without breaking
the API.
Depends: libosmocore I15e1af68616309555d0ed9ac5da027c9833d42e3
Change-Id: I29c5e2fb972896faeb771ba040f015592487fcbe
When testing the file name and the line numbers are output to
stderr, this causes the test to fail when change moves the
lines.
Disable line numbers in the stderror log when testing, also
disable timestamps and colors. Make sure the log category
is print.
Change-Id: I7f1bd9454188f0ca869dada1fcc2877b789cc0ac
Add mgcp_common.h to libosmo-mgcp-client, to not need to share a header file
with libosmo-legacy-mgcp (nor the upcoming libosmo-mgcp). Both libraries use
the enum mgcp_connection_mode (and a for-loop macro and a string mangling
function), so far declared in mgcp.h, and both mgcp-dev and mgcp-client-dev
debian packages would require this header file to be installed. So far the
mgcp-client-dev lacks this header file, which causes the osmo-msc debian
package to fail building. Ways to solve:
- If both -dev debian packages installed the same header file in the same
place, they would conflict if ever installed at the same time.
- mgcp-client-dev can depend on mgcp-dev, but it seems bad to keep such a large
dependency for just one enum and two helpers.
- Instead, this patch solves this by copying the few definitions to
libosmo-mgcp-client.
Once libosmo-mgcp-client has its own copy of those definitions, it is
fully self-contained and depending builds (osmo-msc deb) will succeed.
Copy the few actually common definitions to new header
<osmocom/mgcp_client/mgcp_common.h>. The nature of this .h is that it may be
shared with future libosmo-mgcp without causing linking problems.
Remove libosmo-legacy-mgcp/mgcp_common.c file from the build of
libosmo-mgcp-client, no longer needed.
Add to new mgcp_common.h:
- enum mgcp_connection_mode;
- for_each_non_empty_line() macro.
- mgcp_msg_terminate_nul() as static function. Its complexity is just above
your average static inline, but being inline is a way to use it in both mgcp
and mgcp_client without linking problems.
Replace for_each_line() use in mgcp_client with for_each_non_empty_line()
(for_each_non_empty_line() replaces for_each_line() and uses strtok_r() instead
of a local reinvention).
mgcp_connection_mode_strs are actually only used in libosmo-mgcp-client, so
rename to mgcp_client_ prefix and move to mgcp_client.c.
BTW, the future plan for upcoming libosmo-mgcp is to use the identical header
file, and keep only one copy in the git source tree. The second copy may be
generated during 'make', to avoid code dup while having two distinct headers.
Related: I8e3359bedf973077c0a038aa04f5371a00c48fa0 (fix osmo-msc after this),
I7a5d3b9a2eb90be7e34b95efa529429f2e6c3ed8 (mgcp_common.h)
Change-Id: Ifb8f3fc2b399662a9dbba174e942352a1a21df3f
The name "mgcpgw_client" referred to an MGCP gateway, which is rather an MGW
(Media Gateway). But this client code is more generally a client for the MGCP
protocol, independently from what the server program is called.
Rename the files as well as the function prefixes to drop the "gw". It is
purely cosmetic and not strictly necessary, but a good point in time for fixes
like this.
osmo-msc build will be adjusted by I093ad02ca0e532f659447c785e09678b3e6f220d.
osmo-bsc build will be adjusted by I6402c7cbe58dacae7630f7f03819f8102e54c699.
These should be applied right after this here is merged to avoid fallout.
Change-Id: I99f7faab637cfcc22ece64a1dbcbe590f2042187
Neels Hofmeyr
Pau Espin
Harald Welte
Philipp Maier
Neels Hofmeyr