Change in osmo-pcu[master]: RLC/MAC: implement decoding of EGPRS Packet Channel Request

2020-05-25 Thread fixeria
fixeria has submitted this change. ( 
https://gerrit.osmocom.org/c/osmo-pcu/+/17718 )

Change subject: RLC/MAC: implement decoding of EGPRS Packet Channel Request
..

RLC/MAC: implement decoding of EGPRS Packet Channel Request

According to 3GPP TS 44.004, section 7.4a, two alternative RACH block
formats are specified: 8 bit (1 octet) and 11 bit. This change adds
CSN.1 definitions for 11 bit EGPRS Packet Channel Request as per
3GPP TS 44.060, table 11.2.5a.2.

Change-Id: I96df3352856933c9140177b2801a2c71f4134183
Related: OS#1548
---
M src/gsm_rlcmac.c
M src/gsm_rlcmac.h
M tests/rlcmac/RLCMACTest.cpp
M tests/rlcmac/RLCMACTest.err
M tests/rlcmac/RLCMACTest.ok
5 files changed, 222 insertions(+), 0 deletions(-)

Approvals:
  Jenkins Builder: Verified
  laforge: Looks good to me, but someone else must approve
  pespin: Looks good to me, approved



diff --git a/src/gsm_rlcmac.c b/src/gsm_rlcmac.c
index 9b5ecfe..c4ac4b1 100644
--- a/src/gsm_rlcmac.c
+++ b/src/gsm_rlcmac.c
@@ -5249,6 +5249,59 @@
 CSN_DESCR_END(SI6_RestOctet_t)
 #endif

+/* EGPRS Packet Channel Request (see 3GPP TS 44.060, table 11.2.5a.2) */
+static const
+CSN_DESCR_BEGIN(PacketChannelRequest_MC5P2RB3_t)
+  M_UINT   (PacketChannelRequest_MC5P2RB3_t,  MultislotClass,  5),
+  M_UINT   (PacketChannelRequest_MC5P2RB3_t,  Priority,  2),
+  M_UINT   (PacketChannelRequest_MC5P2RB3_t,  RandomBits,  3),
+CSN_DESCR_END  (PacketChannelRequest_MC5P2RB3_t)
+
+static const
+CSN_DESCR_BEGIN(PacketChannelRequest_MCG3P2RB3_t)
+  M_UINT   (PacketChannelRequest_MCG3P2RB3_t,  MultislotClassGroup,  3),
+  M_UINT   (PacketChannelRequest_MCG3P2RB3_t,  Priority,  2),
+  M_UINT   (PacketChannelRequest_MCG3P2RB3_t,  RandomBits,  3),
+CSN_DESCR_END  (PacketChannelRequest_MCG3P2RB3_t)
+
+static const
+CSN_DESCR_BEGIN(PacketChannelRequest_NOB3P2RB3_t)
+  M_UINT   (PacketChannelRequest_NOB3P2RB3_t,  NumberOfBlocks,  3),
+  M_UINT   (PacketChannelRequest_NOB3P2RB3_t,  Priority,  2),
+  M_UINT   (PacketChannelRequest_NOB3P2RB3_t,  RandomBits,  3),
+CSN_DESCR_END  (PacketChannelRequest_NOB3P2RB3_t)
+
+static const
+CSN_DESCR_BEGIN(PacketChannelRequest_P2RB3_t)
+  M_UINT   (PacketChannelRequest_P2RB3_t,  Priority,  2),
+  M_UINT   (PacketChannelRequest_P2RB3_t,  RandomBits,  3),
+CSN_DESCR_END  (PacketChannelRequest_P2RB3_t)
+
+static const
+CSN_DESCR_BEGIN(PacketChannelRequest_RB5_t)
+  M_UINT   (PacketChannelRequest_RB5_t,  RandomBits,  5),
+CSN_DESCR_END  (PacketChannelRequest_RB5_t)
+
+static const
+CSN_ChoiceElement_t EGPRS_PacketChannelRequest_Choice[] =
+{
+  {1, 0x00, 0, M_TYPE(EGPRS_PacketChannelRequest_t, Content, 
PacketChannelRequest_MC5P2RB3_t)},
+  {3, 0x04, 0, M_TYPE(EGPRS_PacketChannelRequest_t, Content, 
PacketChannelRequest_NOB3P2RB3_t)},
+  {3, 0x05, 0, M_TYPE(EGPRS_PacketChannelRequest_t, Content, 
PacketChannelRequest_MCG3P2RB3_t)},
+  {6, 0x30, 0, M_TYPE(EGPRS_PacketChannelRequest_t, Content, 
PacketChannelRequest_P2RB3_t)},
+  {6, 0x33, 0, M_TYPE(EGPRS_PacketChannelRequest_t, Content, 
PacketChannelRequest_RB5_t)},
+  {6, 0x35, 0, M_TYPE(EGPRS_PacketChannelRequest_t, Content, 
PacketChannelRequest_RB5_t)},
+  {6, 0x36, 0, M_TYPE(EGPRS_PacketChannelRequest_t, Content, 
PacketChannelRequest_RB5_t)},
+  {6, 0x37, 0, M_TYPE(EGPRS_PacketChannelRequest_t, Content, 
PacketChannelRequest_RB5_t)},
+  {6, 0x38, 0, M_TYPE(EGPRS_PacketChannelRequest_t, Content, 
PacketChannelRequest_P2RB3_t)},
+  {6, 0x39, 0, M_TYPE(EGPRS_PacketChannelRequest_t, Content, 
PacketChannelRequest_RB5_t)},
+};
+
+CSN_DESCR_BEGIN  (EGPRS_PacketChannelRequest_t)
+  M_CHOICE   (EGPRS_PacketChannelRequest_t, Type,
+  EGPRS_PacketChannelRequest_Choice, 
ElementsOf(EGPRS_PacketChannelRequest_Choice)),
+CSN_DESCR_END(EGPRS_PacketChannelRequest_t)
+
 // 
 // osmo-pcu RLCMAC APIs
 // 
@@ -6153,3 +6206,46 @@

   return ret;
 }
