Attention is currently required from: jolly.
falconia has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/35132?usp=email )
Change subject: Transmit invalid AMR speech blocks instead of dummy FACCH
......................................................................
Patch Set 3:
(4 comments)
Commit Message:
https://gerrit.osmocom.org/c/osmo-bts/+/35132/comment/a16f6572_4bd42192
PS3, Line 17: playload
Did you perhaps mean "payload"?
Patchset:
PS3:
Aside from typos/grammar, I would give a CR+2 on the code itself at this point.
File src/osmo-bts-trx/sched_lchan_tchf.c:
https://gerrit.osmocom.org/c/osmo-bts/+/35132/comment/63b99f1d_eed0cec1
PS3, Line 541: with
This extraneous use of "with" here is ungrammatical. Simply removing the extra "with" will produce correct English grammar.
File src/osmo-bts-trx/sched_lchan_tchh.c:
https://gerrit.osmocom.org/c/osmo-bts/+/35132/comment/16a07373_c32e788d
PS3, Line 456: with
Same as above,
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/35132?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: I056f379715c91ad968f198e112d363a9009dc1c3
Gerrit-Change-Number: 35132
Gerrit-PatchSet: 3
Gerrit-Owner: jolly <andreas(a)eversberg.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: falconia <falcon(a)freecalypso.org>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: jolly <andreas(a)eversberg.eu>
Gerrit-Comment-Date: Wed, 29 Nov 2023 00:07:11 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: dexter, pespin.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/35151?usp=email )
Change subject: gprs_rlcmac_sched: rewrite logic around idle block skip
......................................................................
Patch Set 2:
(1 comment)
File src/gprs_rlcmac_sched.cpp:
https://gerrit.osmocom.org/c/osmo-pcu/+/35151/comment/fe7bd69a_25c49d21
PS1, Line 486: * way we help BTS energy saving (on TRX!=C0) by sending nothing
> This is how it works: […]
Oh, I overlooked the `skip_idle && trx != 0` part in the old code (left side). I don't remember why we had this constraint, but could you explain why are you removing it?
With this constraint we only have power saving on Cn (n > 0). On the BCCH carrier we always transmit PDCH blocks at full power, no matter if there are active TBFs or not, because we always get DATA.req for the C0.
By removing this constraint in your patch you're making osmo-bts-trx send Dummy Bursts (carrying no info at all) instead of Normal Bursts. Like I explained in my previous comment.
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/35151?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: Iadb62748b18605bf158169b317f880352bc0a5a6
Gerrit-Change-Number: 35151
Gerrit-PatchSet: 2
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-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 28 Nov 2023 22:38:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: dexter, pespin.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/35151?usp=email )
Change subject: gprs_rlcmac_sched: rewrite logic around idle block skip
......................................................................
Patch Set 2:
(1 comment)
File src/gprs_rlcmac_sched.cpp:
https://gerrit.osmocom.org/c/osmo-pcu/+/35151/comment/45e2904c_d3c2aa5a
PS1, Line 486: * way we help BTS energy saving (on TRX!=C0) by sending nothing
> IIRC you can only decrease a given number of dBm at specific points in time, it's specified somewher […]
This is how it works:
* on C0 (the BCCH carrier): osmo-bts-trx transmits Dummy Bursts (not Normal Bursts), which can be transmitted at a lower power level (2 dB or even 4 dB less relative to the full power, IIRC) in the BCCH carrier power saving mode;
* on Cn (n > 0): osmo-bts-trx simply transmits nothing and thus saves energy.
It's actually required by 3GPP specs. to maintain a continuous ("gapless") transmission on the BCCH carrier (C0), so that a phone doing full power scan would not overlook it.
It would have been more logical (and elegant, IMO) to offload the power saving logic to the BTS completely, instead of making such a decision in osmo-pcu. There would be no need to maintain a list of the power saving capable BTS models here, and add lengthy comments explaining what the BTS is supposed to do.
My initial idea/proposal was to simply flag dummy blocks generated in the absence of active TBFs, so that the BTS could decide what to do with such a DATA.req on its own. Unfortunately, I was not able to defend this proposal back then because I was on sick leave. Seeing how much effort/time it's taking to fix OS#6191, I think we should consider getting back to this idea at some point...
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/35151?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: Iadb62748b18605bf158169b317f880352bc0a5a6
Gerrit-Change-Number: 35151
Gerrit-PatchSet: 2
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-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 28 Nov 2023 22:30:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de>
Gerrit-MessageType: comment
laforge has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35154?usp=email )
Change subject: libosmosim: Support Microsoft smart card discovery process
......................................................................
libosmosim: Support Microsoft smart card discovery process
Our class/instruction tables (used mainly by simtrace cardem host
software) only contain support for those instructions permitted
in the related card specification.
Microsoft blindly tries CLA=0xCA with INS=0x00 which is not somethin
that the GSM SIM, ETSI UICC or 3GPP USIM specs specify, and which
hence results in log output like this:
DLINP DEBUG [0] <= osmo_st2_cardem_request_sw_tx(sw=6a88)
DLGLOBAL INFO => DATA: flags=0x01 (HDR ), 00 ca 7f 68 00
DLGLOBAL ERROR Unknown APDU case 0
DLGLOBAL FATAL Failed to recognize APDU, terminating
Let's adjust to microsoft and *always* support their instructions
no matter which osim_cla_ins_card_profile was used.
Special thanks to Eric Wild for pointing me to this unexpected
behaviour of PC/SC on modern Windows.
Change-Id: I424964c0afab643e6a5d7824d91c2c86b0d3f25b
Related: SYS#6617
---
M src/sim/class_tables.c
1 file changed, 47 insertions(+), 0 deletions(-)
Approvals:
Jenkins Builder: Verified
Hoernchen: Looks good to me, approved
fixeria: Looks good to me, but someone else must approve
diff --git a/src/sim/class_tables.c b/src/sim/class_tables.c
index 6d05ede..29c1e40 100644
--- a/src/sim/class_tables.c
+++ b/src/sim/class_tables.c
@@ -350,6 +350,13 @@
[0x88] = 4, /* AUTHENTICATE */
};
+/* https://learn.microsoft.com/en-us/windows-hardware/drivers/smartcard/discov… */
+static const uint8_t microsoft_discovery_ins_tbl[256] = {
+ [0xA4] = 4, /* SELECT FILE */
+ [0xCA] = 2, /* GET DATA */
+ [0xC0] = 2, /* GET RESPONSE */
+};
+
int osim_determine_apdu_case(const struct osim_cla_ins_card_profile *prof,
const uint8_t *hdr)
{
@@ -374,5 +381,16 @@
return rc;
}
}
+ /* special handling for Microsoft who insists to use INS=0xCA in CLA=0x00 which is not
+ * really part of GSM SIM, ETSI UICC or 3GPP USIM specifications, but only ISO7816. Rather than adding
+ * it to each and every card profile, let's add the instructions listed at
+ * https://learn.microsoft.com/en-us/windows-hardware/drivers/smartcard/discov… explicitly
+ * here. They will only be used in case no more specific match was found in the actual profile above. */
+ if (cla == 0x00) {
+ rc = microsoft_discovery_ins_tbl[ins];
+ if (rc)
+ return rc;
+ }
+
return 0;
}
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35154?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I424964c0afab643e6a5d7824d91c2c86b0d3f25b
Gerrit-Change-Number: 35154
Gerrit-PatchSet: 4
Gerrit-Owner: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: Hoernchen <ewild(a)sysmocom.de>
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
Attention is currently required from: dexter.
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35154?usp=email )
Change subject: libosmosim: Support Microsoft smart card discovery process
......................................................................
Patch Set 4:
(1 comment)
Patchset:
PS4:
> It would probably be useful to add the msdn links to the commit message that explain what MS does?
the links are in the code comments, not sure we need them also in the commit message?
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35154?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I424964c0afab643e6a5d7824d91c2c86b0d3f25b
Gerrit-Change-Number: 35154
Gerrit-PatchSet: 4
Gerrit-Owner: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: Hoernchen <ewild(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 28 Nov 2023 22:24:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Hoernchen <ewild(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: fixeria, laforge.
Jenkins Builder has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/35124?usp=email )
Change subject: Add GSMTAP encapsulation of RLP frames in CSD NT mode
......................................................................
Patch Set 6:
(2 comments)
File src/common/l1sap.c:
Robot Comment from checkpatch (run ID jenkins-gerrit-lint-12698):
https://gerrit.osmocom.org/c/osmo-bts/+/35124/comment/21244cd8_c8b32077
PS6, Line 1879: } else {
else is not generally useful after a break or return
Robot Comment from checkpatch (run ID jenkins-gerrit-lint-12698):
https://gerrit.osmocom.org/c/osmo-bts/+/35124/comment/8f7b8427_f0815275
PS6, Line 1893: } else {
else is not generally useful after a break or return
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/35124?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: I6a258458822bcb3fe7290a9b9b3d104beecda219
Gerrit-Change-Number: 35124
Gerrit-PatchSet: 6
Gerrit-Owner: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 28 Nov 2023 22:12:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: fixeria, laforge.
Hello Jenkins Builder, fixeria,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-bts/+/35124?usp=email
to look at the new patch set (#6).
The following approvals got outdated and were removed:
Verified-1 by Jenkins Builder
Change subject: Add GSMTAP encapsulation of RLP frames in CSD NT mode
......................................................................
Add GSMTAP encapsulation of RLP frames in CSD NT mode
In CSD (Circuit Switched Data) NT (Non-Transparent) mode, there
are RLP (Radio Link Protocol) frames inside the modified V.110.
wireshark alrady has a dissector for this, and we've introduced
a GSMTAP type for RLP some time ago. So with this patch, we now
generate such GSMTAP RLP frames.
Change-Id: I6a258458822bcb3fe7290a9b9b3d104beecda219
---
M include/osmo-bts/bts.h
M include/osmo-bts/lchan.h
M src/common/l1sap.c
M src/common/vty.c
M tests/osmo-bts.vty
5 files changed, 124 insertions(+), 4 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/24/35124/6
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/35124?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: I6a258458822bcb3fe7290a9b9b3d104beecda219
Gerrit-Change-Number: 35124
Gerrit-PatchSet: 6
Gerrit-Owner: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: dexter, laforge.
Hoernchen has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35154?usp=email )
Change subject: libosmosim: Support Microsoft smart card discovery process
......................................................................
Patch Set 4:
(1 comment)
Patchset:
PS4:
It would probably be useful to add the msdn links to the commit message that explain what MS does?
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35154?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I424964c0afab643e6a5d7824d91c2c86b0d3f25b
Gerrit-Change-Number: 35154
Gerrit-PatchSet: 4
Gerrit-Owner: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: Hoernchen <ewild(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 28 Nov 2023 22:07:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment