[PATCH] libosmocore[master]: add gsm0808_{enc,dec}_cell_id

[Neels Hofmeyr]

Hello Jenkins Builder,

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

https://gerrit.osmocom.org/7789

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

add gsm0808_{enc,dec}_cell_id

Clarify semantics and micro-optimise for the case of single Cell Identifer IEs.
Test in gsm0808_test.c

So far we have gsm0808_enc_cell_id_list2(), but there also exist instances of
single Cell Identifiers (3GPP TS 48.008 3.2.2.17).

It is possible to decode the same using the cell identifier list API, but this
forces the caller to also keep a full struct gsm0808_cell_id_list2 with all its
127 entries around.

E.g. for handover, there are two Cell Identifiers (Serving and Target); I'd
need two full cell id lists for each, and these would be dynamically allocated
for each handover operation, whether it uses them or not.

Related: OS#2283 (inter-BSC HO, BSC side)
Change-Id: I9f9c528965775698ab62ac386af0516192c4b0cc
---
M include/osmocom/gsm/gsm0808_utils.h
M src/gsm/gsm0808_utils.c
M src/gsm/libosmogsm.map
M tests/gsm0808/gsm0808_test.c
M tests/gsm0808/gsm0808_test.ok
5 files changed, 280 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/89/7789/3

diff --git a/include/osmocom/gsm/gsm0808_utils.h 
b/include/osmocom/gsm/gsm0808_utils.h
index 34eae43..8e71b43 100644
--- a/include/osmocom/gsm/gsm0808_utils.h
+++ b/include/osmocom/gsm/gsm0808_utils.h
@@ -31,20 +31,27 @@
  /*! (225-1)/2 is the maximum number of elements in a cell identifier list. */
 #define GSM0808_CELL_ID_LIST2_MAXLEN   127
 
-/*! Parsed representation of a cell identifier list IE. */
+/*! Instead of this, use either struct gsm0808_cell_id or 
gsm0808_cell_id_list2.
+ * All elements contain parsed representations of the data in the 
corresponding IE, in host-byte order.
+ */
+union gsm0808_cell_id_u {
+   struct osmo_cell_global_id  global;
+   struct osmo_lac_and_ci_id   lac_and_ci;
+   uint16_tci;
+   struct osmo_location_area_idlai_and_lac;
+   uint16_tlac;
+};
+
+/*! Parsed representation of Cell Identifier IE (3GPP TS 48.008 3.2.2.17) */
+struct gsm0808_cell_id {
+   enum CELL_IDENT id_discr;
+   union gsm0808_cell_id_u id;
+};
+
+/*! Parsed representation of a Cell Identifier List IE (3GPP TS 48.008 
3.2.2.27). */
 struct gsm0808_cell_id_list2 {
enum CELL_IDENT id_discr;
-   union {
-   /*!
-* All elements of these arrays contain parsed representations 
of the
-* data in the corresponding IE, in host-byte order.
-*/
-   struct osmo_cell_global_id  global;
-   struct osmo_lac_and_ci_id   lac_and_ci;
-   uint16_tci;
-   struct osmo_location_area_idlai_and_lac;
-   uint16_tlac;
-   } id_list[GSM0808_CELL_ID_LIST2_MAXLEN];
+   union gsm0808_cell_id_u id_list[GSM0808_CELL_ID_LIST2_MAXLEN];
unsigned int id_list_len;
 };
 
@@ -78,6 +85,8 @@
 const uint8_t *elem, uint8_t len)
 OSMO_DEPRECATED("use gsm0808_dec_cell_id_list2 
instead");
 int gsm0808_cell_id_list_add(struct gsm0808_cell_id_list2 *dst, const struct 
gsm0808_cell_id_list2 *src);
+uint8_t gsm0808_enc_cell_id(struct msgb *msg, const struct gsm0808_cell_id 
*ci);
+int gsm0808_dec_cell_id(struct gsm0808_cell_id *ci, const uint8_t *elem, 
uint8_t len);
 int gsm0808_chan_type_to_speech_codec(uint8_t perm_spch);
 int gsm0808_speech_codec_from_chan_type(struct gsm0808_speech_codec *sc,
uint8_t perm_spch);
diff --git a/src/gsm/gsm0808_utils.c b/src/gsm/gsm0808_utils.c
index 996456e..73d9362 100644
--- a/src/gsm/gsm0808_utils.c
+++ b/src/gsm/gsm0808_utils.c
@@ -988,6 +988,57 @@
return added;
 }
 
+/*! Encode Cell Identifier IE (3GPP TS 48.008 3.2.2.17).
+ *  \param[out] msg Message Buffer to which IE is to be appended
+ *  \param[in] ci Cell ID to be encoded
+ *  \returns number of bytes appended to \a msg */
+uint8_t gsm0808_enc_cell_id(struct msgb *msg, const struct gsm0808_cell_id *ci)
+{
+   uint8_t rc;
+   uint8_t *ie_tag;
+   struct gsm0808_cell_id_list2 cil = {
+   .id_discr = ci->id_discr,
+   .id_list = { ci->id },
+   .id_list_len = 1,
+   };
+
+   OSMO_ASSERT(msg);
+   OSMO_ASSERT(ci);
+
+   ie_tag = msg->tail;
+   rc = gsm0808_enc_cell_id_list2(msg, );
+
+   if (rc <= 0)
+   return rc;
+
+   *ie_tag = GSM0808_IE_CELL_IDENTIFIER;
+   return rc;
+}
+
+/*! Decode Cell Identifier IE (3GPP TS 48.008 3.2.2.17).
+ *  \param[out] ci Caller-provided memory to store Cell ID.
+ *  \param[in] elem IE value to be decoded.
+ *  \param[in] len Length of \a elem in bytes.
+ *  \returns number of bytes 

[PATCH] libosmocore[master]: add osmo_cgi_name()

[Neels Hofmeyr]

Hello Jenkins Builder,

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

https://gerrit.osmocom.org/7786

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

add osmo_cgi_name()

This will be used by cell idenitifier list code, like upcoming neighbor_ident
VTY in osmo-bsc and regression tests.

Change-Id: Iebc5cdf61b697b1603900993fc265af3eca0cedf
---
M include/osmocom/gsm/gsm23003.h
M src/gsm/gsm23003.c
M src/gsm/libosmogsm.map
3 files changed, 31 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/86/7786/2

diff --git a/include/osmocom/gsm/gsm23003.h b/include/osmocom/gsm/gsm23003.h
index 02e7971..fd4f369 100644
--- a/include/osmocom/gsm/gsm23003.h
+++ b/include/osmocom/gsm/gsm23003.h
@@ -99,6 +99,8 @@
 const char *osmo_plmn_name(const struct osmo_plmn_id *plmn);
 const char *osmo_plmn_name2(const struct osmo_plmn_id *plmn);
 const char *osmo_lai_name(const struct osmo_location_area_id *lai);
+const char *osmo_cgi_name(const struct osmo_cell_global_id *cgi);
+const char *osmo_cgi_name2(const struct osmo_cell_global_id *cgi);
 
 void osmo_plmn_to_bcd(uint8_t *bcd_dst, const struct osmo_plmn_id *plmn);
 void osmo_plmn_from_bcd(const uint8_t *bcd_src, struct osmo_plmn_id *plmn);
diff --git a/src/gsm/gsm23003.c b/src/gsm/gsm23003.c
index 574400d..2c3b21e 100644
--- a/src/gsm/gsm23003.c
+++ b/src/gsm/gsm23003.c
@@ -133,6 +133,33 @@
return buf;
 }
 
+static const char *_cgi_name(const struct osmo_cell_global_id *cgi, char *buf, 
size_t buflen)
+{
+   snprintf(buf, buflen, "%s-%u", osmo_lai_name(>lai), 
cgi->cell_identity);
+   return buf;
+}
+
+/*! Return MCC-MNC-LAC-CI as string, in a static buffer.
+ * \param[in] cgi  CGI to encode.
+ * \returns Static string buffer.
+ */
+const char *osmo_cgi_name(const struct osmo_cell_global_id *cgi)
+{
+   static char buf[32];
+   return _cgi_name(cgi, buf, sizeof(buf));
+}
+
+/*! Same as osmo_cgi_name(), but uses a different static buffer.
+ * Useful for printing two distinct CGIs in the same printf format.
+ * \param[in] cgi  CGI to encode.
+ * \returns Static string buffer.
+ */
+const char *osmo_cgi_name2(const struct osmo_cell_global_id *cgi)
+{
+   static char buf[32];
+   return _cgi_name(cgi, buf, sizeof(buf));
+}
+
 static void to_bcd(uint8_t *bcd, uint16_t val)
 {
bcd[2] = val % 10;
diff --git a/src/gsm/libosmogsm.map b/src/gsm/libosmogsm.map
index 31717d5..a6ea47d 100644
--- a/src/gsm/libosmogsm.map
+++ b/src/gsm/libosmogsm.map
@@ -277,6 +277,8 @@
 osmo_plmn_name2;
 osmo_lai_name;
 osmo_rai_name;
+osmo_cgi_name;
+osmo_cgi_name2;
 osmo_mnc_from_str;
 osmo_mnc_cmp;
 osmo_plmn_cmp;

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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iebc5cdf61b697b1603900993fc265af3eca0cedf
Gerrit-PatchSet: 2
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder

[PATCH] libosmocore[master]: test_gsm0808_enc_dec_cell_id_list_lac(): validate encoded bytes

[Neels Hofmeyr]

Review at  https://gerrit.osmocom.org/7790

test_gsm0808_enc_dec_cell_id_list_lac(): validate encoded bytes

Change-Id: I81b1ffbe6a5ec566c112492c2cbaf99c018c45bb
---
M tests/gsm0808/gsm0808_test.c
M tests/gsm0808/gsm0808_test.ok
2 files changed, 9 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/90/7790/1

diff --git a/tests/gsm0808/gsm0808_test.c b/tests/gsm0808/gsm0808_test.c
index 13ae3ce..4e91832 100644
--- a/tests/gsm0808/gsm0808_test.c
+++ b/tests/gsm0808/gsm0808_test.c
@@ -750,6 +750,13 @@
msgb_free(msg);
 }
 
+#define EXPECT_ENCODED(hexstr) do { \
+   const char *enc_str = msgb_hexdump(msg); \
+   printf("%s: encoded: %s(rc = %u)\n", __func__, enc_str, 
rc_enc); \
+   OSMO_ASSERT(strcmp(enc_str, hexstr " ") == 0); \
+   OSMO_ASSERT(rc_enc == msg->len); \
+   } while(0)
+
 static void test_gsm0808_enc_dec_cell_id_list_lac()
 {
struct gsm0808_cell_id_list2 enc_cil;
@@ -767,7 +774,7 @@
 
msg = msgb_alloc(1024, "output buffer");
rc_enc = gsm0808_enc_cell_id_list2(msg, _cil);
-   OSMO_ASSERT(rc_enc == 9);
+   EXPECT_ENCODED("1a 07 05 56 78 00 00 00 00");
 
rc_dec = gsm0808_dec_cell_id_list2(_cil, msg->data + 2, msg->len - 
2);
OSMO_ASSERT(rc_dec == 7);
@@ -1255,13 +1262,6 @@
 
printf("--- %s done\n", __func__);
 }
-
-#define EXPECT_ENCODED(hexstr) do { \
-   const char *enc_str = msgb_hexdump(msg); \
-   printf("%s: encoded: %s(rc = %u)\n", __func__, enc_str, 
rc_enc); \
-   OSMO_ASSERT(strcmp(enc_str, hexstr " ") == 0); \
-   OSMO_ASSERT(rc_enc == msg->len); \
-   } while(0)
 
 static void test_gsm0808_enc_dec_cell_id_lac()
 {
diff --git a/tests/gsm0808/gsm0808_test.ok b/tests/gsm0808/gsm0808_test.ok
index 1f51aea..6e5e118 100644
--- a/tests/gsm0808/gsm0808_test.ok
+++ b/tests/gsm0808/gsm0808_test.ok
@@ -19,6 +19,7 @@
 Testing creating Paging Request
 Testing creating DTAP
 Testing prepend DTAP
+test_gsm0808_enc_dec_cell_id_list_lac: encoded: 1a 07 05 56 78 00 00 00 00 (rc 
= 9)
 --- test_cell_id_list_add
  cell_id_list cgi[0] = {
  }

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I81b1ffbe6a5ec566c112492c2cbaf99c018c45bb
Gerrit-PatchSet: 1
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 

[PATCH] libosmocore[master]: test_gsm0808_enc_dec_cell_id_list_lac(): populate all LACs

[Neels Hofmeyr]

Review at  https://gerrit.osmocom.org/7791

test_gsm0808_enc_dec_cell_id_list_lac(): populate all LACs

Change-Id: I7535166a2827c03a954fe72d5d99217e4f25868f
---
M tests/gsm0808/gsm0808_test.c
M tests/gsm0808/gsm0808_test.ok
2 files changed, 4 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/91/7791/1

diff --git a/tests/gsm0808/gsm0808_test.c b/tests/gsm0808/gsm0808_test.c
index 4e91832..8dcb1e0 100644
--- a/tests/gsm0808/gsm0808_test.c
+++ b/tests/gsm0808/gsm0808_test.c
@@ -768,13 +768,13 @@
memset(_cil, 0, sizeof(enc_cil));
enc_cil.id_discr = CELL_IDENT_LAC;
enc_cil.id_list[0].lac = 0x0124;
-   enc_cil.id_list[0].lac = 0xABCD;
-   enc_cil.id_list[0].lac = 0x5678;
+   enc_cil.id_list[1].lac = 0xABCD;
+   enc_cil.id_list[2].lac = 0x5678;
enc_cil.id_list_len = 3;
 
msg = msgb_alloc(1024, "output buffer");
rc_enc = gsm0808_enc_cell_id_list2(msg, _cil);
-   EXPECT_ENCODED("1a 07 05 56 78 00 00 00 00");
+   EXPECT_ENCODED("1a 07 05 01 24 ab cd 56 78");
 
rc_dec = gsm0808_dec_cell_id_list2(_cil, msg->data + 2, msg->len - 
2);
OSMO_ASSERT(rc_dec == 7);
diff --git a/tests/gsm0808/gsm0808_test.ok b/tests/gsm0808/gsm0808_test.ok
index 6e5e118..1dbe2f8 100644
--- a/tests/gsm0808/gsm0808_test.ok
+++ b/tests/gsm0808/gsm0808_test.ok
@@ -19,7 +19,7 @@
 Testing creating Paging Request
 Testing creating DTAP
 Testing prepend DTAP
-test_gsm0808_enc_dec_cell_id_list_lac: encoded: 1a 07 05 56 78 00 00 00 00 (rc 
= 9)
+test_gsm0808_enc_dec_cell_id_list_lac: encoded: 1a 07 05 01 24 ab cd 56 78 (rc 
= 9)
 --- test_cell_id_list_add
  cell_id_list cgi[0] = {
  }

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I7535166a2827c03a954fe72d5d99217e4f25868f
Gerrit-PatchSet: 1
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 

[PATCH] libosmocore[master]: add gsm0808_cell_id_list_add() to combine two cell identifie...

[Neels Hofmeyr]

Review at  https://gerrit.osmocom.org/7787

add gsm0808_cell_id_list_add() to combine two cell identifier lists

This will be used by the upcoming neighbor_ident API in osmo-bsc, where the vty
interface allows composing neihbor BSS cell identifier lists, and we want to
allow adding individual items from individual user commands.

It will also be useful to accumulate cell identifiers in case a subscriber sees
multiple alternative cells from a neighboring BSS, and we want to pass these on
to the MSC in a Handover Required.

Related: OS#2283 (inter-BSC HO, BSC side)
Change-Id: I5781f5fa5339c92ab2e2620489b002829d206925
---
M include/osmocom/gsm/gsm0808_utils.h
M src/gsm/gsm0808_utils.c
M src/gsm/libosmogsm.map
M tests/gsm0808/gsm0808_test.c
M tests/gsm0808/gsm0808_test.ok
5 files changed, 361 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/87/7787/1

diff --git a/include/osmocom/gsm/gsm0808_utils.h 
b/include/osmocom/gsm/gsm0808_utils.h
index 363fc39..34eae43 100644
--- a/include/osmocom/gsm/gsm0808_utils.h
+++ b/include/osmocom/gsm/gsm0808_utils.h
@@ -77,6 +77,7 @@
 int gsm0808_dec_cell_id_list(struct gsm0808_cell_id_list *cil,
 const uint8_t *elem, uint8_t len)
 OSMO_DEPRECATED("use gsm0808_dec_cell_id_list2 
instead");
+int gsm0808_cell_id_list_add(struct gsm0808_cell_id_list2 *dst, const struct 
gsm0808_cell_id_list2 *src);
 int gsm0808_chan_type_to_speech_codec(uint8_t perm_spch);
 int gsm0808_speech_codec_from_chan_type(struct gsm0808_speech_codec *sc,
uint8_t perm_spch);
diff --git a/src/gsm/gsm0808_utils.c b/src/gsm/gsm0808_utils.c
index e4872b8..996456e 100644
--- a/src/gsm/gsm0808_utils.c
+++ b/src/gsm/gsm0808_utils.c
@@ -909,6 +909,85 @@
return (int)(elem - old_elem);
 }
 
+static bool same_cell_id_list_entries(const struct gsm0808_cell_id_list2 *a, 
int ai,
+ const struct gsm0808_cell_id_list2 *b, 
int bi)
+{
+   struct gsm0808_cell_id_list2 tmp = {
+   .id_discr = a->id_discr,
+   .id_list_len = 1,
+   };
+   uint8_t buf_a[32 + sizeof(struct msgb)];
+   uint8_t buf_b[32 + sizeof(struct msgb)];
+   struct msgb *msg_a = (void*)buf_a;
+   struct msgb *msg_b = (void*)buf_b;
+
+   msg_a->data_len = 32;
+   msg_b->data_len = 32;
+   msgb_reset(msg_a);
+   msgb_reset(msg_b);
+
+   if (a->id_discr != b->id_discr)
+   return false;
+   if (ai >= a->id_list_len
+   || bi >= b->id_list_len)
+   return false;
+
+   tmp.id_list[0] = a->id_list[ai];
+   gsm0808_enc_cell_id_list2(msg_a, );
+
+   tmp.id_list[0] = b->id_list[bi];
+   gsm0808_enc_cell_id_list2(msg_b, );
+
+   if (msg_a->len != msg_b->len)
+   return false;
+   if (memcmp(msg_a->data, msg_b->data, msg_a->len))
+   return false;
+
+   return true;
+}
+
+/*! Append entries from one Cell Identifier List to another.
+ * The cell identifier types must be identical between the two lists.
+ * \param dst[out]  Append entries to this list.
+ * \param src[in]   Append these entries to \a dst.
+ * \returns the nr of items added, or negative on error: -EINVAL if the 
id_discr mismatch
+ *   between the lists, -ENOSPC if the destination list does not have enough 
space. If an error is
+ *   returned, \a dst may have already been changed (particularly on -ENOSPC). 
Note that a return value
+ *   of zero may occur when the src->id_list_len is zero, or when all entries 
from \a src already exist
+ *   in \a dst, and does not indicate error per se. */
+int gsm0808_cell_id_list_add(struct gsm0808_cell_id_list2 *dst, const struct 
gsm0808_cell_id_list2 *src)
+{
+   int i, j;
+   int added = 0;
+
+   if (dst->id_list_len == 0
+   && dst->id_discr != CELL_IDENT_BSS)
+   dst->id_discr = src->id_discr;
+   else if (dst->id_discr != src->id_discr)
+   return -EINVAL;
+
+   for (i = 0; i < src->id_list_len; i++) {
+   /* don't add duplicate entries */
+   bool skip = false;
+   for (j = 0; j < dst->id_list_len; j++) {
+   if (same_cell_id_list_entries(dst, j, src, i)) {
+   skip = true;
+   break;
+   }
+   }
+   if (skip)
+   continue;
+
+   if (dst->id_list_len >= ARRAY_SIZE(dst->id_list))
+   return -ENOSPC;
+
+   dst->id_list[dst->id_list_len++] = src->id_list[i];
+   added ++;
+   }
+
+   return added;
+}
+
 /*! Convert the representation of the permitted speech codec identifier
  *  that is used in struct gsm0808_channel_type to the speech codec
  *  representation we use in struct gsm0808_speech_codec.
diff --git a/src/gsm/libosmogsm.map 

[PATCH] libosmocore[master]: add tlv_parse2(), capable of multiple instances of the same IE

[Neels Hofmeyr]

Review at  https://gerrit.osmocom.org/7788

add tlv_parse2(), capable of multiple instances of the same IE

Allow passing multiple struct tlv_parsed in an array, to allow parsing as many
repeated IEs as are expected by the caller.

>From tlv_parse(), call tlv_parse2() with dec_multiple = 1 to yield the previous
behavior. tlv_parse() remains valid API.

An example of multiple IEs is the BSSMAP Handover Request, containing Cell
Identifier (Serving) and Cell Identifier (Target), both defined by 3GPP TS
48.008 3.2.2.17 with identical IE tags; both are mandatory.

Related: OS#2283 (inter-BSC HO, BSC side)
Change-Id: Id04008eaf0a1cafdbdc11b7efc556e3035b1c84d
---
M include/osmocom/gsm/tlv.h
M src/gsm/libosmogsm.map
M src/gsm/tlv_parser.c
M tests/tlv/tlv_test.c
4 files changed, 83 insertions(+), 24 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/88/7788/1

diff --git a/include/osmocom/gsm/tlv.h b/include/osmocom/gsm/tlv.h
index 8654893..996f6aa 100644
--- a/include/osmocom/gsm/tlv.h
+++ b/include/osmocom/gsm/tlv.h
@@ -433,6 +433,9 @@
   const uint8_t *buf, int buf_len);
 int tlv_parse(struct tlv_parsed *dec, const struct tlv_definition *def,
  const uint8_t *buf, int buf_len, uint8_t lv_tag, uint8_t lv_tag2);
+int tlv_parse2(struct tlv_parsed *dec, int dec_multiples,
+  const struct tlv_definition *def, const uint8_t *buf, int 
buf_len,
+  uint8_t lv_tag, uint8_t lv_tag2);
 /* take a master (src) tlv def and fill up all empty slots in 'dst' */
 void tlv_def_patch(struct tlv_definition *dst, const struct tlv_definition 
*src);
 
diff --git a/src/gsm/libosmogsm.map b/src/gsm/libosmogsm.map
index d99121e..4d009e0 100644
--- a/src/gsm/libosmogsm.map
+++ b/src/gsm/libosmogsm.map
@@ -406,6 +406,7 @@
 tlv_def_patch;
 tlv_dump;
 tlv_parse;
+tlv_parse2;
 tlv_parse_one;
 tvlv_att_def;
 vtvlv_gan_att_def;
diff --git a/src/gsm/tlv_parser.c b/src/gsm/tlv_parser.c
index f693971..6e089f7 100644
--- a/src/gsm/tlv_parser.c
+++ b/src/gsm/tlv_parser.c
@@ -220,7 +220,12 @@
return len;
 }
 
