Attention is currently required from: neels, fixeria, dexter.
laforge has posted comments on this change. (
https://gerrit.osmocom.org/c/osmo-bsc/+/31497 )
Change subject: pcu_sock: activate/deactivate PDCH on pcu reconnect
......................................................................
Patch Set 5:
(3 comments)
File src/osmo-bsc/pcu_sock.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/31497/comment/1565225a_6f6796c5
PS2, Line 801:
printf("l1sap_chan_rel(trx,gsm_lchan2chan_nr(ts->lchan));\n");
either keep this printf, or if it should be dropped,
drop it in a separate patch?
Ack
https://gerrit.osmocom.org/c/osmo-bsc/+/31497/comment/968de04c_416d056b
PS2, Line 801: && ts->pchan_on_init == GSM_PCHAN_OSMO_DYN
Why does this apply only to dynamic timeslots? I think
I understand why, but I'd like to see a code […]
Ericsson only has
fully-dynamic timeslots, AFAIR. But yes, mentioning that here might make sense for the
most common reader of this code who has no clue about ericsson specifics.
https://gerrit.osmocom.org/c/osmo-bsc/+/31497/comment/880ebec0_1f12759b
PS2, Line 941: bts = llist_entry(state->net->bts_list.next, struct gsm_bts, list);
rather use llist_first_entry_or_null() […]
I
actually think it's a serious problem that only one BTS appears to be suppored here.
Not only that, but there's an implicit assumption that the first BTS is the Ericsson
BTS served by the PCU?
In general, a BSC-colocated PCU can of course serve any number of BTSs. Until we address
that general problem/shortcoming in our current code, we shouldn't make a blind
assumption, but we should make sure that the user can operate this only in a safe
configuration. In the worst case, we could check if thre are more than one (ericsson) BTS
configured in the BSC, and refuse to operate the PCU socket with an associated error
message.
--
To view, visit
https://gerrit.osmocom.org/c/osmo-bsc/+/31497
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I9ea0c53a5e68a51c781ef43bae71f947cdb95678
Gerrit-Change-Number: 31497
Gerrit-PatchSet: 5
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 28 Feb 2023 18:05:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: comment