Attention is currently required from: laforge, pespin, dexter. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31618 ) Change subject: pcu_sock: handle multiple BTSs with one BSC co-located PCU (in theory) ...................................................................... Patch Set 5: (9 comments) Commit Message: https://gerrit.osmocom.org/c/osmo-bsc/+/31618/comment/e1b1b1a8_c80c6fcc PS5, Line 15: RBS) are configured. how about an osmo-bsc with one Ericsson RBS co-located, and then a bunch of other types of BTS with no BSC-co-located PCU? The position of the Ericsson RBS could be anywhere in the bts_list. The reasoning seems to be exclusively about Ericsson RBS. Patchset: PS5: pau, can you point out the place? i don't see it... File src/osmo-bsc/pcu_sock.c: https://gerrit.osmocom.org/c/osmo-bsc/+/31618/comment/1b76bb41_82bdf365 PS5, Line 810: llist_for_each_entry(bts, &state->net->bts_list, list) { could there be several RBS with several separate co-located PCUs, each with their own pcu_sock? This would then disconnect all of them if only one disconnects. I'm asking because in pcu_sock_init() it seems like one pcu_sock is assigned to a single bts bts->pcu_state = state so seems to me the relation of pcu_sock to bts should be 1:1. Here it is handled as 1:N instead. https://gerrit.osmocom.org/c/osmo-bsc/+/31618/comment/1262db71_ef50ee98 PS5, Line 960: (unusual whitespace after the type cast brace) https://gerrit.osmocom.org/c/osmo-bsc/+/31618/comment/088c4228_d0f2d5c2 PS5, Line 960: rc (would prefer if the return val were named 'fd', because 'man accept' says RETURN VALUE On success, these system calls return a file descriptor [...] and below "close(rc)" looks weird, "close(fd)" would make more sense) https://gerrit.osmocom.org/c/osmo-bsc/+/31618/comment/f1c3cc60_a4a2eba0 PS5, Line 966: if (conn_bfd->fd >= 0) { should this check happen before accept(), so we don't even accept a new connection when there already is one https://gerrit.osmocom.org/c/osmo-bsc/+/31618/comment/921a7285_9b3374b2 PS5, Line 969: osmo_fd_read_disable(&state->listen_bfd); so this disables the other active connection?? https://gerrit.osmocom.org/c/osmo-bsc/+/31618/comment/5a4cc95e_bb4afb34 PS5, Line 986: llist_for_each_entry(bts, &state->net->bts_list, list) { same question as above, pcu to bts relation is 1:1 or 1:N? I guess you should pass the bts pointer that the pcu socket belongs to in pcu_sock_state and only act on that specific bts here. https://gerrit.osmocom.org/c/osmo-bsc/+/31618/comment/bfd9e82c_2651145f PS5, Line 1032: bts->pcu_state = state; here it is pcu to bts relation 1:1? -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31618 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I0b42c2c130106f6ffca2dd08d079e1a7bda41f0b Gerrit-Change-Number: 31618 Gerrit-PatchSet: 5 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Sat, 04 Mar 2023 00:52:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment