[MERGED] osmo-pcu[master]: Revert "Rewrite Packet Downlink Assignment"

2018-03-28 Thread Harald Welte 
Harald Welte has submitted this change and it was merged.

Change subject: Revert "Rewrite Packet Downlink Assignment"
..


Revert "Rewrite Packet Downlink Assignment"

This reverts commit 896574e92bea09ed8d39688b6fdf504e84521746,
I52ec9b07413daabba8cd5f1fba5c7b3af6a33389.

This commit was found (empirically) to be a regression, rendering GPRS service
fatally unreliable.

Related: OS#3013
Change-Id: Idcba0381f70eb7f7c9aefdee9dfeafd5de96a9be
---
M src/encoding.cpp
1 file changed, 48 insertions(+), 73 deletions(-)

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



diff --git a/src/encoding.cpp b/src/encoding.cpp
index 279bbfe..7b711b8 100644
--- a/src/encoding.cpp
+++ b/src/encoding.cpp
@@ -44,21 +44,6 @@
 #define set_H(bv) set_x(bv, H)
 
 /* { 0 | 1 < TIMING_ADVANCE_INDEX : bit (4) > } */
-static inline int write_ta_index(bitvec *dest, int8_t tai)
-{
-   int rc;
-
-   if (tai < 0) /* No TIMING_ADVANCE_INDEX: */
-   set_0(dest);
-
-   /* TIMING_ADVANCE_INDEX: */
-   set_1(dest);
-   rc = bitvec_set_u64(dest, tai, 4, false);
-   check(rc);
-
-   return 0;
-}
-
 static inline bool write_tai(bitvec *dest, unsigned& wp, int8_t tai)
 {
if (tai < 0) { /* No TIMING_ADVANCE_INDEX: */
@@ -174,58 +159,52 @@
return 0;
 }
 
-/* 3GPP TS 44.018 §10.5.2.16 IA Rest Octets ::= Packet Downlink Assignment */
-static inline int write_ia_rest_downlink(const gprs_rlcmac_dl_tbf *tbf, bitvec 
*dest,
-bool polling, bool ta_valid, uint32_t 
fn,
-uint8_t alpha, uint8_t gamma, int8_t 
ta_idx)
+static int write_ia_rest_downlink(
+   gprs_rlcmac_dl_tbf *tbf,
+   bitvec * dest, unsigned& wp,
+   uint8_t polling, bool ta_valid, uint32_t fn,
+   uint8_t alpha, uint8_t gamma, int8_t ta_idx)
 {
-   int rc;
-
-   set_H(dest); set_H(dest);
-   set_0(dest); set_1(dest); /* 00 Packet Downlink Assignment */
-
-   rc = bitvec_set_u64(dest, tbf->tlli(), 32, false); /* TLLI */
-   check(rc);
-
-   set_1(dest);
-   rc = bitvec_set_u64(dest, tbf->tfi(), 5, false);   /* TFI_ASSIGNMENT */
-   check(rc);
-
-   /* RLC acknowledged mode */
-   set_0(dest); /* RLC_MODE */
-
-   rc = write_alpha_gamma(dest, alpha, gamma);/* ALPHA and GAMMA */
-   check(rc);
-
-   rc = bitvec_set_bit(dest, (bit_value)polling); /* POLLING */
-   check(rc);
-
-   /* N. B: NOT related to TAI! */
-   rc = bitvec_set_bit(dest, (bit_value)ta_valid); /* TA_VALID */
-   check(rc);
-
-   rc = write_ta_index(dest, ta_idx);
-   check(rc);
-
+   if (!tbf) {
+   LOGP(DRLCMACDL, LOGL_ERROR,
+   "Cannot encode DL IMMEDIATE ASSIGNMENT without TBF\n");
+   return -EINVAL;
+   }
+   // GSM 04.08 10.5.2.16 IA Rest Octets
+   bitvec_write_field(dest, , 3, 2);   // "HH"
+   bitvec_write_field(dest, , 1, 2);   // "01" Packet Downlink 
Assignment
+   bitvec_write_field(dest, ,tbf->tlli(),32); // TLLI
+   bitvec_write_field(dest, ,0x1,1);   // switch TFI   : on
+   bitvec_write_field(dest, ,tbf->tfi(),5);   // TFI
+   bitvec_write_field(dest, ,0x0,1);   // RLC acknowledged mode
+   if (alpha) {
+   bitvec_write_field(dest, ,0x1,1);   // ALPHA = present
+   bitvec_write_field(dest, ,alpha,4);   // ALPHA
+   } else {
+   bitvec_write_field(dest, ,0x0,1);   // ALPHA = not present
+   }
+   bitvec_write_field(dest, ,gamma,5);   // GAMMA power control 
parameter
+   bitvec_write_field(dest, ,polling,1);   // Polling Bit
+   bitvec_write_field(dest, , ta_valid, 1); // N. B: NOT related to TAI!
+   write_tai(dest, wp, ta_idx);
if (polling) {
-   set_1(dest);
-   rc = write_tbf_start_time(dest, fn);/* TBF_STARTING_TIME */
-   check(rc);
-   } else
-   set_0(dest); /* No TBF_STARTING_TIME */
-
-   set_0(dest); /* No P0 nor PR_MODE */
-
+   bitvec_write_field(dest, ,0x1,1);   // TBF Starting TIME 
present
+   bitvec_write_field(dest, ,(fn / (26 * 51)) % 32,5); // T1'
+   bitvec_write_field(dest, ,fn % 51,6);   // T3
+   bitvec_write_field(dest, ,fn % 26,5);   // T2
+   } else {
+   bitvec_write_field(dest, ,0x0,1);   // TBF Starting TIME 
present
+   }
+   bitvec_write_field(dest, ,0x0,1);   // P0 not present
+   //  bitvec_write_field(dest, ,0x1,1);   // P0 not present
+   //  bitvec_write_field(dest, ,,0xb,4);
if (tbf->is_egprs_enabled()) {
-   set_H(dest);
-   rc = bitvec_set_u64(dest, enc_ws(tbf->window_size()), 5, 
false); /* EGPRS Window Size */
-

osmo-pcu[master]: Revert "Rewrite Packet Downlink Assignment"

2018-03-28 Thread Harald Welte 

Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idcba0381f70eb7f7c9aefdee9dfeafd5de96a9be
Gerrit-PatchSet: 1
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-HasComments: No

osmo-pcu[master]: Revert "Rewrite Packet Downlink Assignment"

2018-02-27 Thread Neels Hofmeyr 

Patch Set 1: Verified+1

verified on sysmoBTS that GPRS works better after the revert of these four 
patches

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idcba0381f70eb7f7c9aefdee9dfeafd5de96a9be
Gerrit-PatchSet: 1
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-HasComments: No

[PATCH] osmo-pcu[master]: Revert "Rewrite Packet Downlink Assignment"

2018-02-27 Thread Neels Hofmeyr 

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

Revert "Rewrite Packet Downlink Assignment"

This reverts commit 896574e92bea09ed8d39688b6fdf504e84521746,
I52ec9b07413daabba8cd5f1fba5c7b3af6a33389.

This commit was found (empirically) to be a regression, rendering GPRS service
fatally unreliable.

Related: OS#3013
Change-Id: Idcba0381f70eb7f7c9aefdee9dfeafd5de96a9be
---
M src/encoding.cpp
1 file changed, 48 insertions(+), 73 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/78/6978/1

diff --git a/src/encoding.cpp b/src/encoding.cpp
index 279bbfe..7b711b8 100644
--- a/src/encoding.cpp
+++ b/src/encoding.cpp
@@ -44,21 +44,6 @@
 #define set_H(bv) set_x(bv, H)
 
 /* { 0 | 1 < TIMING_ADVANCE_INDEX : bit (4) > } */
-static inline int write_ta_index(bitvec *dest, int8_t tai)
-{
-   int rc;
-
-   if (tai < 0) /* No TIMING_ADVANCE_INDEX: */
-   set_0(dest);
-
-   /* TIMING_ADVANCE_INDEX: */
-   set_1(dest);
-   rc = bitvec_set_u64(dest, tai, 4, false);
-   check(rc);
-
-   return 0;
-}
-
 static inline bool write_tai(bitvec *dest, unsigned& wp, int8_t tai)
 {
if (tai < 0) { /* No TIMING_ADVANCE_INDEX: */
@@ -174,58 +159,52 @@
return 0;
 }
 
-/* 3GPP TS 44.018 §10.5.2.16 IA Rest Octets ::= Packet Downlink Assignment */
-static inline int write_ia_rest_downlink(const gprs_rlcmac_dl_tbf *tbf, bitvec 
*dest,
-bool polling, bool ta_valid, uint32_t 
fn,
-uint8_t alpha, uint8_t gamma, int8_t 
ta_idx)
+static int write_ia_rest_downlink(
+   gprs_rlcmac_dl_tbf *tbf,
+   bitvec * dest, unsigned& wp,
+   uint8_t polling, bool ta_valid, uint32_t fn,
+   uint8_t alpha, uint8_t gamma, int8_t ta_idx)
 {
-   int rc;
-
-   set_H(dest); set_H(dest);
-   set_0(dest); set_1(dest); /* 00 Packet Downlink Assignment */
-
-   rc = bitvec_set_u64(dest, tbf->tlli(), 32, false); /* TLLI */
-   check(rc);
-
-   set_1(dest);
-   rc = bitvec_set_u64(dest, tbf->tfi(), 5, false);   /* TFI_ASSIGNMENT */
-   check(rc);
-
-   /* RLC acknowledged mode */
-   set_0(dest); /* RLC_MODE */
-
-   rc = write_alpha_gamma(dest, alpha, gamma);/* ALPHA and GAMMA */
-   check(rc);
-
-   rc = bitvec_set_bit(dest, (bit_value)polling); /* POLLING */
-   check(rc);
-
-   /* N. B: NOT related to TAI! */
-   rc = bitvec_set_bit(dest, (bit_value)ta_valid); /* TA_VALID */
-   check(rc);
-
-   rc = write_ta_index(dest, ta_idx);
-   check(rc);
-
+   if (!tbf) {
+   LOGP(DRLCMACDL, LOGL_ERROR,
+   "Cannot encode DL IMMEDIATE ASSIGNMENT without TBF\n");
+   return -EINVAL;
+   }
+   // GSM 04.08 10.5.2.16 IA Rest Octets
+   bitvec_write_field(dest, , 3, 2);   // "HH"
+   bitvec_write_field(dest, , 1, 2);   // "01" Packet Downlink 
Assignment
+   bitvec_write_field(dest, ,tbf->tlli(),32); // TLLI
+   bitvec_write_field(dest, ,0x1,1);   // switch TFI   : on
+   bitvec_write_field(dest, ,tbf->tfi(),5);   // TFI
+   bitvec_write_field(dest, ,0x0,1);   // RLC acknowledged mode
+   if (alpha) {
+   bitvec_write_field(dest, ,0x1,1);   // ALPHA = present
+   bitvec_write_field(dest, ,alpha,4);   // ALPHA
+   } else {
+   bitvec_write_field(dest, ,0x0,1);   // ALPHA = not present
+   }
+   bitvec_write_field(dest, ,gamma,5);   // GAMMA power control 
parameter
+   bitvec_write_field(dest, ,polling,1);   // Polling Bit
+   bitvec_write_field(dest, , ta_valid, 1); // N. B: NOT related to TAI!
+   write_tai(dest, wp, ta_idx);
if (polling) {
-   set_1(dest);
-   rc = write_tbf_start_time(dest, fn);/* TBF_STARTING_TIME */
-   check(rc);
-   } else
-   set_0(dest); /* No TBF_STARTING_TIME */
-
-   set_0(dest); /* No P0 nor PR_MODE */
-
+   bitvec_write_field(dest, ,0x1,1);   // TBF Starting TIME 
present
+   bitvec_write_field(dest, ,(fn / (26 * 51)) % 32,5); // T1'
+   bitvec_write_field(dest, ,fn % 51,6);   // T3
+   bitvec_write_field(dest, ,fn % 26,5);   // T2
+   } else {
+   bitvec_write_field(dest, ,0x0,1);   // TBF Starting TIME 
present
+   }
+   bitvec_write_field(dest, ,0x0,1);   // P0 not present
+   //  bitvec_write_field(dest, ,0x1,1);   // P0 not present
+   //  bitvec_write_field(dest, ,,0xb,4);
if (tbf->is_egprs_enabled()) {
-   set_H(dest);
-   rc = bitvec_set_u64(dest, enc_ws(tbf->window_size()), 5, 
false); /* EGPRS Window Size */
-   check(rc);
-
-   /* The mobile station shall not report measurements: (see 3GPP 
TS 44.060 Table 11.2.7.1) */
-   set_0(dest); set_0(dest); /*