Project

General

Profile

Actions

Bug #1792

closed

2 MS tests: Issue with TS allocation for DL in PCU

Added by arvind.sirsikar over 7 years ago. Updated about 3 years ago.

Status:
Rejected
Priority:
High
Assignee:
Target version:
-
Start date:
08/09/2016
Due date:
% Done:

0%

Spec Reference:

Description

Hi All,

When 4 TS is configured for DL and testing 2 MSs it was found that 1st MS gets 4 TS where as next MS gets 3 TS. causing issue with DL traffic fairness between MSs.

issue is traced to "static int find_multi_slots" function which considers both UL/DL capacity for TS calculation causing 2 TS allocation for UL and 3 for DL.

as of now

if (capacity <= max_capacity) condition in same function is modified to

if (rx_window < max_dl_slots) to concentrate only on DL.

Thanks,
Aravind Sirsikar

Actions #1

Updated by arvind.sirsikar over 7 years ago

  • Status changed from New to Stalled

waiting for review feed back on Gerrit for

https://gerrit.osmocom.org/#/c/819/

Actions #2

Updated by arvind.sirsikar over 7 years ago

Current PCU implementation aims at supporting "many MSs with lesser resources"compared to "giving all the resources to fewer MSs".

The patch https://gerrit.osmocom.org/#/c/819/ tries to give same(rather max)resources to all the MSs. However it is applicable only for lab testing.

Moving this patch to abandoned state as of now. This patch can be merged if necessary for lab testing.

Actions #3

Updated by zecke over 7 years ago

What would it take to scale this? We should not have a flag that makes it look good in a lab with a a handful of phones while in practice we run into resource conflicts too early.

Actions #4

Updated by laforge over 6 years ago

Actions #5

Updated by laforge over 6 years ago

  • Assignee changed from arvind.sirsikar to 4368
Actions #6

Updated by pespin almost 4 years ago

  • Status changed from Stalled to New

There's a test case in AllocTest showcasing this issue in function test_2_consecutive_dl_tbfs().

So once fixed the following assert should pass when checked against == 4:

OSMO_ASSERT(numTs2 == 3);

See osmo-pcu.git commit e26ee01d56b4c4c2da6abc6b649cb765d5787b98 for more information.

Actions #7

Updated by laforge almost 4 years ago

  • Assignee changed from 4368 to pespin
Actions #8

Updated by pespin about 3 years ago

I have been looking at this ticket for a while today, as well as https://gerrit.osmocom.org/#/c/819/

So the topic here is about whether we want to provide more DL timeslots when allocating TBFs over time (hence more DL-bandwidth optimized), or whether we want to focus on allocating DL timeslots based on capacity penalty on the TRX.

I first tried applying the proposed patch, which basically comes to the following line change in find_multi_slots():

-                                       if (capacity <= max_capacity)
+                                       if (rx_window < max_dl_slots)

When applying the patch, some AllocTest tests allocating lots of TBF cleary drop from 101 expected allocated TBF to a really lower number (I don't remember right now, but something between 7 and 30) due to TFI/USF exhaustion. Given the amount of drop here, I don't think it makes sense to apply the patch, since it would hurt service when number of MS start to rise.

I started giving a try to some possible optimization, which doesn't fix the problem described in this ticket (reproduced in test_2_consecutive_dl_tbfs()), but may help improvidng DL bandwidth allocation in some cases (at the expence of lowering a bit the capacity, from 101->100 in current AllocTest):

diff --git a/src/gprs_rlcmac_ts_alloc.cpp b/src/gprs_rlcmac_ts_alloc.cpp
index 5f0bcc5..5abd921 100644
--- a/src/gprs_rlcmac_ts_alloc.cpp
+++ b/src/gprs_rlcmac_ts_alloc.cpp
@@ -634,12 +634,17 @@ int find_multi_slots(struct gprs_rlcmac_trx *trx, uint8_t mslot_class, uint8_t *
                                                capacity);
 #endif

-                                       if (capacity <= max_capacity)
+                                       if (capacity < max_capacity)
+                                               continue;
+                                       /* On same capacity, prefer higher DL window */
+                                       if (capacity == max_capacity &&
+                                           rx_window <= max_dl_slots)
                                                continue;

I'm still not sure it really improves stuff in lots of cases though, so it's WIP.

Actions #9

Updated by pespin about 3 years ago

After looking more carefuly at the code aroudn the proposed patch, I noticed the proposal would be wrong afacit because it's comparing 2 bitmask. Instead, the counting of those bitmask should be used:

-                                       if (capacity <= max_capacity)
+                                       rx_count = pcu_bitcount(rx_window);
+                                       tx_count = pcu_bitcount(tx_window);
+                                       if (rx_count < max_rx)
+                                               continue;
+                                       if (rx_count == max_rx && tx_count <= max_tx)
                                                continue;

-                                       max_capacity = capacity;
+                                       //max_capacity = capacity;
                                        max_ul_slots = tx_window;
                                        max_dl_slots = rx_window;
+                                       max_rx = rx_count;
+                                       max_tx = tx_count;
                                }