+
+struct value_string egprs_pkt_ch_req_type_names[] = {
+  { EGPRS_PKT_CHAN_REQ_ONE_PHASE,   "One Phase Access" },
+  { EGPRS_PKT_CHAN_REQ_SHORT,   "Short Access" },
+  { EGPRS_PKT_CHAN_REQ_ONE_PHASE_RED_LATENCY,   "One Phase Access (Reduced 
Latency MS)" },
+  { EGPRS_PKT_CHAN_REQ_TWO_PHASE,   "Two Phase Access" },
+  { EGPRS_PKT_CHAN_REQ_SIGNALLING,  "Signalling" },
+  { EGPRS_PKT_CHAN_REQ_ONE_PHASE_UNACK, "One Phase Access (RLC unack 
mode)" },
+  { EGPRS_PKT_CHAN_REQ_DEDICATED_CHANNEL,   "Dedicated Channel Request" },
+  { EGPRS_PKT_CHAN_REQ_EMERGENCY_CALL,  "Emergency call" },
+  { EGPRS_PKT_CHAN_REQ_TWO_PHASE_IPA,   "Two Phase Access (by IPA 
capable MS)" },
+  { EGPRS_PKT_CHAN_REQ_SIGNALLING_IPA,  "Signalling (by IPA capable 
MS)" },
+  { 0, NULL }
+};
+
+int decode_egprs_pkt_ch_req(guint16 

Change in osmo-pcu[master]: RLC/MAC: implement decoding of EGPRS Packet Channel Request

2020-05-25 Thread pespin
pespin has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-pcu/+/17718 )

Change subject: RLC/MAC: implement decoding of EGPRS Packet Channel Request
..


Patch Set 5: Code-Review+2


--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/17718
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I96df3352856933c9140177b2801a2c71f4134183
Gerrit-Change-Number: 17718
Gerrit-PatchSet: 5
Gerrit-Owner: fixeria 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: laforge 
Gerrit-Reviewer: pespin 
Gerrit-Comment-Date: Mon, 25 May 2020 08:50:56 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment


Change in osmo-pcu[master]: RLC/MAC: implement decoding of EGPRS Packet Channel Request

2020-05-24 Thread laforge
laforge has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-pcu/+/17718 )

Change subject: RLC/MAC: implement decoding of EGPRS Packet Channel Request
..


Patch Set 5: Code-Review+1


--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/17718
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I96df3352856933c9140177b2801a2c71f4134183
Gerrit-Change-Number: 17718
Gerrit-PatchSet: 5
Gerrit-Owner: fixeria 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: laforge 
Gerrit-Reviewer: pespin 
Gerrit-Comment-Date: Sun, 24 May 2020 07:59:32 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment


Change in osmo-pcu[master]: RLC/MAC: implement decoding of EGPRS Packet Channel Request

2020-05-23 Thread fixeria
Hello pespin, Jenkins Builder,

I'd like you to reexamine a change. Please visit

https://gerrit.osmocom.org/c/osmo-pcu/+/17718

to look at the new patch set (#5).

Change subject: RLC/MAC: implement decoding of EGPRS Packet Channel Request
..

RLC/MAC: implement decoding of EGPRS Packet Channel Request

According to 3GPP TS 44.004, section 7.4a, two alternative RACH block
formats are specified: 8 bit (1 octet) and 11 bit. This change adds
CSN.1 definitions for 11 bit EGPRS Packet Channel Request as per
3GPP TS 44.060, table 11.2.5a.2.

Change-Id: I96df3352856933c9140177b2801a2c71f4134183
Related: OS#1548
---
M src/gsm_rlcmac.c
M src/gsm_rlcmac.h
M tests/rlcmac/RLCMACTest.cpp
M tests/rlcmac/RLCMACTest.err
M tests/rlcmac/RLCMACTest.ok
5 files changed, 222 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/18/17718/5
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/17718
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I96df3352856933c9140177b2801a2c71f4134183
Gerrit-Change-Number: 17718
Gerrit-PatchSet: 5
Gerrit-Owner: fixeria 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: pespin 
Gerrit-MessageType: newpatchset


Change in osmo-pcu[master]: RLC/MAC: implement decoding of EGPRS Packet Channel Request

2020-05-23 Thread fixeria
fixeria has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-pcu/+/17718 )

Change subject: RLC/MAC: implement decoding of EGPRS Packet Channel Request
..


Patch Set 4:

This change is ready for review.


--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/17718
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I96df3352856933c9140177b2801a2c71f4134183
Gerrit-Change-Number: 17718
Gerrit-PatchSet: 4
Gerrit-Owner: fixeria 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: pespin 
Gerrit-Comment-Date: Sat, 23 May 2020 12:37:05 +
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment


Change in osmo-pcu[master]: RLC/MAC: implement decoding of EGPRS Packet Channel Request

2020-05-21 Thread pespin
pespin has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-pcu/+/17718 )

Change subject: RLC/MAC: implement decoding of EGPRS Packet Channel Request
..


Patch Set 3: Code-Review+1


--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/17718
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I96df3352856933c9140177b2801a2c71f4134183
Gerrit-Change-Number: 17718
Gerrit-PatchSet: 3
Gerrit-Owner: fixeria 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: pespin 
Gerrit-Comment-Date: Thu, 21 May 2020 17:25:54 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment


Change in osmo-pcu[master]: RLC/MAC: implement decoding of EGPRS Packet Channel Request

2020-05-21 Thread fixeria
fixeria has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-pcu/+/17718 )

Change subject: RLC/MAC: implement decoding of EGPRS Packet Channel Request
..


Patch Set 3:

(1 comment)

https://gerrit.osmocom.org/c/osmo-pcu/+/17718/2/src/gsm_rlcmac.h
File src/gsm_rlcmac.h:

https://gerrit.osmocom.org/c/osmo-pcu/+/17718/2/src/gsm_rlcmac.h@5347
PS2, Line 5347: extern struct value_string egprs_pkt_ch_req_type_names[];
> Why isn't getting to wireshark?

Because EGPRS_PacketChannelRequestType_t is not actually needed in Wireshark...



--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/17718
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I96df3352856933c9140177b2801a2c71f4134183
Gerrit-Change-Number: 17718
Gerrit-PatchSet: 3
Gerrit-Owner: fixeria 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: pespin 
Gerrit-Comment-Date: Thu, 21 May 2020 16:43:19 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin 
Comment-In-Reply-To: fixeria 
Gerrit-MessageType: comment


