<p>Harald Welte <strong>merged</strong> this change.</p><p><a href="https://gerrit.osmocom.org/12044">View Change</a></p><div style="white-space:pre-wrap">Approvals:
  Harald Welte: Looks good to me, approved
  Pau Espin Pedrol: Looks good to me, approved
  Jenkins Builder: Verified

</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">gsm0808: add encoder for cause codes and use it<br><br>At the moment the all gsm0808 cause codes are encoded directly using the<br>tlv API directly to put a one byte TLV field. This works ok for most<br>situations where the cause code consists of a single byte. However,<br>gsm0808 specifies a two byte cause code model where cause codes may be<br>extended up to two bytes. Instead of implementing the encoding over and<br>over and again, let's rather have an encoder function we can call.<br><br>- Add an encoder function that can generate single byte and extended<br>  cause codeds and makes the length decision automatically.<br><br>- Use only this function to append cause codes<br><br>Change-Id: I71d58fad89502a43532f60717ca022c15c73f8bb<br>---<br>M include/osmocom/gsm/gsm0808_utils.h<br>M src/gsm/gsm0808.c<br>M src/gsm/gsm0808_utils.c<br>M src/gsm/libosmogsm.map<br>M tests/gsm0808/gsm0808_test.c<br>M tests/gsm0808/gsm0808_test.ok<br>6 files changed, 75 insertions(+), 19 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/include/osmocom/gsm/gsm0808_utils.h b/include/osmocom/gsm/gsm0808_utils.h</span><br><span>index 097bd76..90ff677 100644</span><br><span>--- a/include/osmocom/gsm/gsm0808_utils.h</span><br><span>+++ b/include/osmocom/gsm/gsm0808_utils.h</span><br><span>@@ -77,6 +77,7 @@</span><br><span> int gsm0808_cell_id_u_name(char *buf, size_t buflen,</span><br><span>                         enum CELL_IDENT id_discr, const union gsm0808_cell_id_u *u);</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+uint8_t gsm0808_enc_cause(struct msgb *msg, uint16_t cause);</span><br><span> uint8_t gsm0808_enc_aoip_trasp_addr(struct msgb *msg,</span><br><span>                               const struct sockaddr_storage *ss);</span><br><span> int gsm0808_dec_aoip_trasp_addr(struct sockaddr_storage *ss,</span><br><span>diff --git a/src/gsm/gsm0808.c b/src/gsm/gsm0808.c</span><br><span>index c0d5f39..e951ab1 100644</span><br><span>--- a/src/gsm/gsm0808.c</span><br><span>+++ b/src/gsm/gsm0808.c</span><br><span>@@ -141,7 +141,7 @@</span><br><span>               return NULL;</span><br><span> </span><br><span>     msgb_v_put(msg, BSS_MAP_MSG_RESET);</span><br><span style="color: hsl(0, 100%, 40%);">-     msgb_tlv_put(msg, GSM0808_IE_CAUSE, 1, &cause);</span><br><span style="color: hsl(120, 100%, 40%);">+   gsm0808_enc_cause(msg, cause);</span><br><span>       msg->l3h = msgb_tv_push(msg, BSSAP_MSG_BSS_MANAGEMENT, msgb_length(msg));</span><br><span> </span><br><span>     return msg;</span><br><span>@@ -190,7 +190,7 @@</span><br><span> </span><br><span>        msg->l3h = msgb_tv_put(msg, BSSAP_MSG_BSS_MANAGEMENT, 4);</span><br><span>         msgb_v_put(msg, BSS_MAP_MSG_CLEAR_CMD);</span><br><span style="color: hsl(0, 100%, 40%);">- msgb_tlv_put(msg, GSM0808_IE_CAUSE, 1, &cause);</span><br><span style="color: hsl(120, 100%, 40%);">+   gsm0808_enc_cause(msg, cause);</span><br><span> </span><br><span>   return msg;</span><br><span> }</span><br><span>@@ -273,7 +273,7 @@</span><br><span> </span><br><span>   msgb_v_put(msg, BSS_MAP_MSG_CIPHER_MODE_REJECT);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-    msgb_tlv_put(msg, GSM0808_IE_CAUSE, 1, (const uint8_t *)&cause);</span><br><span style="color: hsl(120, 100%, 40%);">+  gsm0808_enc_cause(msg, cause);</span><br><span> </span><br><span>   msg->l3h = msgb_tv_push(msg, BSSAP_MSG_BSS_MANAGEMENT, msgb_length(msg));</span><br><span> </span><br><span>@@ -286,18 +286,22 @@</span><br><span>  *  \returns callee-allocated msgb with BSSMAP Cipher Mode Reject message */</span><br><span> struct msgb *gsm0808_create_cipher_reject_ext(enum gsm0808_cause_class class, uint8_t ext)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">-     uint8_t c[2];</span><br><span style="color: hsl(120, 100%, 40%);">+ uint16_t cause;</span><br><span>      struct msgb *msg = msgb_alloc_headroom(BSSMAP_MSG_SIZE, BSSMAP_MSG_HEADROOM,</span><br><span>                                                "bssmap: cipher mode reject");</span><br><span>      if (!msg)</span><br><span>            return NULL;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-        c[0] = 0x80 | (class << 4); /* set the high bit to indicate extended cause */</span><br><span style="color: hsl(0, 100%, 40%);">-     c[1] = ext;</span><br><span style="color: hsl(120, 100%, 40%);">+   /* Set cause code class in the upper byte */</span><br><span style="color: hsl(120, 100%, 40%);">+  cause = 0x80 | (class << 4);</span><br><span style="color: hsl(120, 100%, 40%);">+    cause = cause << 8;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+   /* Set cause code extension in the lower byte */</span><br><span style="color: hsl(120, 100%, 40%);">+      cause |= ext;</span><br><span> </span><br><span>    msgb_v_put(msg, BSS_MAP_MSG_CIPHER_MODE_REJECT);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-    msgb_tlv_put(msg, GSM0808_IE_CAUSE, 2, c);</span><br><span style="color: hsl(120, 100%, 40%);">+    gsm0808_enc_cause(msg, cause);</span><br><span> </span><br><span>   msg->l3h = msgb_tv_push(msg, BSSAP_MSG_BSS_MANAGEMENT, msgb_length(msg));</span><br><span> </span><br><span>@@ -572,7 +576,7 @@</span><br><span>               return NULL;</span><br><span> </span><br><span>     msgb_v_put(msg, BSS_MAP_MSG_ASSIGMENT_FAILURE);</span><br><span style="color: hsl(0, 100%, 40%);">- msgb_tlv_put(msg, GSM0808_IE_CAUSE, 1, &cause);</span><br><span style="color: hsl(120, 100%, 40%);">+   gsm0808_enc_cause(msg, cause);</span><br><span> </span><br><span>   /* RR cause 3.2.2.22 */</span><br><span>      if (rr_cause)</span><br><span>@@ -614,7 +618,7 @@</span><br><span>          return NULL;</span><br><span> </span><br><span>     msgb_v_put(msg, BSS_MAP_MSG_CLEAR_RQST);</span><br><span style="color: hsl(0, 100%, 40%);">-        msgb_tlv_put(msg, GSM0808_IE_CAUSE, 1, &cause);</span><br><span style="color: hsl(120, 100%, 40%);">+   gsm0808_enc_cause(msg, cause);</span><br><span>       msg->l3h = msgb_tv_push(msg, BSSAP_MSG_BSS_MANAGEMENT, msgb_length(msg));</span><br><span> </span><br><span>     return msg;</span><br><span>@@ -751,7 +755,7 @@</span><br><span>    msgb_v_put(msg, BSS_MAP_MSG_HANDOVER_REQUIRED);</span><br><span> </span><br><span>  /* Cause, 3.2.2.5 */</span><br><span style="color: hsl(0, 100%, 40%);">-    msgb_tlv_put(msg, GSM0808_IE_CAUSE, params->cause & 0x80? 2 : 1, (const uint8_t*)&params->cause);</span><br><span style="color: hsl(120, 100%, 40%);">+       gsm0808_enc_cause(msg, params->cause);</span><br><span> </span><br><span>        /* Cell Identifier List, 3.2.2.27 */</span><br><span>         gsm0808_enc_cell_id_list2(msg, &params->cil);</span><br><span>@@ -876,7 +880,7 @@</span><br><span>   msgb_v_put(msg, BSS_MAP_MSG_HANDOVER_FAILURE);</span><br><span> </span><br><span>   /* Cause, 3.2.2.5 */</span><br><span style="color: hsl(0, 100%, 40%);">-    msgb_tlv_put(msg, GSM0808_IE_CAUSE, params->cause & 0x80? 2 : 1, (const uint8_t*)&params->cause);</span><br><span style="color: hsl(120, 100%, 40%);">+       gsm0808_enc_cause(msg, params->cause);</span><br><span> </span><br><span>        /* RR Cause, 3.2.2.22 */</span><br><span>     if (params->rr_cause_present)</span><br><span>@@ -907,7 +911,7 @@</span><br><span>       msgb_v_put(msg, BSS_MAP_MSG_HANDOVER_PERFORMED);</span><br><span> </span><br><span>         /* Cause, 3.2.2.5 */</span><br><span style="color: hsl(0, 100%, 40%);">-    msgb_tlv_put(msg, GSM0808_IE_CAUSE, gsm0808_cause_ext(params->cause) ? 2 : 1, (const uint8_t *)&params->cause);</span><br><span style="color: hsl(120, 100%, 40%);">+     gsm0808_enc_cause(msg, params->cause);</span><br><span> </span><br><span>        /* Cell Identifier, 3.2.2.17 */</span><br><span>      gsm0808_enc_cell_id(msg, &params->cell_id);</span><br><span>diff --git a/src/gsm/gsm0808_utils.c b/src/gsm/gsm0808_utils.c</span><br><span>index c58d828..38a8664 100644</span><br><span>--- a/src/gsm/gsm0808_utils.c</span><br><span>+++ b/src/gsm/gsm0808_utils.c</span><br><span>@@ -48,6 +48,32 @@</span><br><span>  *  \file gsm0808_utils.c</span><br><span>  */</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+/*! Encode TS 08.08 AoIP Cause IE</span><br><span style="color: hsl(120, 100%, 40%);">+ *  \param[out] msg Message Buffer to which to append IE</span><br><span style="color: hsl(120, 100%, 40%);">+ *  \param[in] cause Cause code to be used in IE</span><br><span style="color: hsl(120, 100%, 40%);">+ *  \returns number of bytes added to \a msg */</span><br><span style="color: hsl(120, 100%, 40%);">+uint8_t gsm0808_enc_cause(struct msgb *msg, uint16_t cause)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+      /* See also 3GPP TS 48.008 3.2.2.5 Cause */</span><br><span style="color: hsl(120, 100%, 40%);">+   uint8_t *old_tail;</span><br><span style="color: hsl(120, 100%, 40%);">+    bool extended;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+      old_tail = msg->tail;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+    extended = gsm0808_cause_ext(cause >> 8);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+     msgb_put_u8(msg, GSM0808_IE_CAUSE);</span><br><span style="color: hsl(120, 100%, 40%);">+   if (extended) {</span><br><span style="color: hsl(120, 100%, 40%);">+               msgb_put_u8(msg, 2);</span><br><span style="color: hsl(120, 100%, 40%);">+          msgb_put_u16(msg, cause);</span><br><span style="color: hsl(120, 100%, 40%);">+     } else {</span><br><span style="color: hsl(120, 100%, 40%);">+              msgb_put_u8(msg, 1);</span><br><span style="color: hsl(120, 100%, 40%);">+          msgb_put_u8(msg, (uint8_t) (cause & 0xFF));</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 (uint8_t) (msg->tail - old_tail);</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /*! Encode TS 08.08 AoIP transport address IE</span><br><span>  *  \param[out] msg Message Buffer to which to append IE</span><br><span>  *  \param[in] ss Socket Address to be used in IE</span><br><span>diff --git a/src/gsm/libosmogsm.map b/src/gsm/libosmogsm.map</span><br><span>index e9a9e4f..dc4e0a6 100644</span><br><span>--- a/src/gsm/libosmogsm.map</span><br><span>+++ b/src/gsm/libosmogsm.map</span><br><span>@@ -183,6 +183,7 @@</span><br><span> gsm0808_create_handover_failure;</span><br><span> gsm0808_create_handover_performed;</span><br><span> gsm0808_prepend_dtap_header;</span><br><span style="color: hsl(120, 100%, 40%);">+gsm0808_enc_cause;</span><br><span> gsm0808_enc_aoip_trasp_addr;</span><br><span> gsm0808_dec_aoip_trasp_addr;</span><br><span> gsm0808_enc_speech_codec;</span><br><span>diff --git a/tests/gsm0808/gsm0808_test.c b/tests/gsm0808/gsm0808_test.c</span><br><span>index 197ec06..63b8720 100644</span><br><span>--- a/tests/gsm0808/gsm0808_test.c</span><br><span>+++ b/tests/gsm0808/gsm0808_test.c</span><br><span>@@ -30,6 +30,13 @@</span><br><span> #include <arpa/inet.h></span><br><span> #include <errno.h></span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+#define EXPECT_ENCODED(hexstr) do { \</span><br><span style="color: hsl(120, 100%, 40%);">+             const char *enc_str = msgb_hexdump(msg); \</span><br><span style="color: hsl(120, 100%, 40%);">+            printf("%s: encoded: %s(rc = %u)\n", __func__, enc_str, rc_enc); \</span><br><span style="color: hsl(120, 100%, 40%);">+          OSMO_ASSERT(strcmp(enc_str, hexstr " ") == 0); \</span><br><span style="color: hsl(120, 100%, 40%);">+            OSMO_ASSERT(rc_enc == msg->len); \</span><br><span style="color: hsl(120, 100%, 40%);">+ } while(0)</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> #define VERIFY(msg, data, len)                                          \</span><br><span>    if (msgb_l3len(msg) != len) {                                   \</span><br><span>            printf("%s:%d Length don't match: %d vs. %d. %s\n",       \</span><br><span>@@ -65,6 +72,27 @@</span><br><span>       scl->len = 3;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+void test_gsm0808_enc_cause(void)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+       /* NOTE: This must be tested early because many of the following tests</span><br><span style="color: hsl(120, 100%, 40%);">+         * rely on the generation of a proper cause code. */</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+        uint8_t rc_enc;</span><br><span style="color: hsl(120, 100%, 40%);">+       struct msgb *msg;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+   /* Test with a single byte cause code */</span><br><span style="color: hsl(120, 100%, 40%);">+      msg = msgb_alloc(1024, "output buffer");</span><br><span style="color: hsl(120, 100%, 40%);">+    rc_enc = gsm0808_enc_cause(msg, 0x41);</span><br><span style="color: hsl(120, 100%, 40%);">+        EXPECT_ENCODED("04 01 41");</span><br><span style="color: hsl(120, 100%, 40%);">+ msgb_free(msg);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+     /* Test with an extended (two byte) cause code */</span><br><span style="color: hsl(120, 100%, 40%);">+     msg = msgb_alloc(1024, "output buffer");</span><br><span style="color: hsl(120, 100%, 40%);">+    rc_enc = gsm0808_enc_cause(msg, 0x8041);</span><br><span style="color: hsl(120, 100%, 40%);">+      EXPECT_ENCODED("04 02 80 41");</span><br><span style="color: hsl(120, 100%, 40%);">+      msgb_free(msg);</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> static void test_create_layer3(void)</span><br><span> {</span><br><span>    static const uint8_t res[] = {</span><br><span>@@ -824,13 +852,6 @@</span><br><span>        msgb_free(msg);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-#define EXPECT_ENCODED(hexstr) do { \</span><br><span style="color: hsl(0, 100%, 40%);">-           const char *enc_str = msgb_hexdump(msg); \</span><br><span style="color: hsl(0, 100%, 40%);">-              printf("%s: encoded: %s(rc = %u)\n", __func__, enc_str, rc_enc); \</span><br><span style="color: hsl(0, 100%, 40%);">-            OSMO_ASSERT(strcmp(enc_str, hexstr " ") == 0); \</span><br><span style="color: hsl(0, 100%, 40%);">-              OSMO_ASSERT(rc_enc == msg->len); \</span><br><span style="color: hsl(0, 100%, 40%);">-   } while(0)</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> static void test_gsm0808_enc_dec_cell_id_list_lac()</span><br><span> {</span><br><span>       struct gsm0808_cell_id_list2 enc_cil;</span><br><span>@@ -1770,6 +1791,7 @@</span><br><span> int main(int argc, char **argv)</span><br><span> {</span><br><span>        printf("Testing generation of GSM0808 messages\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ test_gsm0808_enc_cause();</span><br><span>    test_create_layer3();</span><br><span>        test_create_layer3_aoip();</span><br><span>   test_create_reset();</span><br><span>diff --git a/tests/gsm0808/gsm0808_test.ok b/tests/gsm0808/gsm0808_test.ok</span><br><span>index a48cf1d..e5833d0 100644</span><br><span>--- a/tests/gsm0808/gsm0808_test.ok</span><br><span>+++ b/tests/gsm0808/gsm0808_test.ok</span><br><span>@@ -1,4 +1,6 @@</span><br><span> Testing generation of GSM0808 messages</span><br><span style="color: hsl(120, 100%, 40%);">+test_gsm0808_enc_cause: encoded: 04 01 41 (rc = 3)</span><br><span style="color: hsl(120, 100%, 40%);">+test_gsm0808_enc_cause: encoded: 04 02 80 41 (rc = 4)</span><br><span> Testing creating Layer3</span><br><span> Testing creating Layer3 (AoIP)</span><br><span> Testing creating Reset</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/12044">change 12044</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/12044"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: libosmocore </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: I71d58fad89502a43532f60717ca022c15c73f8bb </div>
<div style="display:none"> Gerrit-Change-Number: 12044 </div>
<div style="display:none"> Gerrit-PatchSet: 5 </div>
<div style="display:none"> Gerrit-Owner: dexter <pmaier@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Harald Welte <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder (1000002) </div>
<div style="display:none"> Gerrit-Reviewer: Max <msuraev@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Pau Espin Pedrol <pespin@sysmocom.de> </div>