However, it seems to suffer from same issues described initialy. More slots maybe assigned to the X first TBFs, but TBF allocation will end up exhausted far too quickly. See for instance in AllocTest when running against this patch with gdb (32 in master's algo vs proposed change in this ticket):

No USF available
  Successfully allocated 7 UL TBFs, algorithm B class 10..10 (UL and DL)
  Expected 32 TBFs (got 7), algorithm B class 10..10 (UL and DL)
Assert failed counter == expect_num /home/pespin/dev/sysmocom/git/osmo-pcu/tests/alloc/AllocTest.cpp:671

Actions #10

Updated by pespin about 3 years ago

  • Status changed from New to Rejected
I tried again writing a version of my own keeping best from both sides:
  • First of all, optimizing based on max capacity
  • On equal capacity, optimize based on DL num of slots (and on equal num of DL slots, then optimize on UL num of slots).

See following changes:

-                                       if (capacity <= max_capacity)
+                                       if (capacity < max_capacity)
                                                continue;
+                                       rx_count = pcu_bitcount(rx_window);
+                                       tx_count = pcu_bitcount(tx_window);
+                                       if (capacity == max_capacity && rx_count < max_rx)
+                                               continue;
+                                       if (capacity == max_capacity && rx_count == max_rx && tx_count <= max_tx)
+                                               continue;
+
+                                       if (capacity == max_capacity) {
+                                               LOGP(DRLCMAC, LOGL_NOTICE, "PESPIN: updating capcity=%d tx_window=%d rx_window=%d max_dl_slots=%d\n",
+                                               capacity, tx_window, rx_window, max_dl_slots);
+                                       }

                                        max_capacity = capacity;
                                        max_ul_slots = tx_window;
                                        max_dl_slots = rx_window;
+                                       max_rx = rx_count;
+                                       max_tx = tx_count;
                                }

This time, results from exhaustion point of view look better than with original patch, but still the exhaustion of TBFs happens way too early (32 vs 21):

Allocating UL TBF: MS_CLASS=10/0
No USF available
TBF(TFI=21 TLLI=0xffffffff DIR=DL STATE=RELEASING) free
  Successfully allocated 21 UL TBFs, algorithm B class 10..10 (DL and UL)
  Expected 32 TBFs (got 21), algorithm B class 10..10 (DL and UL)
Assert failed counter == expect_num /home/pespin/dev/sysmocom/git/osmo-pcu/tests/alloc/AllocTest.cpp:671
backtrace() returned 11 addresses

That's probably related to the fact that selecting bigger DL num of slots (eg 4 slots for a class 11 MS with 4 PDCH-enabled TRX) ends up allocating USFs on the first PDCHs timeslots always.

So I think it's proven that, based on the fact that we want to optimize in capacity (to avoid exhaustion), the current allocation algo is better than the proposed change in this ticket, at the expense of lossing some DL TS sometimes when allocating TBfs to an MS.

All in all, one can consider the current algo is more fair than the proposal here in the sense that it allows providing service (TBF allocation) to more MS, while the proposal here is more fair only for the set of MS which were provided service (before TBF allocation exhaustion occurs).

Hence, I reject the ticket.

Actions

Also available in: Atom PDF

Add picture from clipboard (Maximum size: 48.8 MB)