Hello Neels Hofmeyr, Jenkins Builder,

I'd like you to reexamine a change.  Please visit

    https://gerrit.osmocom.org/2165

to look at the new patch set (#4).

Add SW Description (de)marshalling

* data structure representing 3GPP TS 52.021 §9.4.62 SW Description
* function to serialize it into msgb
* function to deserialize it from buffer
* function to estimate buffer size
* test harness

There are several similar functions to deal with SW Description in
OpenBSC, there's also need to use similar functionality in
OsmoBTS. Hence it's better to put the code into common library with
proper tests and documentation.

Change-Id: Ib63b6b5e83b8914864fc7edd789f8958cdc993cd
Related: OS#1614
---
M include/osmocom/gsm/protocol/gsm_12_21.h
M src/gsm/abis_nm.c
M src/gsm/libosmogsm.map
M tests/Makefile.am
M tests/msgb/msgb_test.c
M tests/msgb/msgb_test.ok
6 files changed, 157 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/65/2165/4

diff --git a/include/osmocom/gsm/protocol/gsm_12_21.h 
b/include/osmocom/gsm/protocol/gsm_12_21.h
index c88f0b1..3e6bb2b 100644
--- a/include/osmocom/gsm/protocol/gsm_12_21.h
+++ b/include/osmocom/gsm/protocol/gsm_12_21.h
@@ -29,6 +29,7 @@
 /*! \file gsm_12_21.h */
 
 #include <stdint.h>
+#include <stdbool.h>
 #include <osmocom/gsm/tlv.h>
 
 /*! \brief generic header in front of every OML message according to TS 08.59 
*/
@@ -791,6 +792,22 @@
        IPAC_BINF_CELL_ALLOC            = (1 << 2),
 };
 
+/*! \brief 3GPP TS 52.021 §9.4.62 SW Description */
+struct abis_nm_sw_descr {
+       uint8_t file_id[UINT8_MAX];
+       uint8_t file_id_len;
+
+       uint8_t file_version[UINT8_MAX];
+       uint8_t file_version_len;
+};
+
+uint16_t abis_nm_sw_descr_len(const struct abis_nm_sw_descr *sw,
+                             bool put_sw_descr);
+uint16_t abis_nm_put_sw_descr(struct msgb *msg, const struct abis_nm_sw_descr 
*sw,
+                             bool put_sw_descr);
+bool abis_nm_get_sw_descr(struct abis_nm_sw_descr *sw, const uint8_t * buf,
+                         size_t len);
+
 struct msgb *abis_nm_fail_evt_rep(enum abis_nm_event_type t,
                                  enum abis_nm_severity s,
                                  enum abis_nm_pcause_type ct,
diff --git a/src/gsm/abis_nm.c b/src/gsm/abis_nm.c
index 934d7ce..6dbc45a 100644
--- a/src/gsm/abis_nm.c
+++ b/src/gsm/abis_nm.c
@@ -751,6 +751,81 @@
        return nmsg;
 }
 
