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/.
osmith gerrit-no-reply at lists.osmocom.orgosmith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/22309 )
Change subject: Allow multiple bts objects in PCU
......................................................................
Patch Set 4: Code-Review-1
(11 comments)
https://gerrit.osmocom.org/c/osmo-pcu/+/22309/3/src/bts.cpp
File src/bts.cpp:
https://gerrit.osmocom.org/c/osmo-pcu/+/22309/3/src/bts.cpp@1132
PS3, Line 1132: void bts_update_tbf_ta(struct gprs_rlcmac_bts *bts, const char *p, uint32_t fn, uint8_t trx_no, uint8_t ts, int8_t ta, bool is_rach)
(line too long)
https://gerrit.osmocom.org/c/osmo-pcu/+/22309/3/src/gprs_bssgp_pcu.c
File src/gprs_bssgp_pcu.c:
https://gerrit.osmocom.org/c/osmo-pcu/+/22309/3/src/gprs_bssgp_pcu.c@213
PS3, Line 213: /* FIXME: look if MS is attached a specific BTS and then only page on that one? */
Sounds like looping over the attached BTS and their MS would solve it. Is this out of scope for this patch?
(same with _ps below)
https://gerrit.osmocom.org/c/osmo-pcu/+/22309/3/src/gprs_bssgp_pcu.c@823
PS3, Line 823: bts = llist_first_entry_or_null(&the_pcu->bts_list, struct gprs_rlcmac_bts, list);
Why not add bts as parameter to gprs_bssgp_pcu_rx_ptp()?
https://gerrit.osmocom.org/c/osmo-pcu/+/22309/3/src/gprs_rlcmac.h
File src/gprs_rlcmac.h:
https://gerrit.osmocom.org/c/osmo-pcu/+/22309/3/src/gprs_rlcmac.h@116
PS3, Line 116:
(space)
https://gerrit.osmocom.org/c/osmo-pcu/+/22309/3/src/osmo-bts-litecell15/lc15_l1_if.c
File src/osmo-bts-litecell15/lc15_l1_if.c:
https://gerrit.osmocom.org/c/osmo-pcu/+/22309/3/src/osmo-bts-litecell15/lc15_l1_if.c@160
PS3, Line 160: bts = llist_first_entry_or_null(&the_pcu->bts_list, struct gprs_rlcmac_bts, list);
add bts parameter instead, or at least a FIXME comment like above?
Same with all other "bts = llist_first_entry_or_null" lines below / in other files.
Alternatively / additionally to FIXME everywhere, maybe add a note in README under "Current limitations".
https://gerrit.osmocom.org/c/osmo-pcu/+/22309/4/src/osmobts_sock.c
File src/osmobts_sock.c:
https://gerrit.osmocom.org/c/osmo-pcu/+/22309/4/src/osmobts_sock.c@64
PS4, Line 64: bool retry = !llist_empty(&the_pcu->bts_list);
I think the ! is wrong, shouldn't retry be true if the list is empty?
https://gerrit.osmocom.org/c/osmo-pcu/+/22309/4/src/osmobts_sock.c@114
PS4, Line 114: llist_for_each_entry(bts, &the_pcu->bts_list, list) {
My understanding is, that if one bts closes the socket to the pcu, the pcu will give up completely, drop all bts and exit. So this could use another FIXME.
https://gerrit.osmocom.org/c/osmo-pcu/+/22309/4/src/pcu_l1_if.h
File src/pcu_l1_if.h:
https://gerrit.osmocom.org/c/osmo-pcu/+/22309/4/src/pcu_l1_if.h@159
PS4, Line 159: struct gprs_rlcmac_bts;
its declared above already in the "ifdef __cplusplus" section. is that on purpose?
https://gerrit.osmocom.org/c/osmo-pcu/+/22309/4/src/pcu_l1_if.cpp
File src/pcu_l1_if.cpp:
https://gerrit.osmocom.org/c/osmo-pcu/+/22309/4/src/pcu_l1_if.cpp@845
PS4, Line 845: bts = gprs_pcu_get_bts_by_nr(the_pcu, pcu_prim->bts_nr);
this is called twice:
gprs_pcu_get_bts_by_nr(the_pcu, pcu_prim->bts_nr);
https://gerrit.osmocom.org/c/osmo-pcu/+/22309/4/src/pcu_vty.c
File src/pcu_vty.c:
https://gerrit.osmocom.org/c/osmo-pcu/+/22309/4/src/pcu_vty.c@1073
PS4, Line 1073: pcu_vty_show_tbf_all(vty, bts, flags);
Print out the BTS number too? Otherwise it will just be "UL TBFs", "DL TBFs", "UL TBFs", ...
https://gerrit.osmocom.org/c/osmo-pcu/+/22309/4/src/pcu_vty.c@1085
PS4, Line 1085: pcu_vty_show_ms_all(vty, bts);
How about extending show_ms() to mention the BTS number?
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/22309
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I6b10913f46c19d438c4e250a436a7446694b725a
Gerrit-Change-Number: 22309
Gerrit-PatchSet: 4
Gerrit-Owner: pespin <pespin at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: osmith <osmith at sysmocom.de>
Gerrit-Comment-Date: Tue, 19 Jan 2021 16:12:52 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210119/b6183459/attachment.htm>