Skip to content
Success

Changes

Summary

  1. rtp2trau: remove broken acceptance of 0-length payloads (details)
  2. rtp2trau FR & EFR: disallow BFI in DL (details)
  3. rtp2trau HR to DL: validate ToC octet of RFC 5993 (details)
Commit a77d9dd135553db91374b98c9ea9bbe1e9f73fde by falcon
rtp2trau: remove broken acceptance of 0-length payloads

Since the beginning of osmo_rtp2trau() in 2020, this API accepted
0-length RTP payloads for FR, HR and EFR, for both TRAU-DL and
TRAU-UL output.  However, this "support" never worked correctly:

* For FR & EFR in DL direction, previous patch 1d42fcd4ea40
  attempted to emit the idle speech frame of TS 48.060 section 5.4,
  following sections 6.5.2 and 6.5.3 of the same spec.  However,
  the actual combination of osmo_rtp2trau() followed by
  osmo_trau_frame_encode() resulted in an invalid FR/EFR TRAU-DL
  frame being emitted, rather than the intended special idle speech
  frame.

* For FR & EFR in UL direction, the same previous patch would turn
  0-length payload input (or TW-TS-001 No_Data input with later
  patches) into BFI with a peculiar fill pattern.  The fill pattern
  was all-1s in the case of FR or all-1s plus bad CRC in the case of
  EFR.  (The 5 CRCs of EFR TRAU frame format just happen to come out
  bad when all Dn bits are set to 1 - not intentional CRC inversion.)
  TRAU-UL output is not currently used anywhere in Osmocom; there is
  a plan to implement TFO in ThemWi software using Osmocom libraries,
  but that plan does NOT include No_Data input to osmo_rtp2trau().

* For HR speech to TRAU-DL conversion, the effect of 0-length payload
  input was emission of a TRAU-DL frame that looks valid, but has all
  112 bits of payload set to 0, with correct CRC.  All-zeros is a poor
  choice of LPC parameter fill for dummy GSM-HR codec frames, but the
  bigger issue is consistency with FR/EFR: while it would be trivial
  to change the fill pattern to the recently added
  osmo_gsm620_silence_frame[], if we are removing support for NULL
  input to osmo_rtp2trau() in the case of FR and EFR, it would be
  pointless to support such operation just for HR.

Because fixing the case of NULL or No_Data input for {FR,EFR}->TRAU
would entail more complexity and rather arbitrary decision-making in
the library that better belongs in applications (see below), let's
take the Alexandrian solution to this Gordian Knot: simply remove
all bogo-support for 0-length RTP payload input to osmo_rtp2trau()
in the case of speech codecs, and return -EINVAL just like for all
other types of invalid RTP input.

Effect on OsmoMGW-E1: the only time when OsmoMGW could pass a 0-length
payload to osmo_rtp2trau() would be if the RTP stream comes from
OsmoBTS (not from another leg on OsmoMGW-E1) with vty option
'rtp continuous-streaming' enabled.  Prior to the present patch,
the resulting effect would be bogus output on Abis DL; after this
patch, the MGW behavior will be the same as with no RTP input -
the MGW will insert fixed fill frames at the application level,
going directly to I.460 mux.

Further notes:

* If someone does wish to emit the idle speech frame of TS 48.060
  section 5.4 on Abis DL, despite the observation that historical
  TRAUs never emit such frames and the uncertainty of how E1 BTS
  would handle such strangeness, they can use osmo_trau_frame_encode()
  directly, without going through osmo_rtp2trau() - trivial as there
  is no payload in need of conversion.

* In the realm of TRAU-UL output (TFO or emulation of E1 BTS), every
  time a BFI frame is emitted, _some_ payload bit pattern is required:
  there is no provision for No_Data frames in FR/EFR TRAU/TFO frame
  format.  Selecting one particular fill bit pattern to be "special"
  in that the library fills it in when the application passed NULL
  or No_Data is philosophically wrong: no one bit pattern is any more
  special than any other in this situation.  Historical E1 BTS most
  commonly use the buffer method: they maintain an internal buffer
  into which new channel-decoded frames are written, and during
  No_Data conditions (FACCH stealing), the previous buffer content
  is repeated, perhaps corrupted in some peculiar way.  The same
  approach is recommended for applications that need to transform
  from RTP to TRAU-UL/TFO output - but this buffer clearly belongs
  in the application, not in the library.

* For HR codec, there is currently no fully working support for it
  in Osmocom - or more precisely, the combination of OsmoBSC+OsmoMGW
  has no working HR support with E1 BTS.  Therefore, there are no
  backward compatibility concerns, and we still have the freedom
  to change the behavior of TRAU<->RTP library functions to make it
  more sensible.

Change-Id: Ia41ea2eab1ded094cadcd91dbd33de8800af11ee
The file was modifiedsrc/trau/trau_rtp_conv.c
Commit 1752e681257750862e830423a3ed856dd4370cdc by falcon
rtp2trau FR & EFR: disallow BFI in DL

osmo_rtp2trau() accepts TW-TS-001 extended RTP payload format for
FR and EFR, converting to either TRAU-DL or TRAU-UL.  This extended
format is required for TRAU-UL/TFO frame output, but it is also
accepted for conversion to TRAU-DL in order to support the current
form of OsmoMGW-E1 that has no TFO transform between RTP input and
TDM-timed calls to osmo_rtp2trau().

TW-TS-001 can represent not only good frames, but also bad ones -
however, bad frames on Abis are valid only in UL, not in DL.  Prior
to the present patch, if a BFI-marked frame were fed to osmo_rtp2trau()
for conversion to TRAU-DL, the Bad Frame Indicator was dropped and
the garbage payload bits were emitted as if they were a good frame -
bad choice.  Reject BFI frames in TRAU-DL mode, return -EINVAL.

Change-Id: Ic63943d4bde9902b27e4d8fe9a5fb6ccecbf36c6
The file was modifiedsrc/trau/trau_rtp_conv.c
Commit 8c4884c43f0380dfd67c5e2e722eb4f39e0e98ec by falcon
rtp2trau HR to DL: validate ToC octet of RFC 5993

This validation serves two purposes:

* Payloads that just happen to be 15 bytes long but aren't valid
  RFC 5993 should be rejected;

* Super-5993 extensions of TW-TS-002 are valid only in UL, not in DL
  - catch and block them.

Change-Id: Ibbaa1e1e12254eaf75a999dd1b58e2145eff158c
The file was modifiedsrc/trau/trau_rtp_conv.c