Project

General

Profile

Bug #2989

OsmoBTS reports ever-growing TA value

Added by laforge 12 months ago. Updated 29 days ago.

Status:
Stalled
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 codeNew2016-11-18

History

#1 Updated by laforge 12 months ago

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

#2 Updated by laforge 12 months 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

#3 Updated by laforge 12 months ago

  • % Done changed from 0 to 20

#4 Updated by fixeria 12 months 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.

#5 Updated by laforge 12 months 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.

#6 Updated by laforge 12 months ago

Ping?

#7 Updated by dexter 12 months 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.

#8 Updated by laforge 9 months ago

  • Tags set to TTCN3

#9 Updated by laforge 9 months ago

see also: Timing_Advance

#10 Updated by dexter 6 months ago

  • Status changed from In Progress to Stalled

#11 Updated by laforge 4 months ago

  • Assignee changed from dexter to tnt

#12 Updated by tnt about 1 month 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 ...

#13 Updated by fixeria about 1 month 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).

#14 Updated by tnt 29 days 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.

Also available in: Atom PDF

Add picture from clipboard (Maximum size: 48.8 MB)