Change in osmo-bsc[master]: assignment_fsm: fix channel allocator preferences

historical

Neels Hofmeyr has posted comments on this change. ( https://gerrit.osmocom.org/12625 )

Change subject: assignment_fsm: fix channel allocator preferences
......................................................................

Patch Set 2: Code-Review-1

(2 comments)

(let me first post this and follow up with review on the remaining patch later)

https://gerrit.osmocom.org/#/c/12625/2/include/osmocom/bsc/gsm_data.h
File include/osmocom/bsc/gsm_data.h:

https://gerrit.osmocom.org/#/c/12625/2/include/osmocom/bsc/gsm_data.h@121
PS2, Line 121: 	struct channel_mode_and_rate ch_mode_rate_alt;
Are there really only two possible mode,rate combinations?

https://gerrit.osmocom.org/#/c/12625/2/src/osmo-bsc/abis_rsl.c
File src/osmo-bsc/abis_rsl.c:

https://gerrit.osmocom.org/#/c/12625/2/src/osmo-bsc/abis_rsl.c@1391
PS2, Line 1391: 	if (!lchan) {
These changes above make up a separate fix by themselves. I personally understand this change because I explained the mistake I made here to you, but the reason definitely should appear in the commit log.

Let's put this in a separate patch and explain like this:

Fix TCH-as-SDCCH allocation on Channel Request

On rsl_rx_chan_rqd(), so far osmo-bsc tried to preferably assign the lchan type
that was asked for in the RACH. Firstly, this contained a bug, and secondly,
it does not make sense to heed that preference, since we do late assignment.

Ignore the preference for the MS' TCH kind.
We do late assignment to avoid codec mismatches. In the "old days", we would
heed the MS' TCH channel kind, even if the MSC or BSC didn't actually allow
or prefer that channel kind. Hence, in the presence of both TCH/F and TCH/H,
the MS could ask for TCH/F (which we would grant on the MO side) and the BSC
or MSC could prefer TCH/H (which we would apply on the MT side), and hence
fabricate a codec mismatch. Instead, since quite some time now, we *always*
assign an SDCCH first, and only later on do another Assignment to hand out
a proper voice lchan that heeds the MS capability bits as well as MSC's and
BSC's preferences.

By completely ignoring the channel kind the MS asked for in the RACH, we
also eliminate this bug in rsl_rx_chan_rqd():
- If the first "lchan_select_by_type(GSM_LCHAN_SDDCH)" fails (all SDDCH in use),
  we should try to fall back to any TCH instead, to serve as SDCCH.
- the first "if (!lchan && lctype != GSM_LCHAN_SDCCH)" was an attempt to prefer
  a fallback to the lchan type the MS requested.
- the remaining two "if (!lchan && lctype == GSM_LCHAN_SDCCH)" were obviously
  only applied if the MS asked for an SDCCH, and skipped if the type was TCH/*.
- hence, missing was: if the MS asked for a TCH, to also try the *other* TCH
  kind that the MS did not ask for. (Example: all SDCCH in use, MS asks for
  TCH/F, but BSC has only TCH/H lchans; we should assign TCH/H as SDCCH, instead
  we said "no resources")

-- 
To view, visit https://gerrit.osmocom.org/12625
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5239e05c1cfbcb8af28f43373a58fa6c2d216c51
Gerrit-Change-Number: 12625
Gerrit-PatchSet: 2
Gerrit-Owner: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-CC: Vadim Yanitskiy <axilirator at gmail.com>
Gerrit-Comment-Date: Mon, 21 Jan 2019 13:52:44 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190121/07ed56b4/attachment.htm>