openbsc[master]: Use libosmocore for SW Description parsing

[Harald Welte]

Patch Set 9: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib94db414e94a2a1f234ac6f1cb346dca1c7a8be3
Gerrit-PatchSet: 9
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Max 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max 
Gerrit-HasComments: No

openbsc[master]: Use libosmocore for SW Description parsing

[Max]

Patch Set 9:

Yes, tested as follows:
- upgraded nanobts using old code (from master)
- reset nanobts (otherwise next upgrade fails due to lack of space with 
"17392:WARN:SW_DLD:sw_dld_sdp_fsm.c#1000:Unable to allocate Flash region for 
SDP!")
- upgraded nanobts using new code - checked that the result is the same: "LOAD 
END ACK...Foo l2h: 0x5627f3fb1e28 l3h: 0x5627f3fb1e2b... length l2: 55  l3: 52"
I think this can be merged.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib94db414e94a2a1f234ac6f1cb346dca1c7a8be3
Gerrit-PatchSet: 9
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Max 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max 
Gerrit-HasComments: No

openbsc[master]: Use libosmocore for SW Description parsing

[Harald Welte]

Patch Set 9:

has ipa firmware update tested with this patch? if yes, we can +2 it

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib94db414e94a2a1f234ac6f1cb346dca1c7a8be3
Gerrit-PatchSet: 9
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Max 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max 
Gerrit-HasComments: No

openbsc[master]: Use libosmocore for SW Description parsing

[Harald Welte]

Patch Set 8: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib94db414e94a2a1f234ac6f1cb346dca1c7a8be3
Gerrit-PatchSet: 8
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Max 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max 
Gerrit-HasComments: No

openbsc[master]: Use libosmocore for SW Description parsing

[Max]

Patch Set 8:

The changes are related to fw update/select which I don't know how to test. Do 
we have firmware image for nanoBTS somewhere? Tried to search wiki but found 
nothing so far.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib94db414e94a2a1f234ac6f1cb346dca1c7a8be3
Gerrit-PatchSet: 8
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Max 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max 
Gerrit-HasComments: No

[PATCH] openbsc[master]: Use libosmocore for SW Description parsing

[Max]

Hello Jenkins Builder,

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

https://gerrit.osmocom.org/2166

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

Use libosmocore for SW Description parsing

Requires libosmocore with Ib63b6b5e83b8914864fc7edd789f8958cdc993cd.

Change-Id: Ib94db414e94a2a1f234ac6f1cb346dca1c7a8be3
---
M openbsc/include/openbsc/abis_nm.h
M openbsc/src/ipaccess/ipaccess-config.c
M openbsc/src/libbsc/abis_nm.c
M openbsc/tests/abis/abis_test.c
M openbsc/tests/abis/abis_test.ok
5 files changed, 36 insertions(+), 213 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/66/2166/5

diff --git a/openbsc/include/openbsc/abis_nm.h 
b/openbsc/include/openbsc/abis_nm.h
index 2465452..09e0b23 100644
--- a/openbsc/include/openbsc/abis_nm.h
+++ b/openbsc/include/openbsc/abis_nm.h
@@ -68,18 +68,6 @@
int (*sw_act_req)(struct msgb *);
 };
 
-struct abis_nm_sw_descr {
-   /* where does it start? how long is it? */
-   const uint8_t   *start;
-   size_t  len;
-
-   /* the parsed data */
-   const uint8_t   *file_id;
-   uint16_tfile_id_len;
-   const uint8_t   *file_ver;
-   uint16_tfile_ver_len;
-};
-
 extern int abis_nm_rcvmsg(struct msgb *msg);
 
 int abis_nm_tlv_parse(struct tlv_parsed *tp, struct gsm_bts *bts, const 
uint8_t *buf, int len);
@@ -181,8 +169,6 @@
 
 void abis_nm_queue_send_next(struct gsm_bts *bts); /* for bs11_config. */
 
-int abis_nm_parse_sw_config(const uint8_t *data, const size_t len,
-   struct abis_nm_sw_descr *res, const int res_len);
 int abis_nm_select_newest_sw(const struct abis_nm_sw_descr *sw, const size_t 
len);
 
 /* Helper functions for updating attributes */
