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

2018-03-30 Thread Neels Hofmeyr 
Neels Hofmeyr has submitted this change and it was merged.

Change subject: Revert "Rewrite EGPRS Packet Uplink Assignment"
..


Revert "Rewrite EGPRS Packet Uplink Assignment"

This reverts commit 529ce885450946d85d1920fb3d1a994c3efe5849,
I2139fb347b3290621bbc3f6a031f7f213d372e65.

Commit I52ec9b07413daabba8cd5f1fba5c7b3af6a33389 /
896574e92bea09ed8d39688b6fdf504e84521746 was found (empirically) to be a
regression, rendering GPRS service fatally unreliable.

This reverted commit seems to be related to the regression and is reverted
along with it.

Related: OS#3013
Change-Id: I3e8cc0e8ba3ba5bd444124fd4cb95ef92a71fdfb
---
M src/encoding.cpp
1 file changed, 55 insertions(+), 117 deletions(-)

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



diff --git a/src/encoding.cpp b/src/encoding.cpp
index 7b711b8..b2da1e8 100644
--- a/src/encoding.cpp
+++ b/src/encoding.cpp
@@ -36,13 +36,6 @@
 #include 
 #include 
 
-#define check(rc) { if (rc < 0) return rc; }
-#define set_x(bv, x) { if (bitvec_set_bit(bv, x) < 0) return -EOWNERDEAD; }
-#define set_0(bv) set_x(bv, ZERO)
-#define set_1(bv) set_x(bv, ONE)
-#define set_L(bv) set_x(bv, L)
-#define set_H(bv) set_x(bv, H)
-
 /* { 0 | 1 < TIMING_ADVANCE_INDEX : bit (4) > } */
 static inline bool write_tai(bitvec *dest, unsigned& wp, int8_t tai)
 {
@@ -93,70 +86,6 @@
write_ta(dest, wp, ta);
if (write_tai(dest, wp, tai)) /* TIMING_ADVANCE_TIMESLOT_NUMBER: */
bitvec_write_field(dest, , ts, 3);
-}
-
-/* 3GPP TS 44.018 § 10.5.2.16:
-   { 0 | 1 < ALPHA : bit (4) > }
-   < GAMMA : bit (5) >
-*/
-static inline int write_alpha_gamma(bitvec *dest, uint8_t alpha, uint8_t gamma)
-{
-   int rc;
-
-   if (alpha) {
-   set_1(dest);
-   rc = bitvec_set_u64(dest, alpha, 4, false); /* ALPHA */
-   check(rc);
-   } else
-   set_0(dest);/* No ALPHA */
-
-   rc = bitvec_set_u64(dest, gamma, 5, false); /* GAMMA */
-   check(rc);
-
-   return 0;
-}
-
-/* TBF_STARTING_TIME -- same as 3GPP TS 44.018 §10.5.2.38 Starting Time 
without tag: */
-static inline int write_tbf_start_time(bitvec *dest, uint32_t fn)
-{
-   int rc;
-
-   /* Set values according to 3GPP TS 44.018 Table 10.5.2.38.1 */
-   rc = bitvec_set_u64(dest, (fn / (26 * 51)) % 32, 5, false); /* T1' */
-   check(rc);
-   rc = bitvec_set_u64(dest, fn % 51, 6, false);   /* T3  */
-   check(rc);
-   rc = bitvec_set_u64(dest, fn % 26, 5, false);   /* T2  */
-
-   return rc;
-}
-
-/* 3GPP TS 44.018 § 10.5.2.16:
-   < TFI_ASSIGNMENT : bit (5) >
-   < POLLING : bit >
-   0 -- The value '1' was allocated in an earlier version of the protocol and 
shall not be used.
-   < USF: bit (3) >
-   < USF_GRANULARITY : bit >
-   { 0 | 1 < P0 : bit (4) > < PR_MODE : bit (1) > }
-*/
-static inline int write_tfi_usf(bitvec *dest, const gprs_rlcmac_ul_tbf *tbf, 
uint8_t usf)
-{
-   int rc;
-
-   rc = bitvec_set_u64(dest, tbf->tfi(), 5, false); /* TFI_ASSIGNMENT */
-   check(rc);
-
-   set_0(dest); /* POLLING -- no action is required from MS */
-
-   set_0(dest); /* '1' was allocated in an earlier spec version and shall 
not be used */
-
-   rc = bitvec_set_u64(dest, usf, 3, false); /* USF */
-   check(rc);
-   set_0(dest); /* USF_GRANULARITY -- the mobile station shall transmit 
one RLC/MAC block */
-
-   set_0(dest); /* No P0 nor PR_MODE */
-
-   return 0;
 }
 
 static int write_ia_rest_downlink(
@@ -256,60 +185,68 @@
return 0;
 }
 
-/* 3GPP TS 44.018 §10.5.2.16 IA Rest Octets ::= EGPRS Packet Uplink Assignment 
*/
-static inline int write_ia_rest_egprs_uplink(const gprs_rlcmac_ul_tbf *tbf, 
bitvec *dest,
-uint8_t usf, uint32_t fn,
-uint8_t alpha, uint8_t gamma, 
int8_t ta_idx,
-enum ph_burst_type burst_type, 
uint16_t ra)
+static int write_ia_rest_egprs_uplink(
+   gprs_rlcmac_ul_tbf *tbf,
+   bitvec * dest, unsigned& wp,
+   uint8_t usf, uint32_t fn,
+   uint8_t alpha, uint8_t gamma, int8_t ta_idx,
+   enum ph_burst_type burst_type, uint16_t ra)
 {
-   int rc;
+   uint8_t extended_ra = 0;
 
-   set_L(dest); set_H(dest);
-   set_0(dest); set_0(dest); /* 00 EGPRS Packet Uplink Assignment */
+   extended_ra = (ra & 0x1F);
 
-   rc = bitvec_set_u64(dest, ra & 0x1F, 5, false); /* Extended RA */
-   check(rc);
+   bitvec_write_field(dest, , 1, 2);/* LH */
+   bitvec_write_field(dest, , 0, 2);/* 0 EGPRS Uplink Assignment */
+   bitvec_write_field(dest, , extended_ra, 5);/* Extended RA */
+   bitvec_write_field(dest, , 0, 1);/* Access technology Request */
 
-   set_0(dest);

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

2018-03-28 Thread Harald Welte 

Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3e8cc0e8ba3ba5bd444124fd4cb95ef92a71fdfb
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 EGPRS Packet Uplink 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/6979
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I3e8cc0e8ba3ba5bd444124fd4cb95ef92a71fdfb
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 EGPRS Packet Uplink Assignment"

2018-02-27 Thread Neels Hofmeyr 

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

Revert "Rewrite EGPRS Packet Uplink Assignment"

This reverts commit 529ce885450946d85d1920fb3d1a994c3efe5849,
I2139fb347b3290621bbc3f6a031f7f213d372e65.

Commit I52ec9b07413daabba8cd5f1fba5c7b3af6a33389 /
896574e92bea09ed8d39688b6fdf504e84521746 was found (empirically) to be a
regression, rendering GPRS service fatally unreliable.

This reverted commit seems to be related to the regression and is reverted
along with it.

Related: OS#3013
Change-Id: I3e8cc0e8ba3ba5bd444124fd4cb95ef92a71fdfb
---
M src/encoding.cpp
1 file changed, 55 insertions(+), 117 deletions(-)


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

diff --git a/src/encoding.cpp b/src/encoding.cpp
index 7b711b8..b2da1e8 100644
--- a/src/encoding.cpp
+++ b/src/encoding.cpp
@@ -36,13 +36,6 @@
 #include 
 #include 
 
-#define check(rc) { if (rc < 0) return rc; }
-#define set_x(bv, x) { if (bitvec_set_bit(bv, x) < 0) return -EOWNERDEAD; }
-#define set_0(bv) set_x(bv, ZERO)
-#define set_1(bv) set_x(bv, ONE)
-#define set_L(bv) set_x(bv, L)
-#define set_H(bv) set_x(bv, H)
-
 /* { 0 | 1 < TIMING_ADVANCE_INDEX : bit (4) > } */
 static inline bool write_tai(bitvec *dest, unsigned& wp, int8_t tai)
 {
@@ -93,70 +86,6 @@
write_ta(dest, wp, ta);
if (write_tai(dest, wp, tai)) /* TIMING_ADVANCE_TIMESLOT_NUMBER: */
bitvec_write_field(dest, , ts, 3);
-}
-
-/* 3GPP TS 44.018 § 10.5.2.16:
-   { 0 | 1 < ALPHA : bit (4) > }
-   < GAMMA : bit (5) >
-*/
-static inline int write_alpha_gamma(bitvec *dest, uint8_t alpha, uint8_t gamma)
-{
-   int rc;
-
-   if (alpha) {
-   set_1(dest);
-   rc = bitvec_set_u64(dest, alpha, 4, false); /* ALPHA */
-   check(rc);
-   } else
-   set_0(dest);/* No ALPHA */
-
-   rc = bitvec_set_u64(dest, gamma, 5, false); /* GAMMA */
-   check(rc);
-
-   return 0;
-}
-
-/* TBF_STARTING_TIME -- same as 3GPP TS 44.018 §10.5.2.38 Starting Time 
without tag: */
-static inline int write_tbf_start_time(bitvec *dest, uint32_t fn)
-{
-   int rc;
-
-   /* Set values according to 3GPP TS 44.018 Table 10.5.2.38.1 */
-   rc = bitvec_set_u64(dest, (fn / (26 * 51)) % 32, 5, false); /* T1' */
-   check(rc);
-   rc = bitvec_set_u64(dest, fn % 51, 6, false);   /* T3  */
-   check(rc);
-   rc = bitvec_set_u64(dest, fn % 26, 5, false);   /* T2  */
-
-   return rc;
-}
-
-/* 3GPP TS 44.018 § 10.5.2.16:
-   < TFI_ASSIGNMENT : bit (5) >
-   < POLLING : bit >
-   0 -- The value '1' was allocated in an earlier version of the protocol and 
shall not be used.
-   < USF: bit (3) >
-   < USF_GRANULARITY : bit >
-   { 0 | 1 < P0 : bit (4) > < PR_MODE : bit (1) > }
-*/
-static inline int write_tfi_usf(bitvec *dest, const gprs_rlcmac_ul_tbf *tbf, 
uint8_t usf)
-{
-   int rc;
-
-   rc = bitvec_set_u64(dest, tbf->tfi(), 5, false); /* TFI_ASSIGNMENT */
-   check(rc);
-
-   set_0(dest); /* POLLING -- no action is required from MS */
-
-   set_0(dest); /* '1' was allocated in an earlier spec version and shall 
not be used */
-
-   rc = bitvec_set_u64(dest, usf, 3, false); /* USF */
-   check(rc);
-   set_0(dest); /* USF_GRANULARITY -- the mobile station shall transmit 
one RLC/MAC block */
-
-   set_0(dest); /* No P0 nor PR_MODE */
-
-   return 0;
 }
 
 static int write_ia_rest_downlink(
@@ -256,60 +185,68 @@
return 0;
 }
 
-/* 3GPP TS 44.018 §10.5.2.16 IA Rest Octets ::= EGPRS Packet Uplink Assignment 
*/
-static inline int write_ia_rest_egprs_uplink(const gprs_rlcmac_ul_tbf *tbf, 
bitvec *dest,
-uint8_t usf, uint32_t fn,
-uint8_t alpha, uint8_t gamma, 
int8_t ta_idx,
-enum ph_burst_type burst_type, 
uint16_t ra)
+static int write_ia_rest_egprs_uplink(
+   gprs_rlcmac_ul_tbf *tbf,
+   bitvec * dest, unsigned& wp,
+   uint8_t usf, uint32_t fn,
+   uint8_t alpha, uint8_t gamma, int8_t ta_idx,
+   enum ph_burst_type burst_type, uint16_t ra)
 {
-   int rc;
+   uint8_t extended_ra = 0;
 
-   set_L(dest); set_H(dest);
-   set_0(dest); set_0(dest); /* 00 EGPRS Packet Uplink Assignment */
+   extended_ra = (ra & 0x1F);
 
-   rc = bitvec_set_u64(dest, ra & 0x1F, 5, false); /* Extended RA */
-   check(rc);
+   bitvec_write_field(dest, , 1, 2);/* LH */
+   bitvec_write_field(dest, , 0, 2);/* 0 EGPRS Uplink Assignment */
+   bitvec_write_field(dest, , extended_ra, 5);/* Extended RA */
+   bitvec_write_field(dest, , 0, 1);/* Access technology Request */
 
-   set_0(dest);/* No Access 
Technologies Request */
+   if (tbf == NULL) {
 
-   if (tbf) {
-   set_1(dest); /* Single Block Allocation */
+