Bug #3059
closedSystem Information on SACCH missing L2 Pseudo-Length
100%
Description
Something seems odd about our SI5/SI6 messages.
The message on the downlink SACCH is structured as follows:
- two octets L1 header
- two octets "LAPDm B4" frame format: Address + Control Octet
- 23-4=19 octets of L3 payload (as specified in TS 44.018), consisting of
- one octet L2 pseudo-length
- 18 octets of actual L3 message
It appears that the messages that we send from the Osmocom stack are missing the L2 pseudo-length. This maybe related to https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=14105
Checklist
- wireshark patch for RSL
- wireshark patch for SACCH/LAPDm
- patch for legacy openbsc.git
Related issues
Updated by laforge over 6 years ago
- Related to Bug #3057: OsmoBTS fails to schedule SACCH filling like SI5 added
Updated by laforge over 6 years ago
- Related to Bug #2963: Measurement Reports cease to be useful some time into a voice call / after handover (not sure which project has the bug / bugs yet) added
Updated by laforge over 6 years ago
- Status changed from New to In Progress
- % Done changed from 0 to 10
the interesting question is: how does the format look on Abis? According to my reading of the specs:
- L3 Info TLV length given by TS 48.058 for SACCH FILLING is "22", where 1 byte is tag, and 2 bytes are length, i.e. 19 bytes left for actual payload
- this indicates the full 19 bytes including pseudo-length are to be included
- The actual SI5 / SI6 definitions in TS 44.018 also include the L2 pseodo length as part of the SI message
I also found some old pcap files of a proprietary ip.access BSC, which confirms that assumption.
So it seems to me that- OsmoBSC is lacking the L2 pseudo-length field as first byte of the L3 INFO
- the wireshark packet-rsl.c decoder is broken as it assumes no l2 plen
- we impleemnted our code to comply with wireshark, inheriting the bug
- the wireshark LAPDm decoder was "fixed" by me to also comply with that broken assumption
So now, wiershark RSL, wiershark GSMTAP/LAPDm and OsmoBSC are all broken. yay. :(
Updated by laforge over 6 years ago
This appears to be the root of all evil:
commit 6f0e50c8337355eb59033903ede9ab6528890835 Author: Max <msuraev@sysmocom.de> Date: Wed Apr 12 15:30:54 2017 +0200 Prepare for extended SI2quater support
as it overwrites the l2_plen just after it was written.
Updated by laforge over 6 years ago
- Assignee changed from laforge to neels
- % Done changed from 10 to 30
proposed (but yet untested) patch at https://gerrit.osmocom.org/7220
Updated by laforge over 6 years ago
- Project changed from Cellular Network Infrastructure to OsmoBSC
Updated by neels over 6 years ago
- Project changed from OsmoBSC to Cellular Network Infrastructure
- Assignee changed from neels to laforge
- % Done changed from 30 to 80
Just tested with this patch as well as https://gerrit.osmocom.org/#/c/7218/ (it's already merged to osmo-bts master, but let me mention that it was used in the test) and: YES! Finally the measurement reports survive a handover! No matter what I do, the neighbor lists are reliably populated. Excellent! Let me add a few percent there.
Updated by fixeria over 6 years ago
Hi Harald,
this is probably related to:
https://lists.osmocom.org/pipermail/openbsc/2017-December/011545.html
my plan is to revert the:
https://git.osmocom.org/osmocom-bb/commit/?id=1a8a80aeae4c2e5c870ae5b032d9a6ae60b67ac8
because I was referring an outdated spec version and the way of Osmocom stack.
Updated by laforge over 6 years ago
- Checklist item wireshark patch for RSL added
- Checklist item wireshark patch for SACCH/LAPDm added
- Project changed from Cellular Network Infrastructure to OpenBSC
- % Done changed from 80 to 90
ok, have merged the related patch now. Let's abuse this ticket until wireshark patches are written + tested.
Updated by laforge over 6 years ago
- Checklist item patch for legacy openbsc.git added
openbsc.git fix submitted in https://gerrit.osmocom.org/#/c/7226/
Updated by fixeria over 6 years ago
My question was lost because the 7226 was merged, but anyway,
laforge, what do you think about introducing a new structure with l2_plen?
Updated by laforge over 6 years ago
On Mon, Mar 12, 2018 at 01:16:53PM +0000, fixeria [REDMINE] wrote:
My question was lost because the 7226 was merged, but anyway,
it wasn't lost, I read it ;)
laforge, what do you think about introducing a new structure with l2_plen?
I don't think it's worth it. Too much confusion?
Updated by laforge over 6 years ago
- Checklist item wireshark patch for RSL set to Done
Updated by laforge over 6 years ago
wireshark patch for RSL now proposed as https://code.wireshark.org/review/#/c/26797/
Updated by laforge over 6 years ago
- Checklist item wireshark patch for SACCH/LAPDm set to Done
- Status changed from In Progress to Stalled
LAPDm patch submitted at https://code.wireshark.org/review/#/c/26798/1 - waiting for review.
Updated by laforge about 6 years ago
- Status changed from Stalled to Resolved
- % Done changed from 90 to 100
both wireshark patches merged.