diff --git a/openbsc/src/ipaccess/ipaccess-config.c 
b/openbsc/src/ipaccess/ipaccess-config.c
index 0c3f888..ca5f195 100644
--- a/openbsc/src/ipaccess/ipaccess-config.c
+++ b/openbsc/src/ipaccess/ipaccess-config.c
@@ -52,6 +52,7 @@
 #include 
 #include 
 #include 
+#include 
 
 struct gsm_network *bsc_gsmnet;
 
@@ -70,17 +71,9 @@
 static int found_trx = 0;
 static int loop_tests = 0;
 
-struct sw_load {
-   uint8_t file_id[255];
-   uint8_t file_id_len;
-
-   uint8_t file_version[255];
-   uint8_t file_version_len;
-};
-
 static void *tall_ctx_config = NULL;
-static struct sw_load *sw_load1 = NULL;
-static struct sw_load *sw_load2 = NULL;
+static struct abis_nm_sw_descr *sw_load1 = NULL;
+static struct abis_nm_sw_descr *sw_load2 = NULL;
 
 /*
 static uint8_t prim_oml_attr[] = { 0x95, 0x00, 7, 0x88, 192, 168, 100, 11, 
0x00, 0x00 };
@@ -344,19 +337,11 @@
msg->l3h = >l2h[3];
 
/* activate software */
-   if (sw_load1) {
-   msgb_v_put(msg, NM_ATT_SW_DESCR);
-   msgb_tl16v_put(msg, NM_ATT_FILE_ID, 
sw_load1->file_id_len, sw_load1->file_id);
-   msgb_tl16v_put(msg, NM_ATT_FILE_VERSION, 
sw_load1->file_version_len,
-   sw_load1->file_version);
-   }
+   if (sw_load1)
+   abis_nm_put_sw_descr(msg, sw_load1, true);
 
-   if (sw_load2) {
-   msgb_v_put(msg, NM_ATT_SW_DESCR);
-   msgb_tl16v_put(msg, NM_ATT_FILE_ID, 
sw_load2->file_id_len, sw_load2->file_id);
-   msgb_tl16v_put(msg, NM_ATT_FILE_VERSION, 
sw_load2->file_version_len,
-   sw_load2->file_version);
-   }
+   if (sw_load2)
+   abis_nm_put_sw_descr(msg, sw_load2, true);
 
/* fill in the data */
msg->l2h[0] = NM_ATT_IPACC_CUR_SW_CFG;
@@ -618,11 +603,11 @@
return 0;
 }
 
