Project

General

Profile

Actions

Bug #3626

closed

LAPDm code pulls both 'l1h' and 'l2h' of msgb

Added by fixeria over 5 years ago. Updated 7 months ago.

Status:
Resolved
Priority:
Normal
Assignee:
Category:
libosmogsm
Target version:
-
Start date:
10/04/2018
Due date:
% Done:

100%

Spec Reference:

Description

In some cases, it's required to keep some data before the actual MAC-block, e.g. in order to indicate
the FDMA/TDMA info (frame number, ARFCN, etc.) to the upper layers, but the current implementation
doesn't allow this, because it calls msgb_pull_to_l3() stripping all headers. In other words,
when a message buffer is being passed through the current LAPDm code, everything before the
data frame is getting lost.


Related issues

Related to OsmocomBB - Feature #5500: MS-Side GPRS RLC/MAC implementationResolvedpespin07/01/2022

Actions
Related to OsmoBTS - Bug #6142: channels are opened, but nothing happens, sometimes strange DTAP messages appearResolvedpespin08/23/2023

Actions
Blocks OsmocomBB - Bug #6130: modem: Fix Submitting CCCH_DATA.ind with hardcoded fn=0 to libosmo-gprs-rlcmacResolvedpespin08/02/2023

Actions
Actions #1

Updated by laforge over 5 years ago

you can consider using the msgb->cb for this?

Actions #2

Updated by fixeria over 5 years ago

you can consider using the msgb->cb for this?

Possible but not flexible. In case of OsmocomBB, I would like to keep the whole 'l1ctl_info_dl' header.
What is the purpose / reason of stripping both 'l1h' and 'l2h' message parts out?

Actions #3

Updated by fixeria 9 months ago

  • Status changed from New to In Progress
  • Priority changed from Low to Normal

Bumping priority. This problem is blocking us in #5500, where we need to know TDMA Fn in order to distinguish between AGCH (UL TBF assignment) and PCH (DL TBF assignment).

Actions #4

Updated by fixeria 9 months ago

  • Related to Feature #5500: MS-Side GPRS RLC/MAC implementation added
Actions #5

Updated by fixeria 9 months ago

Here is a quick recap on the problem:

  • osmocom-bb.git rx_ph_data_ind(): here we get a msgb from the L1/PHY containing a MAC block and a L1CTL DL header (struct l1ctl_info_dl) with the L1 context information (ARFCN, TDMA Fn, measurements). In this function we:
    • parse the received L1CTL_DATA_IND,
      • msg->l1h is pointing to struct l1ctl_info_dl,
      • msg->l2h is pointing to the MAC block,
    • allocate struct osmo_phsap_prim on stack,
    • make this prim point to the msgb (but not msgb_push it),
    • call lapdm_phsap_up() passing it the primitive.
  • libosmocore.git lapdm_phsap_up(): here we switch based on primitive type, taking case PRIM_PH_DATA.ind.
    • the scope of struct osmo_phsap_prim ends here, it is not getting passed to other functions;
    • for case PRIM_PH_DATA.ind we call l2_ph_data_ind() passing it the msgb, chan_nr and link_id.
  • libosmocore.git LAPDm magic (merging fragments, handling retransmissions, etc.) happens...
    • among with all the magic the code does msgb_pull_to_l3(), removing the struct l1ctl_info_dl (!)
    • once the payload is ready, send_rslms_rll_l3() gets called.
  • libosmocore.git send_rslms_rll_l3(): this function emits complete L3 payload to the upper layers (L3):
    • in this function we push TV16 with chan_nr and link_id by calling rsl_rll_push_l3(),
    • finally, rslms_sendmsg() calls the user-provided callback le->l3_cb.
Actions #6

Updated by laforge 9 months ago

I've added jolly to the ticket, as he is the original author of our LAPD/LAPDm code

Actions #7

Updated by laforge 9 months ago

fixeria wrote in #note-2:

Possible but not flexible. In case of OsmocomBB, I would like to keep the whole 'l1ctl_info_dl' header.
What is the purpose / reason of stripping both 'l1h' and 'l2h' message parts out?

The point is that lapdm is a L2 protocol, and it's user is a L3 protocol, which by definition is interested only in its L3 payload. Making assumptions in L3 that there might be some magic things somewhere earlier in the packet is a layering violation and should be avoided, IMHO.

Actions #8

Updated by laforge 9 months ago

fixeria wrote in #note-5:

  • libosmocore.git LAPDm magic (merging fragments, handling retransmissions, etc.) happens...
    • among with all the magic the code does msgb_pull_to_l3(), removing the struct l1ctl_info_dl (!)
    • once the payload is ready, send_rslms_rll_l3() gets called.

I think this is the key technical explanation for it. L2 has fragmentation/re-assembly. So you cannot even expect the msgb that you receive at L3 to be the same msgb that was passed in at L2. This might be the case in a given implementation for simple processing. However, in case multiple mac-blocks are re-assembled into a larger L3 frame, this could just be a completely new allocated msgb, without any L1/L2 information whatsoever. And L3 cannot make any assumptions about what is before the L3, as there can be any number of L2 frames that contributed to the L3 msgb it receives.

Actions #9

Updated by jolly 9 months ago

And L3 cannot make any assumptions about what is before the L3, as there can be any number of L2 frames that contributed to the L3 msgb it receives.

In case of AGCH and PCH we have RSL_MT_UNIT_DATA_IND type of RSL frames. This means that there is not much magic going on. The message is related to one L1 frame, so it could be related to a frame number and other layer 1 information.

The RSL header's channel number IE does not allow to distinguish between PCH and AGCH. I think RSL layer was not made for the MS side. To fix this, one could push the L1 header in front of the RSL header. (Simple, but not quite RSL conform.) I would suggest to add "Frame Number" IE (08.58 / 9.3.8) after the L3 Info of the UNIT DATA INDICATION message.

Actions #10

Updated by laforge 8 months ago

jolly wrote in #note-9:

I would suggest to add "Frame Number" IE (08.58 / 9.3.8) after the L3 Info of the UNIT DATA INDICATION message.

I think that adding a standard or non-standard RSL IE to the end of the message, or passing information in the msgb->cb[] is both fine.

Actions #11

Updated by fixeria 7 months ago

  • Blocks Bug #6130: modem: Fix Submitting CCCH_DATA.ind with hardcoded fn=0 to libosmo-gprs-rlcmac added
Actions #12

Updated by pespin 7 months ago

I would suggest to add "Frame Number" IE (08.58 / 9.3.8) after the L3 Info of the UNIT DATA INDICATION message.

This IE (RSL_IE_FRAME_NUMBER) cannot be used because it's 16bit, aka an RFN ("absolute frame number (FN) modulo 42432"). Same goes for (RSL_IE_STARTNG_TIME).

To start with, I fixed osmocom-bb to provide the FN field to lapdm, since that was actually missing (the field was already there but not being populated in this specific case):
https://gerrit.osmocom.org/c/osmocom-bb/+/34115 l1ctl: Fill ph_data_param fn field

Actions #13

Updated by pespin 7 months ago

  • Assignee changed from fixeria to pespin
  • % Done changed from 0 to 50

I have a patchset prepared in libosmocore.git branch "pespin/lapdm" which I still need to test.

Actions #14

Updated by fixeria 7 months ago

pespin wrote in #note-12:

I would suggest to add "Frame Number" IE (08.58 / 9.3.8) after the L3 Info of the UNIT DATA INDICATION message.

This IE (RSL_IE_FRAME_NUMBER) cannot be used because it's 16bit, aka an RFN ("absolute frame number (FN) modulo 42432"). Same goes for (RSL_IE_STARTNG_TIME).

I see a WIP patch in your branch adding a new Osmocom specific IE RSL_IE_OSMO_ABS_FRAME_NUMBER:

https://cgit.osmocom.org/libosmocore/commit/?h=pespin/lapdm&id=58800788bbd66e4d1fc70ffd5ce1dd2a670c170c

I am not strictly against this, but not sure if we really want this, given that the idea of using RSL on the MS side is (beautiful, but still) a hack. I see no use for this IE outside the scope of the RSLms, i.e. on the BTS side. How much would it complicate the upper layers' logic if we stay within the limits of standard RSL IEs and pass a reduced Fn value (i.e. Fn % 42432) to the upper layers?

Actions #15

Updated by pespin 7 months ago

fixeria wrote in #note-14:

How much would it complicate the upper layers' logic if we stay within the limits of standard RSL IEs and pass a reduced Fn value (i.e. Fn % 42432) to the upper layers?

The point is precisely to get an absolute frame from the lower layers so that the RFN from TBF Starting Time in the upper layers can actually be computed to an absolute FN. So that means, the RFN value is not useful there.

Ideally RSLms would be a primitive-based interface where the FN would be a field or the primitive .ind and the l3_buffer would be another field. But that's not how the whole thing is implemented so we have no other option than keep using the same approach it is used so far for other fields: Add an IE in the TLV.

Actions #16

Updated by pespin 7 months ago

fixeria we can of course also pass stuff through the msgb->cb as one of the proposals, but other stuff is already passed through IEs so that would also look even more weird, where some stuff is passed trouh cb, some through TLVs, etc.

Actions #17

Updated by pespin 7 months ago

  • Status changed from In Progress to Feedback
  • % Done changed from 50 to 90

I just tested that I can receive proper FNs now with these patches from libosmocore:
remote: https://gerrit.osmocom.org/c/libosmocore/+/34124 tlv: Introduce API msgb_tv32_push()
remote: https://gerrit.osmocom.org/c/libosmocore/+/34130 lapdm: Track fn of primitives in struct lapdm_msg_ctx [NEW]
remote: https://gerrit.osmocom.org/c/libosmocore/+/34131 rsl: Introduce new osmocom extension IE RSL_IE_OSMO_ABS_FRAME_NUMBER [NEW]
remote: https://gerrit.osmocom.org/c/libosmocore/+/34132 lapdm: Append RSL_IE_OSMO_ABS_FRAME_NUMBER to RSLms msgs towards upper layers [NEW

Actions #18

Updated by pespin 7 months ago

  • Status changed from Feedback to Resolved
  • % Done changed from 90 to 100

Merged, closing.

Actions #19

Updated by pespin 7 months ago

  • Related to Bug #6142: channels are opened, but nothing happens, sometimes strange DTAP messages appear added
Actions

Also available in: Atom PDF

Add picture from clipboard (Maximum size: 48.8 MB)