Change in osmo-pcu[master]: RLC/MAC: implement decoding of EGPRS Packet Channel Request

2020-05-21 Thread pespin
pespin has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-pcu/+/17718 )

Change subject: RLC/MAC: implement decoding of EGPRS Packet Channel Request
..


Patch Set 3:

(1 comment)

https://gerrit.osmocom.org/c/osmo-pcu/+/17718/2/src/gsm_rlcmac.h
File src/gsm_rlcmac.h:

https://gerrit.osmocom.org/c/osmo-pcu/+/17718/2/src/gsm_rlcmac.h@5347
PS2, Line 5347: extern struct value_string egprs_pkt_ch_req_type_names[];
> Then we would also need to move EGPRS_PacketChannelRequestType_t, because 
> it's not going to make to  […]
Why isn't getting to wireshark? So nothoing of these new additions is going 
there? If everything's specific for us then it's fine keeping it as it is here 
at the end.



--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/17718
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I96df3352856933c9140177b2801a2c71f4134183
Gerrit-Change-Number: 17718
Gerrit-PatchSet: 3
Gerrit-Owner: fixeria 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: pespin 
Gerrit-Comment-Date: Thu, 21 May 2020 16:11:13 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria 
Comment-In-Reply-To: pespin 
Gerrit-MessageType: comment


Change in osmo-pcu[master]: RLC/MAC: implement decoding of EGPRS Packet Channel Request

2020-05-21 Thread fixeria
fixeria has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-pcu/+/17718 )

Change subject: RLC/MAC: implement decoding of EGPRS Packet Channel Request
..


Patch Set 3:

(1 comment)

https://gerrit.osmocom.org/c/osmo-pcu/+/17718/2/src/gsm_rlcmac.h
File src/gsm_rlcmac.h:

https://gerrit.osmocom.org/c/osmo-pcu/+/17718/2/src/gsm_rlcmac.h@5347
PS2, Line 5347: extern struct value_string egprs_pkt_ch_req_type_names[];
> Further below, were all our encode/decode specific function are :)
Then we would also need to move EGPRS_PacketChannelRequestType_t, because it's 
not going to make to Wireshark. That would break everything, because this enum 
is needed by EGPRS_PacketChannelRequest_t. I believe git is smart enough to 
highlight this little piece of code properly in the diff, isn't it?



--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/17718
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I96df3352856933c9140177b2801a2c71f4134183
Gerrit-Change-Number: 17718
Gerrit-PatchSet: 3
Gerrit-Owner: fixeria 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: pespin 
Gerrit-Comment-Date: Thu, 21 May 2020 16:06:55 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin 
Comment-In-Reply-To: fixeria 
Gerrit-MessageType: comment


Change in osmo-pcu[master]: RLC/MAC: implement decoding of EGPRS Packet Channel Request

2020-05-21 Thread pespin
pespin has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-pcu/+/17718 )

Change subject: RLC/MAC: implement decoding of EGPRS Packet Channel Request
..


Patch Set 3:

(1 comment)

https://gerrit.osmocom.org/c/osmo-pcu/+/17718/2/src/gsm_rlcmac.h
File src/gsm_rlcmac.h:

https://gerrit.osmocom.org/c/osmo-pcu/+/17718/2/src/gsm_rlcmac.h@5347
PS2, Line 5347: extern struct value_string egprs_pkt_ch_req_type_names[];
> It's not even in Wireshark yet... […]
Further below, were all our encode/decode specific function are :)



--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/17718
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I96df3352856933c9140177b2801a2c71f4134183
Gerrit-Change-Number: 17718
Gerrit-PatchSet: 3
Gerrit-Owner: fixeria 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: pespin 
Gerrit-Comment-Date: Thu, 21 May 2020 15:55:17 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria 
Comment-In-Reply-To: pespin 
Gerrit-MessageType: comment


Change in osmo-pcu[master]: RLC/MAC: implement decoding of EGPRS Packet Channel Request

2020-05-21 Thread fixeria
fixeria has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-pcu/+/17718 )

Change subject: RLC/MAC: implement decoding of EGPRS Packet Channel Request
..


Patch Set 3:

(1 comment)

https://gerrit.osmocom.org/c/osmo-pcu/+/17718/2/src/gsm_rlcmac.h
File src/gsm_rlcmac.h:

