Change in osmo-bsc[master]: bsc_vty: add features to disable specific lchans via vty

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 Hofmeyr gerrit-no-reply at lists.osmocom.org
Tue Jan 22 12:53:27 UTC 2019


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

Change subject: bsc_vty: add features to disable specific lchans via vty
......................................................................


Patch Set 5: Code-Review-1

(1 comment)

Thanks for splitting away the other changes!
Now for the "disabled" feature...

https://gerrit.osmocom.org/#/c/12624/5/include/osmocom/bsc/gsm_data.h
File include/osmocom/bsc/gsm_data.h:

https://gerrit.osmocom.org/#/c/12624/5/include/osmocom/bsc/gsm_data.h@619
PS5, Line 619: 	bool disabled;
> Messing with the FSM is what I am trying to avoid.

ok, I understand. It makes sense from the distance, but in my point of view the lchan FSM is designed
to completely wipe its state when it goes to UNUSED. So as soon as we go back out of the
broken state, things will be perfectly the same as before.

Also in the ttcn3 tests, IIUC, the RSL and OML re-connect for each test case, so the FSM instances
will anyway be completely blown away and re-initialized between test cases.

I am asking myself whether I am bikeshedding on this; I really don't want to generate more work for you
than necessary ... <think, think> ... no, it is better to avoid introducing a new flag:
this flag may have to be checked in numerous conditions all around the code base.
Where ever we check the lchan state, we may have to add a side check for the "disabled" flag.
We should keep the lchan state as the single place to indicate lchan use and usability.

I made this kind of mistake with introducing dynamic timeslots: I wanted to avoid messing with the
ts->pchan, so instead of re-using the ts->pchan as the single place to indicate the current ts type,
I added more flags next to it, to be checked in case of a dynamic ts.
That rippled all over the bts and bsc code base and a lot of bugs had to be fixed in a lot of
if-statements that I had overlooked in the first patches, even months and years later.

To implement changing the lchan FSM to the broken state:

- we may have to add a transition from the broken state back to "unused",
  or you may decide to instead completely re-allocate the lchan fsm instance.
- The vty command should probably only change to broken if the lchan is currently UNUSED.
  (at least for now, to avoid complexity.)
- Besides adding a vty command to place lchans *in* the broken state (for testing),
  we also allow *removing* the broken state, which might even be useful for manual recovery
  in tight situations like rhizomatica had back in the days.
  So I would argue to keep that vty command public.

Let me know if I should help with that, since designing that stuff is still fresh in my mind
I should be comparatively quick with implementing. (though I should probably stick with inter-MSC)



-- 
To view, visit https://gerrit.osmocom.org/12624
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: I397e68e26d6a1727890353fa34f4897b54795866
Gerrit-Change-Number: 12624
Gerrit-PatchSet: 5
Gerrit-Owner: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: dexter <pmaier at sysmocom.de>
Gerrit-CC: Harald Welte <laforge at gnumonks.org>
Gerrit-CC: Vadim Yanitskiy <axilirator at gmail.com>
Gerrit-Comment-Date: Tue, 22 Jan 2019 12:53:27 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190122/63194051/attachment.htm>


More information about the gerrit-log mailing list