Commit Graph

7007 Commits

Author SHA1 Message Date
Pau Espin Pedrol
b5a4f45dbf mgw: mgcp_protocol: assert freeing last conn allows creating new conn
Change-Id: Ib8c9089a7f04bc67e5bdfb1148cfc603f0e17bb7
2025-01-07 11:47:56 +01:00
Pau Espin Pedrol
e6bafbf3e3 mgcp_endp: Add helpers accessing endp connections
This has several benefits:
* easily improving/changing the impelemntation later on (eg. by storing
  a count instead of iterating the whole list).
* Remove code complexity from mgcp_protocol.c.

Change-Id: I77c298abf0a1488b91f58c9e76507ef8add0168e
2025-01-07 11:47:56 +01:00
Pau Espin Pedrol
5c7b128166 mgw: constify mgcp_endp_avail() param
Change-Id: I31ed2f3abbcf024ee44f62e130dd4ec01611ae97
2025-01-07 11:47:39 +01:00
Pau Espin Pedrol
5469edc278 cosmetic: mgw: iuup: Update comment
Change-Id: I6e1448bf7e3bb7454754147e2b2e01844c157500
2024-12-23 13:36:31 +01:00
Pau Espin Pedrol
66952183a9 mgw: Avoid 2nd lookup of conn in endp during CRCX
There's no need to lookup (iterate over conns) for rtp_conn in endp, we
have it accessible in constant time.

While at it, simplify some accesses to the pointer after it.

Change-Id: I8316e76a1789d4a8eaa1eb89bdaf86e25dc0a1e8
2024-12-23 13:35:21 +01:00
Pau Espin Pedrol
cc97f58d96 mgw: Clean up access to conn_rtp from conn
Add a new mgcp_conn_get_conn_rtp() and use it everywhere instead of
accessing deep structure fields.

Change-Id: Iee2c19598e9570ea3b1ceba3cdfd2a5f5be2c954
2024-12-23 11:49:21 +01:00
Pau Espin Pedrol
5d6931881a mgw: mgcp_network.c: Simplify use of conn_rtp ptr
Change-Id: I5bab15fc793434173660769a8e60dae4ae4aa4c6
2024-12-23 11:49:21 +01:00
Pau Espin Pedrol
0f4be7699b mgw: Rename and move several get_conn funcs acting on endp object
These functions act on the endp object, hence prefix them with
mgcp_endp_* and move them to mgcp_endp.{c,h}.

Change-Id: Ifff01331db68998e9e23f99d8836d96022d550c2
2024-12-23 11:49:21 +01:00
Pau Espin Pedrol
546983cdbc mgw: Rename and move code freeing endp connection
All the code to free conns in an endpoint is really entanged, which also
makes it inefficient in several places because the list of conns is
iterated twice, or even ~N*2.

* Move code freeing the conn object to its own mgcp_conn_free()
  function.
* Rename functions acting on endp to be mgcp_endp_* prefix, and move
  them to mgcp_endp.{c,h}.
* Remove old mgcp_conn_free(endp, id) since it's not really needed
  anymore.

Change-Id: I9a87a07699dd8aa7856d5036e9b95a4ddf699a15
2024-12-23 11:49:21 +01:00
Pau Espin Pedrol
c04de4e7bb mgcp-client: Fix regression checking null ptr
A recent patch introduced a bug checking incorrectly for null ptr.

Related: Coverity CID#445417
Related: Coverity CID#445418
Fixes: e6ef4e7484
Change-Id: I1a458a34376851aec73c14a658cccd6132a5eb6c
2024-12-23 11:38:33 +01:00
Pau Espin Pedrol
d46cdd80de cosmetic: mgw: Fix indentation whitespace
Change-Id: Ic5b267108b5c5cba064ddddbe688705f9dc29ac8
2024-12-19 18:16:38 +01:00
Pau Espin Pedrol
6676e63c89 mgw: Drop own MGCP extension 'noanswer'
This was added in e3d16bb775 14 years ago:
"""
[mgcp] Protocol extension to not generate answers.

    For the NAT we want to send requests in a send and forget
    way and we are not interested in seeing the answers, so tell
    the gateway to not answer them.
"""

So this code is most probably related to osmo-bsc_mgcp time, and we
don't really need this feature in osmo-mgw, it really makes no sense.
Drop it.

Change-Id: Ica2d3d54cda41e0c6416913ab056843a0cd80c17
2024-12-19 18:14:19 +01:00
Pau Espin Pedrol
1f99c3aaae mgcp-cli: Fix filling in wrong local IP address of SDP Origin o=
If user (VTY) configured the local IP address to use for MGCP, and that
IP address was not the one selected by kernel routing table lookup, the
IP address filled in the MGCP message would be wrong, not matching the
one sending the MGCP message.

Change-Id: I35624db853dc1f0fee85503105960613f70473c6
2024-12-19 16:59:47 +01:00
Pau Espin Pedrol
e6ef4e7484 mgcp-cli: Improve error handling around mgcp_msg_gen() return
Change-Id: Ib9b02b7e6b37472c8e38b3380bc990a2bbac0657
2024-12-19 16:41:05 +01:00
Pau Espin Pedrol
b0fa041a28 mgcp-cli: Mark iofd ptr as NULL when freed
Change-Id: Ie3cd7b49c9cd5514f81289ac5ec1ccee14c8509c
2024-12-19 16:28:22 +01:00
Pau Espin Pedrol
e17bf77bdd mgcp_client_internal.h: Add missing header dependency
struct mgcp_client defined here has a field "struct mgcp_client_conf
actual", which is defined in osmocom/mgcp_client/mgcp_client.h.

Change-Id: I61a67c371dbcfab073fa75ba08a3cac0f46e0ac0
2024-12-19 16:16:41 +01:00
Pau Espin Pedrol
802b1fad61 mgcp-client: Fix wrong value passed to strerror()
errno is positive, hence we need to change the value of "res" being
passed to strerror().
While at it, use thread-safe variant strerror_r().

Change-Id: Ibc278199bc8f66e5521bff72bad150f44716f3eb
2024-12-12 14:05:03 +01:00
Pau Espin Pedrol
8a8973eec2 jenkins.sh: Use --disable-doxygen configure param
Change-Id: I0f7d2b212834a3dea356de45fadb81a7e7176191
2024-12-10 17:04:42 +01:00
Pau Espin Pedrol
17e7ea66e4 jenkins.sh: libosmo-netif no longer depends on libosmo-abis
Change-Id: I64c4d46eae0bc131a7b4133cb79367837cb91c3b
Depends: libosmo-abis.git Change-Id I079dc3999de508301dd37ed03e399356a58d3cab
Depends: libosmo-netif.git Change-Id I13d6e88158f6d9ce017986283183ee9c2cc68cae
2024-11-21 14:51:56 +01:00
Oliver Smith
9c3fad2c42 Bump version: 1.13.0.3-227a → 1.13.1
Change-Id: I8ebbaf6bff440cb233088f4bc226b80db0fd3922
1.13.1
2024-09-12 13:54:42 +02:00
Pau Espin Pedrol
227a800d26 tests/mgcp/mgcp_test: Add some extra asserts in code
This makes it easier to catch possible failures while running the test,
plus makes gcc on OpenSUSE_Factory happy and not warn.

Related: OS#5044
Change-Id: I01c3c84e58a11055f0e0a6955016dc2489c73379
2024-08-20 12:02:02 +02:00
Philipp Maier
24cae67f3e mgcp_network: add missing ntohs
We must use ntohs before passing the RTCP port to osmo_sockaddr_port

Related: OS#6527
Change-Id: I9773c7b1fda8c9b429723ef9f0db0b58894b28fe
2024-08-07 09:48:45 +02:00
Philipp Maier
0d47c408ac mgcp_network: use an uint16_t to store the port number
The comment in struct mgcp_rtp_end says that the RTCP port number has to
be stored in network byte order. Such a port number is 16 bit long, so
let's use an uint16_t instead of an int here.

Related: OS#6527
Change-Id: I392b07b243389a78d4ad1d784cdfdc28ec59b487
2024-08-07 09:47:42 +02:00
Oliver Smith
f1b557988d Bump version: 1.12.1.58-95e50-dirty → 1.13.0
Change-Id: I46cabfda147bf700fb29a0b8eecf54560a1d378d
1.13.0
2024-07-24 16:23:15 +02:00
Mychaela N. Falconia
95e504aa97 fix E1 TS output when used with osmo-e1d
The original code did E1 raw TS output by posting directly to
&ts->raw.tx_queue instead of calling e1inp_ts_send_raw();
doing so bypasses the call to driver->want_write performed in
e1inp layer.  This approach worked for DAHDI where no
select-for-write is used; however, e1inp interface to osmo-e1d
does use select-for-write, hence applications like osmo-mgw
do need to use e1inp_ts_send_raw() API in order to work
correctly.

Change-Id: I6ce9a1dea6834632faf75059e85ca9a0c25d57c2
2024-07-23 14:24:18 +00:00
Neels Hofmeyr
a5ae091fdd do not FAIL on CRCX in sendrecv mode
Currently, a CRCX in sendrecv mode results in:

  DLMGCP ERROR endpoint:rtpbridge/2@mgw CI:7F4C8EDD CRCX: selected connection mode type requires an opposite end! (mgcp_protocol.c:1090)

But it is not actually practical, nor logical to require another conn
for the 'sendrecv' ConnectionMode.

Impractical: If I want to create two conns on an endpoint that both are
in 'sendrecv' mode, I have to send two CRCX, one for each conn. At the
time of the first CRCX, there cannot be any other conn, so I am
currently forced to send a different ConnectionMode in the CRCX,
followed by another MDCX later, just to change the first conn to
sendrecv.

Illogical: In a situation where two conns are currently in sendrecv
mode, if I now DLCX one of them, I can legally reach a state with a
single conn in sendrecv mode, just as currently forbidden for CRCX.

In general, MGCP is not forcing any particular number of connections.

Simply start forwarding RTP as soon as there is a remote conn, and not
require another explicit MDCX.

Related: SYS#6974 SYS#6907
Related: osmo-ttcn3-hacks I00fd854f058f7f53e2f579e8481ca2b9253f08e3
Change-Id: Ic089485543c5c97a35c7ae24fe0f622bf57d1976
2024-07-02 01:54:45 +00:00
Neels Hofmeyr
249f799a88 mgcp_test: add CRCX for IUFP in sendrecv
Note, there are also ttcn3 tests sending the same to osmo-mgw being
submitted along with these patches, and the ttcn3 tests currently fail:
osmo-mgw rejects the connection mode "sendrecv" in the first CRCX.

Change-Id: Icdb1085b8e44ac4cff6c457163349b81e81bf765
2024-07-02 01:54:45 +00:00
Neels Hofmeyr
dc395843ff mgcp_test: test a=ptime:20, not 40
In our GSM world, the packet time is 20 ms, hence 20 is the typical
value that we should test. Maybe we can do one test with 40 at most, but
we'll most probably never see anything else than 20 in practice.

Change-Id: Ia2292198bf6e3b72912afd69607654ca77fd549d
2024-07-02 01:54:45 +00:00
Neels Hofmeyr
c5d003e09e mgcp_test.c: fix various missing '\r' and '\n'
The only valid line endings are '\n' and '\r\n'.
We usually use '\r\n' like we were in MSDOS.

MGCP, RFC3435 3.1:

   Headers and session descriptions are encoded as a set of text lines,
   separated by a carriage return and line feed character (or,
   optionally, a single line-feed character).

SDP, RFC8866 5:

   The sequence CRLF (0x0d0a) is used to end a line,
   although parsers SHOULD be tolerant and also accept lines terminated
   with a single newline character.

There should probably be tests for '\n' line endings, but mixing them in
the same MGCP message is ridiculous.

Change-Id: I6d530535a3a5f1d1a0716ab9e4a8079ba1de242e
2024-07-02 01:54:45 +00:00
Mychaela N. Falconia
e6398d886b E1: support HRv1 codec on both 16k and 8k subslots
HRv1 support in OsmoMGW-E1 was previously broken (couldn't work
on either 16k or 8k subslots) because of inconsistency: the TRAU
frame type was set to OSMO_TRAU16_FT_HR, but the TRAU sync pattern
was set to OSMO_TRAU_SYNCP_8_HR.  However, now that libosmotrau
has proper support for HRv1 TRAU frame encoding and RTP conversion
in both 16k and 8k formats, drive it correctly in OsmoMGW-E1.

While at it, change the code structure to avoid else-after-return.
Was the original code written and merged in a time before strict
linter checks?

Change-Id: Ifadbdc68905178c6ffdd673a6fb71c18610c9847
2024-06-26 17:45:49 +00:00
Neels Hofmeyr
23140d6f94 mgcp_test.c: verify osmo-mgw accepts m=audio 0
When osmo-hnbgw does not yet know the remote port, it wants to send a
CRCX to set up IuUP at the MGW, with the audio port set to zero.
Make sure osmo-mgw accepts port == 0.

Related: SYS#6907 SYS#6974
Change-Id: I42011c2e7256d372a37b6a2fe86af0153038e2d0
2024-06-14 16:14:41 +02:00
Neels Hofmeyr
36c823eef1 mgcp-client: always send 'm=audio' line
Re-add the m=audio line to SDP emitted from libosmo-mgcp-client, even if
the audio port is not set yet

Patch a5acaa68db introduced a presence
flag for the RTP audio port number. This flag, when unset, also omitted
the 'm=audio...' line completely, dropping the PT number definitions.

Correct:

  m=audio 1234 RTP/AVP 96             <--- anounce 96
  a=rtpmap:96 VND.3GPP.IUFP/16000     <--- further specify 96
  a=fmtp:96 ...                       <--- further specify 96

When m=audio is missing, we only have orphaned rtpmap and fmtp entries.
They are supposed to further specify the 96 listed in 'RTP/AVP 96',
instead they are without context:

  a=rtpmap:96 VND.3GPP.IUFP/16000
  a=fmtp:96 ...

When the presence map indicates no port known, we still need to emit the
list of PT numbers; so we're forced to send a port of 0:

  m=audio 0 RTP/AVP 96
  a=rtpmap:96 VND.3GPP.IUFP/16000
  a=fmtp:96 ...

This is an important fix for osmo-hnbgw, which sends the first CRCX with
an IUFP codec, at a time when the remote port is not yet known. osmo-mgw
requires an m=audio line to accept incoming IuUP Initializaition
requests. When m=audio is missing, osmo-mgw does not parse the IUFP
codec, and 3G voice fails completely. This mgcp-client patch will emit
valid codec config also when port == 0, fixing osmo-hnbgw voice, because
osmo-mgw will know about IUFP from the start.

Related: SYS#6907 SYS#6974
Related: osmo-mgw a5acaa68db
Change-Id: Id95b629453aec999100b5af821c6a0b9562bb597
2024-06-14 16:14:25 +02:00
Oliver Smith
fa393fa1e9 doc: example configs: fix deprecation warnings
Fix for:
  range must end at an odd port number, autocorrecting port (16000) to: 16001
  % Deprecated 'sdp audio-payload number <0-255>' config no longer has any effect
  % Deprecated 'sdp audio-payload name NAME' config no longer has any effect
  % Deprecated 'loop (0|1)' config no longer has any effect

Change-Id: I62a4fd119de48039c3c450d5323d8f9b7de8120f
2024-05-15 11:58:52 +00:00
Oliver Smith
e0fc37a437 contrib/systemd: run as osmocom user
I have verified that with AmbientCapabilities=CAP_SYS_NICE, setting
scheduling policy as described in the manual still works as expected.

