Change in osmo-bsc[master]: abis_rsl: prioritize emergency calls over regular calls

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

neels gerrit-no-reply at lists.osmocom.org
Mon Aug 24 11:55:11 UTC 2020


neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/19793 )

Change subject: abis_rsl: prioritize emergency calls over regular calls
......................................................................


Patch Set 1: Code-Review-1

(4 comments)

https://gerrit.osmocom.org/c/osmo-bsc/+/19793/1/src/osmo-bsc/abis_rsl.c 
File src/osmo-bsc/abis_rsl.c:

https://gerrit.osmocom.org/c/osmo-bsc/+/19793/1/src/osmo-bsc/abis_rsl.c@1331 
PS1, Line 1331: 	bo
> I think all other members are self-explanatory, but this one maybe could deserver a one-line comment […]
'releasing' with 'e'


https://gerrit.osmocom.org/c/osmo-bsc/+/19793/1/src/osmo-bsc/abis_rsl.c@1402 
PS1, Line 1402: /* Find any busy TCH/H or TCH/F lchan, prefer established and old lchans */
This is not preferring "old" lchans. The order of the timeslots is no good indicator for the duration of the call, especially when a cell is continuously in use. The first used lchan can often be the most recently established, if another call ended shortly before that and released the first slot, which was then rapidly taken for the next call.

Shouldn't this also favor actually unused lchans?

Shouldn't this also favor an lchan that is currently being released, over an established one?

And, what if by coincidence all lchans are currently being established, then this should probably still return one of them and abort the establishment to service the emergency call instead?


https://gerrit.osmocom.org/c/osmo-bsc/+/19793/1/src/osmo-bsc/abis_rsl.c@1447 
PS1, Line 1447: static int handle_emergency_call(struct chan_rqd *rqd)
the logic in this function confuses me:

- the name suggests that it handles a call, but then when an lchan is available it just exits.
  (maybe it should be called: 'force_free_lchan_for_emergency'?)
- the 'relasing' case sounds like an emergency call is releasing, but still the function looks for an lchan first.
- the negative return code in the end seems to not be an error?


https://gerrit.osmocom.org/c/osmo-bsc/+/19793/1/src/osmo-bsc/lchan_select.c 
File src/osmo-bsc/lchan_select.c:

https://gerrit.osmocom.org/c/osmo-bsc/+/19793/1/src/osmo-bsc/lchan_select.c@254 
PS1, Line 254: lchan_select_avail
> naming.  We are not selecting a channel, as we are not returning it, or performing any action on it. […]
'lchan_avail_by_type()', i.e. just s/select/avail from above function name.

A more elegant arrangement would be this:

  struct gsm_lchan *lchan_avail_by_type(bts, type)
  {
       [...most of current lchan_select_by_type() function body...]
       return lchan;
  }

  struct gsm_lchan *lchan_select_by_type(bts, type)
  {
       lchan = lchan_avail_by_type(bts, type);
       if (lchan)
            lchan->type = type;
            [...]
       return lchan;
  }

i.e. no third static function is necessary, because the 'select' is just a continuation of the 'avail'



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

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: If8651265928797dbda9f528b544931dcfa4a0b36
Gerrit-Change-Number: 19793
Gerrit-PatchSet: 1
Gerrit-Owner: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels <nhofmeyr at sysmocom.de>
Gerrit-CC: laforge <laforge at osmocom.org>
Gerrit-Comment-Date: Mon, 24 Aug 2020 11:55:11 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: laforge <laforge at osmocom.org>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20200824/baedaaf9/attachment.htm>


More information about the gerrit-log mailing list