-/*! Parse an entire buffer of TLV encoded Information Elements
+/*! Parse an entire buffer of TLV encoded Information Elements.
+ * In case of multiple occurences of an IE, keep only the first occurence.
+ * Most GSM related protocols clearly indicate that in case of duplicate
+ * IEs, only the first occurrence shall be used, while any further occurrences
+ * shall be ignored.  See e.g. 3GPP TS 24.008 Section 8.6.3.
+ * For multiple occurences, use tlv_parse2().
  *  \param[out] dec caller-allocated pointer to \ref tlv_parsed
  *  \param[in] def structure defining the valid TLV tags / configurations
  *  \param[in] buf the input data buffer to be parsed
@@ -233,38 +238,77 @@
  const uint8_t *buf, int buf_len, uint8_t lv_tag,
  uint8_t lv_tag2)
 {
+   return tlv_parse2(dec, 1, def, buf, buf_len, lv_tag, lv_tag2);
+}
+
+/*! Like tlv_parse(), but capable of decoding multiple occurences of the same 
IE.
+ * Parse an entire buffer of TLV encoded Information Elements.
+ * To decode multiple occurences of IEs, provide in dec an _array_ of 
tlv_parsed, and
+ * pass the size of that array in dec_multiples. The first occurence of each IE
+ * is stored in dec[0], the second in dec[1] and so forth. If there are more
+ * occurences than the array length given in dec_multiples, the remaining
+ * occurences are dropped.
+ *  \param[out] dec caller-allocated pointer to \ref tlv_parsed
+ *  \param[in] dec_multiples length of the tlv_parsed[] in \a dec.
+ *  \param[in] def structure defining the valid TLV tags / configurations
+ *  \param[in] buf the input data buffer to be parsed
+ *  \param[in] buf_len length of the input data buffer
+ *  \param[in] lv_tag an initial LV tag at the start of the buffer
+ *  \param[in] lv_tag2 a second initial LV tag following the \a lv_tag
+ *  \returns number of TLV entries parsed; negative in case of error
+ */
+int tlv_parse2(struct tlv_parsed *dec, int dec_multiples,
+  const struct tlv_definition *def, const uint8_t *buf, int 
buf_len,
+  uint8_t lv_tag, uint8_t lv_tag2)
+{
int ofs = 0, num_parsed = 0;
uint16_t len;
+   int dec_i;
 
-   memset(dec, 0, sizeof(*dec));
+   for (dec_i = 0; dec_i < dec_multiples; dec_i++)
+   memset([dec_i], 0, sizeof(*dec));
 
if (lv_tag) {
+   const uint8_t *val;
+   uint16_t parsed_len;
if (ofs > buf_len)
return -1;
-   dec->lv[lv_tag].val = [ofs+1];
-   dec->lv[lv_tag].len = buf[ofs];
-   len = dec->lv[lv_tag].len + 1;
-   if (ofs + len > buf_len) {
-   dec->lv[lv_tag].val = NULL;
-   dec->lv[lv_tag].len = 0;
+   val = [ofs+1];
+   len = buf[ofs];
+   parsed_len = len + 1;
+   if (ofs + parsed_len > 

[PATCH] libosmocore[master]: add gsm0808_{enc,dec}_cell_id

[Neels Hofmeyr]

Review at  https://gerrit.osmocom.org/7789

add gsm0808_{enc,dec}_cell_id

Clarify semantics and micro-optimise for the case of single Cell Identifer IEs.
Test in gsm0808_test.c

So far we have gsm0808_enc_cell_id_list2(), but there also exist instances of
single Cell Identifiers (3GPP TS 48.008 3.2.2.17).

It is possible to decode the same using the cell identifier list API, but this
forces the caller to also keep a full struct gsm0808_cell_id_list2 with all its
127 entries around.

E.g. for handover, there are two Cell Identifiers (Serving and Target); I'd
need two full cell id lists for each, and these would be dynamically allocated
for each handover operation, whether it uses them or not.

Related: OS#2283 (inter-BSC HO, BSC side)
Change-Id: I9f9c528965775698ab62ac386af0516192c4b0cc
---
M include/osmocom/gsm/gsm0808_utils.h
M src/gsm/gsm0808_utils.c
M src/gsm/libosmogsm.map
M tests/gsm0808/gsm0808_test.c
M tests/gsm0808/gsm0808_test.ok
5 files changed, 251 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/89/7789/1

diff --git a/include/osmocom/gsm/gsm0808_utils.h 
b/include/osmocom/gsm/gsm0808_utils.h
index 34eae43..8e71b43 100644
--- a/include/osmocom/gsm/gsm0808_utils.h
+++ b/include/osmocom/gsm/gsm0808_utils.h
@@ -31,20 +31,27 @@
  /*! (225-1)/2 is the maximum number of elements in a cell identifier list. */
 #define GSM0808_CELL_ID_LIST2_MAXLEN   127
 
-/*! Parsed representation of a cell identifier list IE. */
+/*! Instead of this, use either struct gsm0808_cell_id or 
gsm0808_cell_id_list2.
+ * All elements contain parsed representations of the data in the 
corresponding IE, in host-byte order.
+ */
+union gsm0808_cell_id_u {
+   struct osmo_cell_global_id  global;
+   struct osmo_lac_and_ci_id   lac_and_ci;
+   uint16_tci;
+   struct osmo_location_area_idlai_and_lac;
+   uint16_tlac;
+};
+
+/*! Parsed representation of Cell Identifier IE (3GPP TS 48.008 3.2.2.17) */
+struct gsm0808_cell_id {
+   enum CELL_IDENT id_discr;
+   union gsm0808_cell_id_u id;
+};
+
+/*! Parsed representation of a Cell Identifier List IE (3GPP TS 48.008 
3.2.2.27). */
 struct gsm0808_cell_id_list2 {
enum CELL_IDENT id_discr;
-   union {
-   /*!
-* All elements of these arrays contain parsed representations 
of the
-* data in the corresponding IE, in host-byte order.
-*/
-   struct osmo_cell_global_id  global;
-   struct osmo_lac_and_ci_id   lac_and_ci;
-   uint16_tci;
-   struct osmo_location_area_idlai_and_lac;
-   uint16_tlac;
-   } id_list[GSM0808_CELL_ID_LIST2_MAXLEN];
+   union gsm0808_cell_id_u id_list[GSM0808_CELL_ID_LIST2_MAXLEN];
unsigned int id_list_len;
 };
 
@@ -78,6 +85,8 @@
 const uint8_t *elem, uint8_t len)
 OSMO_DEPRECATED("use gsm0808_dec_cell_id_list2 
instead");
 int gsm0808_cell_id_list_add(struct gsm0808_cell_id_list2 *dst, const struct 
gsm0808_cell_id_list2 *src);
+uint8_t gsm0808_enc_cell_id(struct msgb *msg, const struct gsm0808_cell_id 
*ci);
+int gsm0808_dec_cell_id(struct gsm0808_cell_id *ci, const uint8_t *elem, 
uint8_t len);
 int gsm0808_chan_type_to_speech_codec(uint8_t perm_spch);
 int gsm0808_speech_codec_from_chan_type(struct gsm0808_speech_codec *sc,
uint8_t perm_spch);
diff --git a/src/gsm/gsm0808_utils.c b/src/gsm/gsm0808_utils.c
index 996456e..1c05576 100644
--- a/src/gsm/gsm0808_utils.c
+++ b/src/gsm/gsm0808_utils.c
@@ -988,6 +988,52 @@
return added;
 }
 
+/*! Encode Cell Identifier IE (3GPP TS 48.008 3.2.2.17).
+ *  \param[out] msg Message Buffer to which IE is to be appended
+ *  \param[in] ci Cell ID to be encoded
+ *  \returns number of bytes appended to \a msg */
+uint8_t gsm0808_enc_cell_id(struct msgb *msg, const struct gsm0808_cell_id *ci)
+{
+   uint8_t rc;
+   uint8_t *ie_tag;
+   struct gsm0808_cell_id_list2 cil = {
+   .id_discr = ci->id_discr,
+   .id_list = { ci->id },
+   .id_list_len = 1,
+   };
+
+   OSMO_ASSERT(msg);
+   OSMO_ASSERT(ci);
+
+   ie_tag = msg->tail;
+   rc = gsm0808_enc_cell_id_list2(msg, );
+
+   if (rc <= 0)
+   return rc;
+
+   *ie_tag = GSM0808_IE_CELL_IDENTIFIER;
+   return rc;
+}
+
+/*! Decode Cell Identifier IE (3GPP TS 48.008 3.2.2.17).
+ *  \param[out] ci Caller-provided memory to store Cell ID.
+ *  \param[in] elem IE value to be decoded.
+ *  \param[in] len Length of \a elem in bytes.
+ *  \returns number of bytes parsed; negative on error */
+int gsm0808_dec_cell_id(struct gsm0808_cell_id *ci, const uint8_t *elem, 

[PATCH] libosmocore[master]: add osmo_cgi_name()

[Neels Hofmeyr]

Review at  https://gerrit.osmocom.org/7786

add osmo_cgi_name()

Change-Id: Iebc5cdf61b697b1603900993fc265af3eca0cedf
---
M include/osmocom/gsm/gsm23003.h
M src/gsm/gsm23003.c
M src/gsm/libosmogsm.map
3 files changed, 22 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/86/7786/1

diff --git a/include/osmocom/gsm/gsm23003.h b/include/osmocom/gsm/gsm23003.h
index 02e7971..fd4f369 100644
--- a/include/osmocom/gsm/gsm23003.h
+++ b/include/osmocom/gsm/gsm23003.h
@@ -99,6 +99,8 @@
 const char *osmo_plmn_name(const struct osmo_plmn_id *plmn);
 const char *osmo_plmn_name2(const struct osmo_plmn_id *plmn);
 const char *osmo_lai_name(const struct osmo_location_area_id *lai);
+const char *osmo_cgi_name(const struct osmo_cell_global_id *cgi);
+const char *osmo_cgi_name2(const struct osmo_cell_global_id *cgi);
 
 void osmo_plmn_to_bcd(uint8_t *bcd_dst, const struct osmo_plmn_id *plmn);
 void osmo_plmn_from_bcd(const uint8_t *bcd_src, struct osmo_plmn_id *plmn);
diff --git a/src/gsm/gsm23003.c b/src/gsm/gsm23003.c
index 574400d..878ebc0 100644
--- a/src/gsm/gsm23003.c
+++ b/src/gsm/gsm23003.c
@@ -133,6 +133,24 @@
return buf;
 }
 
+static const char *_cgi_name(const struct osmo_cell_global_id *cgi, char *buf, 
size_t buflen)
+{
+   snprintf(buf, buflen, "%s-%u", osmo_lai_name(>lai), 
cgi->cell_identity);
+   return buf;
+}
+
+const char *osmo_cgi_name(const struct osmo_cell_global_id *cgi)
+{
+   static char buf[32];
+   return _cgi_name(cgi, buf, sizeof(buf));
+}
+
+const char *osmo_cgi_name2(const struct osmo_cell_global_id *cgi)
+{
+   static char buf[32];
+   return _cgi_name(cgi, buf, sizeof(buf));
+}
+
 static void to_bcd(uint8_t *bcd, uint16_t val)
 {
bcd[2] = val % 10;
diff --git a/src/gsm/libosmogsm.map b/src/gsm/libosmogsm.map
index 31717d5..a6ea47d 100644
--- a/src/gsm/libosmogsm.map
+++ b/src/gsm/libosmogsm.map
@@ -277,6 +277,8 @@
 osmo_plmn_name2;
 osmo_lai_name;
 osmo_rai_name;
+osmo_cgi_name;
+osmo_cgi_name2;
 osmo_mnc_from_str;
 osmo_mnc_cmp;
 osmo_plmn_cmp;

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Iebc5cdf61b697b1603900993fc265af3eca0cedf
Gerrit-PatchSet: 1
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 

[MERGED] osmo-msc[master]: msc conn ref counts: log human readable list of conn owners

[Harald Welte]

Harald Welte has submitted this change and it was merged.

Change subject: msc conn ref counts: log human readable list of conn owners
..


msc conn ref counts: log human readable list of conn owners

Change-Id: I2a09efafbdbdde0399238f7d79feea8612605201
---
M src/libmsc/osmo_msc.c
M tests/msc_vlr/msc_vlr_test_authen_reuse.err
M tests/msc_vlr/msc_vlr_test_call.err
M tests/msc_vlr/msc_vlr_test_gsm_authen.err
M tests/msc_vlr/msc_vlr_test_gsm_ciph.err
M tests/msc_vlr/msc_vlr_test_hlr_reject.err
M tests/msc_vlr/msc_vlr_test_hlr_timeout.err
M tests/msc_vlr/msc_vlr_test_ms_timeout.err
M tests/msc_vlr/msc_vlr_test_no_authen.err
M tests/msc_vlr/msc_vlr_test_reject_concurrency.err
M tests/msc_vlr/msc_vlr_test_rest.err
M tests/msc_vlr/msc_vlr_test_umts_authen.err
12 files changed, 1,107 insertions(+), 1,076 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified




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

Gerrit-MessageType: merged
Gerrit-Change-Id: I2a09efafbdbdde0399238f7d79feea8612605201
Gerrit-PatchSet: 4
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder

osmo-msc[master]: refactor VLR FSM result handling

[Harald Welte]

Patch Set 3: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I27bf8d68737ff1f8dc6d11fb1eac3d391aab0cb1
Gerrit-PatchSet: 3
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-HasComments: No

libosmo-netif[master]: jibuf: Add initial implementation of Jitter Buffer

[Harald Welte]

Patch Set 1: Code-Review-1

(5 comments)

https://gerrit.osmocom.org/#/c/7773/1/include/osmocom/netif/jibuf.h
File include/osmocom/netif/jibuf.h:

Line 1: #ifndef _JIBUF_H_
we genearlly use "#pragma once" these days.


Line 21: struct jibuf {
all new data structures and symbols of osmocom libraries must be prefixed with 
"osmo_", so it should be an "osmo_jibuf", just like all the API calls below.


https://gerrit.osmocom.org/#/c/7773/1/src/jibuf.c
File src/jibuf.c:

Line 1: /* (C) 2017 by Pau Espin Pedrol 
copyright is with your employer sysmocom, you can list yourself as the author 
only, sorry.


Line 6:  * (at your option) any later version.
good idea to add SPDX-License-Identifier to new files right from the beginning.


https://gerrit.osmocom.org/#/c/7773/1/tests/jibuf/jibuf_test.c
File tests/jibuf/jibuf_test.c:

Line 1: /* (C) 207 by Pau Espin Pedrol 
same as my comments in jibuf.c, plus the year is wrong - or I seriously 
underestimated your age :P


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9688ba9c4d5b733b9f29d0f15f73750f9271ef55
Gerrit-PatchSet: 1
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-HasComments: Yes

libosmo-netif[master]: tests: jibuf_tool: Initial commit

[Harald Welte]

Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I92307c8b1483dd488339771462290aae0ae5689a
Gerrit-PatchSet: 1
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-HasComments: No

[MERGED] libosmo-netif[master]: osmux: osmux_xfrm_output_pull: Improve checks and log of mal...

[Harald Welte]

Harald Welte has submitted this change and it was merged.

Change subject: osmux: osmux_xfrm_output_pull: Improve checks and log of 
malformed packets
..


osmux: osmux_xfrm_output_pull: Improve checks and log of malformed packets

Change-Id: I143805bb5ee9f5e3ada46114e380a03ede80df9f
Related: SYS#4182
---
M src/osmux.c
1 file changed, 11 insertions(+), 5 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/src/osmux.c b/src/osmux.c
index a0563d2..03db469 100644
--- a/src/osmux.c
+++ b/src/osmux.c
@@ -85,8 +85,13 @@
case OSMUX_FT_VOICE_AMR:
break;
case OSMUX_FT_DUMMY:
-   msgb_pull(msg, osmux_ft_dummy_size(osmuxh->amr_ft,
-  osmuxh->ctr + 1));
+   len = osmux_ft_dummy_size(osmuxh->amr_ft, osmuxh->ctr + 
1);
+   if (msgb_length(msg) < len) {
+   LOGP(DLMUX, LOGL_ERROR, "Discarding bad Dummy 
FT: %s\n",
+   osmo_hexdump(msg->data, 
msgb_length(msg)));
+   return NULL;
+   }
+   msgb_pull(msg, len);
goto next;
default:
LOGP(DLMUX, LOGL_ERROR, "Discarding unsupported Osmux 
FT %d\n",
@@ -102,9 +107,10 @@
len = osmo_amr_bytes(osmuxh->amr_ft) * (osmuxh->ctr+1) +
sizeof(struct osmux_hdr);
 
-   if (len > msg->len) {
-   LOGP(DLMUX, LOGL_ERROR, "Discarding malformed "
-   "OSMUX message\n");
+   if (msgb_length(msg) < len) {
+   LOGP(DLMUX, LOGL_ERROR,
+   "Discarding malformed OSMUX message: %s\n",
+   osmo_hexdump(msg->data, msgb_length(msg)));
return NULL;
}
 

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I143805bb5ee9f5e3ada46114e380a03ede80df9f
Gerrit-PatchSet: 2
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder

libosmo-netif[master]: osmux: osmux_xfrm_output_pull: Improve checks and log of mal...

[Harald Welte]

Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I143805bb5ee9f5e3ada46114e380a03ede80df9f
Gerrit-PatchSet: 1
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-HasComments: No

libosmo-netif[master]: tests: osmo-pcap-test: Fix pcap includes not found in old ve...

[Harald Welte]

Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I824671a415eb3f35f480c934b9780ff13510011a
Gerrit-PatchSet: 1
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-HasComments: No

[MERGED] libosmo-netif[master]: tests: osmo-pcap-test: Fix pcap includes not found in old ve...

[Harald Welte]

Harald Welte has submitted this change and it was merged.

Change subject: tests: osmo-pcap-test: Fix pcap includes not found in old 
versions
..


tests: osmo-pcap-test: Fix pcap includes not found in old versions

pcap/dlt.h only exists on newer versions of libpcap. On older versions,
same defines are available in pcap/bpf.h, which in newer versions
include pcap/dlt.h, so we are always fine include pcap/bpf.h.
As a side note, there's a lots of comments in pcap/dlt.h stating that
those symbols used to reside in pcap/bpf.h but were moved there at some
point.

Change-Id: I824671a415eb3f35f480c934b9780ff13510011a
---
M tests/osmo-pcap-test/l2_eth.c
M tests/osmo-pcap-test/l2_sll.c
2 files changed, 2 insertions(+), 2 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/tests/osmo-pcap-test/l2_eth.c b/tests/osmo-pcap-test/l2_eth.c
index 3171fd7..34f003a 100644
--- a/tests/osmo-pcap-test/l2_eth.c
+++ b/tests/osmo-pcap-test/l2_eth.c
@@ -12,7 +12,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 
 #include "proto.h"
 
diff --git a/tests/osmo-pcap-test/l2_sll.c b/tests/osmo-pcap-test/l2_sll.c
index 5a600ff..0f44e61 100644
--- a/tests/osmo-pcap-test/l2_sll.c
+++ b/tests/osmo-pcap-test/l2_sll.c
@@ -12,7 +12,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 
 #include "proto.h"
 

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I824671a415eb3f35f480c934b9780ff13510011a
Gerrit-PatchSet: 1
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder

osmo-bsc[master]: fix handling of state changes in acc ramping

[Pau Espin Pedrol]

Patch Set 3: Code-Review-1

(5 comments)

https://gerrit.osmocom.org/#/c/7784/3/src/libbsc/acc_ramp.c
File src/libbsc/acc_ramp.c:

Line 182:   if (trx_is_usable(trx)) /* cross-check with 
operational state */
As discussed a few mins ago, when coming from S_NM_STATECHG_OPER signal in 
abis_nm_rx_statechg_rep, it can be that both operational and administrative 
status change at the same time (same call to this function).

Case: admin LOCKED->UNLOCKED + oper DISABLED->ENABLED, we are not enabling 
ramping here because trx_is_usable() probably checks the current/old status 
which is still DISABLED.


Line 190:   break;
we can probably removed this break to print an error too, unless NULL is really 
a valid state from the spec point of view (one to which we can change to).


Line 201:   if (trx->mo.nm_state.administrative == 
NM_STATE_UNLOCKED)
Again, we need to check after the new one as it could have be different than 
the old/current one.


Line 208:   break;
same, remove break?


Line 222:   if (trigger_ramping && 
!osmo_timer_pending(_ramp->step_timer))
do we really need this osmo_timer_pending check here? This should never happen 
right? better move it inside the if() and add an OSMO_ASSERT() with it.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib3291439655598fb5ddc891a3e4cc35b0bad250f
Gerrit-PatchSet: 3
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-HasComments: Yes

[ABANDON] osmo-ttcn3-hacks[master]: */regen_makefile.sh: drop multiple naming of main ttcn file

[Neels Hofmeyr]

Neels Hofmeyr has abandoned this change.

Change subject: */regen_makefile.sh: drop multiple naming of main ttcn file
..


Abandoned

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: I94f37d64198ce2e36c5cd8892ccd33987e1fba32
Gerrit-PatchSet: 2
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 

osmo-hlr[master]: rewrite subscriber_update_notify() without calls into luop

[Stefan Sperling]

Patch Set 2:

(6 comments)

https://gerrit.osmocom.org/#/c/7743/1/src/hlr.c
File src/hlr.c:

Line 76:if (peer_len < 0) {
> heh, I wasn't actually aware that we keep the GSUP conn peer info in a tlv 
Yes, now done when we receive a location update.


Line 89:LOGP(DMAIN, LOGL_ERROR, "%s: Error: cannot 
encode MSISDN '%s'\n",
> slightly confusing nomenclature ... IDTAG_SERNR is an IPA peer name, we cal
Fixed.


Line 97:if (gsup.cn_domain == OSMO_GSUP_CN_DOMAIN_PS) {
> (syntactically nicer IMHO, above:
OK.


Line 110: 
> if we do this slightly hacky determination whether the peer is ps or not, I
I've added a cn_domain field to the client connection struct.


Line 115:  "IMSI='%s': Cannot notify GSUP client; 
could not allocate msg buffer "
> so this is copied from lu_op_tx_insert_subscr_data(). Now we have the same 
I wrote this as a separate function first because when I started out I did not 
have the slightest idea of what this function would look like when isolated 
from luop.

Figuring out how to merge the two would be my next step.


Line 127:  co && co->conn && co->conn->server? 
co->conn->server->addr : "unset",
> (in an aside, the MSC and SGSN are both supposed to have a VLR, this commen
Fixed.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I06c43ece2b48dc63d599000eb6d6d51e08963067
Gerrit-PatchSet: 2
Gerrit-Project: osmo-hlr
Gerrit-Branch: master
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Reviewer: neels 
Gerrit-HasComments: Yes

[PATCH] osmo-hlr[master]: rewrite subscriber_update_notify() without calls into luop

[Stefan Sperling]

Hello Jenkins Builder,

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

https://gerrit.osmocom.org/7743

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

rewrite subscriber_update_notify() without calls into luop

This function relied on implementation details of the luop code.
Port what is necessary for an independent Insert Subscriber Data
Tx operation from the luop code into this function.

A next possible step would be to try to merge both of these
into a common implementation, but that is left for future work.

The TTCN3 test TC_vty_msisdn_isd is still passing (it currently
triggers the "circuit switched domain" case because it does not
advertise itself as an SGSN in the IPA unit name).

Change-Id: I06c43ece2b48dc63d599000eb6d6d51e08963067
Related: OS#2785
---
M src/gsup_server.h
M src/hlr.c
M src/luop.c
3 files changed, 79 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/43/7743/2

diff --git a/src/gsup_server.h b/src/gsup_server.h
index 74062d4..1f40f21 100644
--- a/src/gsup_server.h
+++ b/src/gsup_server.h
@@ -4,6 +4,7 @@
 #include 
 #include 
 #include 
+#include 
 
 struct osmo_gsup_conn;
 
@@ -33,6 +34,8 @@
struct tlv_parsed ccm;
 
unsigned int auc_3g_ind; /*!< IND index used for UMTS AKA SQN */
+
+   enum osmo_gsup_cn_domain cn_domain; /* Set when first Location Update 
is received. */
 };
 
 
diff --git a/src/hlr.c b/src/hlr.c
index 4fbc268..80330d1 100644
--- a/src/hlr.c
+++ b/src/hlr.c
@@ -62,19 +62,72 @@
return;
 
llist_for_each_entry(co, _hlr->gs->clients, list) {
-   struct lu_operation *luop = lu_op_alloc_conn(co);
-   if (!luop) {
+   struct osmo_gsup_message gsup = {
+   .message_type = OSMO_GSUP_MSGT_INSERT_DATA_REQUEST
+   };
+   uint8_t *peer;
+   int peer_len;
+   uint8_t msisdn_enc[43]; /* TODO use constant; TS 24.008 
10.5.4.7 */
+   uint8_t apn[APN_MAXLEN];
+   int len;
+   struct msgb *msg_out;
+
+   peer_len = osmo_gsup_conn_ccm_get(co, , IPAC_IDTAG_SERNR);
+   if (peer_len < 0) {
LOGP(DMAIN, LOGL_ERROR,
-  "IMSI='%s': Cannot notify GSUP client, cannot 
allocate lu_operation,"
+  "IMSI='%s': Cannot notify GSUP client, cannot 
get peer name "
   " for %s:%u\n", subscr->imsi,
   co && co->conn && co->conn->server? 
co->conn->server->addr : "unset",
   co && co->conn && co->conn->server? 
co->conn->server->port : 0);
continue;
}
-   luop->subscr = *subscr;
-   luop->state = LU_S_LU_RECEIVED; /* Pretend we received a 
location update. */
-   lu_op_tx_insert_subscr_data(luop);
-   lu_op_free(luop);
+
+   osmo_strlcpy(gsup.imsi, subscr->imsi, GSM23003_IMSI_MAX_DIGITS 
+ 1);
+
+   len = gsm48_encode_bcd_number(msisdn_enc, sizeof(msisdn_enc), 
0, subscr->msisdn);
+   if (len < 1) {
+   LOGP(DMAIN, LOGL_ERROR, "%s: Error: cannot encode 
MSISDN '%s'\n",
+subscr->imsi, subscr->msisdn);
+   continue;
+   }
+   gsup.msisdn_enc = msisdn_enc;
+   gsup.msisdn_enc_len = len;
+
+   gsup.cn_domain = co->cn_domain;
+   if (gsup.cn_domain == OSMO_GSUP_CN_DOMAIN_PS) {
+   /* FIXME: PDP infos - use more fine-grained access 
control
+  instead of wildcard APN */
+   len = osmo_apn_from_str(apn, sizeof(apn), "*");
+   if (len > 0) {
+   gsup.pdp_infos[0].apn_enc = apn;
+   gsup.pdp_infos[0].apn_enc_len = len;
+   gsup.pdp_infos[0].have_info = 1;
+   gsup.num_pdp_infos = 1;
+   /* FIXME: use real value: */
+   gsup.pdp_infos[0].context_id = 1;
+   }
+   }
+
+   /* Send ISD to MSC/SGSN */
+   msg_out = msgb_alloc_headroom(1024+16, 16, "GSUP ISD UPDATE");
+   if (msg_out == NULL) {
+   LOGP(DMAIN, LOGL_ERROR,
+  "IMSI='%s': Cannot notify GSUP client; could not 
allocate msg buffer "
+  " for %s:%u\n", subscr->imsi,
+  co && co->conn && co->conn->server? 
co->conn->server->addr : "unset",
+  co && co->conn && co->conn->server? 
co->conn->server->port : 0);
+   continue;
+   }
+
+   osmo_gsup_encode(msg_out, );
+   if (osmo_gsup_addr_send(g_hlr->gs, peer, 

Build failed in Jenkins: master-asn1c » a1=default,a2=default,a3=default,osmocom-master-debian9 #89

[jenkins]

See 


--
[...truncated 2.08 KB...]

+ ./configure
checking build system type... x86_64-unknown-linux-gnu
checking host system type... x86_64-unknown-linux-gnu
checking target system type... x86_64-unknown-linux-gnu
checking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
checking for gawk... gawk
checking whether make sets $(MAKE)... yes
checking whether to enable maintainer-specific portions of Makefiles... no
checking for style of include used by make... GNU
checking for gcc... gcc
checking for C compiler default output file name... a.out
checking whether the C compiler works... yes
checking whether we are cross compiling... no
checking for suffix of executables... 
checking for suffix of object files... o
checking whether we are using the GNU C compiler... yes
checking whether gcc accepts -g... yes
checking for gcc option to accept ISO C89... none needed
checking dependency style of gcc... gcc3
checking for a sed that does not truncate output... /bin/sed
checking for grep that handles long lines and -e... /bin/grep
checking for egrep... /bin/grep -E
checking for ld used by gcc... /usr/bin/ld
checking if the linker (/usr/bin/ld) is GNU ld... yes
checking for /usr/bin/ld option to reload object files... -r
checking for BSD-compatible nm... /usr/bin/nm -B
checking whether ln -s works... yes
checking how to recognise dependent libraries... pass_all
checking how to run the C preprocessor... gcc -E
checking for ANSI C header files... yes
checking for sys/types.h... yes
checking for sys/stat.h... yes
checking for stdlib.h... yes
checking for string.h... yes
checking for memory.h... yes
checking for strings.h... yes
checking for inttypes.h... yes
checking for stdint.h... yes
checking for unistd.h... yes
checking dlfcn.h usability... yes
checking dlfcn.h presence... yes
checking for dlfcn.h... yes
checking for g++... g++
checking whether we are using the GNU C++ compiler... yes
checking whether g++ accepts -g... yes
checking dependency style of g++... gcc3
checking how to run the C++ preprocessor... g++ -E
checking for g77... no
checking for f77... no
checking for xlf... no
checking for frt... no
checking for pgf77... no
checking for cf77... no
checking for fort77... no
checking for fl32... no
checking for af77... no
checking for f90... no
checking for xlf90... no
checking for pgf90... no
checking for pghpf... no
checking for epcf90... no
checking for gfortran... no
checking for g95... no
checking for f95... no
checking for fort... no
checking for xlf95... no
checking for ifort... no
checking for ifc... no
checking for efc... no
checking for pgf95... no
checking for lf95... no
checking for ftn... no
checking whether we are using the GNU Fortran 77 compiler... no
checking whether  accepts -g... no
checking the maximum length of command line arguments... 32768
checking command to parse /usr/bin/nm -B output from gcc object... ok
checking for objdir... .libs
checking for ar... ar
checking for ranlib... ranlib
checking for strip... strip
checking if gcc supports -fno-rtti -fno-exceptions... no
checking for gcc option to produce PIC... -fPIC
checking if gcc PIC flag -fPIC works... yes
checking if gcc static flag -static works... yes
checking if gcc supports -c -o file.o... yes
checking whether the gcc linker (/usr/bin/ld -m elf_x86_64) supports shared 
libraries... yes
checking whether -lc should be explicitly linked in... no
checking dynamic linker characteristics... GNU/Linux ld.so
checking how to hardcode library paths into programs... immediate
checking whether stripping libraries is possible... yes
checking if libtool supports shared libraries... yes
checking whether to build shared libraries... yes
checking whether to build static libraries... yes
configure: creating libtool
appending configuration tag "CXX" to libtool
checking for ld used by g++... /usr/bin/ld -m elf_x86_64
checking if the linker (/usr/bin/ld -m elf_x86_64) is GNU ld... yes
checking whether the g++ linker (/usr/bin/ld -m elf_x86_64) supports shared 
libraries... yes
checking for g++ option to produce PIC... -fPIC
checking if g++ PIC flag -fPIC works... yes
checking if g++ static flag -static works... yes
checking if g++ supports -c -o file.o... yes
checking whether the g++ linker (/usr/bin/ld -m elf_x86_64) supports shared 
libraries... yes
checking dynamic linker characteristics... GNU/Linux ld.so
checking how to hardcode library paths into programs... immediate
appending configuration tag "F77" to libtool
checking for autoconf... /usr/bin/autoconf
checking for autoheader... /usr/bin/autoheader
checking for gcc... (cached) gcc
checking whether we are using the GNU C compiler... (cached) yes
checking whether gcc accepts -g... (cached) yes
checking for gcc option to accept ISO C89... (cached) none needed
checking 

[PATCH] libosmo-netif[master]: tests: osmo-pcap-test: Fix pcap includes not found in old ve...

[Pau Espin Pedrol]

Review at  https://gerrit.osmocom.org/7785

tests: osmo-pcap-test: Fix pcap includes not found in old versions

pcap/dlt.h only exists on newer versions of libpcap. On older versions,
same defines are available in pcap/bpf.h, which in newer versions
include pcap/dlt.h, so we are always fine include pcap/bpf.h.
As a side note, there's a lots of comments in pcap/dlt.h stating that
those symbols used to reside in pcap/bpf.h but were moved there at some
point.

Change-Id: I824671a415eb3f35f480c934b9780ff13510011a
---
M tests/osmo-pcap-test/l2_eth.c
M tests/osmo-pcap-test/l2_sll.c
2 files changed, 2 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/85/7785/1

diff --git a/tests/osmo-pcap-test/l2_eth.c b/tests/osmo-pcap-test/l2_eth.c
index 3171fd7..34f003a 100644
--- a/tests/osmo-pcap-test/l2_eth.c
+++ b/tests/osmo-pcap-test/l2_eth.c
@@ -12,7 +12,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 
 #include "proto.h"
 
diff --git a/tests/osmo-pcap-test/l2_sll.c b/tests/osmo-pcap-test/l2_sll.c
index 5a600ff..0f44e61 100644
--- a/tests/osmo-pcap-test/l2_sll.c
+++ b/tests/osmo-pcap-test/l2_sll.c
@@ -12,7 +12,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 
 #include "proto.h"
 

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I824671a415eb3f35f480c934b9780ff13510011a
Gerrit-PatchSet: 1
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol 

osmo-hlr[master]: rewrite subscriber_update_notify() without calls into luop

[Neels Hofmeyr]

Patch Set 1:

(8 comments)

This review touches on various topics that are not strictly related to this, 
especially determining the cn_domain.

What I'd like to improve in this patch the most: would it not be easy to factor 
out the common parts instead of code dup and copy FIXME code around?

https://gerrit.osmocom.org/#/c/7743/1//COMMIT_MSG
Commit Message:

Line 18: advertise itself as an SGSN in the IPA unit name).
In the osmo_gsup_message, we have a cn_domain indicator. I suppose our GSUP 
connection dance doesn't send this to the GSUP server, does it? If we did that, 
we would save the need to interpret an IPA peer name as PS or not.

Also thinking, so far, a GSUP client is always either PS or not, so as soon as 
we receive the first lu_op request, we can remember the cn_domain globally for 
that peer. That would also save us from string parsing. (and fail loudly as 
soon as  a peer changes its cn_domain; or allow a peer to collect both ps and 
cs flags, while practically MSC would just remain cs and SGSN would just remain 
ps)


https://gerrit.osmocom.org/#/c/7743/1/src/hlr.c
File src/hlr.c:

Line 76:unit_len = osmo_gsup_conn_ccm_get(co, , 
IPAC_IDTAG_UNITNAME);
heh, I wasn't actually aware that we keep the GSUP conn peer info in a tlv ... 
looks like we might want to decode the key tlvs once upon connecting, instead 
of every time we revisit it?


Line 89:   "IMSI='%s': Cannot notify GSUP client, 
cannot get peer address "
slightly confusing nomenclature ... IDTAG_SERNR is an IPA peer name, we call it 
addr in osmo_gsup_server_ccm_cb(), but that might not be a good choice. The log 
message could say IPA peer name instead.


Line 97:gsup.message_type = OSMO_GSUP_MSGT_INSERT_DATA_REQUEST;
(syntactically nicer IMHO, above:

  struct osmo_gsup_msg gsup = {
  .message_type = OSMO,
  };
 
Initializes all remaining members to zero.)


Line 109:   /* XXX Cast to 'char *' avoids a "pointer targets 
differ in signedness" warning from GCC 7.2.0. */
(does this need an "XXX"? it's ok to cast like this, right?)


Line 110:   is_ps = (strncmp((char *)unit, "SGSN", 4) == 0);
if we do this slightly hacky determination whether the peer is ps or not, I'd 
really want this to be in one central place upon the peer connecting, and then 
keep that flag in struct osmo_gsup_conn. (also possible: have one separate 
central function that does the determination, but again it doesn't make sense 
to re-determine every time we visit this peer)

(hmm, osmo_gsup_conn is defined in osmo-hlr; for some time now we want the 
osmo_ prefix reserved for libosmocore definitions. just noting. could rename if 
we had the time)


Line 115:  instead of wildcard APN */
so this is copied from lu_op_tx_insert_subscr_data(). Now we have the same 
FIXME twice. I don't see what's so hard about separating common code into a 
separate function to serve both use cases and avoid code dup?


Line 127:   /* Send ISD to VLR/SGSN */
(in an aside, the MSC and SGSN are both supposed to have a VLR, this comment 
wants to say "to MSC/SGSN")


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I06c43ece2b48dc63d599000eb6d6d51e08963067
Gerrit-PatchSet: 1
Gerrit-Project: osmo-hlr
Gerrit-Branch: master
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: neels 
Gerrit-HasComments: Yes

[PATCH] osmo-bsc[master]: fix handling of state changes in acc ramping

[Stefan Sperling]

fix handling of state changes in acc ramping

Take both the operative and administrative states into account
when deciding whether to start ACC ramping, and examine old/new
state values to avoid triggering ramping for a no-op state change.

This requires a fix to gsm_trx_lock_rf(): This function overwrote
the old administrative state of a trx before enqueuing a state
change request towards the BTS.
The BTS will confirm this request with an ACK, at which time a
signal is generated which the ACC ramp code listens to. We must
not overwrite the old state value until the signal has been handled,
otherwise the signal handler cannot tell what the old state was.

Tested with a virtphy setup.
Still needs to be tested with nanobts and osmo-bts.

Change-Id: Ib3291439655598fb5ddc891a3e4cc35b0bad250f
Related: OS#2591
---
M src/libbsc/abis_nm.c
M src/libbsc/acc_ramp.c
M src/libbsc/bsc_init.c
3 files changed, 72 insertions(+), 28 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/84/7784/3

diff --git a/src/libbsc/abis_nm.c b/src/libbsc/abis_nm.c
index 2ee2e24..e3c4408 100644
--- a/src/libbsc/abis_nm.c
+++ b/src/libbsc/abis_nm.c
@@ -2844,13 +2844,17 @@
 {
uint8_t new_state = locked ? NM_STATE_LOCKED : NM_STATE_UNLOCKED;
 
-   LOGP(DNM, LOGL_NOTICE, "(bts=%d,trx=%d) Changing adm. state %s -> %s 
[%s]\n", trx->bts->nr, trx->nr,
+
+   if (!trx->bts || !trx->bts->oml_link) {
+   /* Set initial state which will be sent when BTS connects. */
+   trx->mo.nm_state.administrative = new_state;
+   return;
+   }
+
+   LOGP(DNM, LOGL_NOTICE, "(bts=%d,trx=%d) Requesting administrative state 
change %s -> %s [%s]\n",
+trx->bts->nr, trx->nr,
 get_value_string(abis_nm_adm_state_names, 
trx->mo.nm_state.administrative),
 get_value_string(abis_nm_adm_state_names, new_state), reason);
-
-   trx->mo.nm_state.administrative = new_state;
-   if (!trx->bts || !trx->bts->oml_link)
-   return;
 
abis_nm_chg_adm_state(trx->bts, NM_OC_RADIO_CARRIER,
  trx->bts->bts_nr, trx->nr, 0xff,
diff --git a/src/libbsc/acc_ramp.c b/src/libbsc/acc_ramp.c
index 7116107..da61094 100644
--- a/src/libbsc/acc_ramp.c
+++ b/src/libbsc/acc_ramp.c
@@ -28,6 +28,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /*
  * Check if an ACC has been permanently barred for a BTS,
@@ -144,6 +145,7 @@
struct nm_statechg_signal_data *nsd = signal_data;
struct acc_ramp *acc_ramp = handler_data;
struct gsm_bts_trx *trx = NULL;
+   bool trigger_ramping = false, abort_ramping = false;
 
/* Handled signals map to an Administrative State Change ACK, or a 
State Changed Event Report. */
if (signal != S_NM_STATECHG_ADM && signal != S_NM_STATECHG_OPER)
@@ -154,6 +156,17 @@
 
trx = nsd->obj;
 
+   if (signal == S_NM_STATECHG_ADM)
+   LOGP(DRSL, LOGL_DEBUG, "(bts=%d,trx=%d) ACC RAMP: 
administrative state %s -> %s\n",
+   acc_ramp->bts->nr, trx->nr,
+   get_value_string(abis_nm_adm_state_names, 
nsd->old_state->administrative),
+   get_value_string(abis_nm_adm_state_names, 
nsd->new_state->administrative));
+   else if (signal == S_NM_STATECHG_OPER)
+   LOGP(DRSL, LOGL_DEBUG, "(bts=%d,trx=%d) ACC RAMP: operational 
state %s -> %s\n",
+   acc_ramp->bts->nr, trx->nr,
+   abis_nm_opstate_name(nsd->old_state->operational),
+   abis_nm_opstate_name(nsd->new_state->operational));
+
/* We only care about state changes of the first TRX. */
if (trx->nr != 0)
return 0;
@@ -162,29 +175,54 @@
if (trx->rsl_link == NULL)
return 0;
 
-   /* Trigger or abort ACC ramping based on the new 'RF lock' state of 
this TRX. */
-   switch (nsd->new_state->administrative) {
-   case NM_STATE_UNLOCKED:
-   /*
-* Do not re-trigger ACC ramping if ramping is already in 
progress.
-* A BTS might send several "unlock" change events: One in the 
Administrative
-* State Change ACK, and/or another in a State Changed Event 
Report.
-* For instance, the nanobts is known to send both.
-*/
-   if (!osmo_timer_pending(_ramp->step_timer))
-   acc_ramp_trigger(acc_ramp);
-   break;
-   case NM_STATE_LOCKED:
-   case NM_STATE_SHUTDOWN:
-   acc_ramp_abort(acc_ramp);
-   break;
-   case NM_STATE_NULL:
-   break;
-   default:
-   LOGP(DRSL, LOGL_NOTICE, "(bts=%d) ACC RAMP: unrecognized 
administrative state '0x%x' reported for TRX 0\n",
-   acc_ramp->bts->nr, nsd->new_state->administrative);
-   break;
+   /* Trigger or abort ACC ramping based on the new state of this TRX. */
+   if 

[PATCH] osmo-bsc[master]: fix handling of state changes in acc ramping

[Stefan Sperling]

fix handling of state changes in acc ramping

Take both the operative and administrative states into account
when deciding whether to start ACC ramping, and examine old/new
state values to avoid triggering ramping for a no-op state change.

This requires a fix to gsm_trx_lock_rf(): This function overwrote
the old administrative state of a trx before enqueuing a state
change request towards the BTS.
The BTS will confirm this request with an ACK, at which time a
signal is generated which the ACC ramp code listens to. We must
not overwrite the old state value until the signal has been handled,
otherwise the signal handler cannot tell what the old state was.

Tested with a virtphy setup.
Still needs to be tested with nanobts and osmo-bts.

Change-Id: Ib3291439655598fb5ddc891a3e4cc35b0bad250f
Related: OS#2591
---
M src/libbsc/abis_nm.c
M src/libbsc/acc_ramp.c
M src/libbsc/bsc_init.c
3 files changed, 72 insertions(+), 28 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/84/7784/2

diff --git a/src/libbsc/abis_nm.c b/src/libbsc/abis_nm.c
index 2ee2e24..66eba12 100644
--- a/src/libbsc/abis_nm.c
+++ b/src/libbsc/abis_nm.c
@@ -2844,13 +2844,17 @@
 {
uint8_t new_state = locked ? NM_STATE_LOCKED : NM_STATE_UNLOCKED;
 
-   LOGP(DNM, LOGL_NOTICE, "(bts=%d,trx=%d) Changing adm. state %s -> %s 
[%s]\n", trx->bts->nr, trx->nr,
+
+   if (!trx->bts || !trx->bts->oml_link) {
+   /* Set initial state which will be sent when BTS connects. */
+   trx->mo.nm_state.administrative = new_state;
+   return;
+   }
+
+   LOGP(DNM, LOGL_NOTICE, "(bts=%d,trx=%d) Requesting administrative state 
change %s -> %s [%s]\n",
+   trx->bts->nr, trx->nr,
 get_value_string(abis_nm_adm_state_names, 
trx->mo.nm_state.administrative),
 get_value_string(abis_nm_adm_state_names, new_state), reason);
-
-   trx->mo.nm_state.administrative = new_state;
-   if (!trx->bts || !trx->bts->oml_link)
-   return;
 
abis_nm_chg_adm_state(trx->bts, NM_OC_RADIO_CARRIER,
  trx->bts->bts_nr, trx->nr, 0xff,
diff --git a/src/libbsc/acc_ramp.c b/src/libbsc/acc_ramp.c
index 7116107..da61094 100644
--- a/src/libbsc/acc_ramp.c
+++ b/src/libbsc/acc_ramp.c
@@ -28,6 +28,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /*
  * Check if an ACC has been permanently barred for a BTS,
@@ -144,6 +145,7 @@
struct nm_statechg_signal_data *nsd = signal_data;
struct acc_ramp *acc_ramp = handler_data;
struct gsm_bts_trx *trx = NULL;
+   bool trigger_ramping = false, abort_ramping = false;
 
/* Handled signals map to an Administrative State Change ACK, or a 
State Changed Event Report. */
if (signal != S_NM_STATECHG_ADM && signal != S_NM_STATECHG_OPER)
@@ -154,6 +156,17 @@
 
trx = nsd->obj;
 
+   if (signal == S_NM_STATECHG_ADM)
+   LOGP(DRSL, LOGL_DEBUG, "(bts=%d,trx=%d) ACC RAMP: 
administrative state %s -> %s\n",
+   acc_ramp->bts->nr, trx->nr,
+   get_value_string(abis_nm_adm_state_names, 
nsd->old_state->administrative),
+   get_value_string(abis_nm_adm_state_names, 
nsd->new_state->administrative));
+   else if (signal == S_NM_STATECHG_OPER)
+   LOGP(DRSL, LOGL_DEBUG, "(bts=%d,trx=%d) ACC RAMP: operational 
state %s -> %s\n",
+   acc_ramp->bts->nr, trx->nr,
+   abis_nm_opstate_name(nsd->old_state->operational),
+   abis_nm_opstate_name(nsd->new_state->operational));
+
/* We only care about state changes of the first TRX. */
if (trx->nr != 0)
return 0;
@@ -162,29 +175,54 @@
if (trx->rsl_link == NULL)
return 0;
 
-   /* Trigger or abort ACC ramping based on the new 'RF lock' state of 
this TRX. */
-   switch (nsd->new_state->administrative) {
-   case NM_STATE_UNLOCKED:
-   /*
-* Do not re-trigger ACC ramping if ramping is already in 
progress.
-* A BTS might send several "unlock" change events: One in the 
Administrative
-* State Change ACK, and/or another in a State Changed Event 
Report.
-* For instance, the nanobts is known to send both.
-*/
-   if (!osmo_timer_pending(_ramp->step_timer))
-   acc_ramp_trigger(acc_ramp);
-   break;
-   case NM_STATE_LOCKED:
-   case NM_STATE_SHUTDOWN:
-   acc_ramp_abort(acc_ramp);
-   break;
-   case NM_STATE_NULL:
-   break;
-   default:
-   LOGP(DRSL, LOGL_NOTICE, "(bts=%d) ACC RAMP: unrecognized 
administrative state '0x%x' reported for TRX 0\n",
-   acc_ramp->bts->nr, nsd->new_state->administrative);
-   break;
+   /* Trigger or abort ACC ramping based on the new state of this TRX. */
+   if 

[PATCH] osmo-bsc[master]: fix handling of state changes in acc ramping

[Stefan Sperling]

Review at  https://gerrit.osmocom.org/7784

fix handling of state changes in acc ramping

Take both the operative and administrative states into account
when deciding whether to start ACC ramping, and examine old/new
state values to avoid triggering ramping for a no-op state change.

This requires a fix to gsm_trx_lock_rf(): This function overwrote
the old administrative state of a trx before enqueuing a state
change request towards the BTS.
The BTS will confirm this request with an ACK, at which time a
signal is generated which the ACC ramp code listens to. We must
not overwrite the old state value until the signal has been handled,
otherwise the signal handler cannot tell what the old state was.

Change-Id: Ib3291439655598fb5ddc891a3e4cc35b0bad250f
Related: OS#2591
---
M src/libbsc/abis_nm.c
M src/libbsc/acc_ramp.c
M src/libbsc/bsc_init.c
3 files changed, 72 insertions(+), 28 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/84/7784/1

diff --git a/src/libbsc/abis_nm.c b/src/libbsc/abis_nm.c
index 2ee2e24..66eba12 100644
--- a/src/libbsc/abis_nm.c
+++ b/src/libbsc/abis_nm.c
@@ -2844,13 +2844,17 @@
 {
uint8_t new_state = locked ? NM_STATE_LOCKED : NM_STATE_UNLOCKED;
 
-   LOGP(DNM, LOGL_NOTICE, "(bts=%d,trx=%d) Changing adm. state %s -> %s 
[%s]\n", trx->bts->nr, trx->nr,
+
+   if (!trx->bts || !trx->bts->oml_link) {
+   /* Set initial state which will be sent when BTS connects. */
+   trx->mo.nm_state.administrative = new_state;
+   return;
+   }
+
+   LOGP(DNM, LOGL_NOTICE, "(bts=%d,trx=%d) Requesting administrative state 
change %s -> %s [%s]\n",
+   trx->bts->nr, trx->nr,
 get_value_string(abis_nm_adm_state_names, 
trx->mo.nm_state.administrative),
 get_value_string(abis_nm_adm_state_names, new_state), reason);
-
-   trx->mo.nm_state.administrative = new_state;
-   if (!trx->bts || !trx->bts->oml_link)
-   return;
 
abis_nm_chg_adm_state(trx->bts, NM_OC_RADIO_CARRIER,
  trx->bts->bts_nr, trx->nr, 0xff,
diff --git a/src/libbsc/acc_ramp.c b/src/libbsc/acc_ramp.c
index 7116107..da61094 100644
--- a/src/libbsc/acc_ramp.c
+++ b/src/libbsc/acc_ramp.c
@@ -28,6 +28,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /*
  * Check if an ACC has been permanently barred for a BTS,
@@ -144,6 +145,7 @@
struct nm_statechg_signal_data *nsd = signal_data;
struct acc_ramp *acc_ramp = handler_data;
struct gsm_bts_trx *trx = NULL;
+   bool trigger_ramping = false, abort_ramping = false;
 
/* Handled signals map to an Administrative State Change ACK, or a 
State Changed Event Report. */
if (signal != S_NM_STATECHG_ADM && signal != S_NM_STATECHG_OPER)
@@ -154,6 +156,17 @@
 
trx = nsd->obj;
 
+   if (signal == S_NM_STATECHG_ADM)
+   LOGP(DRSL, LOGL_DEBUG, "(bts=%d,trx=%d) ACC RAMP: 
administrative state %s -> %s\n",
+   acc_ramp->bts->nr, trx->nr,
+   get_value_string(abis_nm_adm_state_names, 
nsd->old_state->administrative),
+   get_value_string(abis_nm_adm_state_names, 
nsd->new_state->administrative));
+   else if (signal == S_NM_STATECHG_OPER)
+   LOGP(DRSL, LOGL_DEBUG, "(bts=%d,trx=%d) ACC RAMP: operational 
state %s -> %s\n",
+   acc_ramp->bts->nr, trx->nr,
+   abis_nm_opstate_name(nsd->old_state->operational),
+   abis_nm_opstate_name(nsd->new_state->operational));
+
/* We only care about state changes of the first TRX. */
if (trx->nr != 0)
return 0;
@@ -162,29 +175,54 @@
if (trx->rsl_link == NULL)
return 0;
 
-   /* Trigger or abort ACC ramping based on the new 'RF lock' state of 
this TRX. */
-   switch (nsd->new_state->administrative) {
-   case NM_STATE_UNLOCKED:
-   /*
-* Do not re-trigger ACC ramping if ramping is already in 
progress.
-* A BTS might send several "unlock" change events: One in the 
Administrative
-* State Change ACK, and/or another in a State Changed Event 
Report.
-* For instance, the nanobts is known to send both.
-*/
-   if (!osmo_timer_pending(_ramp->step_timer))
-   acc_ramp_trigger(acc_ramp);
-   break;
-   case NM_STATE_LOCKED:
-   case NM_STATE_SHUTDOWN:
-   acc_ramp_abort(acc_ramp);
-   break;
-   case NM_STATE_NULL:
-   break;
-   default:
-   LOGP(DRSL, LOGL_NOTICE, "(bts=%d) ACC RAMP: unrecognized 
administrative state '0x%x' reported for TRX 0\n",
-   acc_ramp->bts->nr, nsd->new_state->administrative);
-   break;
+   /* Trigger or abort ACC ramping based on the new state of this TRX. */
+   if (nsd->old_state->administrative != 

libosmo-netif[master]: jibuf: Add initial implementation of Jitter Buffer

[Pau Espin Pedrol]

Patch Set 1: Code-Review-1

Pushed in here as a side effect (wrong branch). Can be reviewed, open for 
comments, but still shouldn't be merged until further testing is done.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9688ba9c4d5b733b9f29d0f15f73750f9271ef55
Gerrit-PatchSet: 1
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-HasComments: No

[ABANDON] libosmo-netif[master]: osmux: osmux_xfrm_output_pull: Improve checks and log of mal...

[Pau Espin Pedrol]

Pau Espin Pedrol has abandoned this change.

Change subject: osmux: osmux_xfrm_output_pull: Improve checks and log of 
malformed packets
..


Abandoned

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: I7dc3aa2b699a783d86aa9a9452b9e74084d7a97f
Gerrit-PatchSet: 1
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol 

[PATCH] libosmo-netif[master]: osmux: osmux_xfrm_output_pull: Improve checks and log of mal...

[Pau Espin Pedrol]

Review at  https://gerrit.osmocom.org/7783

osmux: osmux_xfrm_output_pull: Improve checks and log of malformed packets

Change-Id: I143805bb5ee9f5e3ada46114e380a03ede80df9f
Related: SYS#4182
---
M src/osmux.c
1 file changed, 11 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/83/7783/1

diff --git a/src/osmux.c b/src/osmux.c
index a0563d2..03db469 100644
--- a/src/osmux.c
+++ b/src/osmux.c
@@ -85,8 +85,13 @@
case OSMUX_FT_VOICE_AMR:
break;
case OSMUX_FT_DUMMY:
-   msgb_pull(msg, osmux_ft_dummy_size(osmuxh->amr_ft,
-  osmuxh->ctr + 1));
+   len = osmux_ft_dummy_size(osmuxh->amr_ft, osmuxh->ctr + 
1);
+   if (msgb_length(msg) < len) {
+   LOGP(DLMUX, LOGL_ERROR, "Discarding bad Dummy 
FT: %s\n",
+   osmo_hexdump(msg->data, 
msgb_length(msg)));
+   return NULL;
+   }
+   msgb_pull(msg, len);
goto next;
default:
LOGP(DLMUX, LOGL_ERROR, "Discarding unsupported Osmux 
FT %d\n",
@@ -102,9 +107,10 @@
len = osmo_amr_bytes(osmuxh->amr_ft) * (osmuxh->ctr+1) +
sizeof(struct osmux_hdr);
 
-   if (len > msg->len) {
-   LOGP(DLMUX, LOGL_ERROR, "Discarding malformed "
-   "OSMUX message\n");
+   if (msgb_length(msg) < len) {
+   LOGP(DLMUX, LOGL_ERROR,
+   "Discarding malformed OSMUX message: %s\n",
+   osmo_hexdump(msg->data, msgb_length(msg)));
return NULL;
}
 

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I143805bb5ee9f5e3ada46114e380a03ede80df9f
Gerrit-PatchSet: 1
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol 

[PATCH] libosmo-netif[master]: jibuf: Estimate src clock skew

[Pau Espin Pedrol]

Review at  https://gerrit.osmocom.org/7781

jibuf: Estimate src clock skew

Change-Id: Ifae633d53107417a8e2f9b0f200d2711db72d199
---
M include/osmocom/netif/jibuf.h
M src/jibuf.c
M tests/jibuf/jibuf_test.c
M tests/jibuf/jibuf_test.ok
M tests/jibuf/jibuf_tool.c
M tests/jibuf/jitter.plt
6 files changed, 1,005 insertions(+), 13 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/81/7781/1

diff --git a/include/osmocom/netif/jibuf.h b/include/osmocom/netif/jibuf.h
index aca319a..64f0824 100644
--- a/include/osmocom/netif/jibuf.h
+++ b/include/osmocom/netif/jibuf.h
@@ -41,6 +41,9 @@
struct timeval last_enqueue_time;
struct timeval next_dequeue_time;
 
+   bool skew_enabled;
+   int32_t skew_us; /* src clock skew, in usec */
+
struct {
uint32_t total_enqueued;
uint64_t total_dropped;
@@ -59,6 +62,8 @@
 void osmo_jibuf_set_min_delay(struct jibuf *jb, uint32_t min_delay);
 void osmo_jibuf_set_max_delay(struct jibuf *jb, uint32_t max_delay);
 
+void osmo_jibuf_enable_skew_compensation(struct jibuf *jb, bool enable);
+
 void osmo_jibuf_set_dequeue_cb(struct jibuf *jb, osmo_jibuf_dequeue_cb 
dequeue_cb, void* cb_data);
 
 /*! @} */
diff --git a/src/jibuf.c b/src/jibuf.c
index 5d568e1..73da04a 100644
--- a/src/jibuf.c
+++ b/src/jibuf.c
@@ -56,6 +56,9 @@
 #define JIBUF_BUFFER_INC_STEP 20
 #define JIBUF_BUFFER_DEC_STEP 5
 
+/* weight of each new packet in calculation of clock skew */
+#define JIBUF_SKEW_WEIGHT ((double)1/32)
+
 struct jibuf_msgb_cb {
struct timeval ts;
unsigned long *old_cb;
@@ -175,6 +178,7 @@
jb->ref_rx_ts = timeval2ms(>last_enqueue_time);
jb->ref_tx_ts = msg_get_timestamp(msg);
jb->ref_tx_seq = msg_get_sequence(msg);
+   jb->skew_us = 0;
 
LOGP(DLJIBUF, LOGL_DEBUG, "New reference (seq=%"PRIu16" rx=%"PRIu32 \
" tx=%"PRIu32")\n", jb->ref_tx_seq, jb->ref_rx_ts, 
jb->ref_tx_ts);
@@ -213,6 +217,14 @@
/* XXX: maybe  try to tune the threshold based on the calculated output 
jitter? */
/* XXX: try to find holes in the list and create fake pkts to improve 
the
 jitter when packets do not arrive on time */
+}
+
+static void recalc_clock_skew(struct jibuf *jb, int32_t rel_delay)
+{
+   if(!jb->skew_enabled)
+   return;
+
+   jb->skew_us = (int32_t) (rel_delay * 1000 * JIBUF_SKEW_WEIGHT + 
jb->skew_us * (1.0 - JIBUF_SKEW_WEIGHT));
 }
 
 static void recalc_threshold_delay(struct jibuf *jb)
@@ -309,6 +321,7 @@
rel_delay = 0;
} else {
rel_delay = calc_pkt_rel_delay(jb, msg);
+   recalc_clock_skew(jb, rel_delay);
}
 
/* Avoid time skew with sender (or drop-everything state),
@@ -316,7 +329,7 @@
//if ((int)(msg_get_sequence(msg) - jb->ref_tx_seq) > 
JIBUF_REFERENCE_TS_FREQ)
//  msg_set_as_reference(jb, msg);
 
-   delay = jb->threshold_delay + rel_delay;
+   delay = jb->threshold_delay + rel_delay - jb->skew_us/1000;
 
/* packet too late, let's drop it and incr buffer size if encouraged */
if (delay < 0) {
@@ -338,8 +351,8 @@
timeradd(>last_enqueue_time, _ts, _ts);
 
LOGP(DLJIBUF, LOGL_DEBUG, "enqueuing packet seq=%"PRIu16" rel=%d 
delay=%d" \
-   " thres=%d {%lu.%06lu -> %lu.%06lu} %s\n",
-   msg_get_sequence(msg), rel_delay, delay, jb->threshold_delay,
+   " skew=%d thres=%d {%lu.%06lu -> %lu.%06lu} %s\n",
+   msg_get_sequence(msg), rel_delay, delay, jb->skew_us, 
jb->threshold_delay,
jb->last_enqueue_time.tv_sec, jb->last_enqueue_time.tv_usec,
sched_ts.tv_sec, sched_ts.tv_usec, msg_get_marker(msg)? "M" : 
"");
 
@@ -394,6 +407,18 @@
jb->threshold_delay = OSMO_MIN(jb->max_delay, jb->threshold_delay);
 }
 
+/*! \brief Toggle use of skew detection and compensation mechanism
+ *  \param[in] jb jitter buffer instance
+ *  \param[in] enable Whether to enable or not (default) the skew estimation 
and compensation mechanism
+ *
+ * When this function is called, the estimated skew is reset.
+ */
+void osmo_jibuf_enable_skew_compensation(struct jibuf *jb, bool enable)
+{
+   jb->skew_enabled = enable;
+   jb->skew_us = 0;
+}
+
 /*! \brief Set dequeue callback for the jitter buffer
  *  \param[in] jb jitter buffer instance
  *  \param[in] dequeue_cb function pointer to call back when the dequeue timer 
for a given packet expires
diff --git a/tests/jibuf/jibuf_test.c b/tests/jibuf/jibuf_test.c
index a9a7ff0..5b5ebad 100644
--- a/tests/jibuf/jibuf_test.c
+++ b/tests/jibuf/jibuf_test.c
@@ -123,13 +123,14 @@
clock_debug("clock_override_set");
 }
 
-static void clock_override_add(long sec, long usec)
+static void clock_override_add_debug(long sec, long usec, bool dbg)
 {
osmo_gettimeofday_override_add(sec, usec);
osmo_clock_override_add(CLOCK_MONOTONIC, sec, usec*1000);
-  

[PATCH] libosmo-netif[master]: tests: jibuf_tool: Add seq.plt

[Pau Espin Pedrol]

Review at  https://gerrit.osmocom.org/7780

tests: jibuf_tool: Add seq.plt

Change-Id: I37bb3ab372b7ad7c3c1d09c8134827c932bf8bf8
---
A tests/jibuf/seq.plt
1 file changed, 50 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/80/7780/1

diff --git a/tests/jibuf/seq.plt b/tests/jibuf/seq.plt
new file mode 100644
index 000..dda8ad0
--- /dev/null
+++ b/tests/jibuf/seq.plt
@@ -0,0 +1,50 @@
+#set terminal png size 1280,1024 enhanced font "Helvetica,20"
+#set output 'output.png'
+#set terminal qt
+#set output
+
+set datafile separator "\t"
+set datafile missing '0'
+
+#set multiplot
+
+# key/legend
+set key top right
+set key box
+set key left bottom
+set key bmargin
+
+
+set title 'Jitter Graph'
+set xlabel 'Timestamp'
+set ylabel 'Seq'
+set ytics nomirror
+
+#set y2label 'delay [ms]'
+#set y2tics nomirror
+
+# For pcap based input, 'pre.delay' makes no sense (it's 0) as we can't know 
tx_delay
+pcap = 1
+if (pcap) {
+plot \
+'/tmp/bla' using 3:1 with linespoints title 'pre.trans' axes x1y1, \
+'/tmp/bla' using 4:1 with linespoints title 'post.trans' axes x1y1, \
+'/tmp/bla' using 3:7 with linespoints title 'pre.jitter' axes x1y1, \
+'/tmp/bla' using 4:8 with linespoints title 'post.jitter' axes x1y1, \
+'/tmp/bla' using 3:10 with linespoints title 'pre.buffer' axes x1y1, \
+'/tmp/bla' using 4:((column(4)-column(3))) with linespoints title 
'post.delay' axes x1y1
+
+} else {
+plot \
+'/tmp/bla' using 3:5 with linespoints title 'pre.trans' axes x1y1, \
+'/tmp/bla' using 4:6 with linespoints title 'post.trans' axes x1y1, \
+'/tmp/bla' using 3:7 with linespoints title 'pre.jitter' axes x1y1, \
+'/tmp/bla' using 4:8 with linespoints title 'post.jitter' axes x1y1, \
+'/tmp/bla' using 3:9 with linespoints title 'pre.dropped' axes x1y1, \
+'/tmp/bla' using 3:10 with linespoints title 'pre.buffer' axes x1y1, \
+'/tmp/bla' using 3:11 with linespoints title 'pre.skew' axes x1y1, \
+'/tmp/bla' using 3:((column(3)-column(2))) with linespoints title 
'pre.delay' axes x1y1, \
+'/tmp/bla' using 4:((column(4)-column(2))) with linespoints title 
'post.delay' axes x1y1
+}
+
+pause mouse close; exit

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I37bb3ab372b7ad7c3c1d09c8134827c932bf8bf8
Gerrit-PatchSet: 1
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol 

[PATCH] libosmo-netif[master]: osmux: osmux_xfrm_output_pull: Improve checks and log of mal...

[Pau Espin Pedrol]

Review at  https://gerrit.osmocom.org/7782

osmux: osmux_xfrm_output_pull: Improve checks and log of malformed packets

Related: SYS#4182

Change-Id: I7dc3aa2b699a783d86aa9a9452b9e74084d7a97f
---
M src/osmux.c
1 file changed, 11 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/82/7782/1

diff --git a/src/osmux.c b/src/osmux.c
index a0563d2..03db469 100644
--- a/src/osmux.c
+++ b/src/osmux.c
@@ -85,8 +85,13 @@
case OSMUX_FT_VOICE_AMR:
break;
case OSMUX_FT_DUMMY:
-   msgb_pull(msg, osmux_ft_dummy_size(osmuxh->amr_ft,
-  osmuxh->ctr + 1));
+   len = osmux_ft_dummy_size(osmuxh->amr_ft, osmuxh->ctr + 
1);
+   if (msgb_length(msg) < len) {
+   LOGP(DLMUX, LOGL_ERROR, "Discarding bad Dummy 
FT: %s\n",
+   osmo_hexdump(msg->data, 
msgb_length(msg)));
+   return NULL;
+   }
+   msgb_pull(msg, len);
goto next;
default:
LOGP(DLMUX, LOGL_ERROR, "Discarding unsupported Osmux 
FT %d\n",
@@ -102,9 +107,10 @@
len = osmo_amr_bytes(osmuxh->amr_ft) * (osmuxh->ctr+1) +
sizeof(struct osmux_hdr);
 
-   if (len > msg->len) {
-   LOGP(DLMUX, LOGL_ERROR, "Discarding malformed "
-   "OSMUX message\n");
+   if (msgb_length(msg) < len) {
+   LOGP(DLMUX, LOGL_ERROR,
+   "Discarding malformed OSMUX message: %s\n",
+   osmo_hexdump(msg->data, msgb_length(msg)));
return NULL;
}
 

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I7dc3aa2b699a783d86aa9a9452b9e74084d7a97f
Gerrit-PatchSet: 1
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol 

[PATCH] libosmo-netif[master]: jibuf: Add initial implementation of Jitter Buffer

[Pau Espin Pedrol]

Review at  https://gerrit.osmocom.org/7773

jibuf: Add initial implementation of Jitter Buffer

Change-Id: I9688ba9c4d5b733b9f29d0f15f73750f9271ef55
---
M include/osmocom/netif/Makefile.am
A include/osmocom/netif/jibuf.h
M src/Makefile.am
A src/jibuf.c
M tests/Makefile.am
A tests/jibuf/jibuf_test.c
A tests/jibuf/jibuf_test.ok
M tests/testsuite.at
8 files changed, 1,402 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/73/7773/1

diff --git a/include/osmocom/netif/Makefile.am 
b/include/osmocom/netif/Makefile.am
index cbaff5c..0db78fb 100644
--- a/include/osmocom/netif/Makefile.am
+++ b/include/osmocom/netif/Makefile.am
@@ -3,6 +3,7 @@
 osmonetif_HEADERS = amr.h  \
channel.h   \
datagram.h  \
+   jibuf.h \
osmux.h \
ipa.h   \
ipa_unit.h  \
diff --git a/include/osmocom/netif/jibuf.h b/include/osmocom/netif/jibuf.h
new file mode 100644
index 000..aca319a
--- /dev/null
+++ b/include/osmocom/netif/jibuf.h
@@ -0,0 +1,66 @@
+#ifndef _JIBUF_H_
+#define _JIBUF_H_
+
+#include 
+#include 
+#include 
+
+#include 
+
+/*! \defgroup jibuf Osmocom Jitter Buffer
+ *  @{
+ */
+
+/*! \file jibuf.h
+ *  \brief Osmocom Jitter Buffer helpers
+ */
+
+typedef void (*osmo_jibuf_dequeue_cb)(struct msgb *msg, void *data);
+
+/*! \brief A structure representing a single instance of a jitter buffer */
+struct jibuf {
+   void *talloc_ctx;
+   bool started;
+   struct osmo_timer_list timer;
+   struct llist_head msg_list; /* sorted by output ts */
+   uint32_t min_delay; /* in msec */
+   uint32_t max_delay; /* in msec */
+   uint32_t threshold_delay; /* in msec */
+
+   osmo_jibuf_dequeue_cb dequeue_cb;
+   void *dequeue_cb_data;
+
+   /* number of pkt drops since we last changed the buffer size */
+   uint32_t last_dropped;
+   uint32_t consecutive_drops;
+
+   uint32_t ref_rx_ts;
+   uint32_t ref_tx_ts;
+   uint16_t ref_tx_seq;
+
+   struct timeval last_enqueue_time;
+   struct timeval next_dequeue_time;
+
+   struct {
+   uint32_t total_enqueued;
+   uint64_t total_dropped;
+   } stats;
+};
+
+
+struct jibuf *osmo_jibuf_alloc(void *talloc_ctx);
+
+void osmo_jibuf_delete(struct jibuf *jb);
+
+int osmo_jibuf_enqueue(struct jibuf *jb, struct msgb *msg);
+
+bool osmo_jibuf_empty(struct jibuf *jb);
+
+void osmo_jibuf_set_min_delay(struct jibuf *jb, uint32_t min_delay);
+void osmo_jibuf_set_max_delay(struct jibuf *jb, uint32_t max_delay);
+
+void osmo_jibuf_set_dequeue_cb(struct jibuf *jb, osmo_jibuf_dequeue_cb 
dequeue_cb, void* cb_data);
+
+/*! @} */
+
+#endif
diff --git a/src/Makefile.am b/src/Makefile.am
index 81a55b4..79e3685 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -18,6 +18,7 @@
  datagram.c\
  ipa.c \
  ipa_unit.c\
+ jibuf.c   \
  osmux.c   \
  rs232.c   \
  rtp.c \
diff --git a/src/jibuf.c b/src/jibuf.c
new file mode 100644
index 000..44a86a5
--- /dev/null
+++ b/src/jibuf.c
@@ -0,0 +1,377 @@
+/* (C) 2017 by Pau Espin Pedrol 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#include 
+
+/*! \addtogroup jibuf Osmocom Jitter Buffer
+ *  @{
+ */
+
+/*! \file jibuf.c
+ *  \brief Osmocom Jitter Buffer helpers
+ */
+
+/* Sampling rate (in Hz) */
+/* TODO: SAMPLE RATE can be guessed from rtp.p_type */
+#define SAMPLE_RATE 8000
+
+/* TUNABLE PARAMETERS: */
+
+/* default {min,max}_delay values if set_{min,max}_delay() is never called */
+#define JIBUF_DEFAULT_MIN_DELAY_MS 60
+#define JIBUF_DEFAULT_MAX_DELAY_MS 200
+
+/* How frequently (num of input packets) do we reselect a new reference? */
+#define JIBUF_REFERENCE_TS_FREQ 60
+
+/* How frequently (num of input packets) do we check if we should adapt the
+ * buffer size (threshold_delay) ? */
+#define JIBUF_BUFFER_RECALC_FREQ 40
+/* How many pkts should be dropped at max every JIBUF_BUFFER_RECALC_FREQ input
+ * pkts? */
+#define JIBUF_ALLOWED_PKT_DROP 3
+/* How many consecutive pkts can be dropped before triggering a buffer size 
incr ? */
+#define JIBUF_ALLOWED_PKT_CONSECUTIVE_DROP 1
+/* How much do we incr/decr the buffer size every time we recalculate it? */
+#define JIBUF_BUFFER_INC_STEP 20
+#define 

[PATCH] libosmo-netif[master]: tests: jibuf_tool: Improve jibuf_test to read pcaps

[Pau Espin Pedrol]

Review at  https://gerrit.osmocom.org/7775

tests: jibuf_tool: Improve jibuf_test to read pcaps

Change-Id: I7a13c823fb70e0adbc5fa0726fd66b15dc40014e
Signed-off-by: Pau Espin Pedrol 
---
M tests/Makefile.am
M tests/jibuf/jibuf_tool.c
M tests/jibuf/jitter.plt
3 files changed, 312 insertions(+), 112 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/75/7775/1

diff --git a/tests/Makefile.am b/tests/Makefile.am
index a4abca2..d6af988 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -9,8 +9,20 @@
 jibuf_jibuf_test_SOURCES = jibuf/jibuf_test.c
 jibuf_jibuf_test_LDADD = $(LIBOSMOCORE_LIBS) 
$(top_builddir)/src/libosmonetif.la
 
-jibuf_jibuf_tool_SOURCES = jibuf/jibuf_tool.c
-jibuf_jibuf_tool_LDADD = $(LIBOSMOCORE_LIBS) 
$(top_builddir)/src/libosmonetif.la
+jibuf_jibuf_tool_SOURCES = \
+   jibuf/jibuf_tool.c \
+   osmo-pcap-test/proto.c \
+   osmo-pcap-test/l2_eth.c \
+   osmo-pcap-test/l2_sll.c \
+   osmo-pcap-test/l3_ipv4.c \
+   osmo-pcap-test/l4_tcp.c \
+   osmo-pcap-test/l4_udp.c \
+   osmo-pcap-test/pcap.c
+
+jibuf_jibuf_tool_LDADD = \
+   $(LIBOSMOCORE_LIBS) \
+   $(top_builddir)/src/libosmonetif.la \
+   -lpcap
 
 # The `:;' works around a Bash 3.2 bug when the output is not writeable.
 $(srcdir)/package.m4: $(top_srcdir)/configure.ac
diff --git a/tests/jibuf/jibuf_tool.c b/tests/jibuf/jibuf_tool.c
index 340c36c..c2461ec 100644
--- a/tests/jibuf/jibuf_tool.c
+++ b/tests/jibuf/jibuf_tool.c
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -25,50 +26,14 @@
 #include 
 #include 
 
-/* Logging related stuff */
-#define INT2IDX(x)   (-1*(x)-1)
-struct log_info_cat jibuf_test_cat[] = {
-   [INT2IDX(DLJIBUF)] = {
-   .name = "DLJIBUF",
-   .description = "Osmocom Jitter Buffer",
-   .enabled = 1, .loglevel = LOGL_DEBUG,
-   },
-};
-const struct log_info jibuf_test_log_info = {
-   .filter_fn = NULL,
-   .cat = jibuf_test_cat,
-   .num_cat = ARRAY_SIZE(jibuf_test_cat),
-};
+#include "../osmo-pcap-test/osmo_pcap.h"
 
-/* RTP packet with AMR payload */
-static uint8_t rtp_pkt[] = {
-   0x80, 0x62, 0x3f, 0xcc, 0x00, 0x01, 0xa7, 0x6f, /* RTP */
-   0x07, 0x09, 0x00, 0x62, 0x20, 0x14, 0xff, 0xd4, /* AMR */
-   0xf9, 0xff, 0xfb, 0xe7, 0xeb, 0xf9, 0x9f, 0xf8,
-   0xf2, 0x26, 0x33, 0x65, 0x54,
-};
-
-static void sigalarm_handler(int foo)
-{
-   printf("FAIL: test did not run successfully\n");
-   exit(EXIT_FAILURE);
-}
-
-#define SAMPLES_PER_PKT160
-#define RTP_FREQ_MS 20
-#define RTP_PKTS_PER_SEC (1000/RTP_FREQ_MS)
-#define NET_DELAY_MS   300
-#define GENERATED_JITTER_MS 160
-#define NUM_PACKETS_TO_SEND 1000
-
-#define TRACE_PACKE_DEBUG 0
-#define TRACE_PACKET_GNUPLOT 1
-#define TRACE_PACKET_TEST_JITTER 0
 
 struct checkpoint {
struct timeval ts;
int transit;
double jitter;
+   uint32_t timestamp;
 };
 
 struct rtp_pkt_info {
@@ -84,14 +49,66 @@
struct rtp_pkt_info *data;
 };
 
-struct jibuf *jb;
-uint16_t rtp_first_seq;
-uint16_t rtp_next_seq;
-uint32_t rtp_next_ts;
-uint32_t packets_sent;
-uint32_t packets_received;
-uint32_t packets_dropped;
-uint32_t packets_too_much_jitter;
+/* Option parameters to the program */
+static bool opt_test_rand;
+static bool opt_debug_human;
+static bool opt_debug_table;
+static char* opt_pcap_file;
+/* - */
+
+/* Logging related stuff */
+#define INT2IDX(x)   (-1*(x)-1)
+struct log_info_cat jibuf_test_cat[] = {
+   [INT2IDX(DLJIBUF)] = {
+   .name = "DLJIBUF",
+   .description = "Osmocom Jitter Buffer",
+   .enabled = 1, .loglevel = LOGL_DEBUG,
+   },
+};
+const struct log_info jibuf_test_log_info = {
+   .filter_fn = NULL,
+   .cat = jibuf_test_cat,
+   .num_cat = ARRAY_SIZE(jibuf_test_cat),
+};
+/* - */
+
+/* Used for test random: */
+#define SAMPLES_PER_PKT160
+#define RTP_FREQ_MS 20
+#define RTP_PKTS_PER_SEC (1000/RTP_FREQ_MS)
+#define NET_DELAY_MS   300
+#define GENERATED_JITTER_MS 160
+#define NUM_PACKETS_TO_SEND 1000
+
+/* RTP packet with AMR payload */
+static uint8_t rtp_pkt[] = {
+   0x80, 0x62, 0x3f, 0xcc, 0x00, 0x01, 0xa7, 0x6f, /* RTP */
+   0x07, 0x09, 0x00, 0x62, 0x20, 0x14, 0xff, 0xd4, /* AMR */
+   0xf9, 0xff, 0xfb, 0xe7, 0xeb, 0xf9, 0x9f, 0xf8,
+   0xf2, 0x26, 0x33, 0x65, 0x54,
+};
+
+static struct jibuf *jb;
+static uint16_t rtp_first_seq;
+static uint16_t rtp_next_seq;
+static uint32_t rtp_next_ts;
+static struct timeval tx_prev_time;
+static uint32_t packets_sent;
+static uint32_t packets_received;
+static uint32_t packets_dropped;
+static uint32_t packets_too_much_jitter;
+/* - */
+
+/* Used for test pcap: */
+static struct osmo_pcap osmo_pcap;
+static bool pcap_finished;
+/* - */
+
+static 

[PATCH] libosmo-netif[master]: tests: jibuf_tool: Add parameters to control size of buffer

[Pau Espin Pedrol]

Review at  https://gerrit.osmocom.org/

tests: jibuf_tool: Add parameters to control size of buffer

Change-Id: I8a7fa39985f8d197e24c32cab80299aba2d03087
---
M tests/jibuf/jibuf_tool.c
1 file changed, 14 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/77//1

diff --git a/tests/jibuf/jibuf_tool.c b/tests/jibuf/jibuf_tool.c
index faa6e38..78eece0 100644
--- a/tests/jibuf/jibuf_tool.c
+++ b/tests/jibuf/jibuf_tool.c
@@ -56,6 +56,8 @@
 static bool opt_debug_table;
 static bool opt_osmux;
 static char* opt_pcap_file;
+uint32_t opt_buffer_min = 60;
+uint32_t opt_buffer_max = 500;
 /* - */
 
 /* Logging related stuff */
@@ -497,8 +499,8 @@
 
jb = osmo_jibuf_alloc(NULL);
osmo_jibuf_set_dequeue_cb(jb, dequeue_cb, NULL);
-   osmo_jibuf_set_min_delay(jb, 60);
-   osmo_jibuf_set_max_delay(jb, 500);
+   osmo_jibuf_set_min_delay(jb, opt_buffer_min);
+   osmo_jibuf_set_max_delay(jb, opt_buffer_max);
 
/* first run */
pcap_pkt_timer_cb(NULL);
@@ -513,20 +515,22 @@
 
 static void print_help(void)
 {
-   printf("jibuf_test [-r] [-p pcap] [-o] [-d] [-g]\n");
+   printf("jibuf_test [-r] [-p pcap] [-o] [-d] [-g] [-m ms] [-M ms]\n");
printf(" -h Print this help message\n");
printf(" -r Run test with randomly generated jitter\n");
printf(" -p Run test with specified pcap file\n");
printf(" -o The pcap contains OSMUX packets isntead of RTP\n");
printf(" -d Enable packet trace debug suitable for humans\n");
printf(" -t Enable packet trace debug suitable for gnuplot\n");
+   printf(" -m Minimum buffer size for the jitter-buffer, in ms (only used 
in -p mode)\n");
+   printf(" -M Maximum buffer size for the jitter-buffer, in ms (only used 
in -p mode)\n");
 }
 
 static int parse_options(int argc, char **argv)
 {
int opt;
 
-   while ((opt = getopt(argc, argv, "hdtrop:")) != -1) {
+   while ((opt = getopt(argc, argv, "hdtrop:m:M:")) != -1) {
switch (opt) {
case 'h':
print_help();
@@ -546,6 +550,12 @@
case 'p':
opt_pcap_file = strdup(optarg);
break;
+   case 'm':
+   opt_buffer_min = (uint32_t) atoi(optarg);
+   break;
+   case 'M':
+   opt_buffer_max = (uint32_t) atoi(optarg);
+   break;
default:
return -1;
}

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I8a7fa39985f8d197e24c32cab80299aba2d03087
Gerrit-PatchSet: 1
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol 

[PATCH] libosmo-netif[master]: tests: jibuf_tool: Initial commit

[Pau Espin Pedrol]

Review at  https://gerrit.osmocom.org/7774

tests: jibuf_tool: Initial commit

Change-Id: I92307c8b1483dd488339771462290aae0ae5689a
---
M tests/Makefile.am
A tests/jibuf/jibuf_tool.c
A tests/jibuf/jitter.plt
3 files changed, 417 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/74/7774/1

diff --git a/tests/Makefile.am b/tests/Makefile.am
index bf1db1f..a4abca2 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -1,7 +1,7 @@
 AM_CFLAGS = -Wall -I$(top_srcdir)/include $(LIBOSMOCORE_CFLAGS) -g
 AM_LDFLAGS = $(LIBOSMOCORE_LDFLAGS)
 
-check_PROGRAMS = osmux/osmux_test jibuf/jibuf_test
+check_PROGRAMS = osmux/osmux_test jibuf/jibuf_test jibuf/jibuf_tool
 
 osmux_osmux_test_SOURCES = osmux/osmux_test.c
 osmux_osmux_test_LDADD = $(LIBOSMOCORE_LIBS) 
$(top_builddir)/src/libosmonetif.la
@@ -9,6 +9,9 @@
 jibuf_jibuf_test_SOURCES = jibuf/jibuf_test.c
 jibuf_jibuf_test_LDADD = $(LIBOSMOCORE_LIBS) 
$(top_builddir)/src/libosmonetif.la
 
+jibuf_jibuf_tool_SOURCES = jibuf/jibuf_tool.c
+jibuf_jibuf_tool_LDADD = $(LIBOSMOCORE_LIBS) 
$(top_builddir)/src/libosmonetif.la
+
 # The `:;' works around a Bash 3.2 bug when the output is not writeable.
 $(srcdir)/package.m4: $(top_srcdir)/configure.ac
:;{ \
diff --git a/tests/jibuf/jibuf_tool.c b/tests/jibuf/jibuf_tool.c
new file mode 100644
index 000..340c36c
--- /dev/null
+++ b/tests/jibuf/jibuf_tool.c
@@ -0,0 +1,378 @@
+/* (C) 2017 by Pau Espin Pedrol 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* Logging related stuff */
+#define INT2IDX(x)   (-1*(x)-1)
+struct log_info_cat jibuf_test_cat[] = {
+   [INT2IDX(DLJIBUF)] = {
+   .name = "DLJIBUF",
+   .description = "Osmocom Jitter Buffer",
+   .enabled = 1, .loglevel = LOGL_DEBUG,
+   },
+};
+const struct log_info jibuf_test_log_info = {
+   .filter_fn = NULL,
+   .cat = jibuf_test_cat,
+   .num_cat = ARRAY_SIZE(jibuf_test_cat),
+};
+
+/* RTP packet with AMR payload */
+static uint8_t rtp_pkt[] = {
+   0x80, 0x62, 0x3f, 0xcc, 0x00, 0x01, 0xa7, 0x6f, /* RTP */
+   0x07, 0x09, 0x00, 0x62, 0x20, 0x14, 0xff, 0xd4, /* AMR */
+   0xf9, 0xff, 0xfb, 0xe7, 0xeb, 0xf9, 0x9f, 0xf8,
+   0xf2, 0x26, 0x33, 0x65, 0x54,
+};
+
+static void sigalarm_handler(int foo)
+{
+   printf("FAIL: test did not run successfully\n");
+   exit(EXIT_FAILURE);
+}
+
+#define SAMPLES_PER_PKT160
+#define RTP_FREQ_MS 20
+#define RTP_PKTS_PER_SEC (1000/RTP_FREQ_MS)
+#define NET_DELAY_MS   300
+#define GENERATED_JITTER_MS 160
+#define NUM_PACKETS_TO_SEND 1000
+
+#define TRACE_PACKE_DEBUG 0
+#define TRACE_PACKET_GNUPLOT 1
+#define TRACE_PACKET_TEST_JITTER 0
+
+struct checkpoint {
+   struct timeval ts;
+   int transit;
+   double jitter;
+};
+
+struct rtp_pkt_info {
+   struct osmo_timer_list timer;
+   struct timeval tx_prev_time;
+   struct timeval tx_time;
+   uint32_t tx_delay;
+   struct checkpoint prequeue;
+   struct checkpoint postqueue;
+};
+
+struct rtp_pkt_info_cb {
+   struct rtp_pkt_info *data;
+};
+
+struct jibuf *jb;
+uint16_t rtp_first_seq;
+uint16_t rtp_next_seq;
+uint32_t rtp_next_ts;
+uint32_t packets_sent;
+uint32_t packets_received;
+uint32_t packets_dropped;
+uint32_t packets_too_much_jitter;
+
+struct rtp_pkt_info *msgb_get_pinfo(struct msgb* msg)
+{
+   struct rtp_pkt_info_cb *cb = (struct rtp_pkt_info_cb *)&((msg)->cb[0]);
+   return cb->data;
+}
+
+static uint32_t timeval2ms(const struct timeval *ts)
+{
+   return ts->tv_sec * 1000 + ts->tv_usec / 1000;
+}
+
+int calc_relative_transmit_time(struct timeval *tx_0, struct timeval *tx_f,
+   struct timeval *rx_0, struct timeval *rx_f)
+{
+   struct timeval txdiff, rxdiff, diff;
+   timersub(rx_f, rx_0, );
+   timersub(tx_f, tx_0, );
+   timersub(, , );
+   return timeval2ms();
+}
+
+void trace_pkt(struct msgb *msg, char* info) {
+   struct timeval now, total_delay;
+   struct rtp_hdr *rtph = osmo_rtp_get_hdr(msg);
+   struct rtp_pkt_info *pinfo = msgb_get_pinfo(msg);
+
+   gettimeofday(, NULL);
+   timersub(, >tx_time, _delay);
+
+#if TRACE_PACKET_DEBUG
+   uint32_t total_delay_ms = timeval2ms(_delay);
+   LOGP(DLJIBUF, LOGL_DEBUG, "%s: seq=%"PRIu16" ts=%"PRIu32" (%ld.%06ld) 
tx_delay=%"PRIu32 \
+   " end_delay=%"PRIu32" pre_trans=%d pre_jitter=%f post_trans=%d 
post_jitter=%f\n",
+   info, ntohs(rtph->sequence), ntohl(rtph->timestamp),
+   

[PATCH] libosmo-netif[master]: jibuf: Take RTP marker into account

[Pau Espin Pedrol]

Review at  https://gerrit.osmocom.org/7778

jibuf: Take RTP marker into account

Change-Id: Ie142acfb45650e0af775f58226fd191beaf8178e
---
M src/jibuf.c
M tests/jibuf/jibuf_test.c
M tests/jibuf/jibuf_test.ok
M tests/jibuf/jibuf_tool.c
4 files changed, 90 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/78/7778/1

diff --git a/src/jibuf.c b/src/jibuf.c
index 44a86a5..aa66302 100644
--- a/src/jibuf.c
+++ b/src/jibuf.c
@@ -111,6 +111,15 @@
 
 }
 
+
+static bool msg_get_marker(struct msgb *msg)
+{
+   /* TODO: make it more generic as a callback so that different types of
+* pkts can be used ? */
+   struct rtp_hdr *rtph = osmo_rtp_get_hdr(msg);
+   return rtph->marker;
+}
+
 static uint16_t msg_get_sequence(struct msgb *msg)
 {
struct rtp_hdr *rtph = osmo_rtp_get_hdr(msg);
@@ -271,7 +280,11 @@
 
clock_gettime_timeval(CLOCK_MONOTONIC, >last_enqueue_time);
 
-   if (!jb->started) {
+   /* If packet contains a mark -> start of talkspurt.
+* A lot of packets may have been suppressed by the sender before it,
+* so let's take it as a reference
+*/
+   if (!jb->started || msg_get_marker(msg)) {
jb->started = true;
msg_set_as_reference(jb, msg);
rel_delay = 0;
@@ -306,10 +319,10 @@
timeradd(>last_enqueue_time, _ts, _ts);
 
LOGP(DLJIBUF, LOGL_DEBUG, "enqueuing packet seq=%"PRIu16" rel=%d 
delay=%d" \
-   " thres=%d {%lu.%06lu -> %lu.%06lu}\n",
+   " thres=%d {%lu.%06lu -> %lu.%06lu} %s\n",
msg_get_sequence(msg), rel_delay, delay, jb->threshold_delay,
jb->last_enqueue_time.tv_sec, jb->last_enqueue_time.tv_usec,
-   sched_ts.tv_sec, sched_ts.tv_usec);
+   sched_ts.tv_sec, sched_ts.tv_usec, msg_get_marker(msg)? "M" : 
"");
 
/* Add scheduled dequeue time in msg->cb so we can check it later */
unsigned long *old_cb = talloc_memdup(jb->talloc_ctx, msg->cb, 
sizeof(msg->cb));
diff --git a/tests/jibuf/jibuf_test.c b/tests/jibuf/jibuf_test.c
index 69a6d2f..a6fe1b9 100644
--- a/tests/jibuf/jibuf_test.c
+++ b/tests/jibuf/jibuf_test.c
@@ -564,6 +564,56 @@
osmo_jibuf_delete(jb);
 }
 
+static void test_rtp_marker(void)
+{
+   int min_delay = 60;
+   struct msgb *msg;
+   struct rtp_hdr *rtph;
+
+   printf("===test_rtp_marker===\n");
+
+   clock_override_enable(true);
+   clock_override_set(0, 0);
+   rtp_init(32, 400);
+   jb = osmo_jibuf_alloc(NULL);
+   osmo_jibuf_set_dequeue_cb(jb, dequeue_cb, NULL);
+   osmo_jibuf_set_min_delay(jb, min_delay);
+   osmo_jibuf_set_max_delay(jb, 200);
+
+   /* First rtp at t=0, should be scheduled in min_delay time */
+   clock_debug("enqueue 1st packet");
+   ENQUEUE_NEXT(jb);
+   clock_override_add(0, TIME_RTP_PKT_MS*1000);
+   clock_debug("enqueue 2nd packet");
+   ENQUEUE_NEXT(jb);
+   clock_override_add(0, min_delay*1000);
+   clock_debug("2 packets dequeued");
+   osmo_select_main(0);
+
+   clock_override_add(0, 40*1000);
+/* We are at t=120, next non-marked (consecutive seq) packet arriving 
at
+ * this time should be dropped, but since marker establishes new ref,
+ * it will be accepted as well an ext paket */
+   clock_debug("enqueue late pkt with marker=1, will be enqueued");
+   msg = rtp_next();
+   rtph = osmo_rtp_get_hdr(msg);
+   rtph->marker = 1;
+   OSMO_ASSERT(osmo_jibuf_enqueue(jb, msg) == 0);
+
+   clock_debug("enqueue late pkt after pkt with marker=1, will be 
enqueued");
+   clock_override_add(0, TIME_RTP_PKT_MS*1000);
+   ENQUEUE_NEXT(jb);
+
+   clock_debug("2 packets dequeued");
+   clock_override_add(0, min_delay*1000);
+   osmo_select_main(0);
+
+   /* t=120, 4 enqueued, 4 dequeued.*/
+   OSMO_ASSERT(osmo_jibuf_empty(jb));
+
+   osmo_jibuf_delete(jb);
+}
+
 int main(int argc, char **argv)
 {
 
@@ -588,6 +638,7 @@
test_buffer_threshold_change();
test_seq_wraparound();
test_timestamp_wraparound();
+   test_rtp_marker();
 
fprintf(stdout, "OK: Test passed\n");
return EXIT_SUCCESS;
diff --git a/tests/jibuf/jibuf_test.ok b/tests/jibuf/jibuf_test.ok
index 5a0f121..3103781 100644
--- a/tests/jibuf/jibuf_test.ok
+++ b/tests/jibuf/jibuf_test.ok
@@ -348,4 +348,21 @@
 sys={0.16}, mono={0.16}: clock_override_add
 sys={0.16}, mono={0.16}: dequeue 5th packet (ts=334)
 sys={0.16}, mono={0.16}: dequeue: seq=37 ts=334 LATEST
+===test_rtp_marker===
+sys={0.00}, mono={0.00}: clock_override_set
+sys={0.00}, mono={0.00}: enqueue 1st packet
+sys={0.02}, mono={0.02}: clock_override_add
+sys={0.02}, mono={0.02}: enqueue 2nd packet
+sys={0.08}, mono={0.08}: clock_override_add
+sys={0.08}, mono={0.08}: 2 packets dequeued

[PATCH] libosmo-netif[master]: jibuf: re-sync clock out of sync timestamps

[Pau Espin Pedrol]

Review at  https://gerrit.osmocom.org/7779

jibuf: re-sync clock out of sync timestamps

Change-Id: I33556b33d7549654442d9bdd7f31128792506652
---
M src/jibuf.c
M tests/jibuf/jibuf_test.c
M tests/jibuf/jibuf_test.ok
M tests/jibuf/jibuf_tool.c
4 files changed, 152 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/79/7779/1

diff --git a/src/jibuf.c b/src/jibuf.c
index aa66302..5d568e1 100644
--- a/src/jibuf.c
+++ b/src/jibuf.c
@@ -32,6 +32,7 @@
 
 /* Sampling rate (in Hz) */
 /* TODO: SAMPLE RATE can be guessed from rtp.p_type */
+#define SAMPLES_PER_PKT 160
 #define SAMPLE_RATE 8000
 
 /* TUNABLE PARAMETERS: */
@@ -146,6 +147,27 @@
uint32_t current_tx_ts = msg_get_timestamp(msg);
 
return samples2ms((current_tx_ts - jb->ref_tx_ts)) - (current_rx_ts - 
jb->ref_rx_ts);
+}
+
+static bool msg_is_in_sequence(struct jibuf *jb, struct msgb *msg)
+{
+   uint32_t current_tx_ts = msg_get_timestamp(msg);
+   uint16_t current_seq = msg_get_sequence(msg);
+   return (current_tx_ts - jb->ref_tx_ts) == (current_seq - 
jb->ref_tx_seq)*SAMPLES_PER_PKT;
+}
+
+/* If packet contains a mark -> start of talkspurt.
+ * A lot of packets may have been suppressed by the sender before it,
+ * so let's take it as a reference
+ * If packet timestamp is not aligned with sequence
+ * number, then we are most probaly starting a talkspurt */
+static bool msg_is_syncpoint(struct jibuf *jb, struct msgb* msg)
+{
+   bool res = msg_get_marker(msg) || !msg_is_in_sequence(jb, msg);
+   if(res)
+   LOGP(DLMIB, LOGL_DEBUG, "syncpoint: %"PRIu16": marker=%d 
in_seq=%d\n",
+   msg_get_sequence(msg), msg_get_marker(msg), 
msg_is_in_sequence(jb, msg));
+   return res;
 }
 
 static void msg_set_as_reference(struct jibuf *jb, struct msgb *msg)
@@ -280,11 +302,8 @@
 
clock_gettime_timeval(CLOCK_MONOTONIC, >last_enqueue_time);
 
-   /* If packet contains a mark -> start of talkspurt.
-* A lot of packets may have been suppressed by the sender before it,
-* so let's take it as a reference
-*/
-   if (!jb->started || msg_get_marker(msg)) {
+   /* Check if it's time to sync, ie. start of talkspurt */
+   if (!jb->started || msg_is_syncpoint(jb, msg)) {
jb->started = true;
msg_set_as_reference(jb, msg);
rel_delay = 0;
diff --git a/tests/jibuf/jibuf_test.c b/tests/jibuf/jibuf_test.c
index a6fe1b9..a9a7ff0 100644
--- a/tests/jibuf/jibuf_test.c
+++ b/tests/jibuf/jibuf_test.c
@@ -614,6 +614,72 @@
osmo_jibuf_delete(jb);
 }
 
+void test_rtp_out_of_sync(unsigned int time_inc_ms, uint16_t seq_nosync_inc, 
uint32_t ts_nosync_inc, bool expect_drop)
+{
+   int min_delay = 60;
+   struct msgb *msg;
+   int rc;
+
+   printf("===test_rtp_out_of_sync(%u, %"PRIu16", %"PRIu32", %d)===\n",
+   time_inc_ms, seq_nosync_inc, ts_nosync_inc, expect_drop);
+
+   clock_override_enable(true);
+   clock_override_set(0, 0);
+   rtp_init(32, 400);
+   jb = osmo_jibuf_alloc(NULL);
+   osmo_jibuf_set_dequeue_cb(jb, dequeue_cb, NULL);
+   osmo_jibuf_set_min_delay(jb, min_delay);
+   osmo_jibuf_set_max_delay(jb, 200);
+
+   /* First rtp at t=0, should be scheduled in min_delay time */
+   clock_debug("enqueue 1st packet (seq=33, ts=560)");
+   ENQUEUE_NEXT(jb);
+   clock_override_add(0, TIME_RTP_PKT_MS*1000);
+   clock_debug("enqueue 2nd packet (seq=34, ts=720)");
+   ENQUEUE_NEXT(jb);
+
+   clock_override_add(0, time_inc_ms*1000);
+   clock_debug("2 packets dequeued");
+   osmo_select_main(0);
+
+/* We are at t=20+time_inc_ms, next pkt would normally be dropped 
since it is
+ * pretty late, but since seq and timestamp are out of sync, which
+ * means the sender had some clock issues, the jibuf is going to take
+ * this new tuple as reference and accept it.
+   */
+   clock_debug("enqueue late pkt with possible sync change");
+   rtp_init(rtp_next_seq + seq_nosync_inc, rtp_next_ts + ts_nosync_inc);
+   msg = rtp_new(rtp_next_seq, rtp_next_ts);
+   rc = osmo_jibuf_enqueue(jb, msg);
+   if (expect_drop) {
+   OSMO_ASSERT(rc < 0);
+   msgb_free(msg);
+   } else {
+   OSMO_ASSERT(rc == 0);
+   }
+
+   clock_debug("enqueue late pkt after possible resync");
+   clock_override_add(0, TIME_RTP_PKT_MS*1000);
+   msg = rtp_next();
+   rc = osmo_jibuf_enqueue(jb, msg);
+   if (expect_drop) {
+   OSMO_ASSERT(rc < 0);
+   msgb_free(msg);
+   } else {
+   OSMO_ASSERT(rc == 0);
+   }
+
+   if (!expect_drop) {
+   clock_debug("2 packets dequeued");
+   clock_override_add(0, min_delay*1000);
+   osmo_select_main(0);
+   }
+
+   OSMO_ASSERT(osmo_jibuf_empty(jb));
+
+   

[PATCH] libosmo-netif[master]: tests: jibuf_tool: Add OSMUX support

[Pau Espin Pedrol]

Review at  https://gerrit.osmocom.org/7776

tests: jibuf_tool: Add OSMUX support

Change-Id: I0f02da0329e6739ff340d31113161bb520b1b760
---
M tests/jibuf/jibuf_tool.c
1 file changed, 42 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/76/7776/1

diff --git a/tests/jibuf/jibuf_tool.c b/tests/jibuf/jibuf_tool.c
index c2461ec..faa6e38 100644
--- a/tests/jibuf/jibuf_tool.c
+++ b/tests/jibuf/jibuf_tool.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "../osmo-pcap-test/osmo_pcap.h"
 
@@ -53,6 +54,7 @@
 static bool opt_test_rand;
 static bool opt_debug_human;
 static bool opt_debug_table;
+static bool opt_osmux;
 static char* opt_pcap_file;
 /* - */
 
@@ -102,6 +104,8 @@
 /* Used for test pcap: */
 static struct osmo_pcap osmo_pcap;
 static bool pcap_finished;
+static struct osmux_out_handle pcap_osmux_h;
+static struct llist_head osmux_list;
 /* - */
 
 static void sigalarm_handler(int foo)
@@ -304,7 +308,7 @@
return cb->data;
 }
 
-void send_rtp_packet()
+void rand_send_rtp_packet()
 {
 
struct rtp_pkt_info *pinfo;
@@ -362,7 +366,7 @@
if (!packets_sent)
gettimeofday(_generated, NULL);
 
-   send_rtp_packet();
+   rand_send_rtp_packet();
packets_sent++;
 
timeradd(_generated, _rate, _ts);
@@ -395,9 +399,33 @@
return 0;
 }
 
+void glue_cb(struct msgb *msg, void *data)
+{
+   pcap_generate_pkt_cb(msg);
+}
+
+int pcap_read_osmux(struct msgb *msg)
+{
+   struct osmux_hdr *osmuxh;
+
+   /* This code below belongs to the osmux receiver */
+   while((osmuxh = osmux_xfrm_output_pull(msg)) != NULL) {
+   osmux_xfrm_output(osmuxh, _osmux_h, _list);
+   osmux_tx_sched(_list, glue_cb, NULL);
+   }
+   msgb_free(msg);
+   return 0;
+}
+
 void pcap_pkt_timer_cb(void *data)
 {
-   if (osmo_pcap_test_run(_pcap, IPPROTO_UDP, pcap_generate_pkt_cb) < 
0) {
+   int (*mycb)(struct msgb *msgb);
+   if(opt_osmux)
+   mycb = pcap_read_osmux;
+   else
+   mycb = pcap_generate_pkt_cb;
+
+   if (osmo_pcap_test_run(_pcap, IPPROTO_UDP, mycb) < 0) {
osmo_pcap_stats_printf();
osmo_pcap_test_close(osmo_pcap.h);
pcap_finished=true;
@@ -462,6 +490,11 @@
 
osmo_pcap.timer.cb = pcap_pkt_timer_cb;
 
+   if(opt_osmux) {
+   INIT_LLIST_HEAD(_list);
+   osmux_xfrm_output_init(_osmux_h, 0);
+   }
+
jb = osmo_jibuf_alloc(NULL);
osmo_jibuf_set_dequeue_cb(jb, dequeue_cb, NULL);
osmo_jibuf_set_min_delay(jb, 60);
@@ -480,10 +513,11 @@
 
 static void print_help(void)
 {
-   printf("jibuf_test [-r] [-p pcap] [-d] [-g]\n");
+   printf("jibuf_test [-r] [-p pcap] [-o] [-d] [-g]\n");
printf(" -h Print this help message\n");
printf(" -r Run test with randomly generated jitter\n");
printf(" -p Run test with specified pcap file\n");
+   printf(" -o The pcap contains OSMUX packets isntead of RTP\n");
printf(" -d Enable packet trace debug suitable for humans\n");
printf(" -t Enable packet trace debug suitable for gnuplot\n");
 }
@@ -492,7 +526,7 @@
 {
int opt;
 
-   while ((opt = getopt(argc, argv, "hdtrp:")) != -1) {
+   while ((opt = getopt(argc, argv, "hdtrop:")) != -1) {
switch (opt) {
case 'h':
print_help();
@@ -506,6 +540,9 @@
case 'r':
opt_test_rand = true;
break;
+   case 'o':
+   opt_osmux = true;
+   break;
case 'p':
opt_pcap_file = strdup(optarg);
break;

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I0f02da0329e6739ff340d31113161bb520b1b760
Gerrit-PatchSet: 1
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol 

osmo-bsc[master]: only trigger acc ramping if trx 0 is usable and unlocked

[Harald Welte]

Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I054829a936f0aa1e3fa34fad6466a1cd6150e307
Gerrit-PatchSet: 1
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-HasComments: No

osmo-ci[master]: jobs: gerrit-osmo-ttcn3-hacks: Make deps before cleaning

[Harald Welte]

Patch Set 3: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I07fee78bba5b07c7f3f4359869e00ef2583e0769
Gerrit-PatchSet: 3
Gerrit-Project: osmo-ci
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-HasComments: No

[MERGED] osmo-bsc[master]: only trigger acc ramping if trx 0 is usable and unlocked

[Harald Welte]

Harald Welte has submitted this change and it was merged.

Change subject: only trigger acc ramping if trx 0 is usable and unlocked
..


only trigger acc ramping if trx 0 is usable and unlocked

Starting an ACC ramping process while TRX 0 is unusable or locked is
pointless. For instance, after loading a config with 'rf_locked 1'
for trx 0, the ramping process was started as soon as the BTS
established RSL, even though the air interface was still down.
ACC ramping should instead be triggered once TRX 0 is unlocked.

Change-Id: I054829a936f0aa1e3fa34fad6466a1cd6150e307
Related: OS#2591
---
M src/libbsc/acc_ramp.c
1 file changed, 9 insertions(+), 4 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/src/libbsc/acc_ramp.c b/src/libbsc/acc_ramp.c
index 16bce3f..7116107 100644
--- a/src/libbsc/acc_ramp.c
+++ b/src/libbsc/acc_ramp.c
@@ -26,6 +26,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 /*
@@ -257,7 +258,7 @@
 
 /*!
  * Determine if ACC ramping should be started according to configuration, and
- * if ACC ramping is enabled, begin the ramping process.
+ * begin the ramping process if the necessary conditions are present.
  * Perform at least one ramping step to allow 'step_size' ACCs.
  * If 'step_size' is ACC_RAMP_STEP_SIZE_MAX, or if ACC ramping is disabled,
  * all ACCs will be allowed immediately.
@@ -269,9 +270,13 @@
acc_ramp_abort(acc_ramp);
 
if (acc_ramp_is_enabled(acc_ramp)) {
-   /* Set all available ACCs to barred and start ramping up. */
-   barr_all_accs(acc_ramp);
-   do_acc_ramping_step(acc_ramp);
+   struct gsm_bts_trx *trx0 = gsm_bts_trx_by_nr(acc_ramp->bts, 0);
+   /* TRX 0 should be usable and unlocked, otherwise starting ACC 
ramping is pointless. */
+   if (trx0 && trx_is_usable(trx0) && 
trx0->mo.nm_state.administrative == NM_STATE_UNLOCKED) {
+   /* Set all available ACCs to barred and start ramping 
up. */
+   barr_all_accs(acc_ramp);
+   do_acc_ramping_step(acc_ramp);
+   }
}
 }
 

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I054829a936f0aa1e3fa34fad6466a1cd6150e307
Gerrit-PatchSet: 2
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 

osmo-pcu[master]: tbf: add frame number to log output

[Harald Welte]

Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5a744dc5cd7c1de1baea13fffac026c83d091429
Gerrit-PatchSet: 2
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Owner: dexter 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-HasComments: No

[ABANDON] osmo-pcu[master]: tbf: hex output of tbf data in log

[dexter]

dexter has abandoned this change.

Change subject: tbf: hex output of tbf data in log
..


Abandoned

Its probably not a good idea to clog up the log with hexdumps.

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: I6c5adda8aaead00b1ba4359c34866e6842ea161f
Gerrit-PatchSet: 1
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Owner: dexter 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder

osmo-bsc[master]: only trigger acc ramping if trx 0 is usable and unlocked

[Stefan Sperling]

Patch Set 1:

(1 comment)

https://gerrit.osmocom.org/#/c/7772/1/src/libbsc/acc_ramp.c
File src/libbsc/acc_ramp.c:

Line 267:   if (trx0 && trx_is_usable(trx0) && 
trx0->mo.nm_state.administrative == NM_STATE_UNLOCKED) {
> I know that we are then enabling ramping when we receive the event to becom
At present trx_is_usable() ignores the administrative state entirely, because 
the nm_is_running() function does not look at it.

So currently we need both checks here. Maybe this should be changed, but 
changing a widely used function like trx_is_usable() is out of scope for this 
patch set.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I054829a936f0aa1e3fa34fad6466a1cd6150e307
Gerrit-PatchSet: 1
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-HasComments: Yes

[MERGED] osmo-msc[master]: libmsc/ussd: don't overwrite rc if decoding failed

[Harald Welte]

Harald Welte has submitted this change and it was merged.

Change subject: libmsc/ussd: don't overwrite rc if decoding failed
..


libmsc/ussd: don't overwrite rc if decoding failed

Change-Id: I344e4b3a9aad617686a7ddbdeae5780fd8b07e58
---
M src/libmsc/ussd.c
1 file changed, 3 insertions(+), 2 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/src/libmsc/ussd.c b/src/libmsc/ussd.c
index ab59571..fe1610f 100644
--- a/src/libmsc/ussd.c
+++ b/src/libmsc/ussd.c
@@ -74,8 +74,9 @@
if (!rc) {
LOGP(DMM, LOGL_ERROR, "SS/USSD message parsing error, "
"rejecting request...\n");
-   rc = gsm0480_send_ussd_reject(conn, );
-   return rc;
+   gsm0480_send_ussd_reject(conn, );
+   /* The GSM 04.80 API uses inverted codes (0 means error) */
+   return -EPROTO;
}
 
/* Interrogation or releaseComplete? */

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I344e4b3a9aad617686a7ddbdeae5780fd8b07e58
Gerrit-PatchSet: 5
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Owner: Vadim Yanitskiy 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder

osmo-mgw[master]: stats: use libosmocore rate counter for in/out_stream.err_ts...

[Harald Welte]

Patch Set 1: Code-Review-1

(2 comments)

https://gerrit.osmocom.org/#/c/7555/1/include/osmocom/mgcp/mgcp_internal.h
File include/osmocom/mgcp/mgcp_internal.h:

Line 207:   struct rate_ctr_group_desc rate_ctr_group_desc;
the rate_ctr_group_desc and rate_ctr_desc should be global data (if not even 
const?).  Wat is the rationale of copying it into each and every mgcp_state?  
Only the "rate_ctr_group *" should be here, AFAICT.


https://gerrit.osmocom.org/#/c/7555/1/src/libosmo-mgcp/mgcp_conn.c
File src/libosmo-mgcp/mgcp_conn.c:

Line 119:   talloc_strdup(conn, "conn_rtp");
all of those strings should be static/const strings, no copying or dynamic 
allocations involved.  Did you check existing examples about how we use 
rate_ctr in the osmocom code?  If there are any examples like thism, please let 
me know, as we need to fix it.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I67aa7a8602f60366ef3ba2c5b1319b1b85719f64
Gerrit-PatchSet: 1
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Owner: dexter 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-HasComments: Yes

osmo-ci[master]: jobs: gerrit-osmo-ttcn3-hacks: Make deps before cleaning

[Pau Espin Pedrol]

Patch Set 3:

ping. Around 2 weeks without review.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I07fee78bba5b07c7f3f4359869e00ef2583e0769
Gerrit-PatchSet: 3
Gerrit-Project: osmo-ci
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-HasComments: No

osmo-trx[master]: jenkins.sh: change qemu-img default location to $HOME/qemu-i...

[Harald Welte]

Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I56f314d78c0ca968b1fef9a91ecd540a7cc8fa86
Gerrit-PatchSet: 1
Gerrit-Project: osmo-trx
Gerrit-Branch: master
Gerrit-Owner: lynxis lazus 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: lynxis lazus 
Gerrit-HasComments: No

[MERGED] osmo-trx[master]: jenkins.sh: change qemu-img default location to $HOME/qemu-i...

[Harald Welte]

Harald Welte has submitted this change and it was merged.

Change subject: jenkins.sh: change qemu-img default location to $HOME/qemu-img 
instead of /opt/qemu-img
..


jenkins.sh: change qemu-img default location to $HOME/qemu-img instead of 
/opt/qemu-img

Change-Id: I56f314d78c0ca968b1fef9a91ecd540a7cc8fa86
---
M contrib/jenkins.sh
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  Pau Espin Pedrol: Looks good to me, but someone else must approve
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/contrib/jenkins.sh b/contrib/jenkins.sh
index 10fc2b1..c4d786d 100755
--- a/contrib/jenkins.sh
+++ b/contrib/jenkins.sh
@@ -28,7 +28,7 @@
 if ! $(substr "arm" "$(uname -m)") && [ "x${INSTR}" = "x--with-neon" 
-o "x${INSTR}" = "x--with-neon-vfpv4" ]; then
 
 OSMOTRX_DIR="$PWD" # we assume we are called as 
contrib/jenkins.sh
-ROOTFS_PREFIX="${ROOTFS_PREFIX:-/opt}"
+ROOTFS_PREFIX="${ROOTFS_PREFIX:-$HOME}"
 ROOTFS="${ROOTFS_PREFIX}/qemu-img"
 mkdir -p "${ROOTFS_PREFIX}"
 

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I56f314d78c0ca968b1fef9a91ecd540a7cc8fa86
Gerrit-PatchSet: 2
Gerrit-Project: osmo-trx
Gerrit-Branch: master
Gerrit-Owner: lynxis lazus 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: lynxis lazus 

docker-playground[master]: jenkins-common: move workspace to /tmp/jenkins when build ou...

[Harald Welte]

Patch Set 1:

I don't mind, but I'm also not sure this is what users want? Opinions, please

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8a139ef2d23dd8115191e51e86ef0346cb97e4cf
Gerrit-PatchSet: 1
Gerrit-Project: docker-playground
Gerrit-Branch: master
Gerrit-Owner: lynxis lazus 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: dexter 
Gerrit-Reviewer: neels 
Gerrit-HasComments: No

docker-playground[master]: Makefile: use $USER/image when depending on a generated image

[Harald Welte]

Patch Set 2: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ied42c3e1de9a2ffaca22ba4cd02e6a398e48e97d
Gerrit-PatchSet: 2
Gerrit-Project: docker-playground
Gerrit-Branch: master
Gerrit-Owner: lynxis lazus 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: lynxis lazus 
Gerrit-HasComments: No

[MERGED] docker-playground[master]: Makefile: use $USER/image when depending on a generated image

[Harald Welte]

Harald Welte has submitted this change and it was merged.

Change subject: Makefile: use $USER/image when depending on a generated image
..


Makefile: use $USER/image when depending on a generated image

Te make scripts will generate docker images like
"$username/foobar-test". When depending on an previous image,
the $username must match or the build will about with image not found.

Change-Id: Ied42c3e1de9a2ffaca22ba4cd02e6a398e48e97d
---
M m3ua-test/Dockerfile
M make/Makefile
M sua-test/Dockerfile
3 files changed, 5 insertions(+), 3 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved; Verified



diff --git a/m3ua-test/Dockerfile b/m3ua-test/Dockerfile
index edd139a..ba5d17d 100644
--- a/m3ua-test/Dockerfile
+++ b/m3ua-test/Dockerfile
@@ -1,4 +1,5 @@
-FROM laforge/sigtran-tests
+ARG USER=osmocom-build
+FROM $USER/sigtran-tests
 
 MAINTAINER Harald Welte 
 
diff --git a/make/Makefile b/make/Makefile
index 21614a8..f492d60 100644
--- a/make/Makefile
+++ b/make/Makefile
@@ -40,7 +40,7 @@
 
 
 docker-build: .release
-   docker build -t $(IMAGE):latest .
+   docker build --build-arg USER=$(USERNAME) -t $(IMAGE):latest .
@DOCKER_MAJOR=$(shell docker -v | sed -e 's/.*version //' -e 's/,.*//' 
| cut -d\. -f1) ; \
DOCKER_MINOR=$(shell docker -v | sed -e 's/.*version //' -e 's/,.*//' | 
cut -d\. -f2) ; \
 
diff --git a/sua-test/Dockerfile b/sua-test/Dockerfile
index 141497e..f7f071a 100644
--- a/sua-test/Dockerfile
+++ b/sua-test/Dockerfile
@@ -1,4 +1,5 @@
-FROM laforge/sigtran-tests
+ARG USER=osmocom-build
+FROM $USER/sigtran-tests
 
 MAINTAINER Harald Welte 
 

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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ied42c3e1de9a2ffaca22ba4cd02e6a398e48e97d
Gerrit-PatchSet: 2
Gerrit-Project: docker-playground
Gerrit-Branch: master
Gerrit-Owner: lynxis lazus 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: lynxis lazus 

[MERGED] docker-playground[master]: add .gitreview

[Harald Welte]

Harald Welte has submitted this change and it was merged.

Change subject: add .gitreview
..


add .gitreview

Allows `git review` to automatic setup gerrit

Change-Id: I179c02d38496bf3d408020db7f0baf79ee064705
---
A .gitreview
1 file changed, 3 insertions(+), 0 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved; Verified



diff --git a/.gitreview b/.gitreview
new file mode 100644
index 000..6a6092f
--- /dev/null
+++ b/.gitreview
@@ -0,0 +1,3 @@
+[gerrit]
+host=gerrit.osmocom.org
+project=docker-playground

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I179c02d38496bf3d408020db7f0baf79ee064705
Gerrit-PatchSet: 2
Gerrit-Project: docker-playground
Gerrit-Branch: master
Gerrit-Owner: lynxis lazus 
Gerrit-Reviewer: Harald Welte 

[MERGED] osmo-gsm-manuals[master]: jenkins.sh: add hostkey for osmocom.org:48

[Harald Welte]

Harald Welte has submitted this change and it was merged.

Change subject: jenkins.sh: add hostkey for osmocom.org:48
..


jenkins.sh: add hostkey for osmocom.org:48

Otherwise it will depend on a setting this up in a seperate step.
This way it's more decoupled from the build host.

Change-Id: Iea1f5810bc7d4370724fdd7eb875c9a27b3d82af
---
M contrib/jenkins.sh
1 file changed, 8 insertions(+), 1 deletion(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/contrib/jenkins.sh b/contrib/jenkins.sh
index 59fcfd7..5cd1c1f 100755
--- a/contrib/jenkins.sh
+++ b/contrib/jenkins.sh
@@ -22,7 +22,14 @@
 if [ "x$publish" = "x--publish" ]; then
   mkdir out/
   cp */*.pdf out/
-  rsync -avz --delete -e "ssh -p 48" ./out/ d...@osmocom.org:web-files/latest/
+
+  cat > "$WORKSPACE/known_hosts" 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: lynxis lazus 

osmo-hlr[master]: rewrite subscriber_update_notify() without calls into luop

[Harald Welte]

Patch Set 1:

leaving this to neels.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I06c43ece2b48dc63d599000eb6d6d51e08963067
Gerrit-PatchSet: 1
Gerrit-Project: osmo-hlr
Gerrit-Branch: master
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels 
Gerrit-HasComments: No

osmo-ttcn3-hacks[master]: */regen_makefile.sh: drop multiple naming of main ttcn file

[Harald Welte]

Patch Set 2: Code-Review-1

> Is there any reason why the main file should be listed first?
 > Couldn't see any adverse effects in running the BSC tests without
 > that.

At least when I started the TTCN-3 developments, there was.  This is why I 
implemented the regen_makefile the way they are.  It might not be required with 
the runtime that we currently use, but there are TITAN configurations that 
require it.  Probably when you're linking one executable and not just building 
all the .so files.  The change from one executable to dozens of .so files was 
done as I believe the junit xml plugin needs it.

As this is purely cosmetic and likely to cause breakage should we ever change 
this again, I'm against this change.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I94f37d64198ce2e36c5cd8892ccd33987e1fba32
Gerrit-PatchSet: 2
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-HasComments: No

osmo-bsc[master]: only trigger acc ramping if trx 0 is usable and unlocked

[Pau Espin Pedrol]

Patch Set 1:

(1 comment)

https://gerrit.osmocom.org/#/c/7772/1/src/libbsc/acc_ramp.c
File src/libbsc/acc_ramp.c:

Line 267:   if (trx0 && trx_is_usable(trx0) && 
trx0->mo.nm_state.administrative == NM_STATE_UNLOCKED) {
I know that we are then enabling ramping when we receive the event to become 
unlocked (NM_STATE_UNLOCKED). What about the other condition here? Do we also 
start it when trx_is_usable() goes from return false to returning true?

Shouldn't the trx_is_usable() already contain the check for the administrative 
state? I'm not sure if it makes sense to state it is usable while it is 
actually locked on the  administrate side.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I054829a936f0aa1e3fa34fad6466a1cd6150e307
Gerrit-PatchSet: 1
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-HasComments: Yes

[MERGED] osmo-bsc[master]: trigger acc ramping on state-changed-event reports

[Harald Welte]

Harald Welte has submitted this change and it was merged.

Change subject: trigger acc ramping on state-changed-event reports
..


trigger acc ramping on state-changed-event reports

Trigger ACC ramping not only when an Administrative State Change
ACK is received from a BTS, but also when an administrative state
change is reported for TRX 0 in a State Changed Event Report.

This should allow ACC ramping to work with any BTS which reports
an administrative state change to 'unlock' using either of these
OML messages.

Tested with a sysmobts and a nanobts.

The sysmobts only reports TRX locked/unlock changes in Administrative
State Change ACKs, not via State Changed Event Reports.

The nanobts is known to send both of these OML messages in quick
succession, so do not re-trigger ramping if it's already in progress.

Change-Id: I097a113a3a63de02bcb8277020beb59cf485b176
Related: OS#2591
---
M src/libbsc/acc_ramp.c
1 file changed, 10 insertions(+), 2 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/src/libbsc/acc_ramp.c b/src/libbsc/acc_ramp.c
index 6d9be59..16bce3f 100644
--- a/src/libbsc/acc_ramp.c
+++ b/src/libbsc/acc_ramp.c
@@ -144,7 +144,8 @@
struct acc_ramp *acc_ramp = handler_data;
struct gsm_bts_trx *trx = NULL;
 
-   if (signal != S_NM_STATECHG_ADM)
+   /* Handled signals map to an Administrative State Change ACK, or a 
State Changed Event Report. */
+   if (signal != S_NM_STATECHG_ADM && signal != S_NM_STATECHG_OPER)
return 0;
 
if (nsd->obj_class != NM_OC_RADIO_CARRIER)
@@ -163,7 +164,14 @@
/* Trigger or abort ACC ramping based on the new 'RF lock' state of 
this TRX. */
switch (nsd->new_state->administrative) {
case NM_STATE_UNLOCKED:
-   acc_ramp_trigger(acc_ramp);
+   /*
+* Do not re-trigger ACC ramping if ramping is already in 
progress.
+* A BTS might send several "unlock" change events: One in the 
Administrative
+* State Change ACK, and/or another in a State Changed Event 
Report.
+* For instance, the nanobts is known to send both.
+*/
+   if (!osmo_timer_pending(_ramp->step_timer))
+   acc_ramp_trigger(acc_ramp);
break;
case NM_STATE_LOCKED:
case NM_STATE_SHUTDOWN:

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I097a113a3a63de02bcb8277020beb59cf485b176
Gerrit-PatchSet: 2
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Stefan Sperling 

osmo-bsc[master]: trigger acc ramping on state-changed-event reports

[Harald Welte]

Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I097a113a3a63de02bcb8277020beb59cf485b176
Gerrit-PatchSet: 2
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Stefan Sperling 
Gerrit-HasComments: No

osmo-ttcn3-hacks[master]: */regen_makefile.sh: drop multiple naming of main ttcn file

[Pau Espin Pedrol]

Patch Set 2:

> Is there any reason why the main file should be listed first?
 > Couldn't see any adverse effects in running the BSC tests without
 > that.

I had the same question while moving stuff to the build/ dir in 
https://gerrit.osmocom.org/#/c/7405/

If it's not needed, then +1 for me for this patch.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I94f37d64198ce2e36c5cd8892ccd33987e1fba32
Gerrit-PatchSet: 2
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-HasComments: No

[PATCH] osmo-bsc[master]: trigger acc ramping on state-changed-event reports

[Stefan Sperling]

Hello Jenkins Builder,

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

https://gerrit.osmocom.org/7770

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

trigger acc ramping on state-changed-event reports

Trigger ACC ramping not only when an Administrative State Change
ACK is received from a BTS, but also when an administrative state
change is reported for TRX 0 in a State Changed Event Report.

This should allow ACC ramping to work with any BTS which reports
an administrative state change to 'unlock' using either of these
OML messages.

Tested with a sysmobts and a nanobts.

The sysmobts only reports TRX locked/unlock changes in Administrative
State Change ACKs, not via State Changed Event Reports.

The nanobts is known to send both of these OML messages in quick
succession, so do not re-trigger ramping if it's already in progress.

Change-Id: I097a113a3a63de02bcb8277020beb59cf485b176
Related: OS#2591
---
M src/libbsc/acc_ramp.c
1 file changed, 10 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/70/7770/2

diff --git a/src/libbsc/acc_ramp.c b/src/libbsc/acc_ramp.c
index 6d9be59..16bce3f 100644
--- a/src/libbsc/acc_ramp.c
+++ b/src/libbsc/acc_ramp.c
@@ -144,7 +144,8 @@
struct acc_ramp *acc_ramp = handler_data;
struct gsm_bts_trx *trx = NULL;
 
-   if (signal != S_NM_STATECHG_ADM)
+   /* Handled signals map to an Administrative State Change ACK, or a 
State Changed Event Report. */
+   if (signal != S_NM_STATECHG_ADM && signal != S_NM_STATECHG_OPER)
return 0;
 
if (nsd->obj_class != NM_OC_RADIO_CARRIER)
@@ -163,7 +164,14 @@
/* Trigger or abort ACC ramping based on the new 'RF lock' state of 
this TRX. */
switch (nsd->new_state->administrative) {
case NM_STATE_UNLOCKED:
-   acc_ramp_trigger(acc_ramp);
+   /*
+* Do not re-trigger ACC ramping if ramping is already in 
progress.
+* A BTS might send several "unlock" change events: One in the 
Administrative
+* State Change ACK, and/or another in a State Changed Event 
Report.
+* For instance, the nanobts is known to send both.
+*/
+   if (!osmo_timer_pending(_ramp->step_timer))
+   acc_ramp_trigger(acc_ramp);
break;
case NM_STATE_LOCKED:
case NM_STATE_SHUTDOWN:

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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I097a113a3a63de02bcb8277020beb59cf485b176
Gerrit-PatchSet: 2
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Stefan Sperling 

osmo-bsc[master]: trigger acc ramping on state-changed-event reports

[Stefan Sperling]

Patch Set 1:

> Please also test with osmo-bts

A sysmobts reports TRX lock/unlock only via Change Administrative State ACK.

Its State Changed Event Reports only contain information about operational and 
availability states (as we would expect from reading the GSM spec).

Unlike the nanobts, a sysmobts does not send a State Changed event Report when 
trx 0 is locked or unlocked. It looks like these messages do not trigger for 
administrative state changes.

Regardless, ACC ramping still works as expected with a sysmobts.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I097a113a3a63de02bcb8277020beb59cf485b176
Gerrit-PatchSet: 1
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Stefan Sperling 
Gerrit-HasComments: No

[PATCH] osmo-bsc[master]: only trigger acc ramping if trx 0 is usable and unlocked

[Stefan Sperling]

Review at  https://gerrit.osmocom.org/7772

only trigger acc ramping if trx 0 is usable and unlocked

Starting an ACC ramping process while TRX 0 is unusable or locked is
pointless. For instance, after loading a config with 'rf_locked 1'
for trx 0, the ramping process was started as soon as the BTS
established RSL, even though the air interface was still down.
ACC ramping should instead be triggered once TRX 0 is unlocked.

Change-Id: I054829a936f0aa1e3fa34fad6466a1cd6150e307
Related: OS#2591
---
M src/libbsc/acc_ramp.c
1 file changed, 9 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/72/7772/1

diff --git a/src/libbsc/acc_ramp.c b/src/libbsc/acc_ramp.c
index 6d9be59..441be68 100644
--- a/src/libbsc/acc_ramp.c
+++ b/src/libbsc/acc_ramp.c
@@ -26,6 +26,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 /*
@@ -249,7 +250,7 @@
 
 /*!
  * Determine if ACC ramping should be started according to configuration, and
- * if ACC ramping is enabled, begin the ramping process.
+ * begin the ramping process if the necessary conditions are present.
  * Perform at least one ramping step to allow 'step_size' ACCs.
  * If 'step_size' is ACC_RAMP_STEP_SIZE_MAX, or if ACC ramping is disabled,
  * all ACCs will be allowed immediately.
@@ -261,9 +262,13 @@
acc_ramp_abort(acc_ramp);
 
if (acc_ramp_is_enabled(acc_ramp)) {
-   /* Set all available ACCs to barred and start ramping up. */
-   barr_all_accs(acc_ramp);
-   do_acc_ramping_step(acc_ramp);
+   struct gsm_bts_trx *trx0 = gsm_bts_trx_by_nr(acc_ramp->bts, 0);
+   /* TRX 0 should be usable and unlocked, otherwise starting ACC 
ramping is pointless. */
+   if (trx0 && trx_is_usable(trx0) && 
trx0->mo.nm_state.administrative == NM_STATE_UNLOCKED) {
+   /* Set all available ACCs to barred and start ramping 
up. */
+   barr_all_accs(acc_ramp);
+   do_acc_ramping_step(acc_ramp);
+   }
}
 }
 

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I054829a936f0aa1e3fa34fad6466a1cd6150e307
Gerrit-PatchSet: 1
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Owner: Stefan Sperling