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:
You are still preventing to configure more than 1 ericsson BTS here. […]
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?