diff mbox series

libosmocore/BSC/MSC: MO voice calls rejected from Siemens S11E (patch included)

Message ID CAOH_NPOAP_doapsjhNMY8SW_2QvxZAxA-53ct6FXwpaZvnjs2A@mail.gmail.com
State New
Headers show
Series libosmocore/BSC/MSC: MO voice calls rejected from Siemens S11E (patch included) | expand

Commit Message

Lennart Müller June 29, 2022, 8:42 p.m. UTC
Hi,

I had a problem placing MO GSM calls from a Siemens S11E: The calls
were dropped immediately; Osmo-MSC reports "Cannot compose Channel
Type from bearer capabilities"

After investigating the SETUP request from the S11E, the phone does
not use octet 3a (no extension bit set in IE 3). Wireshark decodes the
radio channel requirement as "Full rate support only MS/fullrate
speech version 1 supported", so I added a condition to the gsm48_ie.c
function of libosmocore to include at least GSM FR in the list of
available speech_ver in case octet 3 has no extension.

Attached to this message are the Abis-IP PCAP traces of MO calls, and
the patch for gsm48_ie.c.


Regards,
Lennart

Comments

Mychaela Falconia June 30, 2022, 10:18 p.m. UTC | #1
Hi Lennart,

First of all, thank you for being another person who cares about
supporting vintage GSM phones!  As to your proposed patch to gsm48_ie.c
in libosmocore, I have a counterproposal - please see attachment.
Instead of patching gsm48_decode_bearer_cap(), I feel that it would be
more philosophically correct to patch the logic in
mncc_bearer_cap_to_channel_type() in OsmoMSC - I feel that IE encoding
and decoding functions should do just that, perform bit-level encoding
and decoding without higher-level interpretation of the data they
operate on, whereas the desired change is in the interpretation of the
condition of MS not having sent any speech version numbers.

Right now I don't have any phones that are old enough to support only
FR1 codec and not EFR.  As of right now, the oldest working phone in
my collection is Ericsson I888, and even though it's a very old design
(older, higher-voltage internal components than market-release-date
contemporary Nokia DCT3 phones of the famed monitor mode variety), it
already supports both FR1 and EFR.  (Interestingly enough, it does not
support HR codec - but then perhaps I shouldn't be surprised, as I
heard that HR1 is computationally intensive, and whatever ancient DSP
this phone has in it could easily lack the needed horsepower.)

Back to the topic of fixing Osmocom CNI software to work with ancient
FR1-only phones that don't send any octet 3a in their bearer cap IE, I
would appreciate it if someone with more seniority in Osmocom could
weigh in as to the most proper way to do it: would it be Lennart's
proposed patch to libosmocore, or my proposed patch to osmo-msc, or
some other, altogether different approach?

In solidarity for continued preservation and support for GSM/2G,

Lady Mychaela Nadezhda Falconia,
Mother of FreeCalypso,
Dame of the Order of 2G,
Champion of Published Source Code
Harald Welte July 2, 2022, 3:37 p.m. UTC | #2
Hi Lennart and Mychaela,

I think it is indeed best to apply a fix to libosmocore, where the parser
is.  The functional split is a bit ... arbitrary ... there.

The best way to handle this would be to also add a test caes, probably
easiest in terms of a unit test (in libosmocore/tests), showing that
both the longer/later version and the shorter/older version parse
with the correct expectations.  That's not required, but it's the only
way to really ensure it won't break in the future.

Is either of you going to submit the patch (with or without unit test)
via gerrit?
https://osmocom.org/projects/cellular-infrastructure/wiki/Gerrit

Thanks again,
	Harald
Mychaela Falconia July 2, 2022, 6:04 p.m. UTC | #3
Hi Harald,

> I think it is indeed best to apply a fix to libosmocore, where the parser
> is.  The functional split is a bit ... arbitrary ... there.

If we are going to put this logic inside gsm48_decode_bearer_cap(),
then we have to do it fully by the spec, decoding not only the FR1-only
possibility, but also the two possibilities of both FR1 and HR1
supported, with the order of preference determined by the radio channel
requirement bits.  Please see attached patch for a rough first-order
implementation.

In my other proposal (putting the logic in mncc_bearer_cap_to_channel_type()
in OsmoMSC) I was able to "cheat" in terms of omitting HR1 support -
but only because that function already has pre-existing logic that
actively suppresses HR1 as a matter of policy.  But if we are patching
the generic bearer cap IE decoding function, then I argue that we must
not cheat in such manner.

> Is either of you going to submit the patch (with or without unit test)
> via gerrit?

