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.