Attention is currently required from: neels, nt2mku. falconia has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/36784?usp=email ) Change subject: gsm48_encode_bearer_cap(): omit octet 3a if only GSM-FR/GSM-HR v1 is supported ...................................................................... Patch Set 10: (1 comment) Patchset: PS10: This patch is now pedantically correct: it does what it says, follows Osmocom coding style, and produces encoded output that follows the 3GPP spec. However, I am still unsure if it should be merged, or if we should pursue a different solution. If we merge this patch as-is, we will lose the ability to send intentionally invalid speech version lists, and thereby lose the ability to make discoveries like the one made by @vyanitskiy@sysmocom.de in Russia last year. Background: in 2023-09 Vadim discovered (using osmocom-bb as his test instrument) that Russian MNO Tele2 will issue a connection using HRv1 codec (yes, that's right) *only if* the MS transmits a certain carefully crafted spec-invalid speech version list in its bearer cap IE. Per ETSI and 3GPP specs, every MS *must* support FRv1 at the very least, hence a bearer cap IE that says (via any encoding) that "I don't support FRv1" is spec-invalid. When octet 3a is omitted, there is no possible coding that produces this spec-invalid meaning - but if you do include octet 3a, you can produce this oddity by sending a speech version list that omits FRv1 and only includes HRv1, or only HRv1 and AMR-HR as in Vadim's other experiment on that MNO (which resulted in AHS being issued). It is my understanding that Vadim produced those spec-invalid SV lists using osmocom-bb without even realizing at the time that they were spec-invalid. Also irrespective of whether octets 3a etc are included or omitted, every correct MS (outside of experiments where you send intentionally invalid stuff) must set the RRQ bits correctly based on its speech version support. Contrary to common misunderstanding, those bcap->radio bits are *not* there to say in an abstract, codec-independent manner whether the MS supports TCH/H, or which rate it prefers - instead it is there to tell ancient networks (those that predate the addition of octet 3a) whether the MS supports HRv1 or if it is FRv1-only, and if HRv1 is supported, which is preferred. Referring to the same spec PDF version which Vadim cited earlier, please look at logical page 558 (PDF physical page 559), and scrutinize the table at the very bottom of the page (the case with octets 3a etc included) and the table just above it (the case with octet 3a omitted). These two tables make it clear that the two RRQ bits need to be set based *solely* on whether or not the MS supports HRv1, and where HRv1 stands *relative to FRv1* in the order of preference, without any regard for TCH/F or TCH/H support or preference with newer codecs! This logic does not currently exist in gsm48_encode_bearer_cap(), and this functions does not look like the right place for it. My proposal: we leave gsm48_encode_bearer_cap() as it is (don't merge the present patch), but create a wrapper (or preprocessor) function to be called (optionally) just before this encoding function, and have this preener function do the following: * Enforce that FRv1 appears in the provided speech version list, returning an error otherwise; * Compute spec-compliant RRQ bits from the provided speech version list; * Replace the SV list with an empty one (-1 in speech_ver[0]) if only FRv1/HRv1 are supported, and this support has now been encoded in the RRQ bits. I can produce a patch implementing this preener function *if* others agree with the general principle outlined above. P.S. I apologize for not making this thought-out proposal earlier - but my volunteer time isn't infinite, and my previous replies here were somewhat rushed. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/36784?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ia09abb32a8458384151a6ae28744835ea440fc5b Gerrit-Change-Number: 36784 Gerrit-PatchSet: 10 Gerrit-Owner: nt2mku <degrunert.web(a)googlemail.com> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-CC: falconia <falcon(a)freecalypso.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: nt2mku <degrunert.web(a)googlemail.com> Gerrit-Comment-Date: Wed, 29 May 2024 18:46:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment