Attention is currently required from: neels, pespin, fixeria. dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/31145 )
Change subject: bts: add IMMEDIATE ASSIGNMENT via PCH transmission ...................................................................... Patch Set 18: (6 comments) File include/osmocom/pcu/pcuif_proto.h: https://gerrit.osmocom.org/c/osmo-pcu/+/31145/comment/5510a419_bd202104 PS17, Line 48: #define PCU_IF_FLAG_DT (1 << 2)/* use TLLI for confirmation directly */ > I think we agreed in the call this flag was not needed. Thanks for reminding me. I wasn't sure anymore but it certainly makes sense to drop the flag and look at the PCUIIF version number instead. https://gerrit.osmocom.org/c/osmo-pcu/+/31145/comment/e20bdc85_35de6adb PS17, Line 280: uint8_t pgroup[3]; > This can be a int16_t containing a number 0-999. I think this would then make using the struct at the BSC side more complicated, but I am not entirely sure. The thing is that this value ends up at extract_paging_group and this function does a str_to_imsi(imsi_digit_buf) with the value, which then ends up at libosmocore:gsm0502_calc_paging_group, which gets the IMSI as an uint64_t. If we can feed gsm0502_calc_paging_group() the pgroup value from here directly then we can use an uint16_t. (what makes me wonder a bit is that gsm0502_calc_paging_group() returns an unsigned int but we store the returned paging group in an uint8_t.) https://gerrit.osmocom.org/c/osmo-pcu/+/31145/comment/851ea54f_2dfab0f6 PS17, Line 294: struct gsm_pcu_if_data_cnf_dt data_cnf_dt; > isn't it a bit weird that we have a cnf_dt but no data_dt? The message in the direction towards the BSC is sent as data_req under the SAPI PCU_IF_SAPI_PCH_DT. For the confirmation we have specific message types. That is the reason why there is no data_dt. File src/bts.h: https://gerrit.osmocom.org/c/osmo-pcu/+/31145/comment/d48a6ed7_c5eeb3ef PS17, Line 282: /* Use the TLLI directly to handle IMMEDIATE ASSIGNMENT confirmation, otherwise the TLLI is extracted > This is most probably not needed anymore and can be dropped. (see above, we still needed it, but hopefully not very long.) File src/bts.cpp: https://gerrit.osmocom.org/c/osmo-pcu/+/31145/comment/63442e9d_6731f25d PS17, Line 1134: pcu_l1if_tx_pch(bts, immediate_assignment, plen, ms_paging_group(tbf_ms(tbf))); > I thought we agreed on dropping the previous message? If we drop it now, then the PCU will become incompatible with osmo-bts. As far as I remember we decided to add a deprecation warning and upgrade osmo-bts later. Its easy to remove the code pathes later so lets not get blocked by the BTS part and keep compatibility for now. File src/pcu_l1_if.cpp: https://gerrit.osmocom.org/c/osmo-pcu/+/31145/comment/d78cf3c1_09a75e9d PS17, Line 769: > this can be dropped. I have reworked this part and added the deprecation note. I also have created a ticket for the BTS part now. -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/31145 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I2a78651593323e8b9627c39918d949a33497b70f Gerrit-Change-Number: 31145 Gerrit-PatchSet: 18 Gerrit-Owner: dexter <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <[email protected]> Gerrit-Reviewer: laforge <[email protected]> Gerrit-Reviewer: pespin <[email protected]> Gerrit-CC: neels <[email protected]> Gerrit-Attention: neels <[email protected]> Gerrit-Attention: pespin <[email protected]> Gerrit-Attention: fixeria <[email protected]> Gerrit-Comment-Date: Tue, 28 Feb 2023 15:47:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <[email protected]> Gerrit-MessageType: comment
