Hi Keith,
On 12.01.2023 10:05, Keith wrote:
Could somebody take a look at this:
https://gitea.osmocom.org/cellular-infrastructure/osmo-msc/src/branch/master/src/libmsc/transaction.c#L156
where we have:
osmo_store32be(trans->callref, lcls->gcr.cr);
osmo_store16be(use_lac ? trans->msc_a->via_cell.lai.lac :
trans->msc_a->via_cell.cell_identity, lcls->gcr.cr + 3);
I don't have any deep knowledge of the related code, but I see weird
things happening in the code you're pointing to above:
* the lcls->gcr.cr[] is only 5 octets large,
* the code stores 4 + 2 = 6 octets in that buffer.
| 0 | 1 | 2 | 3 | 4 |
^^^^^^^^^^^^xxx CallRef at &lcls->gcr.cr[0]
^^^^^^^ LAC/CID at &lcls->gcr.cr[3]
As can be seen, LAC/CID overwrites last octet of the CallRef. This looks
like a bug to me, because we're basically loosing an essential part of
the CallRef (e.g. 0x80000023 becomes 0x800000??).
Now, If I change the order, such that would seem logical:
osmo_store16be(use_lac ? trans->msc_a->via_cell.lai.lac :
trans->msc_a->via_cell.cell_identity, lcls->gcr.cr + 3);
osmo_store32be(trans->callref, lcls->gcr.cr);
Then I get a different GCR, reflecting the trans->callref for each call.
If you change the order, the CallRef overwrites half of the LAC/CID:
| 0 | 1 | 2 | 3 | 4 |
xxx~~~~ LAC/CID at &lcls->gcr.cr[3]
~~~~~~~~~~~~~~~ CallRef at &lcls->gcr.cr[0]
But am I then maybe overwriting the LAC/CI ?
Exactly.
I guess the limit of 5 octets is defined by the specs? If so, then it's
better to sacrifice the most significant octet (usually 0x80 for CC) of
the CallRef, rather than the least significant one.
== Option a):
| 0 | 1 | 2 | 3 | 4 |
^^^^^^^^^^^ CallRef at &lcls->gcr.cr[0]
^^^^^^^ LAC/CID at &lcls->gcr.cr[3]
lcls->gcr.cr[2] = (trans->callref >> 0) & 0xff;
lcls->gcr.cr[1] = (trans->callref >> 8) & 0xff;
lcls->gcr.cr[0] = (trans->callref >> 16) & 0xff;
osmo_store16be(/* LAC/CID */, &lcls->gcr.cr[3]);
== Option b):
| 0 | 1 | 2 | 3 | 4 |
^^^^^^^^^^^ CallRef at &lcls->gcr.cr[2]
^^^^^^^ LAC/CID at &lcls->gcr.cr[0]
/* NOTE: LAC/CID overwrites MSB of CallRef at &lcls->gcr.cr[1] */
osmo_store32be(/* CallRef */, &lcls->gcr.cr[1]);
osmo_store16be(/* LAC/CID */, &lcls->gcr.cr[0]);
Personally I would prefer option b), because hexadecimal representation
of the resulting lcls->gcr.cr[] would look similar to the CallRef in
hex, so it would be easier to match them in logging.
Best regards,
Vadim.
--
- Vadim Yanitskiy <vyanitskiy at sysmocom.de> http://www.sysmocom.de/
=======================================================================
* sysmocom - systems for mobile communications GmbH
* Alt-Moabit 93
* 10559 Berlin, Germany
* Sitz / Registered office: Berlin, HRB 134158 B
* Geschaeftsfuehrer / Managing Director: Harald Welte
