[S] Change in libosmocore[pespin/rel-1.10.0]: gsm48_encode_bearer_cap(): properly set 'Structure' in octet 4

pespin has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/39345?usp=email ) Change subject: gsm48_encode_bearer_cap(): properly set 'Structure' in octet 4 ...................................................................... gsm48_encode_bearer_cap(): properly set 'Structure' in octet 4 Before commit 714843a45 we hard-coded octet 4 to 0xb8, then we started hard-coding it to 0x88. Neither is correct. According to 3GPP TS 43.010, Annex A.1 "Attribute definition and their values", attribute 'Structure' can take two values: * Service data unit integrity, which is ** applicable for connection element "non transparent"; * Unstructured, which is ** applicable for connection element "transparent". This commit changes the output of tests/gsm0408/gsm0408_test: * Test `CSD 2400/V.22bis/transparent' is now passing. * csd_9600_v110_lv[] revert octet 4 back to 0xb8. Change-Id: I563becfdae75e22de3b5e5de3c810334a74e7ffb Fixes: 714843a45 ("gsm48_ie.c: change bearer cap structure in outgoing CSD calls") Related: OS#5730 (cherry picked from commit aec32dbf288715f102a544d991fe16765642d862) --- M src/gsm/gsm48_ie.c M tests/gsm0408/gsm0408_test.c M tests/gsm0408/gsm0408_test.err M tests/gsm0408/gsm0408_test.ok 4 files changed, 19 insertions(+), 7 deletions(-) Approvals: osmith: Looks good to me, approved Jenkins Builder: Verified neels: Looks good to me, but someone else must approve diff --git a/src/gsm/gsm48_ie.c b/src/gsm/gsm48_ie.c index b95609f..7a41ae8 100644 --- a/src/gsm/gsm48_ie.c +++ b/src/gsm/gsm48_ie.c @@ -339,8 +339,23 @@ case GSM48_BCAP_ITCAP_3k1_AUDIO: case GSM48_BCAP_ITCAP_FAX_G3: lv[i++] |= 0x80; /* last IE of octet 3 etc */ - /* octet 4 */ - lv[i++] = 0x88; + /* octet 4 + * 1... .... = Extension: No Extension + * .0.. .... = Compression: Not Allowed + * ..xx .... = Structure: (see below) + * .... 1... = Duplex mode: Full + * .... .0.. = Configuration: Point-to-point + * .... ..0. = NIRR: No meaning is associated with this value + * .... ...0 = Establishment: Demand + * + * For connection element "non transparent": + * ..00 .... = Structure: Service data unit integrity (0) + * For connection element "transparent": + * ..11 .... = Structure: Unstructured (3) */ + if ((bcap->data.transp & 0x01) == 0) + lv[i++] = 0x88 | (0x03 << 4); + else + lv[i++] = 0x88; /* octet 5 */ lv[i++] = 0x80 | ((bcap->data.rate_adaption & 3) << 3) | (bcap->data.sig_access & 7); diff --git a/tests/gsm0408/gsm0408_test.c b/tests/gsm0408/gsm0408_test.c index 8a89357..7456ed8 100644 --- a/tests/gsm0408/gsm0408_test.c +++ b/tests/gsm0408/gsm0408_test.c @@ -31,7 +31,7 @@ #include <osmocom/core/msgb.h> -static const uint8_t csd_9600_v110_lv[] = { 0x07, 0xa1, 0x88, 0x89, 0x21, 0x15, 0x63, 0x80 }; +static const uint8_t csd_9600_v110_lv[] = { 0x07, 0xa1, 0xb8, 0x89, 0x21, 0x15, 0x63, 0x80 }; static const struct gsm_mncc_bearer_cap bcap_csd_9600_v110 = { .transfer = GSM48_BCAP_ITCAP_UNR_DIG_INF, diff --git a/tests/gsm0408/gsm0408_test.err b/tests/gsm0408/gsm0408_test.err index 0102de0..8aeda3e 100644 --- a/tests/gsm0408/gsm0408_test.err +++ b/tests/gsm0408/gsm0408_test.err @@ -1,6 +1,3 @@ -Incorrect encoded result of CSD 2400/V.22bis/transparent: - should: 07 a2 b8 81 21 13 43 83 - is: 07 a2 88 81 21 13 43 83 Incorrect encoded result of Speech, without octet 3a: should: 01 a0 is: 02 20 80 diff --git a/tests/gsm0408/gsm0408_test.ok b/tests/gsm0408/gsm0408_test.ok index 721e2c5..3580591 100644 --- a/tests/gsm0408/gsm0408_test.ok +++ b/tests/gsm0408/gsm0408_test.ok @@ -1,6 +1,6 @@ Test `CSD 9600/V.110/transparent' passed Test `CSD 4800/RLP/non-transparent' passed -Test `CSD 2400/V.22bis/transparent' failed +Test `CSD 2400/V.22bis/transparent' passed Test `Speech, all codecs' passed Test `Speech, without octet 3a' failed Simple TMSI encoding test....passed -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/39345?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: libosmocore Gerrit-Branch: pespin/rel-1.10.0 Gerrit-Change-Id: I563becfdae75e22de3b5e5de3c810334a74e7ffb Gerrit-Change-Number: 39345 Gerrit-PatchSet: 1 Gerrit-Owner: pespin &lt;pespin(a)sysmocom.de&gt; Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels &lt;nhofmeyr(a)sysmocom.de&gt; Gerrit-Reviewer: osmith &lt;osmith(a)sysmocom.de&gt; Gerrit-Reviewer: pespin &lt;pespin(a)sysmocom.de&gt; Gerrit-CC: fixeria &lt;vyanitskiy(a)sysmocom.de&gt;