Related: OS#4107
Change-Id: Ibb83c231231b39dc6732c0f375aeb3b21f3938ef
2024-05-14 15:25:21 +02:00
Oliver Smith
4aa2b0e35b contrib: remove rpm spec file
Related: https://osmocom.org/news/255
Related: OS#6446
Change-Id: I703e115a426ac1012c80d2e1576ee6dcfbe191a5
2024-05-08 14:41:07 +02:00
Pau Espin Pedrol
e01f6e7f33 iuup: Increment RTP hdr seqnr even if Tx over UDP fails
This way holes can be detected. In practice it's not much important
since it would be really strange that UDP fails for a while and then it
starts working out of the blue...

Related: SYS#6907
Change-Id: I8095f3505c859650c0b83abce405067bef745975
2024-04-24 18:57:02 +02:00
Pau Espin Pedrol
388b48d14e Fix IuUP RTP hdr seqnr field not incremented
Previous commit add osmo_io changed mgcp_udp_send() implementation from
"return sendto()", which is documented as:
"return the number of bytes sent. Otherwise, -1 shall be returned"

to "return mgcp_udp_send_msg()", which in turn calls
"return osmo_iofd_sendto_msgb()", and which is documented as:
"\returns 0 in case of success (takes msgb ownership), -1 on error
(doesn't take msgb ownership)."

So successful return code changed from >0 (bytes sent) to ==0,
but forgot to update mgcp_send_iuup() return code path check (and also
some related function documentation calling mgcp_udp_send()".

This commit fixes all the related aspects of that return code change.

Related: SYS#6907
Fixes: 352b967d1b
Change-Id: I154e1e41cd02fd4d9b88ad98fc7c4d657246c589
2024-04-24 18:56:57 +02:00
Harald Welte
352b967d1b Convert RTP/RTCP/OSMUX I/O from osmo_fd to osmo_io
Converting from osmo_fd to osmo_io allows us to switch to the new
io_uring backend and benefit from related performance benefits.

In a benchmark running 200 concurrent bi-directional voice calls with
GSM-EFR codec, I am observing:

* the code before this patch uses 40..42% of a single core on a
  Ryzen 5950X at 200 calls (=> 200 endpoints with each two connections)

* no increase in CPU utilization before/after this patch, i.e. the
  osmo_io overhead for the osmo_fd backend is insignificant compared
  to the direct osmo_fd mode before

* an almost exactly 50% reduction of CPU utilization when running the
  same osmo-mgw build with LIBOSMO_IO_BACKEND=IO_URING - top shows
  19..21% for the same workload instead of 40..42% with the OSMO_FD
  default backend.

* An increase of about 4 Megabytes in both RSS and VIRT size when
  enabling the OSMO_IO backend.  This is likely the memory-mapped rings.

No memory leakage is observed when using either of the backends.

Change-Id: I8471960d5d8088a70cf105f2f40dfa5d5458169a
2024-04-02 19:40:03 +02:00
Harald Welte
ed4da25f2f cosmetic: make linter happy
Change-Id: Iec8404061588b848f9e597bf4112d6df9597de95
2024-03-20 14:25:49 +01:00
Harald Welte
5abda312ed Change msgb ownership in processing of received msgb
The old approach was: rtp_data_net() reads a msgb from the incomging
socket, calls through whatever function chain and in the end free's it.
So none of the intermediate functions was permitted to take msgb
ownership.

This was a good choice as all processing would happen synchronously,
up to the point where that msgb was written on the output RTP socket.

Let's change this from passing msgb ownership throug the whole call
chain, through rx_rtp() to the various *_dispatch_rtp() functions.

This is required for upcoming migration to osmo_io, as in that case the
write (sendto) calls are asynchronous and hence msgb ownership needs
to be transferred.

Change-Id: I6a331f3c6b2eb51ea312ac6ef8c357185ddb79cf
2024-03-20 14:25:49 +01:00
Harald Welte
75862d3131 remove osmo_fd from mgcp_create_bind()
preparation for osmo_io

Change-Id: I4a3b66a14fdfbc867daca0f0a05f694d5e0d7b66
2024-03-20 14:25:49 +01:00
Harald Welte
8733542b13 simplify unused transcoding/processing call-back
the processing call-back is working with a raw buffer + length,
while we actually work with struct msgb.  Let's simply pass the msgb
into the call-back, and the call-back can then do what they want with
the contents of that msgb.

Change-Id: I002624f9008726e3d754d48aa2282c38e3b42953
2024-03-20 14:25:43 +01:00
Harald Welte
d7aac20cc1 remove strange loop for non-existant transcoding support
The existing support preparing the mgw for transcoding (which doesn't exist)
has some kind of method where the transcoding function might be called
multiple times in a row.  However, as it is not used, it is not entirely
clear how it was intended to work.  Let's remove this unused looping
feature which makes it hard to understand how upcoming osmo_io should
deal with it.

Change-Id: Ie1a629fd31c5ab806fc929d1e6b279c4be5b8246
2024-03-20 13:25:22 +00:00
Harald Welte
8b5361412c don't log useless "transcoding disabled" message
The entire mgw has no transcoding support.  So printing that message is
useless to begin with.  And printing it for *every RTP packet* is even
more useless.  Let's remove it.

Change-Id: If0ee2607404afc3a00665a5cf22a9e0eb62eb476
2024-03-20 12:34:38 +00:00
Harald Welte
3a971ba0d1 mgw: Add our usual SIGABRT, SIGUSR1 signal handlers
This is mostly related to talloc reports.

Change-Id: Idc35444d2b8a0bc52c267b468dfa3c1b59f9187a
2024-03-19 18:16:58 +01:00
Neels Hofmeyr
c4b90354ca mgw: do not fail MGCP on codec mismatch
Before this patch, when an CRCX+MDCX wants to set a codec list that has
no match with the codecs for the other conn of that same endpoint,
osmo-mgw returns an MGCP "FAIL" response.

When a client wants to change the codec, it has to do that one RTP port
at a time. So osmo-mgw *must* allow to configure an MGCP conn with a
codec choice that mismatches the other conn.

This is crucial to allow codec negotiation in osmo-msc: if MO has
already assigned a specific codec, and later wants to re-assign to the
codec that MT has chosen, the codec needs to be changed at osmo-mgw.

This patch is the minimal fix required to get re-assignment to a
different codec to work (via osmo-msc). There is more work to be done
about this bit of code in osmo-mgw, but keep that to a separate patch.

In detail, before this patch, we fail both
- when a side has no codecs,
- or when there is no single match between codecs of the two sides of
  the endpoint.
Remove only the second condition; after this patch, still fail when a
side has no codecs -- this allows mgcp_test.c to still pass.

Related: OS#6293
Related: osmo-msc I8760feaa8598047369ef8c3ab2673013bac8ac8a
Change-Id: I3d1163fe622bdd7dc42a485f796072524ab39db9
2024-03-19 03:38:38 +00:00
Harald Welte
28fd236044 migrate mgcp_client from osmo_wqueue to osmo_io
The new osmo_io framework means that we can [optionally] make use
of the io_uring backend, which greatly reduces the syscall load
compared to the legacy osmo_wqueue + osmo_select_main + read/write.

We only use features already present in the intiial osmo_io support
of libosmocore 1.9.0, so no entry in TODO-RELEASE is needed.

Closes: OS#5754
Related: OS#5755
Change-Id: I766224da4691695c023d4d08d042a4bbeba05e47
2024-03-07 19:44:59 +01:00
Neels Hofmeyr
17b5701f19 mgcp_test: fix false negatives in test output
If one test fails, do not print failure for all following tests as well.

Change-Id: I196880b4b34a672ef45042c25f89bc1684363567
2024-02-06 03:17:50 +01:00
Neels Hofmeyr
9bea6eb78c tweak DEBUG log
Printing the debug log line a little later will include the MGCP verb
information.

Change-Id: Icb230cf4d623cdbc4ab52bd52d2a72525c0168c7
2024-02-06 03:17:50 +01:00
Neels Hofmeyr
fd57bd5f6f mgcp_codec_decide: remove redundant lookup
We already did a lookup from conn_src[i] and found a matching
codec_conn_dst, no need to do another reverse lookup to end up at the
same conn_src[i] codec.

Change-Id: Iecc7f22c551fd17b23db434fdb177266407d2621
2024-02-06 03:14:56 +01:00