<p>laforge <strong>submitted</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/osmo-pcu/+/17245">View Change</a></p><div style="white-space:pre-wrap">Approvals:
laforge: Looks good to me, approved
pespin: Looks good to me, but someone else must approve
Jenkins Builder: Verified
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">tests/rlcmac: also verify encoding of MS RA Capability<br><br>The main idea of this change is to demonstrate a weakness of the<br>CSN.1 codec that most likely causes a unit test breakage in [1].<br><br>The problem seems to be that the transitional structures, where<br>the CSN.1 decoder stores the results, do not contain any details<br>about presence of the optional fields (such as M_UINT_OR_NULL).<br><br>In other words, it's impossible to know whether some optional<br>field is omitted in the encoded message (NULL), or is it just<br>set to 0. This means that the encoder will always include all<br>optional fields, even if they're not present in the original<br>message.<br><br>[1] Ibb4cbd3f5865415fd547e95fc24ff31df1aed4c0<br><br>Change-Id: Ic46d6e56768f516203d27d8e7a5adb77afdf32b7<br>---<br>M src/gsm_rlcmac.cpp<br>M tests/rlcmac/RLCMACTest.cpp<br>M tests/rlcmac/RLCMACTest.err<br>M tests/rlcmac/RLCMACTest.ok<br>4 files changed, 58 insertions(+), 7 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/gsm_rlcmac.cpp b/src/gsm_rlcmac.cpp</span><br><span>index 09a46e8..e60ccc7 100644</span><br><span>--- a/src/gsm_rlcmac.cpp</span><br><span>+++ b/src/gsm_rlcmac.cpp</span><br><span>@@ -5766,3 +5766,27 @@</span><br><span> }</span><br><span> return ret;</span><br><span> }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+/* This function is not actually used by osmo-pcu itself, and only needed for</span><br><span style="color: hsl(120, 100%, 40%);">+ * the RLCMAC unit test. Having it here is better than making the internal</span><br><span style="color: hsl(120, 100%, 40%);">+ * CSN.1 definitions (in particular, MS_Radio_Access_capability_t) non-static. */</span><br><span style="color: hsl(120, 100%, 40%);">+int encode_gsm_ra_cap(bitvec *vector, MS_Radio_Access_capability_t *data)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ unsigned writeIndex = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ csnStream_t ar;</span><br><span style="color: hsl(120, 100%, 40%);">+ int ret;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ csnStreamInit(&ar, 0, vector->data_len * 8);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* recursive csnStreamEncoder call uses LOGPC everywhere, so we need to start the log somewhere... */</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DCSN1, LOGL_INFO, "csnStreamEncoder (RAcap): ");</span><br><span style="color: hsl(120, 100%, 40%);">+ ret = csnStreamEncoder(&ar, CSNDESCR(MS_Radio_Access_capability_t), vector, &writeIndex, data);</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPC(DCSN1, LOGL_INFO, "\n");</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ret > 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DRLCMACDATA, LOGL_NOTICE, "Got %d remaining bits unhandled by encoder at the end of bitvec\n", ret);</span><br><span style="color: hsl(120, 100%, 40%);">+ ret = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ return ret;</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span>diff --git a/tests/rlcmac/RLCMACTest.cpp b/tests/rlcmac/RLCMACTest.cpp</span><br><span>index de63fe6..b8506b1 100644</span><br><span>--- a/tests/rlcmac/RLCMACTest.cpp</span><br><span>+++ b/tests/rlcmac/RLCMACTest.cpp</span><br><span>@@ -214,12 +214,16 @@</span><br><span> msgb_free(m);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+int encode_gsm_ra_cap(bitvec * vector, MS_Radio_Access_capability_t * data);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> void testRAcap(void *test_ctx)</span><br><span> {</span><br><span> printf("*** %s ***\n", __func__);</span><br><span> MS_Radio_Access_capability_t data;</span><br><span> memset(&data, 0, sizeof(data));</span><br><span style="color: hsl(0, 100%, 40%);">- bitvec *vector = bitvec_alloc(23, test_ctx);</span><br><span style="color: hsl(120, 100%, 40%);">+ bitvec *bv_dec = bitvec_alloc(23, test_ctx);</span><br><span style="color: hsl(120, 100%, 40%);">+ bitvec *bv_enc = bitvec_alloc(23, test_ctx);</span><br><span style="color: hsl(120, 100%, 40%);">+ unsigned int len_dec, len_enc;</span><br><span> int rc;</span><br><span> /*</span><br><span> MS RA capability 1</span><br><span>@@ -235,9 +239,10 @@</span><br><span> .... ..00 011. .... = GPRS multislot class: Max Rx-Slot/TDMA:2 Max Tx-Slot/TDMA:2 Max-Sum-Slot/TDMA:3 Tta:3 Ttb:2 Tra:3 Trb:1 Type:1 (3)</span><br><span> ...0 .... = GPRS Extended Dynamic Allocation Capability: Not Implemented</span><br><span> */</span><br><span style="color: hsl(0, 100%, 40%);">- bitvec_unhex(vector, "13a5146200");</span><br><span style="color: hsl(120, 100%, 40%);">+ bitvec_unhex(bv_dec, "13a5146200");</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- rc = decode_gsm_ra_cap(vector, &data);</span><br><span style="color: hsl(120, 100%, 40%);">+ printf("=== Test decoding of MS RA Capability ===\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ rc = decode_gsm_ra_cap(bv_dec, &data);</span><br><span> OSMO_ASSERT(rc == 0);</span><br><span> </span><br><span> /* Make sure there's 1 value (currently fails due to failed decoding) */</span><br><span>@@ -247,14 +252,30 @@</span><br><span> printf("GPRS multislot class = %u\n", Decoding::get_ms_class_by_capability(&data));</span><br><span> printf("EGPRS multislot class = %u\n", Decoding::get_egprs_ms_class_by_capability(&data));</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ /* Test encoding of decoded MS RA Capability */</span><br><span style="color: hsl(120, 100%, 40%);">+ printf("=== Test encoding of MS RA Capability ===\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ rc = encode_gsm_ra_cap(bv_enc, &data);</span><br><span style="color: hsl(120, 100%, 40%);">+ printf("encode_gsm_ra_cap() returns %d\n", rc);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ bv_dec->cur_bit = 4;</span><br><span style="color: hsl(120, 100%, 40%);">+ len_dec = bitvec_get_uint(bv_dec, 7);</span><br><span style="color: hsl(120, 100%, 40%);">+ bv_enc->cur_bit = 4;</span><br><span style="color: hsl(120, 100%, 40%);">+ len_enc = bitvec_get_uint(bv_enc, 7);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* NOTE: vector2 is expected to be different because there is actually no</span><br><span style="color: hsl(120, 100%, 40%);">+ * way to distinguish between NULL and 0 in MS_Radio_Access_capability_t.</span><br><span style="color: hsl(120, 100%, 40%);">+ * The difference is in length indicator: 29 bits vs 50 bits. */</span><br><span style="color: hsl(120, 100%, 40%);">+ printf("vector1 (len_ind=%u) = %s\n", len_dec, osmo_hexdump(bv_dec->data, bv_dec->data_len));</span><br><span style="color: hsl(120, 100%, 40%);">+ printf("vector2 (len_ind=%u) = %s\n", len_enc, osmo_hexdump(bv_enc->data, bv_enc->data_len));</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /* Mangle the length indicator (set it to 21) */</span><br><span> unsigned int writeIndex = 4;</span><br><span style="color: hsl(0, 100%, 40%);">- rc = bitvec_write_field(vector, &writeIndex, 21, 7);</span><br><span style="color: hsl(120, 100%, 40%);">+ rc = bitvec_write_field(bv_dec, &writeIndex, 21, 7);</span><br><span> OSMO_ASSERT(rc == 0);</span><br><span> </span><br><span> /* Make sure decoding attempt fails */</span><br><span style="color: hsl(0, 100%, 40%);">- printf("Test decoding of a malformed vector (short length indicator)\n");</span><br><span style="color: hsl(0, 100%, 40%);">- rc = decode_gsm_ra_cap(vector, &data);</span><br><span style="color: hsl(120, 100%, 40%);">+ printf("=== Test decoding of a malformed vector (short length indicator) ===\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ rc = decode_gsm_ra_cap(bv_dec, &data);</span><br><span> printf("decode_gsm_ra_cap() returns %d\n", rc);</span><br><span> }</span><br><span> </span><br><span>diff --git a/tests/rlcmac/RLCMACTest.err b/tests/rlcmac/RLCMACTest.err</span><br><span>index 52a3cc3..a7e501e 100644</span><br><span>--- a/tests/rlcmac/RLCMACTest.err</span><br><span>+++ b/tests/rlcmac/RLCMACTest.err</span><br><span>@@ -30,4 +30,5 @@</span><br><span> DCSN1 INFO csnStreamEncoder (type=2): PayloadType = 1 | spare = 0 | R = 0 | MESSAGE_TYPE = 2 | DOWNLINK_TFI = 20 | : Ack_Nack_Description | FINAL_ACK_INDICATION = 1 | STARTING_SEQUENCE_NUMBER = 1 | RECEIVED_BLOCK_BITMAP[0] = 0 | RECEIVED_BLOCK_BITMAP[1] = 0 | RECEIVED_BLOCK_BITMAP[2] = 0 | RECEIVED_BLOCK_BITMAP[3] = 0 | RECEIVED_BLOCK_BITMAP[4] = 0 | RECEIVED_BLOCK_BITMAP[5] = 0 | RECEIVED_BLOCK_BITMAP[6] = 0 | RECEIVED_BLOCK_BITMAP[7] = 1 | : End Ack_Nack_Description | Exist_Channel_Request_Description = 1 | : Channel_Request_Description | PEAK_THROUGHPUT_CLASS = 0 | RADIO_PRIORITY = 0 | RLC_MODE = 0 | LLC_PDU_TYPE = 1 | RLC_OCTET_COUNT = 18 | : End Channel_Request_Description | : Channel_Quality_Report | C_VALUE = 40 | RXQUAL = 1 | SIGN_VAR = 0 | Slot[0].Exist = 0 | Slot[1].Exist = 0 | Slot[2].Exist = 0 | Slot[3].Exist = 0 | Slot[4].Exist = 0 | Slot[5].Exist = 0 | Slot[6].Exist = 0 | Slot[7].Exist = 0 | : End Channel_Quality_Report | Exist_AdditionsR99 = 1 | : AdditionsR99 | Exist_PFI = 0 | : End AdditionsR99 | Padding = 3|43|43|43|43|43|</span><br><span> DCSN1 INFO csnStreamDecoder (type=8): PayloadType = 1 | spare = 0 | R = 0 | MESSAGE_TYPE = 8 | DOWNLINK_TFI = 0 | MS_OUT_OF_MEMORY = 0 | Exist_EGPRS_ChannelQualityReport = 1 | : EGPRS_ChannelQualityReport | : EGPRS_BEP_LinkQualityMeasurements | Exist_MEAN_CV_BEP_GMSK = 0 | Exist_MEAN_CV_BEP_8PSK = 1 | MEAN_BEP_8PSK = 31 | CV_BEP_8PSK = 7 | : End EGPRS_BEP_LinkQualityMeasurements | C_VALUE = 58 | : EGPRS_TimeslotLinkQualityMeasurements | Exist_BEP_MEASUREMENTS = 0 | Exist_INTERFERENCE_MEASUREMENTS = 0 | : End EGPRS_TimeslotLinkQualityMeasurements | : End EGPRS_ChannelQualityReport | Exist_ChannelRequestDescription = 1 | : ChannelRequestDescription | PEAK_THROUGHPUT_CLASS = 6 | RADIO_PRIORITY = 0 | RLC_MODE = 0 | LLC_PDU_TYPE = 1 | RLC_OCTET_COUNT = 62 | : End ChannelRequestDescription | Exist_PFI = 0 | Exist_ExtensionBits = 0 | : EGPRS_AckNack | Desc = 0 | : Desc | FINAL_ACK_INDICATION = 0 | BEGINNING_OF_WINDOW = 1 | END_OF_WINDOW = 1 | STARTING_SEQUENCE_NUMBER = 1187 | Exist_CRBB = 0 | URBB = 127 | URBB = 255 | URBB = 255 | URBB = 238 | URBB = 0 | URBB = 0 | URBB = 0 | URBB = 0 | URBB = 0 | URBB = 0 | URBB = 0 | URBB = 0 | URBB = 0 | : End Desc | : End EGPRS_AckNack | Padding = </span><br><span> DCSN1 INFO csnStreamDecoder (RAcap): MS_RA_capability_value { | Choice MS_RA_capability_value_Choice = 1 | u.Content length = 29 | offset = 4 | RF_Power_Capability = 1 | Exist_A5_bits = 0 | ES_IND = 1 | PS = 0 | VGCS = 0 | VBS = 0 | Exist_Multislot_capability = 1 | : Multislot_capability | Exist_HSCSD_multislot_class = 0 | Exist_GPRS_multislot_class = 1 | GPRS_multislot_class = 3 | GPRS_Extended_Dynamic_Allocation_Capability = 0 | Exist_SM = 0 | Exist_ECSD_multislot_class = 0 | Exist_EGPRS_multislot_class = 1 | EGPRS_multislot_class = 0 | EGPRS_Extended_Dynamic_Allocation_Capability = 0 | Exist_DTM_GPRS_multislot_class = 0 | : End Multislot_capability | Exist_Eight_PSK_Power_Capability = 0 | COMPACT_Interference_Measurement_Capability = 0 | Revision_Level_Indicator = NULL | UMTS_FDD_Radio_Access_Technology_Capability = NULL | UMTS_384_TDD_Radio_Access_Technology_Capability = NULL | CDMA2000_Radio_Access_Technology_Capability = NULL | UMTS_128_TDD_Radio_Access_Technology_Capability = NULL | GERAN_Feature_Package_1 = NULL | Modulation_based_multislot_class_support = NULL | GMSK_MultislotPowerProfile = NULL | EightPSK_MultislotProfile = NULL | MultipleTBF_Capability = NULL | DownlinkAdvancedReceiverPerformance = NULL | ExtendedRLC_MAC_ControlMessageSegmentionsCapability = NULL | DTM_EnhancementsCapability = NULL | PS_HandoverCapability = NULL | MS_RA_capability_value } | </span><br><span style="color: hsl(120, 100%, 40%);">+DCSN1 INFO csnStreamEncoder (RAcap): MS_RA_capability_value { | u.Content = 1 | RF_Power_Capability = 1 | Exist_A5_bits = 0 | ES_IND = 1 | PS = 0 | VGCS = 0 | VBS = 0 | Exist_Multislot_capability = 1 | : Multislot_capability | Exist_HSCSD_multislot_class = 0 | Exist_GPRS_multislot_class = 1 | GPRS_multislot_class = 3 | GPRS_Extended_Dynamic_Allocation_Capability = 0 | Exist_SM = 0 | Exist_ECSD_multislot_class = 0 | Exist_EGPRS_multislot_class = 1 | EGPRS_multislot_class = 0 | EGPRS_Extended_Dynamic_Allocation_Capability = 0 | Exist_DTM_GPRS_multislot_class = 0 | : End Multislot_capability | Exist_Eight_PSK_Power_Capability = 0 | COMPACT_Interference_Measurement_Capability = 0 | Revision_Level_Indicator = 0 | UMTS_FDD_Radio_Access_Technology_Capability = 0 | UMTS_384_TDD_Radio_Access_Technology_Capability = 0 | CDMA2000_Radio_Access_Technology_Capability = 0 | UMTS_128_TDD_Radio_Access_Technology_Capability = 0 | GERAN_Feature_Package_1 = 0 | Exist_Extended_DTM_multislot_class = 0 | Modulation_based_multislot_class_support = 0 | Exist_HighMultislotCapability = 0 | Exist_GERAN_lu_ModeCapability = 0 | GMSK_MultislotPowerProfile = 0 | EightPSK_MultislotProfile = 0 | MultipleTBF_Capability = 0 | DownlinkAdvancedReceiverPerformance = 0 | ExtendedRLC_MAC_ControlMessageSegmentionsCapability = 0 | DTM_EnhancementsCapability = 0 | Exist_DTM_GPRS_HighMultislotClass = 0 | PS_HandoverCapability = 0 | u.Content length = 50 | MS_RA_capability_value } | </span><br><span> DCSN1 INFO csnStreamDecoder (RAcap): MS_RA_capability_value { | Choice MS_RA_capability_value_Choice = 1 | u.Content length = 21 | offset = 4 | RF_Power_Capability = 1 | Exist_A5_bits = 0 | ES_IND = 1 | PS = 0 | VGCS = 0 | VBS = 0 | Exist_Multislot_capability = 1 | : Multislot_capability | Exist_HSCSD_multislot_class = 0 | Exist_GPRS_multislot_class = 1 | GPRS_multislot_class = 3 | GPRS_Extended_Dynamic_Allocation_Capability = 0 | Exist_SM = 0 | Exist_ECSD_multislot_class = 0 | Exist_EGPRS_multislot_class = 1 | DCSN1 ERROR csnStreamDecoder: error NEED_MORE BITS TO UNPACK (-5) at EGPRS_multislot_class (idx 31): End Multislot_capability | </span><br><span>diff --git a/tests/rlcmac/RLCMACTest.ok b/tests/rlcmac/RLCMACTest.ok</span><br><span>index 96c1bfb..2aea569 100644</span><br><span>--- a/tests/rlcmac/RLCMACTest.ok</span><br><span>+++ b/tests/rlcmac/RLCMACTest.ok</span><br><span>@@ -124,7 +124,12 @@</span><br><span> vector1 == vector2 : TRUE</span><br><span> *** testCsnLeftAlignedVarBmpBounds ***</span><br><span> *** testRAcap ***</span><br><span style="color: hsl(120, 100%, 40%);">+=== Test decoding of MS RA Capability ===</span><br><span> GPRS multislot class = 3</span><br><span> EGPRS multislot class = 0</span><br><span style="color: hsl(0, 100%, 40%);">-Test decoding of a malformed vector (short length indicator)</span><br><span style="color: hsl(120, 100%, 40%);">+=== Test encoding of MS RA Capability ===</span><br><span style="color: hsl(120, 100%, 40%);">+encode_gsm_ra_cap() returns 0</span><br><span style="color: hsl(120, 100%, 40%);">+vector1 (len_ind=29) = 13 a5 14 62 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 </span><br><span style="color: hsl(120, 100%, 40%);">+vector2 (len_ind=50) = 16 45 14 62 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 </span><br><span style="color: hsl(120, 100%, 40%);">+=== Test decoding of a malformed vector (short length indicator) ===</span><br><span> decode_gsm_ra_cap() returns -5</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-pcu/+/17245">change 17245</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/osmo-pcu/+/17245"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-pcu </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: Ic46d6e56768f516203d27d8e7a5adb77afdf32b7 </div>
<div style="display:none"> Gerrit-Change-Number: 17245 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: Vadim Yanitskiy <axilirator@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: Vadim Yanitskiy <axilirator@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>