-static struct sw_load *create_swload(struct sdp_header *header)
+static struct abis_nm_sw_descr *create_swload(struct sdp_header *header)
 {
-   struct sw_load *load;
+   struct abis_nm_sw_descr *load;
 
-   load = talloc_zero(tall_ctx_config, struct sw_load);
+   load = talloc_zero(tall_ctx_config, struct abis_nm_sw_descr);
 
strncpy((char *)load->file_id, header->firmware_info.sw_part, 20);
load->file_id_len = strlen(header->firmware_info.sw_part) + 1;
diff --git a/openbsc/src/libbsc/abis_nm.c b/openbsc/src/libbsc/abis_nm.c
index 33a23a2..cd848b0 100644
--- a/openbsc/src/libbsc/abis_nm.c
+++ b/openbsc/src/libbsc/abis_nm.c
@@ -413,93 +413,29 @@
 
 /* Activate the specified software into the BTS */
 static int ipacc_sw_activate(struct gsm_bts *bts, uint8_t obj_class, uint8_t 
i0, uint8_t i1,
-uint8_t i2, const uint8_t *sw_desc, uint8_t 
swdesc_len)
+uint8_t i2, const struct abis_nm_sw_descr *sw_desc)
 {
struct abis_om_hdr *oh;
struct msgb *msg = nm_msgb_alloc();
-   uint8_t len = swdesc_len;
-   uint8_t *trailer;
+   uint16_t len = abis_nm_sw_descr_len(sw_desc, true);
 
oh = (struct 

[PATCH] openbsc[master]: Use libosmocore for SW Description parsing

[Max]

Hello Jenkins Builder,

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

https://gerrit.osmocom.org/2166

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

Use libosmocore for SW Description parsing

Requires libosmocore with Ib63b6b5e83b8914864fc7edd789f8958cdc993cd.

Change-Id: Ib94db414e94a2a1f234ac6f1cb346dca1c7a8be3
---
M openbsc/include/openbsc/abis_nm.h
M openbsc/src/ipaccess/ipaccess-config.c
M openbsc/src/libbsc/abis_nm.c
M openbsc/tests/abis/abis_test.c
M openbsc/tests/abis/abis_test.ok
5 files changed, 74 insertions(+), 124 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/66/2166/4

diff --git a/openbsc/include/openbsc/abis_nm.h 
b/openbsc/include/openbsc/abis_nm.h
index 2465452..9dcb91f 100644
--- a/openbsc/include/openbsc/abis_nm.h
+++ b/openbsc/include/openbsc/abis_nm.h
@@ -68,18 +68,6 @@
int (*sw_act_req)(struct msgb *);
 };
 
-struct abis_nm_sw_descr {
-   /* where does it start? how long is it? */
-   const uint8_t   *start;
-   size_t  len;
-
-   /* the parsed data */
-   const uint8_t   *file_id;
-   uint16_tfile_id_len;
-   const uint8_t   *file_ver;
-   uint16_tfile_ver_len;
-};
-
 extern int abis_nm_rcvmsg(struct msgb *msg);
 
 int abis_nm_tlv_parse(struct tlv_parsed *tp, struct gsm_bts *bts, const 
uint8_t *buf, int len);
@@ -182,7 +170,7 @@
 void abis_nm_queue_send_next(struct gsm_bts *bts); /* for bs11_config. */
 
 int abis_nm_parse_sw_config(const uint8_t *data, const size_t len,
-   struct abis_nm_sw_descr *res, const int res_len);
+   struct abis_nm_sw_descr *res, const int res_len);
 int abis_nm_select_newest_sw(const struct abis_nm_sw_descr *sw, const size_t 
len);
 
 /* Helper functions for updating attributes */
diff --git a/openbsc/src/ipaccess/ipaccess-config.c 
b/openbsc/src/ipaccess/ipaccess-config.c
index 0c3f888..ca5f195 100644
--- a/openbsc/src/ipaccess/ipaccess-config.c
+++ b/openbsc/src/ipaccess/ipaccess-config.c
@@ -52,6 +52,7 @@
 #include 
 #include 
 #include 
+#include 
 
 struct gsm_network *bsc_gsmnet;
 
@@ -70,17 +71,9 @@
 static int found_trx = 0;
 static int loop_tests = 0;
 
-struct sw_load {
-   uint8_t file_id[255];
-   uint8_t file_id_len;
-
-   uint8_t file_version[255];
-   uint8_t file_version_len;
-};
-
 static void *tall_ctx_config = NULL;
-static struct sw_load *sw_load1 = NULL;
-static struct sw_load *sw_load2 = NULL;
+static struct abis_nm_sw_descr *sw_load1 = NULL;
+static struct abis_nm_sw_descr *sw_load2 = NULL;
 
 /*
 static uint8_t prim_oml_attr[] = { 0x95, 0x00, 7, 0x88, 192, 168, 100, 11, 
0x00, 0x00 };
@@ -344,19 +337,11 @@
msg->l3h = >l2h[3];
 
/* activate software */
-   if (sw_load1) {
-   msgb_v_put(msg, NM_ATT_SW_DESCR);
-   msgb_tl16v_put(msg, NM_ATT_FILE_ID, 
sw_load1->file_id_len, sw_load1->file_id);
-   msgb_tl16v_put(msg, NM_ATT_FILE_VERSION, 
sw_load1->file_version_len,
-   sw_load1->file_version);
-   }
+   if (sw_load1)
+   abis_nm_put_sw_descr(msg, sw_load1, true);
 