https://gerrit.osmocom.org/c/osmo-pcu/+/17718/2/src/gsm_rlcmac.h@5347
PS2, Line 5347: extern struct value_string egprs_pkt_ch_req_type_names[];
> Please avoid mixing our stuff from stuff shared with wireshark, so it's 
> easier to spot differences o […]
It's not even in Wireshark yet... Where am I supposed to place this declaration?



--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/17718
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I96df3352856933c9140177b2801a2c71f4134183
Gerrit-Change-Number: 17718
Gerrit-PatchSet: 3
Gerrit-Owner: fixeria 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: pespin 
Gerrit-Comment-Date: Thu, 21 May 2020 15:33:48 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin 
Gerrit-MessageType: comment


Change in osmo-pcu[master]: RLC/MAC: implement decoding of EGPRS Packet Channel Request

2020-05-21 Thread fixeria
Hello pespin, Jenkins Builder,

I'd like you to reexamine a change. Please visit

https://gerrit.osmocom.org/c/osmo-pcu/+/17718

to look at the new patch set (#3).

Change subject: RLC/MAC: implement decoding of EGPRS Packet Channel Request
..

RLC/MAC: implement decoding of EGPRS Packet Channel Request

According to 3GPP TS 44.004, section 7.4a, two alternative RACH block
formats are specified: 8 bit (1 octet) and 11 bit. This change adds
CSN.1 definitions for 11 bit EGPRS Packet Channel Request as per
3GPP TS 44.060, table 11.2.5a.2.

Change-Id: I96df3352856933c9140177b2801a2c71f4134183
Related: OS#1548
---
M src/gsm_rlcmac.c
M src/gsm_rlcmac.h
M tests/rlcmac/RLCMACTest.cpp
M tests/rlcmac/RLCMACTest.err
M tests/rlcmac/RLCMACTest.ok
5 files changed, 222 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/18/17718/3
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/17718
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I96df3352856933c9140177b2801a2c71f4134183
Gerrit-Change-Number: 17718
Gerrit-PatchSet: 3
Gerrit-Owner: fixeria 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: pespin 
Gerrit-MessageType: newpatchset


Change in osmo-pcu[master]: RLC/MAC: implement decoding of EGPRS Packet Channel Request

2020-05-21 Thread pespin
pespin has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-pcu/+/17718 )

Change subject: RLC/MAC: implement decoding of EGPRS Packet Channel Request
..


Patch Set 2: Code-Review+1

(1 comment)

https://gerrit.osmocom.org/c/osmo-pcu/+/17718/2/src/gsm_rlcmac.h
File src/gsm_rlcmac.h:

https://gerrit.osmocom.org/c/osmo-pcu/+/17718/2/src/gsm_rlcmac.h@5347
PS2, Line 5347: extern struct value_string egprs_pkt_ch_req_type_names[];
Please avoid mixing our stuff from stuff shared with wireshark, so it's easier 
to spot differences over time.



--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/17718
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I96df3352856933c9140177b2801a2c71f4134183
Gerrit-Change-Number: 17718
Gerrit-PatchSet: 2
Gerrit-Owner: fixeria 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: pespin 
Gerrit-Comment-Date: Thu, 21 May 2020 15:28:48 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment


Change in osmo-pcu[master]: RLC/MAC: implement decoding of EGPRS Packet Channel Request

2020-05-21 Thread fixeria
fixeria has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-pcu/+/17718 )

Change subject: RLC/MAC: implement decoding of EGPRS Packet Channel Request
..


Patch Set 2:

This change is ready for review.


--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/17718
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I96df3352856933c9140177b2801a2c71f4134183
Gerrit-Change-Number: 17718
Gerrit-PatchSet: 2
Gerrit-Owner: fixeria 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: pespin 
Gerrit-Comment-Date: Thu, 21 May 2020 15:10:34 +
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment


Change in osmo-pcu[master]: RLC/MAC: implement decoding of [EGPRS] Packet Channel Request

2020-04-04 Thread fixeria
fixeria has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-pcu/+/17718 )

Change subject: RLC/MAC: implement decoding of [EGPRS] Packet Channel Request
..


Patch Set 1:

