Attention is currently required from: dexter, osmith, pespin.
Hello Jenkins Builder, pespin,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-bts/+/34648?usp=email
to look at the new patch set (#7).
The following approvals got outdated and were removed:
Verified-1 by Jenkins Builder
Change subject: pcuif_proto: signal BTS model via PCUIF
......................................................................
pcuif_proto: signal BTS model via PCUIF
At the moment the PCU has no way of knowing with which BTS model it is
used with. However, some BTS models may require slightly different
behaviour by the PCU, depending on which BTS model is used. So, lets add
an additional bts_model field to struct gsm_pcu_if_info_ind in order to
convey the exact BTS model to the PCU.
Related: OS#6191
Depends: osmo-pcu.git I48eb75f65ab54fdec41ef913e24c1f18cd4a4047
Change-Id: Ib51238a0e09d4484a539a7f822864189872698b6
---
M include/osmo-bts/pcuif_proto.h
M src/common/pcu_sock.c
2 files changed, 51 insertions(+), 1 deletion(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/48/34648/7
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/34648?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: Ib51238a0e09d4484a539a7f822864189872698b6
Gerrit-Change-Number: 34648
Gerrit-PatchSet: 7
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: arehbein.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31878?usp=email )
Change subject: Make RLC timing data configurable
......................................................................
Patch Set 2:
(1 comment)
File src/osmo-bsc/net_init.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/31878/comment/972c5b9b_d5c3c520
PS2, Line 51: { .T = 3142, .default_val = 20,
> > So we really need to add a osmo_tdef group per bts object. You can have a look at > asp->cfg. […]
Have a look also ag libosmo-sccp "inst->tdefs" inside osmo_sccp_instance_create(),
and related sccp_timer_cmd in sccp_vty.c
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31878?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I2c24110d8c977d6cc74c3c8e77bcc709ad9d2675
Gerrit-Change-Number: 31878
Gerrit-PatchSet: 2
Gerrit-Owner: arehbein <arehbein(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: arehbein <arehbein(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 24 Oct 2023 15:13:12 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: arehbein <arehbein(a)sysmocom.de>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
dexter has submitted this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/34706?usp=email )
Change subject: pcuif_proto: drop support for PCUIF v10
......................................................................
pcuif_proto: drop support for PCUIF v10
We now use PCUIF v11 in the TTCN3 tests exclusively and also osmo-bts
and osmo-bsc only support PCUIF v11. There is no longer a need to
maintain a backward compatibility to PCUIF v10 in osmo-pcu.
Related: OS#5927
Change-Id: I68a3f59d5c960ae3a4fbd74f9d4a894295cb9ed8
---
M include/osmocom/pcu/pcuif_proto.h
M src/bts.cpp
M src/gprs_rlcmac.c
M src/pcu_l1_if.cpp
M tests/app_info/AppInfoTest.err
5 files changed, 34 insertions(+), 123 deletions(-)
Approvals:
Jenkins Builder: Verified
pespin: Looks good to me, but someone else must approve
osmith: Looks good to me, approved
diff --git a/include/osmocom/pcu/pcuif_proto.h b/include/osmocom/pcu/pcuif_proto.h
index 9df85d9..1133ca6 100644
--- a/include/osmocom/pcu/pcuif_proto.h
+++ b/include/osmocom/pcu/pcuif_proto.h
@@ -13,7 +13,6 @@
/* msg_type */
#define PCU_IF_MSG_DATA_REQ 0x00 /* send data to given channel */
-#define PCU_IF_MSG_DATA_CNF 0x01 /* confirm (e.g. transmission on PCH) */
#define PCU_IF_MSG_DATA_IND 0x02 /* receive data from given channel */
#define PCU_IF_MSG_SUSP_REQ 0x03 /* BTS forwards GPRS SUSP REQ to PCU */
#define PCU_IF_MSG_APP_INFO_REQ 0x04 /* BTS asks PCU to transmit APP INFO via PACCH */
@@ -35,8 +34,6 @@
/* sapi */
#define PCU_IF_SAPI_RACH 0x01 /* channel request on CCCH */
-#define PCU_IF_SAPI_AGCH 0x02 /* assignment on AGCH */
-#define PCU_IF_SAPI_PCH 0x03 /* paging/assignment on PCH */
#define PCU_IF_SAPI_BCCH 0x04 /* SI on BCCH */
#define PCU_IF_SAPI_PDTCH 0x05 /* packet data/control/ccch block */
#define PCU_IF_SAPI_PRACH 0x06 /* packet random access channel */
@@ -297,7 +294,6 @@
union {
struct gsm_pcu_if_data data_req;
- struct gsm_pcu_if_data data_cnf;
struct gsm_pcu_if_data_cnf data_cnf2;
struct gsm_pcu_if_data data_ind;
struct gsm_pcu_if_susp_req susp_req;
diff --git a/src/bts.cpp b/src/bts.cpp
index 50c21a3..0fe8fbc 100644
--- a/src/bts.cpp
+++ b/src/bts.cpp
@@ -1033,10 +1033,7 @@
rip->burst_type);
bts_do_rate_ctr_inc(bts, CTR_IMMEDIATE_ASSIGN_UL_TBF);
if (plen >= 0) {
- if (the_pcu->pcu_if_version >= 0x0b)
- pcu_l1if_tx_agch2(bts, bv, plen, false, GSM_RESERVED_TMSI);
- else
- pcu_l1if_tx_agch(bts, bv, plen);
+ pcu_l1if_tx_agch2(bts, bv, plen, false, GSM_RESERVED_TMSI);
rc = 0;
} else {
rc = plen;
@@ -1050,12 +1047,8 @@
bv, rip->ra, rip->rfn, rip->burst_type,
(uint8_t)osmo_tdef_get(bts->T_defs_bts, 3142, OSMO_TDEF_S, -1));
bts_do_rate_ctr_inc(bts, CTR_IMMEDIATE_ASSIGN_REJ);
- if (plen >= 0) {
- if (the_pcu->pcu_if_version >= 0x0b)
- pcu_l1if_tx_agch2(bts, bv, plen, false, GSM_RESERVED_TMSI);
- else
- pcu_l1if_tx_agch(bts, bv, plen);
- }
+ if (plen >= 0)
+ pcu_l1if_tx_agch2(bts, bv, plen, false, GSM_RESERVED_TMSI);
bitvec_free(bv);
/* rc was already properly set before goto */
return rc;
@@ -1136,24 +1129,20 @@
if (plen >= 0) {
bts_do_rate_ctr_inc(bts, CTR_IMMEDIATE_ASSIGN_DL_TBF);
- if (the_pcu->pcu_if_version >= 0x0b) {
- if (ms_imsi_is_valid(tbf->ms())) {
- pcu_l1if_tx_pch2(bts, immediate_assignment, plen, true, tbf->imsi(), tbf->tlli());
- } else {
- /* During GMM ATTACH REQUEST, the IMSI is not yet known to the PCU or SGSN. (It is
- * requested after the GMM ATTACH REQUEST with the GMM IDENTITY REQUEST.) When the PCU
- * has to assign a DL TBF but the IMSI is not known, then the IMMEDIATE ASSIGNMENT is
- * sent on the AGCH. The reason for this is that without an IMSI we can not calculate
- + the paging group, which would be necessary for transmission on PCH. Since the IMSI
- * is usually only unknown during the GMM ATTACH REQUEST, we may assume that the MS
- * is in non-DRX mode and hence it is listening on all CCCH blocks, including AGCH.
- *
- * See also: 3gpp TS 44.060, section 5.5.1.5
- * 3gpp TS 45.002, section 6.5.3, 6.5.6 */
- pcu_l1if_tx_agch2(bts, immediate_assignment, plen, true, tbf->tlli());
- }
+ if (ms_imsi_is_valid(tbf->ms())) {
+ pcu_l1if_tx_pch2(bts, immediate_assignment, plen, true, tbf->imsi(), tbf->tlli());
} else {
- pcu_l1if_tx_pch(bts, immediate_assignment, plen, tbf->imsi());
+ /* During GMM ATTACH REQUEST, the IMSI is not yet known to the PCU or SGSN. (It is
+ * requested after the GMM ATTACH REQUEST with the GMM IDENTITY REQUEST.) When the PCU
+ * has to assign a DL TBF but the IMSI is not known, then the IMMEDIATE ASSIGNMENT is
+ * sent on the AGCH. The reason for this is that without an IMSI we can not calculate
+ * the paging group, which would be necessary for transmission on PCH. Since the IMSI
+ * is usually only unknown during the GMM ATTACH REQUEST, we may assume that the MS
+ * is in non-DRX mode and hence it is listening on all CCCH blocks, including AGCH.
+ *
+ * See also: 3gpp TS 44.060, section 5.5.1.5
+ * 3gpp TS 45.002, section 6.5.3, 6.5.6 */
+ pcu_l1if_tx_agch2(bts, immediate_assignment, plen, true, tbf->tlli());
}
}
diff --git a/src/gprs_rlcmac.c b/src/gprs_rlcmac.c
index d15445e..b2cbeca 100644
--- a/src/gprs_rlcmac.c
+++ b/src/gprs_rlcmac.c
@@ -44,11 +44,7 @@
}
bts_do_rate_ctr_inc(bts, CTR_PCH_REQUESTS);
- if (the_pcu->pcu_if_version >= 0x0b)
- pcu_l1if_tx_pch2(bts, paging_request, plen, false, imsi, GSM_RESERVED_TMSI);
- else
- pcu_l1if_tx_pch(bts, paging_request, plen, imsi);
-
+ pcu_l1if_tx_pch2(bts, paging_request, plen, false, imsi, GSM_RESERVED_TMSI);
bitvec_free(paging_request);
return 0;
diff --git a/src/pcu_l1_if.cpp b/src/pcu_l1_if.cpp
index 4fcec31..9557f66 100644
--- a/src/pcu_l1_if.cpp
+++ b/src/pcu_l1_if.cpp
@@ -249,22 +249,6 @@
pcu_tx_data_req(bts, trx, ts, PCU_IF_SAPI_PTCCH, arfcn, fn, block_nr, data, data_len);
}
-void pcu_l1if_tx_agch(struct gprs_rlcmac_bts *bts, bitvec *block, int plen)
-{
- /* TODO: When PCUIF v.11 has become mainline, we will use pcu_l1if_tx_agch2() exclusively.
- * This will make this function obsolote, so we can remove it. */
- uint8_t data[GSM_MACBLOCK_LEN]; /* prefix PLEN */
-
- /* FIXME: why does OpenBTS has no PLEN and no fill in message? */
- bitvec_pack(block, data + 1);
- data[0] = (plen << 2) | 0x01;
-
- if (the_pcu->gsmtap_categ_mask & (1 << PCU_GSMTAP_C_DL_AGCH))
- gsmtap_send(the_pcu->gsmtap, 0, 0, GSMTAP_CHANNEL_AGCH, 0, 0, 0, 0, data, GSM_MACBLOCK_LEN);
-
- pcu_tx_data_req(bts, 0, 0, PCU_IF_SAPI_AGCH, 0, 0, 0, data, sizeof(data));
-}
-
/* Send a MAC block via the access grant channel. This will (obviously) only work for MAC blocks that contain
* an IMMEDIATE ASSIGNMENT. In case the confirm flag is set, the receiving end is required to send a confirmation
* back when the IMMEDIATE ASSIGNMENT has been sent. */
@@ -283,43 +267,6 @@
pcu_tx_data_req(bts, 0, 0, PCU_IF_SAPI_AGCH_2, 0, 0, 0, (uint8_t*)&agch, sizeof(agch));
}
-#define IMSI_DIGITS_FOR_PAGING 3
-/* Send a MAC block via the paging channel. (See also comment below) */
-void pcu_l1if_tx_pch(struct gprs_rlcmac_bts *bts, bitvec *block, int plen, const char *imsi)
-{
- /* TODO: When PCUIF v.11 has become mainline, we will use pcu_l1if_tx_pch2() exclusively.
- * This will make this function obsolote, so we can remove it. */
-
- uint8_t data[IMSI_DIGITS_FOR_PAGING + GSM_MACBLOCK_LEN];
-
- /* prepend last three IMSI digits (if present) from which BTS/BSC will calculate the paging group */
- if (imsi && strlen(imsi) >= IMSI_DIGITS_FOR_PAGING)
- memcpy(data, imsi + strlen(imsi) - IMSI_DIGITS_FOR_PAGING, IMSI_DIGITS_FOR_PAGING);
- else
- memset(data, '0', IMSI_DIGITS_FOR_PAGING);
-
- /* OS#6097: if strlen(imsi) == 0: We assume the MS is in non-DRX
- * mode (TS 44.060 5.5.1.5) and hence it is listening on all CCCH blocks
- * (TS 45.002 6.5.3, 6.5.6).
- * Hence, pgroup 000 is taken "randomly" to send it over it. This of
- * course not optimal since it can actually be sent on any CCCH blocks,
- * so we are delaying the ImmAss for no good reason. But anyway,
- * pcu_l1if_tx_pch() is deprecated and pcu_l1if_tx_pch2() should be
- * used instead, which doesn't suffer from this problem.
- */
-
- /* block provided by upper layer comes without first byte (plen), prepend it manually: */
- OSMO_ASSERT(sizeof(data) >= IMSI_DIGITS_FOR_PAGING + 1 + block->data_len);
- data[IMSI_DIGITS_FOR_PAGING] = (plen << 2) | 0x01;
- bitvec_pack(block, data + IMSI_DIGITS_FOR_PAGING + 1);
-
- if (the_pcu->gsmtap_categ_mask & (1 << PCU_GSMTAP_C_DL_PCH))
- gsmtap_send(the_pcu->gsmtap, 0, 0, GSMTAP_CHANNEL_PCH, 0, 0, 0, 0,
- data + IMSI_DIGITS_FOR_PAGING, GSM_MACBLOCK_LEN);
-
- pcu_tx_data_req(bts, 0, 0, PCU_IF_SAPI_PCH, 0, 0, 0, data, sizeof(data));
-}
-
/* Send a MAC block via the paging channel. This will (obviously) only work for MAC blocks that contain an
* IMMEDIATE ASSIGNMENT or a PAGING COMMAND message. In case the MAC block contains an IMMEDIATE ASSIGNMENT
* message, the receiving end is required to confirm when the IMMEDIATE ASSIGNMENT has been sent. */
@@ -556,26 +503,6 @@
return rc;
}
-static int pcu_rx_data_cnf(struct gprs_rlcmac_bts *bts, struct gsm_pcu_if_data *data_cnf)
-{
- int rc = 0;
-
- LOGP(DL1IF, LOGL_DEBUG, "Data confirm received: sapi=%d\n", data_cnf->sapi);
-
- switch (data_cnf->sapi) {
- case PCU_IF_SAPI_PCH:
- if (data_cnf->data[2] == GSM48_MT_RR_IMM_ASS)
- bts_rcv_imm_ass_cnf(bts, data_cnf->data, GSM_RESERVED_TMSI);
- break;
- default:
- LOGP(DL1IF, LOGL_ERROR, "Received PCU data confirm with "
- "unsupported sapi %d\n", data_cnf->sapi);
- rc = -EINVAL;
- }
-
- return rc;
-}
-
static int pcu_rx_data_cnf2(struct gprs_rlcmac_bts *bts, struct gsm_pcu_if_data_cnf *data_cnf)
{
int rc = 0;
@@ -809,14 +736,7 @@
LOGP(DL1IF, LOGL_DEBUG, "Info indication received:\n");
- /* NOTE: The classic way to confirm an IMMEDIATE assignment is to send the whole MAC block payload back to the
- * PCU. So it is the MAC block itsself that serves a reference for the confirmation. This method has certain
- * disadvantages so it was replaced with a method that uses the TLLI as a reference (msg_id). This new
- * method will replace the old one. The code that handles the old method will be removed in the foreseeable
- * future. (see also OS#5927) */
- if (info_ind->version == 0x0a) {
- LOGP(DL1IF, LOGL_NOTICE, "PCUIF version 10 is deprecated. OS#5927\n");
- } else if (info_ind->version != PCU_IF_VERSION) {
+ if (info_ind->version != PCU_IF_VERSION) {
fprintf(stderr, "PCU interface version number of BTS/BSC (%u) is different (%u).\nPlease use a BTS/BSC with a compatble interface!\n",
info_ind->version, PCU_IF_VERSION);
exit(-1);
@@ -1294,10 +1214,6 @@
CHECK_IF_MSG_SIZE(pcu_prim_length, pcu_prim->u.data_ind);
rc = pcu_rx_data_ind(bts, &pcu_prim->u.data_ind);
break;
- case PCU_IF_MSG_DATA_CNF:
- CHECK_IF_MSG_SIZE(pcu_prim_length, pcu_prim->u.data_cnf);
- rc = pcu_rx_data_cnf(bts, &pcu_prim->u.data_cnf);
- break;
case PCU_IF_MSG_DATA_CNF_2:
CHECK_IF_MSG_SIZE(pcu_prim_length, pcu_prim->u.data_cnf2);
rc = pcu_rx_data_cnf2(bts, &pcu_prim->u.data_cnf2);
diff --git a/tests/app_info/AppInfoTest.err b/tests/app_info/AppInfoTest.err
index 6428556..7cb89f4 100644
--- a/tests/app_info/AppInfoTest.err
+++ b/tests/app_info/AppInfoTest.err
@@ -23,7 +23,7 @@
ws(64)
MS(TA-220:MSCLS-10-11) Attaching DL TBF: TBF(DL:TFI-0-0-0:E){NEW}
MS(TA-220:MSCLS-10-11:DL): + tbf: now used by 1 (tbf)
-(bts=0,trx=0,ts=0) FN=0 Sending data request: sapi=3 arfcn=0 cur_fn=-1 block=0 data=30 30 30 2d 06 3f 30 0e 00 00 7d 80 00 1c 00 df ff ff ff f8 00 00 03 2b 2b 2b
+(bts=0,trx=0,ts=0) FN=0 Sending data request: sapi=9 arfcn=0 cur_fn=-1 block=0 data=ff ff ff ff 2d 06 3f 30 0e 00 00 7d 80 00 1c 00 df ff ff ff f8 00 00 03 2b 2b 2b 01
Creating MS object
Modifying MS object, TLLI = 0xffffffff, MS class 0 -> 12
Modifying MS object, TLLI = 0xffffffff, EGPRS MS class 0 -> 13
@@ -33,7 +33,7 @@
ws(64)
MS(TA-220:MSCLS-12-13) Attaching DL TBF: TBF(DL:TFI-0-0-1:E){NEW}
MS(TA-220:MSCLS-12-13:DL): + tbf: now used by 1 (tbf)
-(bts=0,trx=0,ts=0) FN=0 Sending data request: sapi=3 arfcn=0 cur_fn=-1 block=0 data=30 30 30 2d 06 3f 30 0d 00 00 7d 80 00 1c 00 df ff ff ff f8 40 00 03 2b 2b 2b
+(bts=0,trx=0,ts=0) FN=0 Sending data request: sapi=9 arfcn=0 cur_fn=-1 block=0 data=ff ff ff ff 2d 06 3f 30 0d 00 00 7d 80 00 1c 00 df ff ff ff f8 40 00 03 2b 2b 2b 01
--- test_sched_app_info_ok ---
Application Information Request received: type=0x00000000 len=15
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/34706?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I68a3f59d5c960ae3a4fbd74f9d4a894295cb9ed8
Gerrit-Change-Number: 34706
Gerrit-PatchSet: 3
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: merged
Attention is currently required from: pespin.
arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31878?usp=email )
Change subject: Make RLC timing data configurable
......................................................................
Patch Set 2:
(1 comment)
File src/osmo-bsc/net_init.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/31878/comment/03fbfef7_b9faad9b
PS2, Line 51: { .T = 3142, .default_val = 20,
> So we really need to add a osmo_tdef group per bts object. You can have a look at > asp->cfg.T_defs_lm in libosmo-sccp/src/osmo_ss7_asp.c:673 to find out how to add > a T_def instance per object (as opposed to a single structure per process like > the gsm_network_T_defs here). Basically:
> """
> asp->cfg.T_defs_lm = talloc_memdup(asp, ss7_asp_lm_timer_defaults,
> sizeof(ss7_asp_lm_timer_defaults));
> osmo_tdefs_reset(asp->cfg.T_defs_lm);
> """
I have written some of the per-bts part on my local working copy, but I thought I'd ask before proceeding.
I added an array of tdefs and an array of timer groups like in net_init.c, but since it's per BTS, I can't just add a tdef group to the one in net_init.c and use the tdef API with that to generate VTY commands automatically (the VTY command wouldn't be per BTS).
I moved that code to bts_init.c - thinking I could use `osmo_tdef_vty_groups_init()` from libosmocore.git:src/vty/tdef_vty.c (which unfortunately didn't work out, because or reliance on a global variable `global_tdef_groups` that cannot be changed without breaking a previously set up tdef group).
So my thought now is basically to replicate the tdef API code - pretty much staying the same except for I'd probably have to change usage of `global_tdef_groups` into usage of some BTS-dependent arrays of tdef groups automatically generating the commands for all tdef groups (I'm thinking "rlc", "bssgp" (other upcoming patch) and "nse" (other upcoming patch) would be appropriate groups for each BTS).
So, to sum it up, the approach I would go for something like this (for each BTS):
- Copy and adapt code from tdef_vty.c in libosmocore to enable per BTS tdef API functionality (put it in osmo-bsc.git:src/osmo-bsc/bts_vty.c )
- Set up an array of tdefs (copied from a default set)
*(in theory we could make that configurable so that if a bts config is missing config values, it can take those from a 'global' bts timer config put in `global_tdef_groups` set up in the net_init.c code)*
- Set up an array of tdef groups referenced in the bts struct
I did look at the example in libosmo-sccp/src/osmo_ss7_asp.c, but since we're adding quite a few more timers here, using timer numbers from the specs and also with BSSGP and NSE timers in two other upcoming patches, I assume it makes more sence to go for the above mentioned approach instead of writing out each VTY timer command manually.
What do you think?
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31878?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I2c24110d8c977d6cc74c3c8e77bcc709ad9d2675
Gerrit-Change-Number: 31878
Gerrit-PatchSet: 2
Gerrit-Owner: arehbein <arehbein(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 24 Oct 2023 14:55:58 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: arehbein <arehbein(a)sysmocom.de>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: dexter.
Hello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/pysim/+/34884?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Verified-1 by Jenkins Builder
Change subject: pySim-shell: don't get trapped in applications without file system
......................................................................
pySim-shell: don't get trapped in applications without file system
When we traverse the file system, we may also end up selecting
applications (ADF), which do not support an USIM/ISIM like file system.
This will leave us without the ability to select the MF (or any other
file) again. The only way out is to select the ISIM or USIM application
again to get the access to the file system again.
Related: OS#5418
Change-Id: Ia2fdd65f430c07acb1afdaf265d24c6928b654e0
---
M pySim-shell.py
1 file changed, 42 insertions(+), 2 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/pysim refs/changes/84/34884/2
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/34884?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: Ia2fdd65f430c07acb1afdaf265d24c6928b654e0
Gerrit-Change-Number: 34884
Gerrit-PatchSet: 2
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Jenkins Builder has posted comments on this change. ( https://gerrit.osmocom.org/c/pysim/+/34884?usp=email )
Change subject: pySim-shell: don't get trapped in applications without file system
......................................................................
Patch Set 1:
(1 comment)
File pySim-shell.py:
Robot Comment from checkpatch (run ID jenkins-gerrit-lint-12083):
https://gerrit.osmocom.org/c/pysim/+/34884/comment/286d2213_543ba27b
PS1, Line 615: # To automatize this escape-route while traversing the file system we will check whether
'automatize' may be misspelled - perhaps 'automate'?
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/34884?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: Ia2fdd65f430c07acb1afdaf265d24c6928b654e0
Gerrit-Change-Number: 34884
Gerrit-PatchSet: 1
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-CC: Jenkins Builder
Gerrit-Comment-Date: Tue, 24 Oct 2023 14:26:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
dexter has uploaded this change for review. ( https://gerrit.osmocom.org/c/pysim/+/34884?usp=email )
Change subject: pySim-shell: don't get trapped in applications without file system
......................................................................
pySim-shell: don't get trapped in applications without file system
When we traverse the file system, we may also end up selecting
applications (ADF), which do not support an USIM/ISIM like file system.
This will leave us without the ability to select the MF (or any other
file) again. The only way out is to select the ISIM or USIM application
again to get the access to the file system again.
Related: OS#5418
Change-Id: Ia2fdd65f430c07acb1afdaf265d24c6928b654e0
---
M pySim-shell.py
1 file changed, 42 insertions(+), 2 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/pysim refs/changes/84/34884/1
diff --git a/pySim-shell.py b/pySim-shell.py
index 8a1ad01..2368f95 100755
--- a/pySim-shell.py
+++ b/pySim-shell.py
@@ -55,7 +55,7 @@
from pySim.utils import sanitize_pin_adm, tabulate_str_list, boxed_heading_str, Hexstr, dec_iccid
from pySim.card_handler import CardHandler, CardHandlerAuto
-from pySim.filesystem import CardDF, CardADF, CardModel, CardApplication
+from pySim.filesystem import CardMF, CardDF, CardADF, CardModel, CardApplication
from pySim.runtime import RuntimeState
from pySim.profile import CardProfile
from pySim.cdma_ruim import CardProfileRUIM
@@ -599,7 +599,31 @@
# below, so we must not move up.
if skip_df == False:
self.walk(indent + 1, action_ef, action_df, context, **kwargs)
- fcp_dec = self._cmd.lchan.select("..", self._cmd)
+
+ parent = self._cmd.lchan.selected_file.get_selectables()[".."]
+ df = self._cmd.lchan.selected_file
+ if isinstance(parent, CardMF) and (df.name not in [ "ADF.USIM", "ADF.ISIM"]):
+ # Not every application that may be present on a GlobalPlatform card will support the SELECT
+ # command as we know it from ETSI TS 102 221, section 11.1.1. In fact the only subset of
+ # SELECT we may rely on is the OPEN SELECT command as specified in GlobalPlatform Card
+ # Specification, section 11.9. Unfortunately the OPEN SELECT command only supports the
+ # "select by name" method, which means we can only select an application and not a file.
+ # The consequence of this is that we may get trapped in an application that does not have
+ # ISIM/USIM like file system support and the only way to leave that application is to select
+ # an ISIM/USIM application in order to get the file system access back.
+ #
+ # To automatize this escape-route while traversing the file system we will check whether
+ # the parent file is the MF. When this is the case and the selected ADF is not ADF.ISIM or
+ # ADF.USIM, we will select ADF.USIM or ADF.ISIM (depending on what is available) and from
+ # from there we will then select the MF.
+ for app in parent.applications:
+ if app in [ "a0000000871002", "a0000000871004"]:
+ adf_xsim = app
+ self._cmd.lchan.select(adf_xsim, self._cmd)
+ self._cmd.lchan.select("MF", self._cmd)
+ else:
+ # Normal DF/ADF selection
+ fcp_dec = self._cmd.lchan.select("..", self._cmd)
elif action_ef:
df_before_action = self._cmd.lchan.selected_file
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/34884?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: Ia2fdd65f430c07acb1afdaf265d24c6928b654e0
Gerrit-Change-Number: 34884
Gerrit-PatchSet: 1
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-MessageType: newchange