[PATCH] osmo-pcu[master]: Simplify TS alloc: split off RX mask computation

2017-09-14 Thread Max
Hello Jenkins Builder,

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

https://gerrit.osmocom.org/3913

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

Simplify TS alloc: split off RX mask computation

Move computation of RX mask into separate function and document it. This
allows to significantly shrink find_multi_slot() function and overall
improve code readability.

Since the test output requires cosmetic adjustment anyway due to change
in the sequence of log messages, use this opportunity to better group
and format log message.

Change-Id: I731726a096bba7ee97499e5cbe3e7401869d7392
Related: OS#2282
---
M src/gprs_rlcmac_ts_alloc.cpp
M tests/tbf/TbfTest.err
2 files changed, 58 insertions(+), 56 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/13/3913/4

diff --git a/src/gprs_rlcmac_ts_alloc.cpp b/src/gprs_rlcmac_ts_alloc.cpp
index 5ca0c29..c310830 100644
--- a/src/gprs_rlcmac_ts_alloc.cpp
+++ b/src/gprs_rlcmac_ts_alloc.cpp
@@ -604,6 +604,56 @@
return (win | win >> 8) & 0xFF;
 }
 
+enum { MASK_TT, MASK_TR };
+
+/*! Fill in RX mask table for a given MS Class
+ *
+ *  \param[in] ms_cl MS Class pointer
+ *  \param[in] num_tx Number of TX slots to consider
+ *  \param[out] rx_mask RX mask table
+ */
+static inline void fill_rx_mask(const struct gprs_ms_multislot_class *ms_cl, 
uint8_t num_tx, uint8_t *rx_mask)
+{
+   static const char *digit[10] = { "0", "1", "2", "3", "4", "5", "6", 
"7", "8", "9" };
+   uint8_t Tta = ms_cl->ta, Ttb = ms_cl->tb, Tra = ms_cl->ra, Trb = 
ms_cl->rb, /* Minimum Number of Slots */
+   Tx = ms_cl->tx, Sum = ms_cl->sum, /* Maximum Number of Slots: 
Tx, Sum = Rx + Tx */
+   Type = ms_cl->type; /* Type of Mobile */
+
+   /* MS_A maps to 0 if frequency hopping is disabled */
+   /* TODO: Set it to 1 if FH is implemented and enabled */
+   if (Ttb == MS_A)
+   Ttb = 0;
+   if (Trb == MS_A)
+   Trb = 0;
+
+   /* MS_A and MS_B are 0 iff FH is disabled and there is no Tx/Rx change.
+* This is never the case with the current implementation, so 1 will 
always be used. */
+   if (Ttb == MS_B)
+   Ttb = 1;
+   if (Trb == MS_C)
+   Trb = 1;
+
+   if (num_tx == 1) /* it's enough to log this once per TX slot set 
iteration */
+   LOGP(DRLCMAC, LOGL_DEBUG, " Rx=%u, Tx=%u, Sum Rx + Tx=%s 
[Tta=%s Ttb=%u] [Tra=%u Trb=%u] Type=%u\n",
+ms_cl->rx, Tx,
+(Sum == MS_NA) ? "N/A" : digit[Sum],
+(Tta == MS_NA) ? "N/A" : digit[Tta], Ttb, Tra, Trb, Type);
+
+   if (ms_cl->type == 1) {
+   rx_mask[MASK_TT] = (0x100 >> OSMO_MAX(Ttb, Tta)) - 1;
+   rx_mask[MASK_TT] &= ~((1 << (Trb + num_tx)) - 1);
+   rx_mask[MASK_TR] = (0x100 >> Ttb) - 1;
+   rx_mask[MASK_TR] &= ~((1 << (OSMO_MAX(Trb, Tra) + num_tx)) - 1);
+   } else {
+   /* Class type 2 MS have independant RX and TX */
+   rx_mask[MASK_TT] = 0xff;
+   rx_mask[MASK_TR] = 0xff;
+   }
+
+   rx_mask[MASK_TT] = (rx_mask[MASK_TT] << 3) | (rx_mask[MASK_TT] >> 5);
+   rx_mask[MASK_TR] = (rx_mask[MASK_TR] << 3) | (rx_mask[MASK_TR] >> 5);
+}
+
 /*! Find set of slots available for allocation while taking MS class into 
account
  *
  *  \param[in] trx Pointer to TRX object
@@ -615,17 +665,12 @@
 static int find_multi_slots(const struct gprs_rlcmac_trx *trx, const GprsMs 
*ms, uint8_t *ul_slots, uint8_t *dl_slots)
 {
const struct gprs_ms_multislot_class *ms_class;
-   uint8_t Tx, Sum;/* Maximum Number of Slots: RX, Tx, Sum Rx+Tx */
-   uint8_t Tta, Ttb, Tra, Trb; /* Minimum Number of Slots */
-   uint8_t Type; /* Type of Mobile */
uint8_t max_slots, num_tx, mask_sel, pdch_slots, ul_ts, dl_ts;
int16_t rx_window, tx_window;
-   static const char *digit[10] = { 
"0","1","2","3","4","5","6","7","8","9" };
char slot_info[9] = {0};
-   int max_capacity;
-   uint8_t max_ul_slots;
-   uint8_t max_dl_slots;
-   enum {MASK_TT, MASK_TR};
+   int max_capacity = -1;
+   uint8_t max_ul_slots = 0;
+   uint8_t max_dl_slots = 0;
 
if (ms->ms_class() >= 32) {
LOGP(DRLCMAC, LOGL_ERROR, "Multislot class %d out of range.\n",
@@ -646,33 +691,6 @@
return -EINVAL;
}
 
-   Tx = ms_class->tx;
-   Sum = ms_class->sum;
-   Tta = ms_class->ta;
-   Ttb = ms_class->tb;
-   Tra = ms_class->ra;
-   Trb = ms_class->rb;
-   Type = ms_class->type;
-
-   /* MS_A maps to 0 if frequency hopping is disabled */
-   /* TODO: Set it to 1 if FH is implemented and enabled */
-   if (Ttb == MS_A)
-   Ttb = 0;
-   if (Trb == MS_A)
-   Trb = 0;
-
-   /* MS_A and MS_B are 0 iff FH is disabled and there is no Tx/Rx change.
-* This is never the case with the current implementation, so 1

[PATCH] osmo-pcu[master]: Simplify TS alloc: split off RX mask computation

2017-09-20 Thread Max
Hello Jenkins Builder,

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

https://gerrit.osmocom.org/3913

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

Simplify TS alloc: split off RX mask computation

Move computation of RX mask into separate function and document it. This
allows to significantly shrink find_multi_slot() function and overall
improve code readability.

Since the test output requires cosmetic adjustment anyway due to change
in the sequence of log messages, use this opportunity to better group
and format log message.

Change-Id: I731726a096bba7ee97499e5cbe3e7401869d7392
Related: OS#2282
---
M src/gprs_rlcmac_ts_alloc.cpp
M tests/tbf/TbfTest.err
2 files changed, 58 insertions(+), 57 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/13/3913/5

diff --git a/src/gprs_rlcmac_ts_alloc.cpp b/src/gprs_rlcmac_ts_alloc.cpp
index f4c4ef0..693a72a 100644
--- a/src/gprs_rlcmac_ts_alloc.cpp
+++ b/src/gprs_rlcmac_ts_alloc.cpp
@@ -604,6 +604,56 @@
return (win | win >> 8) & 0xFF;
 }
 
+enum { MASK_TT, MASK_TR };
+
+/*! Fill in RX mask table for a given MS Class
+ *
+ *  \param[in] ms_cl MS Class pointer
+ *  \param[in] num_tx Number of TX slots to consider
+ *  \param[out] rx_mask RX mask table
+ */
+static inline void fill_rx_mask(const struct gprs_ms_multislot_class *ms_cl, 
uint8_t num_tx, uint8_t *rx_mask)
+{
+   static const char *digit[10] = { "0", "1", "2", "3", "4", "5", "6", 
"7", "8", "9" };
+   uint8_t Tta = ms_cl->ta, Ttb = ms_cl->tb, Tra = ms_cl->ra, Trb = 
ms_cl->rb, /* Minimum Number of Slots */
+   Tx = ms_cl->tx, Sum = ms_cl->sum, /* Maximum Number of Slots: 
Tx, Sum = Rx + Tx */
+   Type = ms_cl->type; /* Type of Mobile */
+
+   /* MS_A maps to 0 if frequency hopping is disabled */
+   /* TODO: Set it to 1 if FH is implemented and enabled */
+   if (Ttb == MS_A)
+   Ttb = 0;
+   if (Trb == MS_A)
+   Trb = 0;
+
+   /* MS_A and MS_B are 0 iff FH is disabled and there is no Tx/Rx change.
+* This is never the case with the current implementation, so 1 will 
always be used. */
+   if (Ttb == MS_B)
+   Ttb = 1;
+   if (Trb == MS_C)
+   Trb = 1;
+
+   if (num_tx == 1) /* it's enough to log this once per TX slot set 
iteration */
+   LOGP(DRLCMAC, LOGL_DEBUG, " Rx=%u, Tx=%u, Sum Rx + Tx=%s 
[Tta=%s Ttb=%u] [Tra=%u Trb=%u] Type=%u\n",
+ms_cl->rx, Tx,
+(Sum == MS_NA) ? "N/A" : digit[Sum],
+(Tta == MS_NA) ? "N/A" : digit[Tta], Ttb, Tra, Trb, Type);
+
+   if (ms_cl->type == 1) {
+   rx_mask[MASK_TT] = (0x100 >> OSMO_MAX(Ttb, Tta)) - 1;
+   rx_mask[MASK_TT] &= ~((1 << (Trb + num_tx)) - 1);
+   rx_mask[MASK_TR] = (0x100 >> Ttb) - 1;
+   rx_mask[MASK_TR] &= ~((1 << (OSMO_MAX(Trb, Tra) + num_tx)) - 1);
+   } else {
+   /* Class type 2 MS have independant RX and TX */
+   rx_mask[MASK_TT] = 0xff;
+   rx_mask[MASK_TR] = 0xff;
+   }
+
+   rx_mask[MASK_TT] = (rx_mask[MASK_TT] << 3) | (rx_mask[MASK_TT] >> 5);
+   rx_mask[MASK_TR] = (rx_mask[MASK_TR] << 3) | (rx_mask[MASK_TR] >> 5);
+}
+
 /*! Find set of slots available for allocation while taking MS class into 
account
  *
  *  \param[in] trx Pointer to TRX object
@@ -615,17 +665,12 @@
 static int find_multi_slots(const struct gprs_rlcmac_trx *trx, const GprsMs 
*ms, uint8_t *ul_slots, uint8_t *dl_slots)
 {
const struct gprs_ms_multislot_class *ms_class;
-   uint8_t Tx, Sum;/* Maximum Number of Slots: RX, Tx, Sum Rx+Tx */
-   uint8_t Tta, Ttb, Tra, Trb; /* Minimum Number of Slots */
-   uint8_t Type; /* Type of Mobile */
uint8_t max_slots, num_tx, mask_sel, pdch_slots, ul_ts, dl_ts;
int16_t rx_window, tx_window;
-   static const char *digit[10] = { 
"0","1","2","3","4","5","6","7","8","9" };
char slot_info[9] = {0};
-   int max_capacity;
-   uint8_t max_ul_slots;
-   uint8_t max_dl_slots;
-   enum {MASK_TT, MASK_TR};
+   int max_capacity = -1;
+   uint8_t max_ul_slots = 0;
+   uint8_t max_dl_slots = 0;
 
if (ms->ms_class() >= 32) {
LOGP(DRLCMAC, LOGL_ERROR, "Multislot class %d out of range.\n",
@@ -646,33 +691,6 @@
return -EINVAL;
}
 
-   Tx = ms_class->tx;
-   Sum = ms_class->sum;
-   Tta = ms_class->ta;
-   Ttb = ms_class->tb;
-   Tra = ms_class->ra;
-   Trb = ms_class->rb;
-   Type = ms_class->type;
-
-   /* MS_A maps to 0 if frequency hopping is disabled */
-   /* TODO: Set it to 1 if FH is implemented and enabled */
-   if (Ttb == MS_A)
-   Ttb = 0;
-   if (Trb == MS_A)
-   Trb = 0;
-
-   /* MS_A and MS_B are 0 iff FH is disabled and there is no Tx/Rx change.
-* This is never the case with the current implementation, so 1

[PATCH] osmo-pcu[master]: Simplify TS alloc: split off RX mask computation

2017-09-28 Thread Max
Hello Jenkins Builder,

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

https://gerrit.osmocom.org/3913

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

Simplify TS alloc: split off RX mask computation

Move computation of RX mask into separate function and document it. This
allows to significantly shrink find_multi_slot() function and overall
improve code readability.

Since the test output requires cosmetic adjustment anyway due to change
in the sequence of log messages, use this opportunity to better group
and format log message.

Change-Id: I731726a096bba7ee97499e5cbe3e7401869d7392
Related: OS#2282
---
M src/gprs_rlcmac_ts_alloc.cpp
M tests/tbf/TbfTest.err
2 files changed, 58 insertions(+), 58 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/13/3913/6

diff --git a/src/gprs_rlcmac_ts_alloc.cpp b/src/gprs_rlcmac_ts_alloc.cpp
index 2763100..57faebf 100644
--- a/src/gprs_rlcmac_ts_alloc.cpp
+++ b/src/gprs_rlcmac_ts_alloc.cpp
@@ -612,6 +612,56 @@
return (win | win >> 8) & 0xFF;
 }
 
+enum { MASK_TT, MASK_TR };
+
+/*! Fill in RX mask table for a given MS Class
+ *
+ *  \param[in] ms_cl MS Class pointer
+ *  \param[in] num_tx Number of TX slots to consider
+ *  \param[out] rx_mask RX mask table
+ */
+static inline void fill_rx_mask(const struct gprs_ms_multislot_class *ms_cl, 
uint8_t num_tx, uint8_t *rx_mask)
+{
+   static const char *digit[10] = { "0", "1", "2", "3", "4", "5", "6", 
"7", "8", "9" };
+   uint8_t Tta = ms_cl->ta, Ttb = ms_cl->tb, Tra = ms_cl->ra, Trb = 
ms_cl->rb, /* Minimum Number of Slots */
+   Tx = ms_cl->tx, Sum = ms_cl->sum, /* Maximum Number of Slots: 
Tx, Sum = Rx + Tx */
+   Type = ms_cl->type; /* Type of Mobile */
+
+   /* MS_A maps to 0 if frequency hopping is disabled */
+   /* TODO: Set it to 1 if FH is implemented and enabled */
+   if (Ttb == MS_A)
+   Ttb = 0;
+   if (Trb == MS_A)
+   Trb = 0;
+
+   /* MS_A and MS_B are 0 iff FH is disabled and there is no Tx/Rx change.
+* This is never the case with the current implementation, so 1 will 
always be used. */
+   if (Ttb == MS_B)
+   Ttb = 1;
+   if (Trb == MS_C)
+   Trb = 1;
+
+   if (num_tx == 1) /* it's enough to log this once per TX slot set 
iteration */
+   LOGP(DRLCMAC, LOGL_DEBUG, " Rx=%u, Tx=%u, Sum Rx + Tx=%s 
[Tta=%s Ttb=%u] [Tra=%u Trb=%u] Type=%u\n",
+ms_cl->rx, Tx,
+(Sum == MS_NA) ? "N/A" : digit[Sum],
+(Tta == MS_NA) ? "N/A" : digit[Tta], Ttb, Tra, Trb, Type);
+
+   if (ms_cl->type == 1) {
+   rx_mask[MASK_TT] = (0x100 >> OSMO_MAX(Ttb, Tta)) - 1;
+   rx_mask[MASK_TT] &= ~((1 << (Trb + num_tx)) - 1);
+   rx_mask[MASK_TR] = (0x100 >> Ttb) - 1;
+   rx_mask[MASK_TR] &= ~((1 << (OSMO_MAX(Trb, Tra) + num_tx)) - 1);
+   } else {
+   /* Class type 2 MS have independant RX and TX */
+   rx_mask[MASK_TT] = 0xff;
+   rx_mask[MASK_TR] = 0xff;
+   }
+
+   rx_mask[MASK_TT] = (rx_mask[MASK_TT] << 3) | (rx_mask[MASK_TT] >> 5);
+   rx_mask[MASK_TR] = (rx_mask[MASK_TR] << 3) | (rx_mask[MASK_TR] >> 5);
+}
+
 /*! Find set of slots available for allocation while taking MS class into 
account
  *
  *  \param[in] trx Pointer to TRX object
@@ -623,17 +673,12 @@
 static int find_multi_slots(const struct gprs_rlcmac_trx *trx, const GprsMs 
*ms, uint8_t *ul_slots, uint8_t *dl_slots)
 {
const struct gprs_ms_multislot_class *ms_class;
-   uint8_t Tx, Sum;/* Maximum Number of Slots: RX, Tx, Sum Rx+Tx */
-   uint8_t Tta, Ttb, Tra, Trb; /* Minimum Number of Slots */
-   uint8_t Type; /* Type of Mobile */
uint8_t max_slots, num_tx, mask_sel, pdch_slots, ul_ts, dl_ts;
int16_t rx_window, tx_window;
-   static const char *digit[10] = { 
"0","1","2","3","4","5","6","7","8","9" };
char slot_info[9] = {0};
-   int max_capacity;
-   uint8_t max_ul_slots;
-   uint8_t max_dl_slots;
-   enum {MASK_TT, MASK_TR};
+   int max_capacity = -1;
+   uint8_t max_ul_slots = 0;
+   uint8_t max_dl_slots = 0;
 
if (ms->ms_class() >= 32) {
LOGP(DRLCMAC, LOGL_ERROR, "Multislot class %d out of range.\n",
@@ -657,34 +702,6 @@
return -EINVAL;
}
 
-   Tx = ms_class->tx;
-   Sum = ms_class->sum;
-   Tta = ms_class->ta;
-   Ttb = ms_class->tb;
-   Tra = ms_class->ra;
-   Trb = ms_class->rb;
-   Type = ms_class->type;
-
-   /* MS_A maps to 0 if frequency hopping is disabled */
-   /* TODO: Set it to 1 if FH is implemented and enabled */
-   if (Ttb == MS_A)
-   Ttb = 0;
-   if (Trb == MS_A)
-   Trb = 0;
-
-   /* MS_A and MS_B are 0 iff FH is disabled and there is no Tx/Rx change.
-* This is never the case with the current implementation, so 1

[PATCH] osmo-pcu[master]: Simplify TS alloc: split off RX mask computation

2018-01-31 Thread Max
Hello Harald Welte, Jenkins Builder,

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

https://gerrit.osmocom.org/3913

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

Simplify TS alloc: split off RX mask computation

Move computation of RX mask into separate function and document it. This
allows to significantly shrink find_multi_slot() function and overall
improve code readability.

Since the test output requires cosmetic adjustment anyway due to change
in the sequence of log messages, use this opportunity to better group
and format log message.

Change-Id: I731726a096bba7ee97499e5cbe3e7401869d7392
Related: OS#2282
---
M src/gprs_rlcmac_ts_alloc.cpp
M src/mslot_class.c
M src/mslot_class.h
M tests/tbf/TbfTest.err
4 files changed, 55 insertions(+), 46 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/13/3913/11

diff --git a/src/gprs_rlcmac_ts_alloc.cpp b/src/gprs_rlcmac_ts_alloc.cpp
index 1e70c6f..e290a14 100644
--- a/src/gprs_rlcmac_ts_alloc.cpp
+++ b/src/gprs_rlcmac_ts_alloc.cpp
@@ -498,11 +498,9 @@
  */
 int find_multi_slots(struct gprs_rlcmac_trx *trx, uint8_t mslot_class, uint8_t 
*ul_slots, uint8_t *dl_slots)
 {
-   uint8_t Tx, Sum;/* Maximum Number of Slots: RX, Tx, Sum Rx+Tx */
-   uint8_t Tta, Ttb, Tra, Trb; /* Minimum Number of Slots */
-   uint8_t Type; /* Type of Mobile */
+   uint8_t Tx = mslot_class_get_tx(mslot_class),   /* Max number of Tx 
slots */
+   Sum = mslot_class_get_sum(mslot_class); /* Max number of Tx + 
Rx slots */
int rx_window, tx_window, pdch_slots;
-   static const char *digit[10] = { 
"0","1","2","3","4","5","6","7","8","9" };
char slot_info[9] = {0};
int max_capacity = -1;
uint8_t max_ul_slots = 0, max_dl_slots = 0;
@@ -510,35 +508,17 @@
 
unsigned ul_ts, dl_ts;
unsigned num_tx;
-   enum {MASK_TT, MASK_TR};
unsigned mask_sel;
 
if (mslot_class)
LOGP(DRLCMAC, LOGL_DEBUG, "Slot Allocation (Algorithm B) for 
class %d\n",
 mslot_class);
 
-   Tx = mslot_class_get_tx(mslot_class);
-   Sum = mslot_class_get_sum(mslot_class);
-   Tta = mslot_class_get_ta(mslot_class);
-   Ttb = mslot_class_get_tb(mslot_class);
-
-   /* FIXME: use actual TA offset for computation - make sure to adjust "1 
+ MS_TO" accordingly
-  see also "Offset required" bit in 3GPP TS 24.008 §10.5.1.7 */
-   Tra = mslot_class_get_ra(mslot_class, 0);
-   Trb = mslot_class_get_rb(mslot_class, 0);
-
-   Type = mslot_class_get_type(mslot_class);
-
if (Tx == MS_NA) {
LOGP(DRLCMAC, LOGL_NOTICE, "Multislot class %d not 
applicable.\n",
 mslot_class);
return -EINVAL;
}
-
-   LOGP(DRLCMAC, LOGL_DEBUG, "- Rx=%d Tx=%d Sum Rx+Tx=%s  Tta=%s Ttb=%d "
-   " Tra=%d Trb=%d Type=%d\n", mslot_class_get_rx(mslot_class), Tx,
-   (Sum == MS_NA) ? "N/A" : digit[Sum],
-   (Tta == MS_NA) ? "N/A" : digit[Tta], Ttb, Tra, Trb, Type);
 
max_slots = OSMO_MAX(mslot_class_get_rx(mslot_class), Tx);
 
@@ -561,29 +541,12 @@
 
/* Check for each UL (TX) slot */
 
-   max_capacity = -1;
-   max_ul_slots = 0;
-   max_dl_slots = 0;
-
/* Iterate through possible numbers of TX slots */
for (num_tx = 1; num_tx <= mslot_class_get_tx(mslot_class); num_tx += 
1) {
uint16_t tx_valid_win = (1 << num_tx) - 1;
+   uint8_t rx_mask[MASK_TR + 1];
 
-   uint8_t rx_mask[MASK_TR+1];
-   if (Type == 1) {
-   rx_mask[MASK_TT] = (0x100 >> OSMO_MAX(Ttb, Tta)) - 1;
-   rx_mask[MASK_TT] &= ~((1 << (Trb + num_tx)) - 1);
-   rx_mask[MASK_TR] = (0x100 >> Ttb) - 1;
-   rx_mask[MASK_TR] &=
-   ~((1 << (OSMO_MAX(Trb, Tra) + num_tx)) - 1);
-   } else {
-   /* Class type 2 MS have independant RX and TX */
-   rx_mask[MASK_TT] = 0xff;
-   rx_mask[MASK_TR] = 0xff;
-   }
-
-   rx_mask[MASK_TT] = (rx_mask[MASK_TT] << 3) | (rx_mask[MASK_TT] 
>> 5);
-   rx_mask[MASK_TR] = (rx_mask[MASK_TR] << 3) | (rx_mask[MASK_TR] 
>> 5);
+   mslot_fill_rx_mask(mslot_class, num_tx, rx_mask);
 
/* Rotate group of TX slots: UUU-, -UUU, ..., UU-U */
for (ul_ts = 0; ul_ts < 8; ul_ts += 1, tx_valid_win <<= 1) {
@@ -651,7 +614,7 @@
/* Whether to skip this round doesn not only depend on the bit
 * sets but also on mask_sel. Therefore this check must be done
 * before doing the test_and_set_bit shortcut. */
-   if (Type == 1) {
+   if (mslot_class_get_type(mslot_class) == 1) {
unsigned slot_sum = rx_slot_count + tx_slot_count;
/* As

[PATCH] osmo-pcu[master]: Simplify TS alloc: split off RX mask computation

2018-02-05 Thread Max
Hello Harald Welte, Jenkins Builder,

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

https://gerrit.osmocom.org/3913

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

Simplify TS alloc: split off RX mask computation

Move computation of RX mask into separate function and document it. This
allows to significantly shrink find_multi_slot() function and overall
improve code readability.

Since the test output requires cosmetic adjustment anyway due to change
in the sequence of log messages, use this opportunity to better group
and format log message.

Change-Id: I731726a096bba7ee97499e5cbe3e7401869d7392
Related: OS#2282
---
M src/gprs_rlcmac_ts_alloc.cpp
M src/mslot_class.c
M src/mslot_class.h
M tests/tbf/TbfTest.err
4 files changed, 57 insertions(+), 49 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/13/3913/13

diff --git a/src/gprs_rlcmac_ts_alloc.cpp b/src/gprs_rlcmac_ts_alloc.cpp
index c45aa39..19f78ad 100644
--- a/src/gprs_rlcmac_ts_alloc.cpp
+++ b/src/gprs_rlcmac_ts_alloc.cpp
@@ -471,48 +471,27 @@
  */
 int find_multi_slots(struct gprs_rlcmac_trx *trx, uint8_t mslot_class, uint8_t 
*ul_slots, uint8_t *dl_slots)
 {
-   uint8_t Tx, Sum;/* Maximum Number of Slots: RX, Tx, Sum Rx+Tx */
-   uint8_t Tta, Ttb, Tra, Trb; /* Minimum Number of Slots */
-   uint8_t Type; /* Type of Mobile */
+   uint8_t Tx = mslot_class_get_tx(mslot_class),   /* Max number of Tx 
slots */
+   Sum = mslot_class_get_sum(mslot_class); /* Max number of Tx + 
Rx slots */
int rx_window, tx_window, pdch_slots;
-   static const char *digit[10] = { 
"0","1","2","3","4","5","6","7","8","9" };
char slot_info[9] = {0};
-   int max_capacity;
-   uint8_t max_ul_slots;
-   uint8_t max_dl_slots;
+   int max_capacity = -1;
+   uint8_t max_ul_slots = 0, max_dl_slots = 0;
unsigned max_slots;
 
unsigned ul_ts, dl_ts;
unsigned num_tx;
-   enum {MASK_TT, MASK_TR};
unsigned mask_sel;
 
if (mslot_class)
LOGP(DRLCMAC, LOGL_DEBUG, "Slot Allocation (Algorithm B) for 
class %d\n",
 mslot_class);
 
-   Tx = mslot_class_get_tx(mslot_class);
-   Sum = mslot_class_get_sum(mslot_class);
-   Tta = mslot_class_get_ta(mslot_class);
-   Ttb = mslot_class_get_tb(mslot_class);
-
-   /* FIXME: use actual TA offset for computation - make sure to adjust "1 
+ MS_TO" accordingly
-  see also "Offset required" bit in 3GPP TS 24.008 §10.5.1.7 */
-   Tra = mslot_class_get_ra(mslot_class, 0);
-   Trb = mslot_class_get_rb(mslot_class, 0);
-
-   Type = mslot_class_get_type(mslot_class);
-
if (Tx == MS_NA) {
LOGP(DRLCMAC, LOGL_NOTICE, "Multislot class %d not 
applicable.\n",
 mslot_class);
return -EINVAL;
}
-
-   LOGP(DRLCMAC, LOGL_DEBUG, "- Rx=%d Tx=%d Sum Rx+Tx=%s  Tta=%s Ttb=%d "
-   " Tra=%d Trb=%d Type=%d\n", mslot_class_get_rx(mslot_class), Tx,
-   (Sum == MS_NA) ? "N/A" : digit[Sum],
-   (Tta == MS_NA) ? "N/A" : digit[Tta], Ttb, Tra, Trb, Type);
 
max_slots = OSMO_MAX(mslot_class_get_rx(mslot_class), Tx);
 
@@ -535,29 +514,12 @@
 
/* Check for each UL (TX) slot */
 
-   max_capacity = -1;
-   max_ul_slots = 0;
-   max_dl_slots = 0;
-
/* Iterate through possible numbers of TX slots */
for (num_tx = 1; num_tx <= mslot_class_get_tx(mslot_class); num_tx += 
1) {
uint16_t tx_valid_win = (1 << num_tx) - 1;
+   uint8_t rx_mask[MASK_TR + 1];
 
-   uint8_t rx_mask[MASK_TR+1];
-   if (Type == 1) {
-   rx_mask[MASK_TT] = (0x100 >> OSMO_MAX(Ttb, Tta)) - 1;
-   rx_mask[MASK_TT] &= ~((1 << (Trb + num_tx)) - 1);
-   rx_mask[MASK_TR] = (0x100 >> Ttb) - 1;
-   rx_mask[MASK_TR] &=
-   ~((1 << (OSMO_MAX(Trb, Tra) + num_tx)) - 1);
-   } else {
-   /* Class type 2 MS have independant RX and TX */
-   rx_mask[MASK_TT] = 0xff;
-   rx_mask[MASK_TR] = 0xff;
-   }
-
-   rx_mask[MASK_TT] = (rx_mask[MASK_TT] << 3) | (rx_mask[MASK_TT] 
>> 5);
-   rx_mask[MASK_TR] = (rx_mask[MASK_TR] << 3) | (rx_mask[MASK_TR] 
>> 5);
+   mslot_fill_rx_mask(mslot_class, num_tx, rx_mask);
 
/* Rotate group of TX slots: UUU-, -UUU, ..., UU-U */
for (ul_ts = 0; ul_ts < 8; ul_ts += 1, tx_valid_win <<= 1) {
@@ -626,7 +588,7 @@
/* Whether to skip this round doesn not only depend on the bit
 * sets but also on mask_sel. Therefore this check must be done
 * before doing the test_and_set_bit shortcut. */
-   if (Type == 1) {
+   if (mslot_class_get_type(mslot_class) == 1) {
   

[PATCH] osmo-pcu[master]: Simplify TS alloc: split off RX mask computation

2018-01-26 Thread Max
Hello Jenkins Builder,

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

https://gerrit.osmocom.org/3913

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

Simplify TS alloc: split off RX mask computation

Move computation of RX mask into separate function and document it. This
allows to significantly shrink find_multi_slot() function and overall
improve code readability.

Since the test output requires cosmetic adjustment anyway due to change
in the sequence of log messages, use this opportunity to better group
and format log message.

Change-Id: I731726a096bba7ee97499e5cbe3e7401869d7392
Related: OS#2282
---
M src/gprs_rlcmac_ts_alloc.cpp
M tests/tbf/TbfTest.err
2 files changed, 53 insertions(+), 47 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/13/3913/7

diff --git a/src/gprs_rlcmac_ts_alloc.cpp b/src/gprs_rlcmac_ts_alloc.cpp
index 4c96e9a..c9c9d8f 100644
--- a/src/gprs_rlcmac_ts_alloc.cpp
+++ b/src/gprs_rlcmac_ts_alloc.cpp
@@ -568,6 +568,49 @@
return (win | win >> 8) & 0xFF;
 }
 
+enum { MASK_TT, MASK_TR };
+
+/*! Fill in RX mask table for a given MS Class
+ *
+ *  \param[in] ms_cl MS Class pointer
+ *  \param[in] num_tx Number of TX slots to consider
+ *  \param[out] rx_mask RX mask table
+ */
+static inline void fill_rx_mask(uint8_t mslot_class, uint8_t num_tx, uint8_t 
*rx_mask)
+{
+   static const char *digit[10] = { 
"0","1","2","3","4","5","6","7","8","9" };
+   uint8_t Tx = mslot_class_get_tx(mslot_class), /* Max number of Tx 
slots */
+   Sum = mslot_class_get_sum(mslot_class),   /* Max number of Tx + 
Rx slots */
+   Type = mslot_class_get_type(mslot_class), /* Type of Mobile */
+   Tta = mslot_class_get_ta(mslot_class),/* Minimum number of 
slots */
+   Ttb = mslot_class_get_tb(mslot_class),
+   /* FIXME: use actual TA offset for computation - make sure to 
adjust "1 + MS_TO" accordingly
+  see also "Offset required" bit in 3GPP TS 24.008 §10.5.1.7 */
+   Tra = mslot_class_get_ra(mslot_class, 0),
+   Trb = mslot_class_get_rb(mslot_class, 0);
+
+   if (num_tx == 1) /* it's enough to log this once per TX slot set 
iteration */
+   LOGP(DRLCMAC, LOGL_DEBUG,
+"Rx=%d Tx=%d Sum Rx+Tx=%s, Tta=%s Ttb=%d, Tra=%d Trb=%d, 
Type=%d\n",
+mslot_class_get_rx(mslot_class), Tx,
+(Sum == MS_NA) ? "N/A" : digit[Sum],
+(Tta == MS_NA) ? "N/A" : digit[Tta], Ttb, Tra, Trb, Type);
+
+   if (Type == 1) {
+   rx_mask[MASK_TT] = (0x100 >> OSMO_MAX(Ttb, Tta)) - 1;
+   rx_mask[MASK_TT] &= ~((1 << (Trb + num_tx)) - 1);
+   rx_mask[MASK_TR] = (0x100 >> Ttb) - 1;
+   rx_mask[MASK_TR] &= ~((1 << (OSMO_MAX(Trb, Tra) + num_tx)) - 1);
+   } else {
+   /* Class type 2 MS have independant RX and TX */
+   rx_mask[MASK_TT] = 0xff;
+   rx_mask[MASK_TR] = 0xff;
+   }
+
+   rx_mask[MASK_TT] = (rx_mask[MASK_TT] << 3) | (rx_mask[MASK_TT] >> 5);
+   rx_mask[MASK_TR] = (rx_mask[MASK_TR] << 3) | (rx_mask[MASK_TR] >> 5);
+}
+
 /*! Find set of slots available for allocation while taking MS class into 
account
  *
  *  \param[in] trx Pointer to TRX object
@@ -578,44 +621,24 @@
  */
 int find_multi_slots(struct gprs_rlcmac_trx *trx, uint8_t mslot_class, uint8_t 
*ul_slots, uint8_t *dl_slots)
 {
-   uint8_t Tx, Sum;/* Maximum Number of Slots: RX, Tx, Sum Rx+Tx */
-   uint8_t Tta, Ttb, Tra, Trb; /* Minimum Number of Slots */
-   uint8_t Type; /* Type of Mobile */
+   uint8_t Tx = mslot_class_get_tx(mslot_class),   /* Max number of Tx 
slots */
+   Sum = mslot_class_get_sum(mslot_class); /* Max number of Tx + 
Rx slots */
uint8_t max_slots, num_tx, mask_sel, pdch_slots, ul_ts, dl_ts;
int16_t rx_window, tx_window;
-   static const char *digit[10] = { 
"0","1","2","3","4","5","6","7","8","9" };
char slot_info[9] = {0};
-   int max_capacity;
-   uint8_t max_ul_slots;
-   uint8_t max_dl_slots;
-   enum {MASK_TT, MASK_TR};
+   int max_capacity = -1;
+   uint8_t max_ul_slots = 0;
+   uint8_t max_dl_slots = 0;
 
if (mslot_class)
LOGP(DRLCMAC, LOGL_DEBUG, "Slot Allocation (Algorithm B) for 
class %d\n",
 mslot_class);
-
-   Tx = mslot_class_get_tx(mslot_class);
-   Sum = mslot_class_get_sum(mslot_class);
-   Tta = mslot_class_get_ta(mslot_class);
-   Ttb = mslot_class_get_tb(mslot_class);
-
-   /* FIXME: use actual TA offset for computation - make sure to adjust "1 
+ MS_TO" accordingly
-  see also "Offset required" bit in 3GPP TS 24.008 §10.5.1.7 */
-   Tra = mslot_class_get_ra(mslot_class, 0);
-   Trb = mslot_class_get_rb(mslot_class, 0);
-
-   Type = mslot_class_get_type(mslot_class);
 
if (Tx == MS_NA) {
LOG

[PATCH] osmo-pcu[master]: Simplify TS alloc: split off RX mask computation

2018-01-29 Thread Max
Hello Harald Welte, Jenkins Builder,

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

https://gerrit.osmocom.org/3913

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

Simplify TS alloc: split off RX mask computation

Move computation of RX mask into separate function and document it. This
allows to significantly shrink find_multi_slot() function and overall
improve code readability.

Since the test output requires cosmetic adjustment anyway due to change
in the sequence of log messages, use this opportunity to better group
and format log message.

Change-Id: I731726a096bba7ee97499e5cbe3e7401869d7392
Related: OS#2282
---
M src/gprs_rlcmac_ts_alloc.cpp
M src/mslot_class.c
M src/mslot_class.h
M tests/tbf/TbfTest.err
4 files changed, 55 insertions(+), 47 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/13/3913/8

diff --git a/src/gprs_rlcmac_ts_alloc.cpp b/src/gprs_rlcmac_ts_alloc.cpp
index 438ec93..fa3f788 100644
--- a/src/gprs_rlcmac_ts_alloc.cpp
+++ b/src/gprs_rlcmac_ts_alloc.cpp
@@ -533,44 +533,24 @@
  */
 int find_multi_slots(struct gprs_rlcmac_trx *trx, uint8_t mslot_class, uint8_t 
*ul_slots, uint8_t *dl_slots)
 {
-   uint8_t Tx, Sum;/* Maximum Number of Slots: RX, Tx, Sum Rx+Tx */
-   uint8_t Tta, Ttb, Tra, Trb; /* Minimum Number of Slots */
-   uint8_t Type; /* Type of Mobile */
+   uint8_t Tx = mslot_class_get_tx(mslot_class),   /* Max number of Tx 
slots */
+   Sum = mslot_class_get_sum(mslot_class); /* Max number of Tx + 
Rx slots */
uint8_t max_slots, num_tx, mask_sel, pdch_slots, ul_ts, dl_ts;
int16_t rx_window, tx_window;
-   static const char *digit[10] = { 
"0","1","2","3","4","5","6","7","8","9" };
char slot_info[9] = {0};
-   int max_capacity;
-   uint8_t max_ul_slots;
-   uint8_t max_dl_slots;
-   enum {MASK_TT, MASK_TR};
+   int max_capacity = -1;
+   uint8_t max_ul_slots = 0;
+   uint8_t max_dl_slots = 0;
 
if (mslot_class)
LOGP(DRLCMAC, LOGL_DEBUG, "Slot Allocation (Algorithm B) for 
class %d\n",
 mslot_class);
-
-   Tx = mslot_class_get_tx(mslot_class);
-   Sum = mslot_class_get_sum(mslot_class);
-   Tta = mslot_class_get_ta(mslot_class);
-   Ttb = mslot_class_get_tb(mslot_class);
-
-   /* FIXME: use actual TA offset for computation - make sure to adjust "1 
+ MS_TO" accordingly
-  see also "Offset required" bit in 3GPP TS 24.008 §10.5.1.7 */
-   Tra = mslot_class_get_ra(mslot_class, 0);
-   Trb = mslot_class_get_rb(mslot_class, 0);
-
-   Type = mslot_class_get_type(mslot_class);
 
if (Tx == MS_NA) {
LOGP(DRLCMAC, LOGL_NOTICE, "Multislot class %d not 
applicable.\n",
 mslot_class);
return -EINVAL;
}
-
-   LOGP(DRLCMAC, LOGL_DEBUG, "- Rx=%d Tx=%d Sum Rx+Tx=%s  Tta=%s Ttb=%d "
-   " Tra=%d Trb=%d Type=%d\n", mslot_class_get_rx(mslot_class), Tx,
-   (Sum == MS_NA) ? "N/A" : digit[Sum],
-   (Tta == MS_NA) ? "N/A" : digit[Tta], Ttb, Tra, Trb, Type);
 
max_slots = OSMO_MAX(mslot_class_get_rx(mslot_class), Tx);
 
@@ -593,29 +573,12 @@
 
/* Check for each UL (TX) slot */
 
-   max_capacity = -1;
-   max_ul_slots = 0;
-   max_dl_slots = 0;
-
/* Iterate through possible numbers of TX slots */
for (num_tx = 1; num_tx <= mslot_class_get_tx(mslot_class); num_tx += 
1) {
uint16_t tx_valid_win = (1 << num_tx) - 1;
+   uint8_t rx_mask[MASK_TR + 1];
 
-   uint8_t rx_mask[MASK_TR+1];
-   if (Type == 1) {
-   rx_mask[MASK_TT] = (0x100 >> OSMO_MAX(Ttb, Tta)) - 1;
-   rx_mask[MASK_TT] &= ~((1 << (Trb + num_tx)) - 1);
-   rx_mask[MASK_TR] = (0x100 >> Ttb) - 1;
-   rx_mask[MASK_TR] &=
-   ~((1 << (OSMO_MAX(Trb, Tra) + num_tx)) - 1);
-   } else {
-   /* Class type 2 MS have independant RX and TX */
-   rx_mask[MASK_TT] = 0xff;
-   rx_mask[MASK_TR] = 0xff;
-   }
-
-   rx_mask[MASK_TT] = (rx_mask[MASK_TT] << 3) | (rx_mask[MASK_TT] 
>> 5);
-   rx_mask[MASK_TR] = (rx_mask[MASK_TR] << 3) | (rx_mask[MASK_TR] 
>> 5);
+   mslot_fill_rx_mask(mslot_class, num_tx, rx_mask);
 
/* Rotate group of TX slots: UUU-, -UUU, ..., UU-U */
for (ul_ts = 0; ul_ts < 8; ul_ts += 1, tx_valid_win <<= 1) {
diff --git a/src/mslot_class.c b/src/mslot_class.c
index f8441e4..24f6de6 100644
--- a/src/mslot_class.c
+++ b/src/mslot_class.c
@@ -21,6 +21,7 @@
  */
 
 #include 
+#include 
 
 #include 
 #include 
@@ -205,3 +206,44 @@
 
return rx_good & rx_valid_win;
 }
+
+/*! Fill in RX mask table for a given MS Class
+ *
+ *  \param[in] ms_cl MS Class pointer
+ *  \param[in] num_tx Number