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 <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 28 Feb 2023 15:47:43 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment