Project

General

Profile

Actions

Bug #2989

closed

OsmoBTS reports ever-growing TA value

Added by laforge about 6 years ago. Updated over 4 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
-
Target version:
-
Start date:
02/23/2018
Due date:
% Done:

90%

Spec Reference:
Tags:

Description

While writing some OsmoBTS tests in TTCN-3, I encountered the issue that the reported MS Timing Offset in RSL MEAS RES messages keeps increasing despite the MS never moving

I'm using trxcon+fake_trx, but this shouldn't matter. In this setup I can control the erported timing offset on uplink bursts. As soon as only the slightest timing offset is reported, the reported MS TO value keeps increasing magically by itself ?!?

It seems the culprit is Osmo-BTS Change-Id: I2e0dfd13b53e8aa2822985f12bf2985e683ab553 "fix measurement computation" which can be found at http://git.osmocom.org/osmo-bts/commit/?id=d5fdcfe6d95f52fb76c4f4201969347a062fc9fd

It introduces a new function lchan_meas_update_ordered_TA whose functionality I still haven't yet managed to understand. It appears to be adjusting the requested timing advance (lchan->rqd_ta) but outside osmo-bts-trx/loops.c code. This is odd, as rqd_ta is a state variable of that loops.c code.

So for one part, it is a failure of encapsulation. The TA loop code should be self-contained, particularly as it is only used for omso-bts-trx, and not for the other BTS models. The new lchan_meas_update_ordered_TA() function is used from common code, applicable to all BTS models.

The next part that strikes me about this function is that it
  • has an input parameter "taqb_sum" which is actually not a sum but an average
  • contains a computation like ms_timing_offset = taqb_sum + (lchan->meas.l1_info[1] - lchan->rqd_ta); where we mix taqb_sum (taqb meaning timing advance quarter-bits with elements such as lchan->rqd_ta which is an unsigned integer in units of symbols not quarter-bits or quarter-symbols. I haven't looked at the unit of lchan->meas.l1_info[1] yet, but I think it also is in full symbols, not quarters.

Related issues

Related to OsmoBTS - Feature #1851: generalize power control and TA loop codeResolvedpespin11/18/2016

Actions
Actions #1

Updated by laforge about 6 years ago

  • Related to Feature #1851: generalize power control and TA loop code added
Actions #2

Updated by laforge about 6 years ago

  • Status changed from New to In Progress

I'm going to revert the related commit now, as not only it doesn't seem to make sense to me regarding loops.c encapsulation, but it even has obvious problems in terms of mixing variables of different units in a computation.

See https://gerrit.osmocom.org/6868

Actions #3

Updated by laforge about 6 years ago

  • % Done changed from 0 to 20
Actions #4

Updated by fixeria about 6 years ago

I've experienced the same problem of continuously increasing
Timing Advance value, despite the ToA value remains the same.
It was happening only with the real hardware and regular
phones, while I couldn't reproduce the problem in fake_trx :/

Great that you managed to do that.

Actions #5

Updated by laforge about 6 years ago

  • Assignee changed from laforge to dexter
  • % Done changed from 20 to 90

revert has been meged.

leaving this open for dexter to comment, maybe he can clarify the related code.

Actions #6

Updated by laforge about 6 years ago

Ping?

Actions #7

Updated by dexter about 6 years ago

The patch in question dates back to code that was contributed by Octasic inc. I have reviewed it and it seemed plausible. Unfortunately at that time there was no way to verify the correct functionality of timing advance so I had to rely on the review of Octasic.

Actions #8

Updated by laforge almost 6 years ago

  • Tags set to TTCN3
Actions #9

Updated by laforge almost 6 years ago

see also: Timing_Advance

Actions #10

Updated by dexter over 5 years ago

  • Status changed from In Progress to Stalled
Actions #11

Updated by laforge over 5 years ago

  • Assignee changed from dexter to tnt
Actions #12

Updated by tnt about 5 years ago

No matter how I look at this patch, I can't figure out what it's trying to do ... AFAICT it's just plain wrong.

  • It's a different timing loop that will obviously mess with the one in loop.c
  • The algorithm has no hysteresis, it will inevitably oscillate
  • AFAICT it mixes quarter-bit values (taqb) and full bits (TA values)
Now the current loop is not perfect, I can see a couple of issues :
  • The TOA for the last SACCH burst is going to be used twice in the average. Once added in rx_data_fn and then again in ta_val (via the call to trx_loop_sacch_input)
  • When the reported used TA doesn't match the requested one, the loop does nothing (that's fine), but it should also probably clear the accumulated toa measurement in the channel state because those are for bursts sent with a different TA value and so are not really valid. Either we drop them and wait for the TA used by the phone to be updated, or we need to account for the difference between requested TA and actually used TA before we use them to determine if we should increase/decrease the TA
  • The TOA values for TCH bursts (either speech or FACCH) don't seem to be used at all ?

But the patch above just doesn't fix any of those issues ...

Actions #13

Updated by fixeria about 5 years ago

I think this issue should have been fixed now. AFAIR, this bug could be (probably, still can be) triggered if at least one Measurement Report from the MS is lost. Last time I experienced this bug was when trxcon didn't support caching of Measurement Reports (dummy LAPDm frames were being sent instead). Now it does (see If1b8dc74ced746d6270676fdde75fcda32f91a3d, #2988).

Actions #14

Updated by tnt about 5 years ago

Yeah, AFAIK the only thing left in this ticket was figure out the intent of the patch the Harald reverted to fix the growing TA issue.

And ... I don't think there is any. At least anymore. If this code did anything that the code in loop.c did at the time, that's not the case any more.

So unless someone has any objections, I would just close this.

Actions #15

Updated by laforge over 4 years ago

  • Status changed from Stalled to Closed
Actions #16

Updated by taylorwilson over 4 years ago

Actions

Also available in: Atom PDF

Add picture from clipboard (Maximum size: 48.8 MB)