Change in osmo-ttcn3-hacks[master]: pcu: Introduce test TC_t3172_*

[fixeria]

fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/26256 )

Change subject: pcu: Introduce test TC_t3172_*
......................................................................


Patch Set 1: Code-Review-1

(9 comments)

https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/26256/1/library/RLCMAC_CSN1_Templates.ttcn 
File library/RLCMAC_CSN1_Templates.ttcn:

https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/26256/1/library/RLCMAC_CSN1_Templates.ttcn@23 
PS1, Line 23: tr_RlcMacDlCtrl_PKT_ACC_REJ_ID_TLLI
Not critical, but having 'RlcMacDlCtrl' in the name of this template looks a bit confusing to me, because it may look like a template of 'RlcmacDlCtrlMsg'. I would expect something like 'tr_PacketAccessRejectStruct_TLLI' or so.


https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/26256/1/library/RLCMAC_CSN1_Templates.ttcn@102 
PS1, Line 102: ;
Why are you adding this semicolon here (in this patch I mean)?


https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/26256/1/library/RLCMAC_CSN1_Types.ttcn 
File library/RLCMAC_CSN1_Types.ttcn:

https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/26256/1/library/RLCMAC_CSN1_Types.ttcn@756 
PS1, Line 756: PacketAccessRejectStruct
I had a quick look into the specs: the '< Reject struct >' actually contains an ID *and* the optional '< WAIT_INDICATION >', while in your definition only an ID is present. So I propose to move the '< WAIT_INDICATION >' stuff here. After that, we can easily implement the recursion '{ 1 < Reject struct > ** 0 }'.


https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/26256/1/library/RLCMAC_CSN1_Types.ttcn@764 
PS1, Line 764: 		uint8_t			wait_ind,
Missing 'optional' keyword for both 'wait_ind' and 'wait_ind_size'.


https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/26256/1/pcu/PCU_Tests.ttcn 
File pcu/PCU_Tests.ttcn:

https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/26256/1/pcu/PCU_Tests.ttcn@2073 
PS1, Line 2073: 	var EGPRSPktChRequest req := {
Is there a reason why you're specifically using EGPRS Packet Channel Request here?


https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/26256/1/pcu/PCU_Tests.ttcn@2082 
PS1, Line 2082: 	/* We send 7 requests, the IUT gives us all available USFs (0..6) */
You could simply tell f_init_gprs_ms() to allocate 7 GprsMS instances, and then just call f_multi_ms_establish_tbf(). This would make the test case shorter and easier to follow/read. I mean, you were the one introducing multi-GprsMS capable API, but somehow you're not using it :P


https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/26256/1/pcu/PCU_Tests.ttcn@2086 
PS1, Line 2086: 	}
So if we send another RACH.ind here, the IUT would send the reject over the BCCH/AGCH in form of Immediate Assignment Reject. This message does contain the Wait Indication too, however as per 3GPP TS 44.018 section 9.1.20.3 it reflects the value of T3142, not T3172. And while writing this comment, I found out that we already test it implicitly in TC_egprs_pkt_chan_req_reject_exhaustion. Good.


https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/26256/1/pcu/PCU_Tests.ttcn@2101 
PS1, Line 2101: 	/* Since all USF are taken, we should receive a Reject: */
Took me a while to understand what exactly causes the Reject. I looked above, read the comment 'ACK the DL block', and was like: what? How could an ACK/NACK message trigger the resource allocation? Then I noticed that it actually contains an optional ChannelReqDescription! I didn't know it was possible to request an Uplink resource allocation using the ACK/NACK messages. Thanks for giving me a chance to learn something new, really.

Now I am wondering if it would have been simpler to use one of the allocated Uplink TBFs to request additional resources instead of using the ACK/NACK message? Just wondering, not saying that it must be done this way.


https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/26256/1/pcu/PCU_Tests.ttcn@2127 
PS1, Line 2127: De
Due



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

Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: I3f4368c99b00453b471c3d741fecb8864ecdc628
Gerrit-Change-Number: 26256
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy at sysmocom.de>
Gerrit-Comment-Date: Tue, 16 Nov 2021 00:32:33 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20211116/6bb40b10/attachment.htm>