confused by osmo-bts rsl.c:bts_supports_cm()

[Neels Hofmeyr]

On Sun, Sep 09, 2018 at 08:33:44PM +0200, Keith wrote:
> posting to the list here rather than going down the line of a ticket as
> I'm using the nitb to configure my bts and so this may not be a bug, but
> rather a missing back port to legacy. (which may not be appreciated
> here, I'm aware!)

Well, it's rather the other way round: "Our" opinion is that it makes little
sense to do it, but whoever wants to spend time on osmo-nitb is welcome to do
so! "we" I guess being most of the Osmocom core dev team.

And I appreciate you finding below Chan Mode Modif bug:

> <0000> rsl.c:1503 invalid mode/codec instructed by BSC, check BSC
> configuration.
> 
>  I added some logging in osmo-bts to check what was being passed into
> this line in rsl.c:
> 
> if (bts_supports_cm(lchan->ts->trx->bts, lchan->ts->pchan,
> lchan->tch_mode) != 1)
> 
> and the values are 0 (zero), GSM_PCHAN_TCH_F_TCH_H_PDCH  and
> GSM48_CMODE_SPEECH_AMR but bts_supports_cm() only checks against
> GSM_PCHAN_TCH_F and GSM_PCHAN_TCH_H

At first glance, there should not be a difference between osmo-nitb and
osmo-bsc there.

However, looking more closely, your error message happens during an RSL Mode
Modif. This occurs if the MS already is on an lchan suitable for the voice
call, but just needs a more accurate chan mode. The point being that current
osmo-bsc never reaches that:

a) we almost never assign a TCH/F for immediate assignment even if a phone
asked for it so we usually will not encounter this situation that a phone
already has a TCH/F for early signalling to begin with;

b) the code path that would trigger the mode modif currently doesn't, and
instead we assign a new lchan (
http://git.osmocom.org/osmo-bsc/tree/src/osmo-bsc/assignment_fsm.c?id=981f8b934771460354163dd148a5ecab46dd4476#n347
).

c) Even though current osmo-bsc never invokes Chan Mode Modif, I kind of
expected osmo-bts code to also invoke the same function somewhere for the chan
act case. But it doesn't :P


osmo-nitb, however, doesn't do late assignment, so commonly reaches the Chan
Mode Modif code path.

Either way, even though osmo-bsc doesn't currently use this code path, we still
want it fixed for the future, not only for osmo-nitb.


Solution: bts_supports_cm() should never be fed with dynamic pchan kinds.  To
test my patch, I'd need to setup an osmo-nitb ... Keith, can you take over the
test whether this fixes the problem instead?

Placed it on branch neels/dyn_modif in osmo-bts; it's just:

-       if (bts_supports_cm(lchan->ts->trx->bts, lchan->ts->pchan, lchan->tch_mode) != 1) {
+       if (bts_supports_cm(lchan->ts->trx->bts, ts_pchan(lchan->ts), lchan->tch_mode) != 1) {

If it's any difficulty to build osmo-bts, I do have a compile for sysmoBTS
ready to fire up and could also test myself if you ask me back.

(My initial implementation of dyn TS made using the right pchan value a bit
hard, for hysterical raisins. First there were the TCH/F_PDCH dynamic
timeslots; the implementation I took over from jolly used bit flags to indicate
the current actual channel mode.  Then I added TCH/F_TCH/H_PDCH also with
separate state. The better thing to do from the start would have been to keep
the currently used pchan state in the central ts->pchan, and only code that
needs to know about the dynamic nature of TS should need to be aware of them.
So a code refactoring would be good to make it easier to add new code around
checking pchan types -- https://osmocom.org/issues/1902. Such a refactoring has
already happened in osmo-bsc, but that's unrelated to this issue.)

~N
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.osmocom.org/pipermail/openbsc/attachments/20180910/343b2e53/attachment.bin>