+/*! \brief Compute length of given 3GPP TS 52.021 §9.4.62 SW Description.
+ *  \param[in] sw SW Description struct
+ *  \param[in] put_sw_descr boolean, whether to put NM_ATT_SW_DESCR IE or not
+ *  \returns length of buffer space necessary to store sw
+ */
+uint16_t abis_nm_sw_descr_len(const struct abis_nm_sw_descr *sw,
+                             bool put_sw_descr)
+{
+       /* +3: type is 1-byte, length is 2-byte */
+       return (put_sw_descr ? 1 : 0) + (sw->file_id_len + 3) +
+              (sw->file_version_len + 3);
+}
+
+/*! \brief Put given 3GPP TS 52.021 §9.4.62 SW Description into msgb.
+ *  \param[out] msg message buffer
+ *  \param[in] sw SW Description struct
+ *  \param[in] put_sw_descr boolean, whether to put NM_ATT_SW_DESCR IE or not
+ *  \param[in] simulate boolean, whether to actually modify msg or not
+ *  \returns length of buffer space necessary to store sw
+ */
+uint16_t abis_nm_put_sw_descr(struct msgb *msg, const struct abis_nm_sw_descr 
*sw,
+                             bool put_sw_descr)
+{
+       if (put_sw_descr)
+               msgb_v_put(msg, NM_ATT_SW_DESCR);
+       msgb_tl16v_put(msg, NM_ATT_FILE_ID, sw->file_id_len, sw->file_id);
+       msgb_tl16v_put(msg, NM_ATT_FILE_VERSION, sw->file_version_len,
+                      sw->file_version);
+
+       return abis_nm_sw_descr_len(sw, put_sw_descr);
+}
+
+/*! \brief Parse 3GPP TS 52.021 §9.4.62 SW Description from buffer.
+ *  \param[out] sw SW Description struct
+ *  \param[in] buf buffer
+ *  \param[in] len buffer length
+ *  \returns true if parsing succeeded, else otherwise
+ */
+bool abis_nm_get_sw_descr(struct abis_nm_sw_descr *sw, const uint8_t * buf,
+                         size_t len)
+{
+       static struct tlv_parsed tp;
+       const struct tlv_definition sw_tlvdef = {
+               .def = {
+                       [NM_ATT_SW_DESCR] =             { TLV_TYPE_TV },
+                       [NM_ATT_FILE_ID] =              { TLV_TYPE_TL16V },
+                       [NM_ATT_FILE_VERSION] =         { TLV_TYPE_TL16V },
+               },
+       };
+
+       /* Note: the return value is ignored here because SW Description tag
+          itself is considered optional. */
+       tlv_parse(&tp, &sw_tlvdef, buf, len, 0, 0);
+
+       /* Parsing SW Description is tricky for current implementation of TLV
+          parser which fails to properly handle TV when V has following
+          structure: | TL16V | TL16V |. Hence, the need for 2nd call: */
+       if (tlv_parse(&tp, &sw_tlvdef, buf + TLVP_LEN(&tp, NM_ATT_SW_DESCR),
+                     len - TLVP_LEN(&tp, NM_ATT_SW_DESCR), 0, 0) < 0)
+               return false;
+
+       if (!TLVP_PRESENT(&tp, NM_ATT_FILE_ID) ||
+           !TLVP_PRESENT(&tp, NM_ATT_FILE_VERSION))
+               return false;
+
+       sw->file_id_len = TLVP_LEN(&tp, NM_ATT_FILE_ID);
+       sw->file_version_len = TLVP_LEN(&tp, NM_ATT_FILE_VERSION);
+
+       memcpy(sw->file_id, TLVP_VAL(&tp, NM_ATT_FILE_ID), sw->file_id_len);
+       memcpy(sw->file_version, TLVP_VAL(&tp, NM_ATT_FILE_VERSION),
+              sw->file_version_len);
+
+       return true;
+}
+
 /*! \brief Obtain OML Channel Combination for phnsical channel config */
 int abis_nm_chcomb4pchan(enum gsm_phys_chan_config pchan)
 {
diff --git a/src/gsm/libosmogsm.map b/src/gsm/libosmogsm.map
index 5649e71..a8ad892 100644
--- a/src/gsm/libosmogsm.map
+++ b/src/gsm/libosmogsm.map
@@ -31,6 +31,9 @@
 abis_nm_pcause_type_names;
 abis_nm_msgtype_names;
 abis_nm_att_names;
+abis_nm_sw_descr_len;
+abis_nm_put_sw_descr;
+abis_nm_get_sw_descr;
 
 osmo_sitype_strs;
 osmo_c4;
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 352b5a7..16b45ea 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -74,7 +74,7 @@
 lapd_lapd_test_LDADD = $(top_builddir)/src/libosmocore.la 
$(top_builddir)/src/gsm/libosmogsm.la
 
 msgb_msgb_test_SOURCES = msgb/msgb_test.c
-msgb_msgb_test_LDADD = $(top_builddir)/src/libosmocore.la
+msgb_msgb_test_LDADD = $(top_builddir)/src/libosmocore.la 
$(top_builddir)/src/gsm/libosmogsm.la
 
 msgfile_msgfile_test_SOURCES = msgfile/msgfile_test.c
 msgfile_msgfile_test_LDADD = $(top_builddir)/src/libosmocore.la
diff --git a/tests/msgb/msgb_test.c b/tests/msgb/msgb_test.c
index ac10382..abdcfee 100644
--- a/tests/msgb/msgb_test.c
+++ b/tests/msgb/msgb_test.c
@@ -23,10 +23,12 @@
 #include <osmocom/core/logging.h>
 #include <osmocom/core/utils.h>
 #include <osmocom/core/msgb.h>
+#include <osmocom/gsm/protocol/gsm_12_21.h>
+
 #include <setjmp.h>
 
 #include <errno.h>
-
+#include <stdbool.h>
 #include <string.h>
 
 #define CHECK_RC(rc)   \
@@ -177,6 +179,57 @@
        msgb_free(msg2);
 }
 