(5 comments)

> Are you planning to submit similar to wireshark?

Yes, as soon as I find time for that...

https://gerrit.osmocom.org/c/osmo-pcu/+/17718/1/src/gsm_rlcmac.c
File src/gsm_rlcmac.c:

https://gerrit.osmocom.org/c/osmo-pcu/+/17718/1/src/gsm_rlcmac.c@5337
PS1, Line 5337:   M_CHOICE   (PacketChannelRequest_11b_t, Type,
> I think you need to first parse Type, or are you doing it somewhere else? See 
> for instance: […]
Actually M_CHOICE does parsing for us, check out 'case CSN_CHOICE' in 
csnStreamDecoder(). The definition of IA_EGPRS_t looks wrong to me. None of the 
other definitions is using M_CHOICE like that.


https://gerrit.osmocom.org/c/osmo-pcu/+/17718/1/src/gsm_rlcmac.c@6319
PS1, Line 6319:   struct bitvec bv = {
> If we have an API to allocate better use that, in order not to assume 
> implementation details.
Lifetime of this structure is limited by the execution scope of this function, 
so I don't think we really need to allocate it on heap just because we have an 
API for that. Moreover, this is not the only place where a bitvector is 
allocated on stack, there is at least one such allocation in libosmocore and 
one more in osmo-pcu.


https://gerrit.osmocom.org/c/osmo-pcu/+/17718/1/src/gsm_rlcmac.c@6325
PS1, Line 6325:   switch (req->type) {
> In decode functions afaik the data structure is memzeroes by default. […]
You mean other decoding functions in this file or csnStreamDecoder()? The later 
does not zero-initialize the structure, and that's nice because we can catch 
bugs with AddressSanitizer or Valgrind. As you probably know, I am against 
using zero-initialization without the real need for that.


https://gerrit.osmocom.org/c/osmo-pcu/+/17718/1/src/gsm_rlcmac.c@6333
PS1, Line 6333: descr = CSNDESCR(EGPRS_PacketChannelRequest_t);
> Better have one function for each initial csnStreamInit like we do with 
> others.
Why is it better? What would we benefit?


https://gerrit.osmocom.org/c/osmo-pcu/+/17718/1/tests/rlcmac/RLCMACTest.cpp
File tests/rlcmac/RLCMACTest.cpp:

https://gerrit.osmocom.org/c/osmo-pcu/+/17718/1/tests/rlcmac/RLCMACTest.cpp@481
PS1, Line 481:  static const uint16_t GPRSPktCh11bReqs[] = {
> Did you think about implementing encoding and using the struct and pass it to 
> the encoder then to th […]
Since we're on the BTS/PCU side, we never need to encode RACH blocks on 
practice. So I think it's worthless to add encoding API just for a unit test. I 
am happy with decoding results.



--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/17718
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I96df3352856933c9140177b2801a2c71f4134183
Gerrit-Change-Number: 17718
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: pespin 
Gerrit-Comment-Date: Sat, 04 Apr 2020 14:12:07 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin 
Gerrit-MessageType: comment


Change in osmo-pcu[master]: RLC/MAC: implement decoding of [EGPRS] Packet Channel Request

2020-04-04 Thread pespin
pespin has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-pcu/+/17718 )

Change subject: RLC/MAC: implement decoding of [EGPRS] Packet Channel Request
..


Patch Set 1:

(5 comments)

Are you planning to submit similar to wireshark?

https://gerrit.osmocom.org/c/osmo-pcu/+/17718/1/src/gsm_rlcmac.c
File src/gsm_rlcmac.c:

https://gerrit.osmocom.org/c/osmo-pcu/+/17718/1/src/gsm_rlcmac.c@5337
PS1, Line 5337:   M_CHOICE   (PacketChannelRequest_11b_t, Type,
I think you need to first parse Type, or are you doing it somewhere else? See 
for instance:
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)


https://gerrit.osmocom.org/c/osmo-pcu/+/17718/1/src/gsm_rlcmac.c@6319
PS1, Line 6319:   struct bitvec bv = {
If we have an API to allocate better use that, in order not to assume 
implementation details.


https://gerrit.osmocom.org/c/osmo-pcu/+/17718/1/src/gsm_rlcmac.c@6325
PS1, Line 6325:   switch (req->type) {
In decode functions afaik the data structure is memzeroes by default. I'd 
rather pass this type as a differnet param if needed, and assign it here.


https://gerrit.osmocom.org/c/osmo-pcu/+/17718/1/src/gsm_rlcmac.c@6333
PS1, Line 6333: descr = CSNDESCR(EGPRS_PacketChannelRequest_t);
Better have one function for each initial csnStreamInit like we do with others.


https://gerrit.osmocom.org/c/osmo-pcu/+/17718/1/tests/rlcmac/RLCMACTest.cpp 
File tests/rlcmac/RLCMACTest.cpp:

https://gerrit.osmocom.org/c/osmo-pcu/+/17718/1/tests/rlcmac/RLCMACTest.cpp@481
PS1, Line 481:  static const uint16_t GPRSPktCh11bReqs[] = {
Did you think about implementing encoding and using the struct and pass it to 
the encoder then to the decoder?



--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/17718
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I96df3352856933c9140177b2801a2c71f4134183
Gerrit-Change-Number: 17718
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: pespin 
Gerrit-Comment-Date: Sat, 04 Apr 2020 12:46:09 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment


Change in osmo-pcu[master]: RLC/MAC: implement decoding of [EGPRS] Packet Channel Request

2020-04-03 Thread fixeria
fixeria has uploaded this change for review. ( 
https://gerrit.osmocom.org/c/osmo-pcu/+/17718 )


Change subject: RLC/MAC: implement decoding of [EGPRS] Packet Channel Request
..

RLC/MAC: implement decoding of [EGPRS] Packet Channel Request

According to 3GPP TS 44.004, section 7.4a, two alternative RACH
block formats are specified: 8 bit (1 octet) and 11 bit. This
change implements decoding of the following messages:

  - Packet Channel Request, 11 bit block format (table 11.2.5.1),
  - Packet Channel Request, 8 bit block format (table 11.2.5.2), and
  - EGPRS Packet Channel Request, 11 bit block format (table 11.2.5a.2)

as specified in 3GPP TS 44.060.

Change-Id: I96df3352856933c9140177b2801a2c71f4134183
Related: OS#1548
---
M src/gsm_rlcmac.c
M src/gsm_rlcmac.h
M tests/rlcmac/RLCMACTest.cpp
M tests/rlcmac/RLCMACTest.err
M tests/rlcmac/RLCMACTest.ok
5 files changed, 475 insertions(+), 0 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/18/17718/1

diff --git a/src/gsm_rlcmac.c b/src/gsm_rlcmac.c
index 9b5ecfe..2caf4f0 100644
--- a/src/gsm_rlcmac.c
+++ b/src/gsm_rlcmac.c
@@ -5249,6 +5249,159 @@
 CSN_DESCR_END(SI6_RestOctet_t)
 #endif

+/* Common (unified) fields for [EGPRS] Packet Channel Request
+ * (see 3GPP TS 44.060, tables 11.2.5.1, 11.2.5.2, and 11.2.5a.2) */
+static const
+CSN_DESCR_BEGIN(PacketChannelRequest_MC5P2RB3_t)
+  M_UINT   (PacketChannelRequest_MC5P2RB3_t,  MultislotClass,  5),
+  M_UINT   (PacketChannelRequest_MC5P2RB3_t,  Priority,  2),
+  M_UINT   (PacketChannelRequest_MC5P2RB3_t,  RandomBits,  3),
+CSN_DESCR_END  (PacketChannelRequest_MC5P2RB3_t)
+
+static const
+CSN_DESCR_BEGIN(PacketChannelRequest_MC5RB2_t)
+  M_UINT   (PacketChannelRequest_MC5RB2_t,  MultislotClass,  5),
+  M_UINT   (PacketChannelRequest_MC5RB2_t,  RandomBits,  2),
+CSN_DESCR_END  (PacketChannelRequest_MC5RB2_t)
+
+static const
+CSN_DESCR_BEGIN(PacketChannelRequest_MCG3P2RB3_t)
+  M_UINT   (PacketChannelRequest_MCG3P2RB3_t,  MultislotClassGroup,  3),
+  M_UINT   (PacketChannelRequest_MCG3P2RB3_t,  Priority,  2),
+  M_UINT   (PacketChannelRequest_MCG3P2RB3_t,  RandomBits,  3),
+CSN_DESCR_END  (PacketChannelRequest_MCG3P2RB3_t)
+
+static const
+CSN_DESCR_BEGIN(PacketChannelRequest_NOB3P2RB3_t)
+  M_UINT   (PacketChannelRequest_NOB3P2RB3_t,  NumberOfBlocks,  3),
+  M_UINT   (PacketChannelRequest_NOB3P2RB3_t,  Priority,  2),
+  M_UINT   (PacketChannelRequest_NOB3P2RB3_t,  RandomBits,  3),
+CSN_DESCR_END  (PacketChannelRequest_NOB3P2RB3_t)
+
+static const
+CSN_DESCR_BEGIN(PacketChannelRequest_NOB3RB3_t)
+  M_UINT   (PacketChannelRequest_NOB3RB3_t,  NumberOfBlocks,  3),
+  M_UINT   (PacketChannelRequest_NOB3RB3_t,  RandomBits,  3),
+CSN_DESCR_END  (PacketChannelRequest_NOB3RB3_t)
+
+static const
+CSN_DESCR_BEGIN(PacketChannelRequest_P2RB3_t)
+  M_UINT   (PacketChannelRequest_P2RB3_t,  Priority,  2),
+  M_UINT   (PacketChannelRequest_P2RB3_t,  RandomBits,  3),
+CSN_DESCR_END  (PacketChannelRequest_P2RB3_t)
+
+static const
+CSN_DESCR_BEGIN(PacketChannelRequest_RB5_t)
+  M_UINT   (PacketChannelRequest_RB5_t,  RandomBits,  5),
+CSN_DESCR_END  (PacketChannelRequest_RB5_t)
+
+static const
+CSN_DESCR_BEGIN(PacketChannelRequest_RB3_t)
+  M_UINT   (PacketChannelRequest_RB3_t,  RandomBits,  3),
+CSN_DESCR_END  (PacketChannelRequest_RB3_t)
+
+static const
+CSN_DESCR_BEGIN(PacketChannelRequest_RB2_t)
+  M_UINT   (PacketChannelRequest_RB2_t,  RandomBits,  2),
+CSN_DESCR_END  (PacketChannelRequest_RB2_t)
+
+/* Packet Channel Request (11 bit block, see table 11.2.5.1) */
+static const
+CSN_ChoiceElement_t PacketChannelRequest_11b_Choice[] =
+{
+  {1, 0x00, 0, M_TYPE(PacketChannelRequest_11b_t, u.OnePhase,
+  PacketChannelRequest_MC5P2RB3_t)},
+  {3, 0x04, 0, M_TYPE(PacketChannelRequest_11b_t, u.Short,
+  PacketChannelRequest_NOB3P2RB3_t)},
+  {6, 0x30, 0, M_TYPE(PacketChannelRequest_11b_t, u.TwoPhase,
+  PacketChannelRequest_P2RB3_t)},
+  {6, 0x31, 0, M_TYPE(PacketChannelRequest_11b_t, u.PageResponse,
+  PacketChannelRequest_RB5_t)},
+  {6, 0x32, 0, M_TYPE(PacketChannelRequest_11b_t, u.CellUpdate,
+  PacketChannelRequest_RB5_t)},
+  {6, 0x33, 0, M_TYPE(PacketChannelRequest_11b_t, u.MMProcedure,
+  PacketChannelRequest_RB5_t)},
+  {6, 0x34, 0, M_TYPE(PacketChannelRequest_11b_t, u.SingleBlockNoTBF,
+  PacketChannelRequest_RB5_t)},
+  {6, 0x35, 0, M_TYPE(PacketChannelRequest_11b_t, u.OnePhaseUnack,
+  PacketChannelRequest_RB5_t)},
+  {6, 0x36, 0, M_TYPE(PacketChannelRequest_11b_t, u.DedicatedChannel,
+  PacketChannelRequest_RB5_t)},
+  {6, 0x37, 0, M_TYPE(PacketChannelRequest_11b_t, u.EmergencyCall,
+  PacketChannelRequest_RB5_t)},
+  {6, 0x38, 0,