-   if (sw_load2) {
-   msgb_v_put(msg, NM_ATT_SW_DESCR);
-   msgb_tl16v_put(msg, NM_ATT_FILE_ID, 
sw_load2->file_id_len, sw_load2->file_id);
-   msgb_tl16v_put(msg, NM_ATT_FILE_VERSION, 
sw_load2->file_version_len,
-   sw_load2->file_version);
-   }
+   if (sw_load2)
+   abis_nm_put_sw_descr(msg, sw_load2, true);
 
/* fill in the data */
msg->l2h[0] = NM_ATT_IPACC_CUR_SW_CFG;
@@ -618,11 +603,11 @@
return 0;
 }
 
-static struct sw_load *create_swload(struct sdp_header *header)
+static struct abis_nm_sw_descr *create_swload(struct sdp_header *header)
 {
-   struct sw_load *load;
+   struct abis_nm_sw_descr *load;
 
-   load = talloc_zero(tall_ctx_config, struct sw_load);
+   load = talloc_zero(tall_ctx_config, struct abis_nm_sw_descr);
 
strncpy((char *)load->file_id, header->firmware_info.sw_part, 20);
load->file_id_len = strlen(header->firmware_info.sw_part) + 1;
diff --git a/openbsc/src/libbsc/abis_nm.c b/openbsc/src/libbsc/abis_nm.c
index 33a23a2..fb9c123 100644
--- a/openbsc/src/libbsc/abis_nm.c
+++ b/openbsc/src/libbsc/abis_nm.c
@@ -413,93 +413,50 @@
 
 /* Activate the specified software into the BTS */
 static int ipacc_sw_activate(struct gsm_bts *bts, uint8_t obj_class, uint8_t 
i0, uint8_t i1,
-uint8_t i2, const uint8_t *sw_desc, uint8_t 
swdesc_len)
+uint8_t i2, const struct abis_nm_sw_descr *sw_desc)
 {
struct abis_om_hdr *oh;
struct msgb *msg = nm_msgb_alloc();
-   uint8_t len = swdesc_len;
-   uint8_t *trailer;
+   

[PATCH] openbsc[master]: Use libosmocore for SW Description parsing

[Max]

Hello Jenkins Builder,

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

https://gerrit.osmocom.org/2166

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

Use libosmocore for SW Description parsing

Requires libosmocore with Ib63b6b5e83b8914864fc7edd789f8958cdc993cd.

Change-Id: Ib94db414e94a2a1f234ac6f1cb346dca1c7a8be3
---
M openbsc/include/openbsc/abis_nm.h
M openbsc/src/ipaccess/ipaccess-config.c
M openbsc/src/libbsc/abis_nm.c
M openbsc/tests/abis/abis_test.c
M openbsc/tests/abis/abis_test.ok
5 files changed, 68 insertions(+), 130 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/66/2166/3

diff --git a/openbsc/include/openbsc/abis_nm.h 
b/openbsc/include/openbsc/abis_nm.h
index 2465452..9dcb91f 100644
--- a/openbsc/include/openbsc/abis_nm.h
+++ b/openbsc/include/openbsc/abis_nm.h
@@ -68,18 +68,6 @@
int (*sw_act_req)(struct msgb *);
 };
 
-struct abis_nm_sw_descr {
-   /* where does it start? how long is it? */
-   const uint8_t   *start;
-   size_t  len;
-
-   /* the parsed data */
-   const uint8_t   *file_id;
-   uint16_tfile_id_len;
-   const uint8_t   *file_ver;
-   uint16_tfile_ver_len;
-};
-
 extern int abis_nm_rcvmsg(struct msgb *msg);
 
 int abis_nm_tlv_parse(struct tlv_parsed *tp, struct gsm_bts *bts, const 
uint8_t *buf, int len);
@@ -182,7 +170,7 @@
 void abis_nm_queue_send_next(struct gsm_bts *bts); /* for bs11_config. */
 
 int abis_nm_parse_sw_config(const uint8_t *data, const size_t len,
-   struct abis_nm_sw_descr *res, const int res_len);
+   struct abis_nm_sw_descr *res, const int res_len);
 int abis_nm_select_newest_sw(const struct abis_nm_sw_descr *sw, const size_t 
len);
 
 /* Helper functions for updating attributes */
diff --git a/openbsc/src/ipaccess/ipaccess-config.c 
b/openbsc/src/ipaccess/ipaccess-config.c
index 0c3f888..ca5f195 100644
--- a/openbsc/src/ipaccess/ipaccess-config.c
+++ b/openbsc/src/ipaccess/ipaccess-config.c
@@ -52,6 +52,7 @@
 #include 
 #include 
 #include 
+#include 
 
 struct gsm_network *bsc_gsmnet;
 
@@ -70,17 +71,9 @@
 static int found_trx = 0;
 static int loop_tests = 0;
 
-struct sw_load {
-   uint8_t file_id[255];
-   uint8_t file_id_len;
-
-   uint8_t file_version[255];
-   uint8_t file_version_len;
-};
-
 static void *tall_ctx_config = NULL;
-static struct sw_load *sw_load1 = NULL;
-static struct sw_load *sw_load2 = NULL;
+static struct abis_nm_sw_descr *sw_load1 = NULL;
+static struct abis_nm_sw_descr *sw_load2 = NULL;
 
 /*
 static uint8_t prim_oml_attr[] = { 0x95, 0x00, 7, 0x88, 192, 168, 100, 11, 
0x00, 0x00 };
@@ -344,19 +337,11 @@
msg->l3h = >l2h[3];
 
/* activate software */
-   if (sw_load1) {
-   msgb_v_put(msg, NM_ATT_SW_DESCR);
-   msgb_tl16v_put(msg, NM_ATT_FILE_ID, 
sw_load1->file_id_len, sw_load1->file_id);
-   msgb_tl16v_put(msg, NM_ATT_FILE_VERSION, 
sw_load1->file_version_len,
-   sw_load1->file_version);
-   }
+   if (sw_load1)
+   abis_nm_put_sw_descr(msg, sw_load1, true);
 
-   if (sw_load2) {
-   msgb_v_put(msg, NM_ATT_SW_DESCR);
-   msgb_tl16v_put(msg, NM_ATT_FILE_ID, 
sw_load2->file_id_len, sw_load2->file_id);
-   msgb_tl16v_put(msg, NM_ATT_FILE_VERSION, 
sw_load2->file_version_len,
-   sw_load2->file_version);
-   }
+   if (sw_load2)
+   abis_nm_put_sw_descr(msg, sw_load2, true);
 
/* fill in the data */
msg->l2h[0] = NM_ATT_IPACC_CUR_SW_CFG;
@@ -618,11 +603,11 @@
return 0;
 }
 
-static struct sw_load *create_swload(struct sdp_header *header)
+static struct abis_nm_sw_descr *create_swload(struct sdp_header *header)
 {
-   struct sw_load *load;
+   struct abis_nm_sw_descr *load;
 
-   load = talloc_zero(tall_ctx_config, struct sw_load);
+   load = talloc_zero(tall_ctx_config, struct abis_nm_sw_descr);
 
strncpy((char *)load->file_id, header->firmware_info.sw_part, 20);
load->file_id_len = strlen(header->firmware_info.sw_part) + 1;
diff --git a/openbsc/src/libbsc/abis_nm.c b/openbsc/src/libbsc/abis_nm.c
index 33a23a2..944ece6 100644
--- a/openbsc/src/libbsc/abis_nm.c
+++ b/openbsc/src/libbsc/abis_nm.c
@@ -413,93 +413,46 @@
 
 /* Activate the specified software into the BTS */
 static int ipacc_sw_activate(struct gsm_bts *bts, uint8_t obj_class, uint8_t 
i0, uint8_t i1,
-uint8_t i2, const uint8_t *sw_desc, uint8_t 
swdesc_len)
+uint8_t i2, const struct abis_nm_sw_descr *sw_desc)
 {
struct abis_om_hdr *oh;
struct msgb *msg = nm_msgb_alloc();
-   uint8_t len = swdesc_len;
-   uint8_t *trailer;
+