+static inline void print_chk(const char *what, uint8_t len1, uint8_t len2,
+                            const uint8_t *x1, const uint8_t *x2)
+{
+       int cmp = memcmp(x1, x2, len2);
+       printf("\tFILE %s [%u == %u -> %d, %s] %d => %s\n", what, len1, len2,
+              len1 == len2, len1 != len2 ? "fail" : "ok",
+              cmp, cmp != 0 ? "FAIL" : "OK");
+}
+
+static inline void chk_descr(struct msgb *msg, struct abis_nm_sw_descr *get,
+                            struct abis_nm_sw_descr *put, bool h)
+{
+       bool res;
+       uint16_t len = abis_nm_put_sw_descr(msg, put, h);
+
+       printf("msgb [%u/%u/%u - %s]: SW DESCR (with%s header) %s\n", msg->len,
+              msg->data_len, len, len != msg->len ? "fail" : "ok",
+              h ? "" : "out",
+              len > put->file_version_len + put->file_id_len ? "OK" : "FAIL");
+
+       res = abis_nm_get_sw_descr(get, msgb_data(msg), msg->len);
+       if (res) {
+               print_chk("ID", get->file_id_len, put->file_id_len, 
get->file_id,
+                         put->file_id);
+               print_chk("VERSION", get->file_version_len,
+                         put->file_version_len, get->file_version,
+                         put->file_version);
+       } else
+               printf("SW DESCR (with%s header) parsing error!\n",
+                      h ? "" : "out");
+
+       msgb_reset(msg);
+}
+
+static void test_msg_sw()
+{
+       const char *f_id = "TEST.L0L", *f_ver = 
"0.1.666~deadbeeffacefeed-dirty";
+       uint8_t lf_id = strlen(f_id), lf_ver = strlen(f_ver);
+       struct msgb *msg = msgb_alloc_headroom(4096, 128, "sw");
+       struct abis_nm_sw_descr sw_get1 = { 0 }, sw_get2 = { 0 }, sw_put = {
+               .file_id_len = lf_id,
+               .file_version_len = lf_ver,
+       };
+
+       memcpy(sw_put.file_id, f_id, lf_id);
+       memcpy(sw_put.file_version, f_ver, lf_ver);
+
+       chk_descr(msg, &sw_get1, &sw_put, true);
+       chk_descr(msg, &sw_get2, &sw_put, false);
+}
+
 static void test_msgb_resize_area()
 {
        struct msgb *msg = msgb_alloc_headroom(4096, 128, "data");
@@ -287,6 +340,7 @@
        test_msgb_api_errors();
        test_msgb_copy();
        test_msgb_resize_area();
+       test_msg_sw();
 
        printf("Success.\n");
 
diff --git a/tests/msgb/msgb_test.ok b/tests/msgb/msgb_test.ok
index 6603fe7..2a35407 100644
--- a/tests/msgb/msgb_test.ok
+++ b/tests/msgb/msgb_test.ok
@@ -32,4 +32,10 @@
 Original: [L1]> 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f 10 11 12 13 
[L2]> 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f 20 21 22 23 24 25 26 27 [L3]> 28 29 
2a 2b 2c 2d 2e 2f 30 31 32 33 34 35 36 37 38 39 3a 3b [L4]> 3c 3d 3e 3f 40 41 
42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e 4f 
 Extended: [L1]> 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f 10 11 12 13 
[L2]> 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f 20 21 22 23 24 25 26 27 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
[L3]> 28 29 2a 2b 2c 2d 2e 2f 30 31 32 33 34 35 36 37 38 39 3a 3b [L4]> 3c 3d 
3e 3f 40 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e 4f 
 Shrinked: [L1]> 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f 10 11 12 13 
[L2]> 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f 20 21 22 23 24 25 26 27 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
[L3]> 28 29 2a 2b 2c 2d 2e 2f 30 31 [L4]> 3c 3d 3e 3f 40 41 42 43 44 45 46 47 
48 49 4a 4b 4c 4d 4e 4f 
+msgb [45/4096/45 - ok]: SW DESCR (with header) OK
+       FILE ID [8 == 8 -> 1, ok] 0 => OK
+       FILE VERSION [30 == 30 -> 1, ok] 0 => OK
+msgb [44/4096/44 - ok]: SW DESCR (without header) OK
+       FILE ID [8 == 8 -> 1, ok] 0 => OK
+       FILE VERSION [30 == 30 -> 1, ok] 0 => OK
 Success.

-- 
To view, visit https://gerrit.osmocom.org/2165
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib63b6b5e83b8914864fc7edd789f8958cdc993cd
Gerrit-PatchSet: 4
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Owner: Max <msur...@sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max <msur...@sysmocom.de>
Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de>

Reply via email to