Change in osmo-bts[master]: l1sap: add repeated downlink FACCH

[laforge]

laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/21014 )

Change subject: l1sap: add repeated downlink FACCH
......................................................................


Patch Set 1:

(3 comments)

this is a good start.  However, beyond the specific comments below, I think there are the following missing parts:

* "If an MS has not signalled a Repeated ACCH Capability bit as '1' the BSS may only use Repeated
Downlink FACCH to send command frames."  So we need to look at that "Repeated ACCH capability bit" of the MS and only consider LAPDm commands (based on C/R bit of the LAPDm header) by default as "eligible" for our "history of 2 lapdm frames queue".

* the decision on when to repeate FACCH is a dynamic one.  Enablign it globally in the VTY doesn't make much sense beyond testing.  Thre needs to be some kind of "implementatoon-based criteria" that enables/disables repeated FACCH.  I would guess a vty-configurable RxQual threshold would be a good idea.  If the reported downlink link quality (as per UL meas rep) is below that threshold, enable repeated FACCH for this logical channel.  If not, disable it.

* T200 impact.  As stated, whenever we enable the FACCH retransmission, we must increase the T200 dynamically.  My assumption is that we scale the T200 value of this given lchan by a factor of 2 while repeated FACCH is active, and then remove that scaling when repeated FACCH is disabled again.

https://gerrit.osmocom.org/c/osmo-bts/+/21014/1/include/osmo-bts/gsm_data.h 
File include/osmo-bts/gsm_data.h:

https://gerrit.osmocom.org/c/osmo-bts/+/21014/1/include/osmo-bts/gsm_data.h@261 
PS1, Line 261: /* SLOT #1 */
             : 			struct msgb *msg_1;
             : 			uint32_t fn_1;
             : 
             : 			/* SLOT #2 */
             : 			struct msgb *msg_2;
             : 			uint32_t fn_2;
why not have an array[2] of a structure?


https://gerrit.osmocom.org/c/osmo-bts/+/21014/1/src/common/l1sap.c 
File src/common/l1sap.c:

https://gerrit.osmocom.org/c/osmo-bts/+/21014/1/src/common/l1sap.c@925 
PS1, Line 925: <= fn
why <=8 ?  The spec says "M+8 or M+9 (in case of SACCH or IDLE)".  So there is exactly one frame number at which the FACCH is to be repeated.  I would hope there also is a better way to trigger than to use this 8/9 count, but to look at the logical block numbers within the 26-multiframe?


https://gerrit.osmocom.org/c/osmo-bts/+/21014/1/src/common/l1sap.c@947 
PS1, Line 947: for TCH/H only one
what do you mean by 'instances' here?  Which part in Osmo-BTS is ensuring this never happens?  If this could happen by the BSC (or whatever talks Abis to us) sends more FACCH messages in a given time interval, then we should not assert here.



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

Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I72f0cf7eaaef9f80fc35e752c90ae0e2d24d0c75
Gerrit-Change-Number: 21014
Gerrit-PatchSet: 1
Gerrit-Owner: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: laforge <laforge at osmocom.org>
Gerrit-Comment-Date: Mon, 02 Nov 2020 08:54:33 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20201102/49389f42/attachment.htm>