Actions
Bug #3854
closedOsmoPCU uses wrong CellID in BSSGP
Start date:
03/21/2019
Due date:
% Done:
100%
Spec Reference:
Description
While writing some unrelated PCU tests, I discovered that somehow the CellID on the BSSGP side is wrong.
Whatever the BTS passes in on the PCU interface side ends uo endian-swapped in the BVC-RESET on BSSGP.
This is really odd, as all of PCUIF is in host byte order.
Looking at the commitlog, it appears that the following patch is to be blamed:
commit bdc55fad62e333951a0eb1fc7b96aaaec16dd6ff Author: Neels Hofmeyr <neels@hofmeyr.de> Date: Wed Feb 21 00:39:07 2018 +0100 implement support for 3-digit MNC with leading zeros Receive the mnc_3_digits flag from the PCU interface. Bump the PCU interface to 9. This is one part of the three identical pcuif_proto.h patches: - I49cd762c3c9d7ee6a82451bdf3ffa2a060767947 (osmo-bts) - I787fed84a7b613158a5618dd5cffafe4e4927234 (osmo-pcu) - I78f30aef7aa224b2e9db54c3a844d8f520b3aee0 (osmo-bsc) Add 3-digit flags and use the new RAI and LAI API from libosmocore throughout the code base to be able to handle an MNC < 100 that has three digits (leading zeros). Depends: Id2240f7f518494c9df6c8bda52c0d5092f90f221 (libosmocore), Ib7176b1d65a03b76f41f94bc9d3293a8a07d24c6 (libosmocore) Change-Id: I787fed84a7b613158a5618dd5cffafe4e4927234
And with all due respect, it's a prime example of how we fail
- a completely unrelated change (changing the endinanness of cell_id) is made as part of implementing 3 digits MNC. This should clearly have been unrelated patches
- the reviewer (myself in this case) didn't spot or at least not reject this "making two unrelated changes in one commit"
- not having test coverage for OsmoPCU
So for more than one year, we've been reporting the wrong CellID to the SGSN:/
We should learn from this that we absolutely never accept unrelated changes in the same patch. In order to introduce 3 digit MNC support, there was no need to shift the htons() around.
Related issues
Actions