I don't have a time budget to do all that formal process right now [1],
but if no one else beats me to it, I will come back to this issue at
some later time.  If Lennart or Harald or anyone else would like to
use the patch attached to this email as a starting point (or even to
submit it exactly as-is), please have at it.

M~

[1] In relation to Osmocom CNI, my current work consists of implementing
a voice call gateway between an Osmocom-based GSM network and USA PSTN,
the latter accessed via a wholesale SIP trunk service provider such as
bulkvs.com.  I shall discuss this work in another thread.
Vadim Yanitskiy July 2, 2022, 11:56 p.m. UTC | #4
Hi all,

On 7/3/22 01:04, Mychaela Falconia wrote:
> I don't have a time budget to do all that formal process right now [1],
> but if no one else beats me to it, I will come back to this issue at
> some later time.  If Lennart or Harald or anyone else would like to
> use the patch attached to this email as a starting point (or even to
> submit it exactly as-is), please have at it.

as was suggested by Harald, I added a testcase (IE LV from PCAP):

https://gerrit.osmocom.org/c/libosmocore/+/28506 gsm0408_test: add a 
testcase for gsm48_decode_bearer_cap()

and submitted Mychaela's gsm48_ie.c.patch2:

https://gerrit.osmocom.org/c/libosmocore/+/28508 gsm48_ie: fix parsing 
of Bearer capability IE without octet 3a

I changed this patch a bit to make our linter happy about coding style.

Best regards,
Vadim.
rokj42540@gmail.com Jan. 31, 2024, 7:49 a.m. UTC | #5
After studying the SETUP request, it is obvious that octet three lacks an extension bit, mainly for deciphering blunders in Wireshark. To resolve this, I've changed the gsm48_ie.C feature in libosmocore, ensuring GSM FR is protected even if octet three lacks an extension. This enhancement should rectify the Cannot Compose Channel Type from the bearer talents blunders.  Please permit me to create my essay in this successful troubleshooting process.
Visit here : https://bestessaywriter.co.uk/make-my-essay
Tomcsányi, Domonkos Jan. 31, 2024, 10:48 p.m. UTC | #6
Wow, these generative AI-backed spambots are getting better every time, I almost started trying to understand what it tried to say.
Eventually we will need to run our own AI that detects these, right? Haha.

Cheers,
Domi

> 2024. jan. 31. dátummal, 8:49 időpontban rokj42540@gmail.com írta:
> 
> After studying the SETUP request, it is obvious that octet three lacks an extension bit, mainly for deciphering blunders in Wireshark. To resolve this, I've changed the gsm48_ie.C feature in libosmocore, ensuring GSM FR is protected even if octet three lacks an extension. This enhancement should rectify the Cannot Compose Channel Type from the bearer talents blunders.  Please permit me to create my essay in this successful troubleshooting process.
> Visit here : https://bestessaywriter.co.uk/make-my-essay
diff mbox series

Patch

diff --git a/src/gsm/gsm48_ie.c b/src/gsm/gsm48_ie.c
index 6d40bec4..b3e56075 100644
--- a/src/gsm/gsm48_ie.c
+++ b/src/gsm/gsm48_ie.c
@@ -203,16 +203,25 @@  int gsm48_decode_bearer_cap(struct gsm_mncc_bearer_cap *bcap,
 	case GSM_MNCC_BCAP_SPEECH:
 		i = 1;
 		s = 0;
-		while(!(lv[i] & 0x80)) {
-			i++; /* octet 3a etc */
-			if (in_len < i)
-				return 0;
-			bcap->speech_ver[s++] = lv[i] & 0x0f;
+		if((lv[1] & 0x80) != 0)
+		{
+			bcap->speech_ver[s++] = GSM48_BCAP_SV_FR; /* GSM FR only */
 			bcap->speech_ver[s] = -1; /* end of list */
-			if (i == 2) /* octet 3a */
-				bcap->speech_ctm = (lv[i] & 0x20) >> 5;
-			if (s == 7) /* maximum speech versions + end of list */
-				return 0;
+			return 0;
+		}
+		else
+		{
+			while(!(lv[i] & 0x80)) {
+				i++; /* octet 3a etc */
+				if (in_len < i)
+					return 0;
+				bcap->speech_ver[s++] = lv[i] & 0x0f;
+				bcap->speech_ver[s] = -1; /* end of list */
+				if (i == 2) /* octet 3a */
+					bcap->speech_ctm = (lv[i] & 0x20) >> 5;
+				if (s == 7) /* maximum speech versions + end of list */
+					return 0;
+			}
 		}
 		break;
 	case GSM_MNCC_BCAP_UNR_DIG: