fixeria has uploaded this change for review.

View Change

rlcmac: fix coding of EGPRS Packet Uplink Assignment in IA RestOctets

Thanks to the unit tests, it was found that decoding of the LH part,
specifically the EGPRS Packet Uplink Assignment structure, is done
incorrectly. This patch fixes incorrect decoding.

Double usage of the UnionType element in the IA_EGPRS_t has a side
effect: the M_UINT actually consumes one bit, so the remaining bit
stream is shifted and decoded incorrectly.

Replace a combination of M_UINT and M_CHOICE with two M_UNIONs.
Remove struct IA_EGPRS_t, which is not used anywhere yet.

For reference, see 3GPP TS 44.018, table 10.5.2.16.1.

Change-Id: I7fedbad1ac4d744d1d3941553e573d4202e9d24a
Fixes: I39a29dc9b5b22ce4374ae33336696014e326d012
Related: OS#5500
---
M include/osmocom/gprs/rlcmac/gprs_rlcmac.h
M src/rlcmac/ts_44_018.c
M tests/rlcmac/ts_44_018_test.err
3 files changed, 25 insertions(+), 33 deletions(-)

git pull ssh://gerrit.osmocom.org:29418/libosmo-gprs refs/changes/98/30798/1
diff --git a/include/osmocom/gprs/rlcmac/gprs_rlcmac.h b/include/osmocom/gprs/rlcmac/gprs_rlcmac.h
index 5e6613b..d5912c9 100644
--- a/include/osmocom/gprs/rlcmac/gprs_rlcmac.h
+++ b/include/osmocom/gprs/rlcmac/gprs_rlcmac.h
@@ -677,17 +677,6 @@

