Cellular Network Infrastructure » cni-legacy » OpenBSC » OsmoNITB

On which GNU/Linux distribution this was observed?

Should be fixed by 8c119f7a0510b75e7fa1b96a37f2a6650e13824f in libosmo-abis.

let's close the issue and re-open only in case users report it as still broken.

Notice: the error was re-introduced in c77c2a6aa13accbc558888ab788d1148eb9aeb1a.

AFAICT c77c2a6aa13accbc558888ab788d1148eb9aeb1a is quite clearly a
fix of erratic use of libortp (at least version 0.25).

The commit being reverted, 8c119f7a0510b75e7fa1b96a37f2a6650e13824f, says:

Set connected mode after setting remote address

ortp: according to oRTP documentation rtp_session_set_connected_mode()
uses the address set by rtp_session_set_remote_addr() - move the
function call accordingly.

Fixes: OS#1661

Looking at the ortp code, that is definitely not the case.
rtp_session_set_connected_mode() simply sets a flag which is used in
rtp_session_set_remote_addr().

void rtp_session_set_connected_mode(RtpSession *session, bool_t yesno){
        session->use_connect=yesno;
}

static int
_rtp_session_set_remote_addr_full ([...]) { [called by rtp_session_set_remote_addr()]
[...]
        if (can_connect(session)){
                if (try_connect(session->rtp.gs.socket,(struct sockaddr*)&session->rtp.gs.rem_addr,session->rtp.gs.rem_addrlen))
                        session->flags|=RTP_SOCKET_CONNECTED;
[...]
}

#define can_connect(s)  ( (s)->use_connect && !(s)->symmetric_rtp)

I wonder how introducing this bug could fix this issue, and how fixing the bug
could again introduce the problem observed in OS#1661. It seems to be some
unintended side effect.

Can you clarify?

According to API docs - see http://download-mirror.savannah.gnu.org/releases/linphone/ortp/docs/rtpsession_8h.html#a84cdf75e48fea6e42b67da998e67fbda for instance:

"If yesno is TRUE, thus a connect() syscall is done on the socket to the destination address set by rtp_session_set_remote_addr()" which I interpret as "rtp_session_set_remote_addr() have to be called before rtp_session_set_connected_mode()".

The tests so far have showed that this is indeed the right order for later ortp versions. I'm not sure yet how to please both old and new - or if it's worth the efforts at all: we can just fix on latest stable version ortp (0.22.0 in Debian stable for example).

At the same time examples supplied with ortp do the opposite - see src/tests/rtpsend.c:81
It's really hard to work with the library when library's documentation contradict library's examples.

Already did - no response so far. From previous experience it could be weeks before upstream devs react if at all.

I have analysed a bit further. The starting point was that I switched to using the
SysmoBTS SDK version 1.5.4, which includes a newer libortp version, and with that
voice was broken (only background noise).

Then Max said:

This might be related to commit c77c2a6aa13accbc558888ab788d1148eb9aeb1a
in osmobts [libosmo-abis]

If you roll-back this commit, does voice re-appear?

Wow, it actually does! But when looking at the ortp code as described in detail
in OS#1661, this makes no sense at all. It should not fix anything.
I tried to understand the ortp code:

rtp_session_set_connected_mode() simply sets a flag (use_connect=1) which is
used in rtp_session_set_remote_addr(). Setting the flag only later amounts to
setting a different flag value (use_connect=0) for the first connection. The
main difference: with use_connect=1, rtp_session_set_remote_addr() establishes
a connect() already and sets the "is connected" flag, and with use_connect=0
it seems only the socket is opened.

But the use_connect flag is also queried during rtp_process_incoming_packet().
This calls rtp_session_update_remote_sock_addr(), to update the address to the
remote counterpart, and will connect only if the "is connected" flag is not set yet.

So I suspect this:

When use_connect 1, rtp_session_set_remote_addr() connects and sets the
connection flag. Then the rtp_session_update_remote_sock_addr() wants to update
to the real remote counterpart, but this has no effect, since the flag that
we're connected is already set, and the code does not reconnect the socket.
We still have a connect() to the initial wrong remote address.

When use_connect 0, rtp_session_set_remote_addr() does not connect(), and
the rtp_session_update_remote_sock_addr() has an effect, since the connection
is established only in rtp_process_incoming_packet(). So by setting the
use_connect only after rtp_session_set_remote_addr(), the first incoming
packet does the connect() when it knows the proper remote address.

This is confirmed by a hack: with use_connect == 1 at the top, voice is broken;
if I then set the internal ortp flags to not-connected, voice works: the code
reconnects with the updated address upon receiving a packet.

--- a/src/trau/osmo_ortp.c
+++ b/src/trau/osmo_ortp.c
@@ -401,6 +401,9 @@ int osmo_rtp_socket_connect(struct osmo_rtp_socket *rs, const char *ip, uint16_t
        if (rc < 0)
                return rc;

+       rs->sess->flags &= ~(1<<8); // RTP_SOCKET_CONNECTED=1<<8
+       rs->sess->flags &= ~(1<<9); // RTCP_SOCKET_CONNECTED=1<<9
+
        if (rs->flags & OSMO_RTP_F_POLL)
                return rc;
        else

What appears ugly at this point:

- each rtp_process_incoming_packet() call unconditionally does
rtp_session_update_remote_sock_addr(), but it appears to have an effect only
the first time it is called.

- rtp_process_incoming_packet() should probably update the address on the first
time only.

- rtp_process_incoming_packet() should detect whether an address change is
needed and disconnect the socket (so that it is reconnected in the code
below).

Hence it would be possible to set the use_connect flag a priori and the other
code would automagically know whether a reconnect is necessary.

But in summary, setting the use_connect flag "too late" actually fixes voice
for newer libortp.

The rtp_process_incoming_packet() was introduced after libortp 0.16.5 which
might explain why setting use_connect = 1 at the top doesn't break voice for
libortp 0.16.5.

Todo: check whether litecell15 also works with a recent ortp-0.25 with https://gerrit.osmocom.org/1011 applied.
(The instructions were to use 0.16.5)

To illustrate the landscape a bit further ...

• libortp versions:
  • The SDK for Litecell 1.5 recommends using libortp v0.16.5.
  • Recent libosmo-abis has moved to requiring a minimum of libortp v0.22.0

• c77c2a6aa13accbc is a patch originally suggested by the Litecell 1.5 SDK.
  It looked like a fix at first to me, so I readily applied it, but now the
  situation turns out to be far more complex.

• sysmoBTS SDK versions:
  • The sysmoBTS SDK v1.5.3 works fine with c77c2a6aa13accbc, presumably
    because it still includes an older libortp.
  • The sysmoBTS SDK v1.5.4 produces no voice when c77c2a6aa13accbc is applied,
    presumably because it includes a more recent libortp version.

It seems "recent" libortp internals around the set_connected_mode flag have
either introduced a regression, or alternatively a (seemingly bad) design
choice forces us to take more care when setting that flag.

We should clarify these:

• Why does c77c2a6aa13accbc matter when changing to a recent libortp? Is that
  situation intended by libortp upstream or actually a regression?

• Why exactly does Litecell 1.5 need libortp 0.16.5?
  • Can we accept Litecell 1.5 to stick to a libortp version that current
    libosmo-abis master clearly rejects as too old?
  • What would it take to pull Litecell 1.5 up to a recent libortp?

• Furthermore, why does Litecell 1.5 need c77c2a6aa13accbc on top of an old
  libortp 0.16.5? (Asking because for sysmoBTS it seems that an older libortp
  makes c77c2a6aa13accbc not matter much.)

The latest litecell sdk has oRTP version 0.22.0 which I've tested recently and it works fine. They promised that documentation update will follow but it has not materialized yet. Hence we can focus on testing with oRTP 0.22.0+ ignoring older versions which are not supported by upstream anyway. See for SYS#2569 details.

As original bug refers to osmo-bts-octphy, it got to be tested as well.

Checked it with libortp 0.25.0 on OCTBTS3500 with most recent firmware (02.07.00-B1039) and it works fine. However, the recent libortp (0.27.0 and above) there will be problems because of API changes.

As it works for osmo-bts-octphy again I suggest to close this and move further lc15-specific discussion to linked #1785.