Upcoming patches introduce copying notify information. Prepare by combining
notify info into a separate sub-struct.
Change-Id: I47c0dd112011b8cb4dc88e8efd010466d4ba6308
If an MGCP operation on one conn of an endpoint fails, no longer carry out
other pending requests for that endpoint. Only allow pending DLCX to be sent.
If the caller schedules two CRCX at the same time, the first CRCX is sent with
a wildcarded endpoint name like "rtpbridge/*@mgw". Only when the OK for that
returns an allocated endpoint, will the second CRCX be sent, using that actual
allocated endpoint name. But, if the first CRCX fails, then we should not send
another wildcard CRCX, but rather assume both as failed.
Since a failed MGCP message means that the endpoint becomes unusable /
undefined and typically deallocates directly, we can actually discard all other
pending requests except for DLCX.
Change-Id: Icb1d485224bb486b84eff6329f0bd95932e63246
During FSM instance cleanup, a DLCX message composition may fail if a preceding
received MGCP message was missing parameters. If that occurs, don't crash, just
log an error and deallocate.
Change-Id: Ic1c3c4deeb4703b60e870af9d5d7be216a87fff8
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
If a ptmap appears in the SDP, always store it in the ptmap array. No longer
attempt to drop entries if they match the conventional payload type number.
- One reason is that the past code only matched full explicit "FOO/8000/1"
strings, while the channel number "/1" can be omitted to imply 1; by simply
storing everything received in the SDP, there is no need to add complexity
to match both "FOO/8000" and "FOO/8000/1".
- The other reason is to rather parse exactly what was received, instead of
filtering entries, to take away a degree of implied magic.
Change-Id: I2a69c21e68c602daf804744212d335ab1eafd81b
- move the error logging up to the actual errors. Each appear only once, no
goto labels needed.
- instead of strstr("rtpmap"), use osmo_str_startswith("a=rtpmap:") to more
concisely trigger on the actual syntax of the audio parameters. Same for
"a=ptime:".
Change-Id: I730111e245da8485c1b5e8811f75d140e379cec6
There are allocated bits in conn->end.codecs[], free them.
This is not fixing a memleak, since mgcp_rtp_conn_cleanup() is currently only
called from mgcp_conn_free(), which soon after frees the conn; the conn serves
as talloc parent for the codec strings freed in this patch.
The rationale: it is better style to explicitly free them, to also guard
against future callers of mgcp_rtp_conn_cleanup() which might expect complete
cleanup.
Change-Id: Ic471107ce6e94d9ce582d887429c744ff93e3053
In codec_set(), for each 'goto error', log the specific error cause.
Also add a TODO and a FIXME comment about inventing dynamic payload type
numbers.
Change-Id: I0b44b574c814882b6f8ae7cd738a6f481cd721fd
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
The audio_name and subtype_name are allocated from talloc, so they need to be
freed before resetting the codec array. Use mgcp_codec_free() to ensure this.
Change-Id: I07f207dcb7ce66bbf3445a30af41e696677b384f
Both are used only in the same .c file, so make them static.
Move codec_set() guts into codec_add(): codec_set is only called by codec_add.
If codec_set were left separate, it'd look like the codec_init() is a bug and
lacks a codec_free() first. When looking at the entire context in codec_add(),
it becomes obvious that codec_init() should be called, not codec_free(),
because it is populating a previously unused entry.
Preparation to fix a memleak in a conn's codec list.
Change-Id: I120cab0a352a1e7b31c8f9c720c47b2c291311d7
If mgcp_send() runs a transcoder loop, break the loop if rfc5993_hr_convert()
or amr_oa_bwe_convert() return with error. Possibly fixes an infinite loop
situation for erratic packets? (Didn't check for that in detail.)
Change-Id: Iba115a0b1d74e7cefba5dcdd777e98ddea9eba8c
Remove various OSMO_ASSERT() on size of incoming packets. Doing an assert on
incoming data is a DoS attack vector, absolute no-go. Instead, return -EINVAL
and keep running.
Change some return values to be able to distinguish successful operation from
invalid RTP sizes. In rtp_data_net(), make sure to return negative if the RTP
packet was invalid.
Some of the error return codes implemented here will only be used in upcoming
patch Iba115a0b1d74e7cefba5dcdd777e98ddea9eba8c.
Change-Id: I6bc6ee950ce07bcc2c585c30fad02b81153bdde2
The name 'cmp' implies a return value of -1, 0, 1 to indicate smaller, match or
larger. Since this function returns bool, it should not be named with 'cmp'.
Change-Id: I2d41b1a32300e295551e85d3f9ab82dd2b0e86b8
Current code uses APIs like osmux_xfrm_output_init2() and osmo_amr_*(),
that are only available in libosmo-netif 0.6.0 onwards. Let's update
configure.ac accordingly.
Change-Id: I3bc0196bb6b5c5e5cf96935422fd56c4582ed7f5
Since March 15th 2017, libosmocore API logging_vty_add_cmds() had its
parameter removed (c65c5b4ea075ef6cef11fff9442ae0b15c1d6af7). However,
definition in C file doesn't contain "(void)", which means number of
parameters is undefined and thus compiler doesn't complain. Let's remove
parameters from all callers before enforcing "(void)" on it.
API osmo_stats_vty_add_cmds never had a param list but has seem problem
(no "void"), so some users decided to pass a parameter to it.
Change-Id: Icf4d18969488c9eacca7a597d4071828e649e772
Related: OS#4138
Return variable specified by strtoul() is "unsigned long int". If
"unsigned int" is used, according to Coverity the return value can never
be ULONG_MAX:
CID 202173: Integer handling issues (CONSTANT_EXPRESSION_RESULT)
"pt == 18446744073709551615UL /* 9223372036854775807L * 2UL + 1UL */" is always false regardless of the values of its operands. This occurs as the logical second operand of "&&".
Furthermore, PT is 7 bit in RTP header [1], so let's avoid accepting
incorrect values.
[1] https://tools.ietf.org/html/rfc3550#section-5
Fixes: c5c1430a1c ("Catch unsigned integer MGCP parsing errors with strtoul")
Fixes: Coverity CID#202172
FIxes: Coverity CID#202173
Change-Id: Ice9eee6a252fab73dbab5ebf3cfc83c1b354fd08
There is unfortunately no way to suppres this witha pragma,
and gcc 9 uncovers quite a few new instaces with enabled LTO that can't/won't be fixed
Related: OS#4123
Change-Id: Ib157bd2e110b271dbe5ac928c98251e016477f56
Mark osmocom/mgcp_client/mgcp_common.h as nodist, so "make dist" will
not try to include it in the source tarball. This caused "make dist" to
fail in a clean osmo-mgw source tree with:
make[2]: *** No rule to make target 'osmocom/mgcp_client/mgcp_common.h', needed by 'distdir'. Stop.
The file gets copied during make from osmocom/mgcp/mgcp_common.h (see
include/osmocom/mgcp_client/Makefile.am). Therefore it is not included
in the source tree and we don't need to distribute it.
Related: OS#4084
Change-Id: Ia1d7b051c0924a785b0f7ec0195192e3a852ed70
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: I637cb20f0af4de33ebf6589b1aff260d57d03e7b
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: Ifc1b3bfe6ff6922df478cea89bbbb291b5fa5706
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: I7d1e55faddafa3c3093d38513d4a434ecf5ea5bd
Otherwise it would not catch a duplicate if first the param is
introduced in upper case and later in lower case, or the other way
around.
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: I254bfa3a2d2562441ca3a576cc8e1e7967d9c495
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: I4f7b07b77c2946e9cd6f0eeca00011bd905126dd
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
It can be used together with LCLS, just make sure to also enable
keep-alive packets.
In OS#3429 it was pointed out, that during LCLS the media path remains
active but is not used. Without any traffic flowing, this looks like a
timed out connection and so it will be killed if conn-timeout is set.
However, OsmoBSC and OsmoMSC have an option to enable RTP keep-alive
packets (through libosmo-mgcp, originally intended to keep connections
behind NAT open). If that option is enabled, the keep-alive packets
should also prevent the conn-timeout.
Related: OS#3783
Change-Id: Ib4d19104d558a26a444a80fb36f4b7b33bc5cc59
VTY command to disable conn-timeout again, after it has been enabled.
conn-timeout was introduced in [1].
[1] Change-Id I18886052e090466f73829133c24f011806cc1fe0.
Change-Id: I7dee7dafaaf4bb93fd692ea06b52b9e012beac6d
Move code in RTP specific path to generic dispatch_rtp_cb. This way
loopback logic is applied both for Osmux and RTP connections.
Change-Id: Ia30f5a14f150e4d151eac4d1046ea834f1685a5f
Neels Hofmeyr
Pau Espin
Harald Welte
Oliver Smith
Daniel Willmann