typedef struct
{
- guint8 UnionType;
- union
- {
- IA_EGPRS_00_t IA_EGPRS_PUA; /* 00 < EGPRS Packet Uplink Assignment >*/
- guint8 IA_EGPRS_01; /* 01 reserved for future use */
- guint8 IA_EGPRS_1; /* 1 reserved for future use */
- } u;
-} IA_EGPRS_t;
-
-typedef struct
-{
guint8 Length;
guint8 MAIO;
guint8 MobileAllocation[62];
@@ -825,7 +814,18 @@

typedef struct
{
- IA_EGPRS_t EGPRS;
+ guint8 UnionType;
+ union {
+ IA_EGPRS_00_t IA_EGPRS_PUA;
+ /* TODO: <Multiple Blocks Packet Downlink Assignment> */
+ } u;
+} IA_RestOctetsLH0x_t;
+
+typedef struct
+{
+ guint8 UnionType; /* only one variant: lh0x */
+ IA_RestOctetsLH0x_t lh0x;
+
IA_AdditionsR13_t AdditionsR13;
} IA_RestOctetsLH_t;

diff --git a/src/rlcmac/ts_44_018.c b/src/rlcmac/ts_44_018.c
index aa43b07..5481166 100644
--- a/src/rlcmac/ts_44_018.c
+++ b/src/rlcmac/ts_44_018.c
@@ -228,24 +228,6 @@
CSN_DESCR_END (IA_EGPRS_00_t)

static const
-CSN_ChoiceElement_t IA_EGPRS_Choice[] =
-{
- {2, 0x00, 0, M_TYPE (IA_EGPRS_t, u.IA_EGPRS_PUA, IA_EGPRS_00_t)},
- {2, 0x01, 0, CSN_ERROR(IA_EGPRS_t, "01 <Packet Downlink Assignment>", CSN_ERROR_STREAM_NOT_SUPPORTED)},
- {1, 0x01, 0, CSN_ERROR(IA_EGPRS_t, "1 <Second Part Packet Assignment>", CSN_ERROR_STREAM_NOT_SUPPORTED)},
-};
-
-/* Please observe the double usage of UnionType element.
- * First, it is used to store the second bit of LL/LH identification of EGPRS contents.
- * Thereafter, UnionType will be used to store the index to detected choice.
- */
-static const
-CSN_DESCR_BEGIN(IA_EGPRS_t)
- M_UINT (IA_EGPRS_t, UnionType , 1 ),
- M_CHOICE (IA_EGPRS_t, UnionType, IA_EGPRS_Choice, ElementsOf(IA_EGPRS_Choice)),
-CSN_DESCR_END (IA_EGPRS_t)
-
-static const
CSN_DESCR_BEGIN(IA_FreqParamsBeforeTime_t)
M_UINT (IA_FreqParamsBeforeTime_t, Length, 6),
M_UINT (IA_FreqParamsBeforeTime_t, MAIO, 6),
@@ -373,8 +355,18 @@
CSN_DESCR_END (IA_RestOctetsLL_t)

static const
+CSN_DESCR_BEGIN (IA_RestOctetsLH0x_t)
+ M_UNION (IA_RestOctetsLH0x_t, 2),
+ M_TYPE (IA_RestOctetsLH0x_t, u.IA_EGPRS_PUA, IA_EGPRS_00_t),
+ CSN_ERROR (IA_RestOctetsLH0x_t, "01 <Multiple Blocks Packet Downlink Assignment>", CSN_ERROR_STREAM_NOT_SUPPORTED),
+CSN_DESCR_END (IA_RestOctetsLH0x_t)
+
+static const
CSN_DESCR_BEGIN (IA_RestOctetsLH_t)
- M_TYPE (IA_RestOctetsLH_t, EGPRS, IA_EGPRS_t),
+ M_UNION (IA_RestOctetsLH_t, 2),
+ M_TYPE (IA_RestOctetsLH_t, lh0x, IA_RestOctetsLH0x_t),
+ CSN_ERROR (IA_RestOctetsLH_t, "1 -- reserved for future use", CSN_ERROR_STREAM_NOT_SUPPORTED),
+
M_TYPE_OR_NULL (IA_RestOctetsLH_t, AdditionsR13, IA_AdditionsR13_t),
CSN_DESCR_END (IA_RestOctetsLH_t)

diff --git a/tests/rlcmac/ts_44_018_test.err b/tests/rlcmac/ts_44_018_test.err
index 46348f9..8b804a5 100644
--- a/tests/rlcmac/ts_44_018_test.err
+++ b/tests/rlcmac/ts_44_018_test.err
@@ -6,5 +6,5 @@
DLCSN1 INFO osmo_csn1_stream_decode (IA Rest Octets): u.hh = 3 | : u.hh | u.UplinkDownlinkAssignment = 0 | : u.UplinkDownlinkAssignment | ul_dl.Packet_Uplink_ImmAssignment = 0 | : ul_dl.Packet_Uplink_ImmAssignment | Access.DynamicOrFixedAllocation = 1 | : Access.DynamicOrFixedAllocation | TFI_ASSIGNMENT = 3 | POLLING = 0 | Allocation.DynamicAllocation = 0 | : Allocation.DynamicAllocation | USF = 1 | USF_GRANULARITY = 0 | Exist_P0_PR_MODE = 1 | P0 = 0 | PR_MODE = 1 | : End Allocation.DynamicAllocation | CHANNEL_CODING_COMMAND = 1 | TLLI_BLOCK_CHANNEL_CODING = 1 | Exist_ALPHA = 0 | GAMMA = 15 | Exist_TIMING_ADVANCE_INDEX = 1 | TIMING_ADVANCE_INDEX = 0 | Exist_TBF_STARTING_TIME = 0 | : End Access.DynamicOrFixedAllocation | Exist_AdditionsR99 = 0 | : End ul_dl.Packet_Uplink_ImmAssignment | : End u.UplinkDownlinkAssignment | Exist_AdditionsR10 = 0 | Exist_AdditionsR13 = 0 | : End u.hh | Padding = 43|43|43|43|43|0|
DLCSN1 INFO osmo_csn1_stream_decode (IA Rest Octets): u.hh = 3 | : u.hh | u.UplinkDownlinkAssignment = 0 | : u.UplinkDownlinkAssignment | ul_dl.Packet_Uplink_ImmAssignment = 0 | : ul_dl.Packet_Uplink_ImmAssignment | Access.SingleBlockAllocation = 0 | : Access.SingleBlockAllocation | Exist_ALPHA = 0 | GAMMA = 15 | 0x01 = 1 | : TBF_STARTING_TIME | N32 = 14 | N51 = 50 | N26 = 13 | : End TBF_STARTING_TIME | Exist_P0_BTS_PWR_CTRL_PR_MODE = 0 | : End Access.SingleBlockAllocation | Exist_AdditionsR99 = 0 | : End ul_dl.Packet_Uplink_ImmAssignment | : End u.UplinkDownlinkAssignment | Exist_AdditionsR10 = 0 | Exist_AdditionsR13 = 0 | : End u.hh | Padding = 21|149|149|149|149|128|-22|
DLCSN1 INFO osmo_csn1_stream_decode (IA Rest Octets): u.hh = 3 | : u.hh | u.UplinkDownlinkAssignment = 0 | : u.UplinkDownlinkAssignment | ul_dl.Packet_Downlink_ImmAssignment = 1 | : ul_dl.Packet_Downlink_ImmAssignment | TLLI = 0xd6e1ae5a | Exist_TFI_to_TA_VALID = 1 | TFI_ASSIGNMENT = 3 | RLC_MODE = 0 | Exist_ALPHA = 0 | GAMMA = 15 | POLLING = 0 | TA_VALID = 0 | Exist_TIMING_ADVANCE_INDEX = 0 | Exist_TBF_STARTING_TIME = 0 | Exist_P0_PR_MODE = 1 | P0 = 0 | BTS_PWR_CTRL_MODE = 0 | PR_MODE = 1 | Exist_AdditionsR99 = 0 | : End ul_dl.Packet_Downlink_ImmAssignment | : End u.UplinkDownlinkAssignment | Exist_AdditionsR10 = 0 | Exist_AdditionsR13 = 0 | : End u.hh | Padding = 0|86|86|-22|
-DLCSN1 INFO osmo_csn1_stream_decode (IA Rest Octets): u.lh = 1 | : u.lh | : EGPRS | UnionType = 0 | Choice IA_EGPRS_Choice = 0 | : u.IA_EGPRS_PUA | ExtendedRA = 25 | AccessTechnologyType = 0 | Access.TwoPhaseAccess = 0 | : Access.TwoPhaseAccess | Exist_ALPHA = 0 | GAMMA = 9 | : TBF_STARTING_TIME | N32 = 20 | N51 = 56 | N26 = 24 | : End TBF_STARTING_TIME | NR_OF_RADIO_BLOCKS_ALLOCATED = 1 | Exist_P0_BTS_PWR_CTRL_PR_MODE = 0 | : End Access.TwoPhaseAccess | : End u.IA_EGPRS_PUA | : End EGPRS | : AdditionsR13 | Exist_AdditionsR13 = 1 | ImplicitRejectPS = 1 | PEO_BCCH_CHANGE_MARK = 1 | RCC = 3 | : End AdditionsR13 | : End u.lh | Padding = 2|27|3|43|43|43|
-DLCSN1 INFO osmo_csn1_stream_decode (IA Rest Octets): u.lh = 1 | : u.lh | : EGPRS | UnionType = 0 | Choice IA_EGPRS_Choice = 0 | : u.IA_EGPRS_PUA | ExtendedRA = 17 | AccessTechnologyType = 0 | Access.TwoPhaseAccess = 0 | : Access.TwoPhaseAccess | Exist_ALPHA = 0 | GAMMA = 9 | : TBF_STARTING_TIME | N32 = 16 | N51 = 54 | N26 = 18 | : End TBF_STARTING_TIME | NR_OF_RADIO_BLOCKS_ALLOCATED = 3 | Exist_P0_BTS_PWR_CTRL_PR_MODE = 0 | : End Access.TwoPhaseAccess | : End u.IA_EGPRS_PUA | : End EGPRS | : AdditionsR13 | Exist_AdditionsR13 = 1 | ImplicitRejectPS = 0 | PEO_BCCH_CHANGE_MARK = 0 | RCC = 0 | : End AdditionsR13 | : End u.lh | Padding = 11|43|43|43|43|43|
+DLCSN1 INFO osmo_csn1_stream_decode (IA Rest Octets): u.lh = 1 | : u.lh | lh0x = 0 | : lh0x | u.IA_EGPRS_PUA = 0 | : u.IA_EGPRS_PUA | ExtendedRA = 12 | AccessTechnologyType = Exist | AccessTechnologyType = 0 | AccessTechnologyType = Exist | AccessTechnologyType = 3 | AccessTechnologyType = 0 | Access.OnePhaseAccess = 1 | : Access.OnePhaseAccess | TFI_ASSIGNMENT = 7 | POLLING = 0 | Allocation.DynamicAllocation = 0 | : Allocation.DynamicAllocation | USF = 3 | USF_GRANULARITY = 0 | Exist_P0_PR_MODE = 0 | : End Allocation.DynamicAllocation | EGPRS_CHANNEL_CODING_COMMAND = 2 | TLLI_BLOCK_CHANNEL_CODING = 1 | Exist_BEP_PERIOD2 = 1 | BEP_PERIOD2 = 5 | RESEGMENT = 1 | EGPRS_WindowSize = 4 | Exist_ALPHA = 0 | GAMMA = 13 | Exist_TIMING_ADVANCE_INDEX = 1 | TIMING_ADVANCE_INDEX = 0 | Exist_TBF_STARTING_TIME = 0 | : End Access.OnePhaseAccess | : End u.IA_EGPRS_PUA | : End lh0x | : AdditionsR13 | Exist_AdditionsR13 = 0 | : End AdditionsR13 | : End u.lh | Padding = 0|172|172|-22|
+DLCSN1 INFO osmo_csn1_stream_decode (IA Rest Octets): u.lh = 1 | : u.lh | lh0x = 0 | : lh0x | u.IA_EGPRS_PUA = 0 | : u.IA_EGPRS_PUA | ExtendedRA = 8 | AccessTechnologyType = Exist | AccessTechnologyType = 0 | AccessTechnologyType = Exist | AccessTechnologyType = 3 | AccessTechnologyType = 0 | Access.TwoPhaseAccess = 0 | : Access.TwoPhaseAccess | Exist_ALPHA = 0 | GAMMA = 13 | : TBF_STARTING_TIME | N32 = 20 | N51 = 45 | N26 = 0 | : End TBF_STARTING_TIME | NR_OF_RADIO_BLOCKS_ALLOCATED = 1 | Exist_P0_BTS_PWR_CTRL_PR_MODE = 0 | : End Access.TwoPhaseAccess | : End u.IA_EGPRS_PUA | : End lh0x | : AdditionsR13 | Exist_AdditionsR13 = 0 | : End AdditionsR13 | : End u.lh | Padding = 0|86|86|86|86|-22|

To view, visit change 30798. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: libosmo-gprs
Gerrit-Branch: master
Gerrit-Change-Id: I7fedbad1ac4d744d1d3941553e573d4202e9d24a
Gerrit-Change-Number: 30798
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <vyanitskiy@sysmocom.de>
Gerrit-MessageType: newchange