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
laforge has submitted this change. ( https://gerrit.osmocom.org/c/pysim/+/34849?usp=email )
(
3 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
)Change subject: prevent SimCardCommands.select_adf_by_aid bypassing lchan
......................................................................
prevent SimCardCommands.select_adf_by_aid bypassing lchan
Now that pySim-shell is aware of logical channels and issues almost
all of its APDUs on the currently selected channel, we must also make
sure that ADF selection by AID (implemented by the CardBase class)
issues the SELECT on the respective logical channel.
Before this patch, SELECT ADF by AID would always be issued on the
primary logical channel (0), irrespective of the currently active
RuntimeLchan.
Change-Id: Idf05c297e6a2e24ca539408b8912e348c0782bb4
Related: OS#6230
---
M pySim/cards.py
M pySim/runtime.py
2 files changed, 26 insertions(+), 5 deletions(-)
Approvals:
Jenkins Builder: Verified
dexter: Looks good to me, approved
diff --git a/pySim/cards.py b/pySim/cards.py
index 84f53e1..4573616 100644
--- a/pySim/cards.py
+++ b/pySim/cards.py
@@ -146,8 +146,10 @@
return True
return False
- def select_adf_by_aid(self, adf: str = "usim") -> Tuple[Optional[Hexstr], Optional[SwHexstr]]:
+ def select_adf_by_aid(self, adf: str = "usim", scc: Optional[SimCardCommands] = None) -> Tuple[Optional[Hexstr], Optional[SwHexstr]]:
"""Select ADF.U/ISIM in the Card using its full AID"""
+ # caller may pass a custom scc; we fall back to default
+ scc = scc or self._scc
if is_hex(adf):
aid = adf
else:
@@ -155,10 +157,10 @@
if aid:
aid_full = self._complete_aid(aid)
if aid_full:
- return self._scc.select_adf(aid_full)
+ return scc.select_adf(aid_full)
else:
# If we cannot get the full AID, try with short AID
- return self._scc.select_adf(aid)
+ return scc.select_adf(aid)
return (None, None)
def card_detect(scc: SimCardCommands) -> Optional[CardBase]:
diff --git a/pySim/runtime.py b/pySim/runtime.py
index 27c2ef1..a54a1b6 100644
--- a/pySim/runtime.py
+++ b/pySim/runtime.py
@@ -299,7 +299,7 @@
for p in inter_path:
try:
if isinstance(p, CardADF):
- (data, sw) = self.rs.card.select_adf_by_aid(p.aid)
+ (data, sw) = self.rs.card.select_adf_by_aid(p.aid, scc=self.scc)
self.selected_adf = p
else:
(data, sw) = self.scc.select_file(p.fid)
@@ -343,7 +343,7 @@
f = sels[name]
try:
if isinstance(f, CardADF):
- (data, sw) = self.rs.card.select_adf_by_aid(f.aid)
+ (data, sw) = self.rs.card.select_adf_by_aid(f.aid, scc=self.scc)
else:
(data, sw) = self.scc.select_file(f.fid)
self.selected_file = f
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/34849?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: Idf05c297e6a2e24ca539408b8912e348c0782bb4
Gerrit-Change-Number: 34849
Gerrit-PatchSet: 4
Gerrit-Owner: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: merged