Bug #3626
closedLAPDm code pulls both 'l1h' and 'l2h' of msgb
100%
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
Updated by laforge over 5 years ago
you can consider using the msgb->cb for this?
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?
Updated by fixeria 10 months ago
- Related to Feature #5500: MS-Side GPRS RLC/MAC implementation added
Updated by fixeria 10 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 tostruct 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.
- parse the received
- libosmocore.git
lapdm_phsap_up()
: here we switch based on primitive type, takingcase 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 calll2_ph_data_ind()
passing it the msgb, chan_nr and link_id.
- the scope of
- libosmocore.git LAPDm magic (merging fragments, handling retransmissions, etc.) happens...
- among with all the magic the code does
msgb_pull_to_l3()
, removing thestruct l1ctl_info_dl
(!) - once the payload is ready,
send_rslms_rll_l3()
gets called.
- among with all the magic the code does
- 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 callbackle->l3_cb
.
- in this function we push TV16 with chan_nr and link_id by calling
Updated by laforge 10 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.
Updated by laforge 10 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 thestruct 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.
Updated by jolly 10 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.
Updated by laforge 10 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.
Updated by pespin 9 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
Updated by fixeria 9 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
:
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?
Updated by pespin 9 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.
Updated by pespin 9 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