October 2023 - gerrit-log - lists.osmocom.org

[M] Change in libosmocore[master]: gsmtap_util: Use Osmo IO instead of Osmo write queues
by daniel 24 Oct '23

24 Oct '23

Attention is currently required from: arehbein, laforge, pespin. daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email ) Change subject: gsmtap_util: Use Osmo IO instead of Osmo write queues ...................................................................... Patch Set 8: (1 comment) File src/core/gsmtap_util.c: https://gerrit.osmocom.org/c/libosmocore/+/34743/comment/8a07c099_d0014ba2 PS5, Line 478: osmo_iofd_setup(gti, gti->sink_fd, "gsmtap_inst.out", OSMO_IO_FD_MODE_READ_WRITE, &gsmtap_sink_ops, NULL); > In that case I guess we want to limit somehow the read buffer queue in the kernel to the minimum (even 0) to avoid wasting kb/mb of memory for each gsmtap socket sink with filled message that are never read. Yeah, might be a good idea, but out of scope for this patch. @arehbein@sysmocom.de gsmtap_source_add_sink is called in logging_gsmtap.c and also in osmo_pcu and osmo_bts. In libosmocore/src/vty/logging_vty.c cfg_log_gsmtap calls log_target_create_gsmtap with false, true as last parameters. That means ofd_wq_mode false and add_sink true. We should change the ofd_wq_mode parameter to true here, otherwise it doesn't use osmo_io at all. Not sure if we need a VTY parameter for that. Since add_sink is true the code will call gsmtap_source_add_sink() which creates the sink only if the gsmtap target is local. So simply enabling gsmtap logging to 127.0.0.1 should call the code in question. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34743?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: Iadbbef74e3add7001d84dd6b68f51eac293e44d0 Gerrit-Change-Number: 34743 Gerrit-PatchSet: 8 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Tue, 24 Oct 2023 12:05:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: arehbein <arehbein(a)sysmocom.de> Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: daniel <dwillmann(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in osmo-ttcn3-hacks[master]: DIAMETER_Templates: Add S6b AAR and AAA message templates
by osmith 24 Oct '23

24 Oct '23

Attention is currently required from: lynxis lazus, pespin. osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/34860?usp=email ) Change subject: DIAMETER_Templates: Add S6b AAR and AAA message templates ...................................................................... Patch Set 3: Code-Review+1 (1 comment) File library/DIAMETER_Templates.ttcn: https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/34860/comment/4e7601fb_34ff… PS2, Line 1532: private function f_tr_AVP_AuthRequestType(template (present) BASE_NONE_Auth_Request_Type auth_req_type) > why is this commented out? Done -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/34860?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I302d9a94e6ffbb62b6db81e1b14484355e55ebe5 Gerrit-Change-Number: 34860 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Comment-Date: Tue, 24 Oct 2023 11:56:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: osmith <osmith(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in libgtpnl[master]: Add IPv6 support
by pespin 24 Oct '23

24 Oct '23

Attention is currently required from: fixeria, laforge, osmith. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libgtpnl/+/34735?usp=email ) Change subject: Add IPv6 support ...................................................................... Patch Set 5: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/libgtpnl/+/34735?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libgtpnl Gerrit-Branch: master Gerrit-Change-Id: If864c9170f74af52a95cbc4cdb1b866e0309306b Gerrit-Change-Number: 34735 Gerrit-PatchSet: 5 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Tue, 24 Oct 2023 11:56:18 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in libgtpnl[master]: Add IPv6 support
by osmith 24 Oct '23

24 Oct '23

Attention is currently required from: fixeria, laforge, osmith, pespin. Hello Jenkins Builder, fixeria, laforge, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libgtpnl/+/34735?usp=email to look at the new patch set (#5). The following approvals got outdated and were removed: Verified+1 by Jenkins Builder, Verified+1 by osmith Change subject: Add IPv6 support ...................................................................... Add IPv6 support Implement IPv6 in libgtpnl and the gtp-tunnel testing tool. Allow to combine: - GTPA_MS_ADDRESS and GTPA_PEER_ADDR6 - GTPA_MS_ADDR6 and GTPA_PEER_ADDRESS to specify IPv4-in-IPv6-GTP and IPv6-in-IPv4-GTP in the tunnel declaration from control plane. This patch is based on multiple patches from Pablo in OS#6123. I decided to squash them to directly implement v4-in-v6 and vice versa, instead of implementing another variant first and then changing it again. Co-developed-by: Pablo Neira Ayuso <pablo(a)netfilter.org> Related: OS#6096 Change-Id: If864c9170f74af52a95cbc4cdb1b866e0309306b --- M include/libgtpnl/gtp.h M include/linux/gtp.h M src/gtp-genl.c M src/gtp.c M src/libgtpnl.map M tools/gtp-tunnel.c 6 files changed, 145 insertions(+), 30 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libgtpnl refs/changes/35/34735/5 -- To view, visit https://gerrit.osmocom.org/c/libgtpnl/+/34735?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libgtpnl Gerrit-Branch: master Gerrit-Change-Id: If864c9170f74af52a95cbc4cdb1b866e0309306b Gerrit-Change-Number: 34735 Gerrit-PatchSet: 5 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

[M] Change in libgtpnl[master]: Add IPv6 support
by osmith 24 Oct '23

24 Oct '23

Attention is currently required from: fixeria, laforge, pespin. osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/libgtpnl/+/34735?usp=email ) Change subject: Add IPv6 support ...................................................................... Patch Set 4: Verified+1 (1 comment) This change is ready for review. Patchset: PS4: I've verified that there is no regression with ttcn-3 tests from master and this new libgtpnl version, and Pablo's kernel changes. -- To view, visit https://gerrit.osmocom.org/c/libgtpnl/+/34735?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libgtpnl Gerrit-Branch: master Gerrit-Change-Id: If864c9170f74af52a95cbc4cdb1b866e0309306b Gerrit-Change-Number: 34735 Gerrit-PatchSet: 4 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Tue, 24 Oct 2023 11:52:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in libgtpnl[master]: Use struct gtp_addr for ms_addr and sgsn_addr
by osmith 24 Oct '23

24 Oct '23

osmith has submitted this change. ( https://gerrit.osmocom.org/c/libgtpnl/+/34734?usp=email ) Change subject: Use struct gtp_addr for ms_addr and sgsn_addr ...................................................................... Use struct gtp_addr for ms_addr and sgsn_addr Prepare for IPv6 support by using a new struct for MS and SGSN addresses, in which either an IPv4 or IPv6 can be stored. Related: OS#6096 Change-Id: Ifc7e3b03a723fb544d1c7b789101102b2c27b60e --- M src/gtp-genl.c M src/gtp.c M src/internal.h 3 files changed, 42 insertions(+), 17 deletions(-) Approvals: pespin: Looks good to me, but someone else must approve laforge: Looks good to me, approved Jenkins Builder: Verified diff --git a/src/gtp-genl.c b/src/gtp-genl.c index 9c9e8d3..82ef8ab 100644 --- a/src/gtp-genl.c +++ b/src/gtp-genl.c @@ -48,10 +48,10 @@ if (t->ifns >= 0) mnl_attr_put_u32(nlh, GTPA_NET_NS_FD, t->ifns); mnl_attr_put_u32(nlh, GTPA_LINK, t->ifidx); - if (t->sgsn_addr.s_addr) - mnl_attr_put_u32(nlh, GTPA_PEER_ADDRESS, t->sgsn_addr.s_addr); - if (t->ms_addr.s_addr) - mnl_attr_put_u32(nlh, GTPA_MS_ADDRESS, t->ms_addr.s_addr); + if (t->sgsn_addr.ip4.s_addr) + mnl_attr_put_u32(nlh, GTPA_PEER_ADDRESS, t->sgsn_addr.ip4.s_addr); + if (t->ms_addr.ip4.s_addr) + mnl_attr_put_u32(nlh, GTPA_MS_ADDRESS, t->ms_addr.ip4.s_addr); if (t->gtp_version == GTP_V0) { mnl_attr_put_u64(nlh, GTPA_TID, t->u.v0.tid); mnl_attr_put_u16(nlh, GTPA_FLOW, t->u.v0.flowid); @@ -116,8 +116,8 @@ uint32_t o_tei; } v1; } u; - struct in_addr sgsn_addr; - struct in_addr ms_addr; + struct gtp_addr sgsn_addr; + struct gtp_addr ms_addr; }; static int genl_gtp_validate_cb(const struct nlattr *attr, void *data) @@ -167,11 +167,13 @@ if (tb[GTPA_O_TEI]) pdp.u.v1.o_tei = mnl_attr_get_u32(tb[GTPA_O_TEI]); if (tb[GTPA_PEER_ADDRESS]) { - pdp.sgsn_addr.s_addr = + pdp.sgsn_addr.family = AF_INET; + pdp.sgsn_addr.ip4.s_addr = mnl_attr_get_u32(tb[GTPA_PEER_ADDRESS]); } if (tb[GTPA_MS_ADDRESS]) { - pdp.ms_addr.s_addr = mnl_attr_get_u32(tb[GTPA_MS_ADDRESS]); + pdp.ms_addr.family = AF_INET; + pdp.ms_addr.ip4.s_addr = mnl_attr_get_u32(tb[GTPA_MS_ADDRESS]); } if (tb[GTPA_VERSION]) { pdp.version = mnl_attr_get_u32(tb[GTPA_VERSION]); @@ -179,15 +181,15 @@ printf("version %u ", pdp.version); if (pdp.version == GTP_V0) { - inet_ntop(AF_INET, &pdp.ms_addr, buf, sizeof(buf)); + inet_ntop(AF_INET, &pdp.ms_addr.ip4, buf, sizeof(buf)); printf("tid %"PRIu64" ms_addr %s ", pdp.u.v0.tid, buf); } else if (pdp.version == GTP_V1) { - inet_ntop(AF_INET, &pdp.ms_addr, buf, sizeof(buf)); + inet_ntop(AF_INET, &pdp.ms_addr.ip4, buf, sizeof(buf)); printf("tei %u/%u ms_addr %s ", pdp.u.v1.i_tei, pdp.u.v1.o_tei, buf); } - inet_ntop(AF_INET, &pdp.sgsn_addr, buf, sizeof(buf)); + inet_ntop(AF_INET, &pdp.sgsn_addr.ip4, buf, sizeof(buf)); printf("sgsn_addr %s\n", buf); return MNL_CB_OK; diff --git a/src/gtp.c b/src/gtp.c index e65b0b6..4eadc29 100644 --- a/src/gtp.c +++ b/src/gtp.c @@ -60,13 +60,15 @@ void gtp_tunnel_set_ms_ip4(struct gtp_tunnel *t, struct in_addr *ms_addr) { - t->ms_addr = *ms_addr; + t->ms_addr.family = AF_INET; + t->ms_addr.ip4 = *ms_addr; } EXPORT_SYMBOL(gtp_tunnel_set_ms_ip4); void gtp_tunnel_set_sgsn_ip4(struct gtp_tunnel *t, struct in_addr *sgsn_addr) { - t->sgsn_addr = *sgsn_addr; + t->sgsn_addr.family = AF_INET; + t->sgsn_addr.ip4 = *sgsn_addr; } EXPORT_SYMBOL(gtp_tunnel_set_sgsn_ip4); @@ -114,13 +116,13 @@ const struct in_addr *gtp_tunnel_get_ms_ip4(struct gtp_tunnel *t) { - return &t->ms_addr; + return &t->ms_addr.ip4; } EXPORT_SYMBOL(gtp_tunnel_get_ms_ip4); const struct in_addr *gtp_tunnel_get_sgsn_ip4(struct gtp_tunnel *t) { - return &t->sgsn_addr; + return &t->sgsn_addr.ip4; } EXPORT_SYMBOL(gtp_tunnel_get_sgsn_ip4); diff --git a/src/internal.h b/src/internal.h index 1754e3b..ce8a683 100644 --- a/src/internal.h +++ b/src/internal.h @@ -12,11 +12,19 @@ #include <stdint.h> #include <netinet/in.h> +struct gtp_addr { + sa_family_t family; + union { + struct in_addr ip4; + struct in6_addr ip6; + }; +}; + struct gtp_tunnel { int ifns; uint32_t ifidx; - struct in_addr ms_addr; - struct in_addr sgsn_addr; + struct gtp_addr ms_addr; + struct gtp_addr sgsn_addr; int gtp_version; union { struct { -- To view, visit https://gerrit.osmocom.org/c/libgtpnl/+/34734?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libgtpnl Gerrit-Branch: master Gerrit-Change-Id: Ifc7e3b03a723fb544d1c7b789101102b2c27b60e Gerrit-Change-Number: 34734 Gerrit-PatchSet: 2 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 0

[M] Change in osmo-ttcn3-hacks[master]: DIAMETER_Templates: Add S6b AAR and AAA message templates
by pespin 24 Oct '23

24 Oct '23

Attention is currently required from: lynxis lazus, osmith, pespin. Hello Jenkins Builder, lynxis lazus, osmith, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/34860?usp=email to look at the new patch set (#3). The following approvals got outdated and were removed: Code-Review+1 by osmith, Verified+1 by Jenkins Builder Change subject: DIAMETER_Templates: Add S6b AAR and AAA message templates ...................................................................... DIAMETER_Templates: Add S6b AAR and AAA message templates Related: OS#6229 Change-Id: I302d9a94e6ffbb62b6db81e1b14484355e55ebe5 --- M library/DIAMETER_Templates.ttcn 1 file changed, 87 insertions(+), 3 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks refs/changes/60/34860/3 -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/34860?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I302d9a94e6ffbb62b6db81e1b14484355e55ebe5 Gerrit-Change-Number: 34860 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu> Gerrit-MessageType: newpatchset

1 0

[M] Change in libosmocore[master]: gsmtap_util: Use Osmo IO instead of Osmo write queues
by pespin 24 Oct '23

24 Oct '23

Attention is currently required from: arehbein, daniel, laforge. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email ) Change subject: gsmtap_util: Use Osmo IO instead of Osmo write queues ...................................................................... Patch Set 8: (1 comment) File src/core/gsmtap_util.c: https://gerrit.osmocom.org/c/libosmocore/+/34743/comment/1e28f701_1c259d04 PS5, Line 478: osmo_iofd_setup(gti, gti->sink_fd, "gsmtap_inst.out", OSMO_IO_FD_MODE_READ_WRITE, &gsmtap_sink_ops, NULL); > it may make sense to look at what shutdown(sink_fd, SHUT_RD) may do to a UDP socket, perhaps it alre […] hmm, looks like shutdown is not really implemented in UDP sockets: https://stackoverflow.com/questions/60105082/using-shutdown-for-a-udp-socket In that case I guess we want to limit somehow the read buffer queue in the kernel to the minimum (even 0) to avoid wasting kb/mb of memory for each gsmtap socket sink with filled message that are never read. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34743?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: Iadbbef74e3add7001d84dd6b68f51eac293e44d0 Gerrit-Change-Number: 34743 Gerrit-PatchSet: 8 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Comment-Date: Tue, 24 Oct 2023 11:31:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: arehbein <arehbein(a)sysmocom.de> Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: daniel <dwillmann(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in libosmocore[master]: gsmtap_util: Use Osmo IO instead of Osmo write queues
by pespin 24 Oct '23

24 Oct '23

Attention is currently required from: arehbein, daniel, laforge. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email ) Change subject: gsmtap_util: Use Osmo IO instead of Osmo write queues ...................................................................... Patch Set 8: (1 comment) File src/core/gsmtap_util.c: https://gerrit.osmocom.org/c/libosmocore/+/34743/comment/814c540a_ef231d55 PS5, Line 478: osmo_iofd_setup(gti, gti->sink_fd, "gsmtap_inst.out", OSMO_IO_FD_MODE_READ_WRITE, &gsmtap_sink_ops, NULL); > I have restored pre-patch functionality for the sink in this patch and added https://gerrit.osmocom. […] it may make sense to look at what shutdown(sink_fd, SHUT_RD) may do to a UDP socket, perhaps it already instructs the kernel to directly discard wathever is received. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34743?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: Iadbbef74e3add7001d84dd6b68f51eac293e44d0 Gerrit-Change-Number: 34743 Gerrit-PatchSet: 8 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Comment-Date: Tue, 24 Oct 2023 11:09:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: arehbein <arehbein(a)sysmocom.de> Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Comment-In-Reply-To: daniel <dwillmann(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in pysim[master]: commands.py: Add support for multiple logical channels.
by dexter 24 Oct '23

24 Oct '23

Attention is currently required from: laforge. dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/pysim/+/34846?usp=email ) Change subject: commands.py: Add support for multiple logical channels. ...................................................................... Patch Set 2: (3 comments) File pySim/commands.py: https://gerrit.osmocom.org/c/pysim/+/34846/comment/6e5fcbd2_098448ac PS2, Line 42: if cla >> 4 in [0x0, 0xA, 0x8]: I find this a bit difficult to read. Maybe cla & 0xF0 in [0x00, 0x0A, 0x80] is better. Also I am a bit lost here. The positions of the logical channel match ISO/IEC 7816, section 5.1.1 but 0xA0 and 0x80 are proprietary classes that are not encoded as per ISO/IEC 7816? Maybe a spec reference would help? https://gerrit.osmocom.org/c/pysim/+/34846/comment/0701763c_40903ebc PS2, Line 93: if not cla: the caller must decide when to access the cache and when not? https://gerrit.osmocom.org/c/pysim/+/34846/comment/c4f8e6be_6324b2ab PS2, Line 526: return self._tp.send_apdu_checksw(self.cla_byte + self.cla4lchan('e0') + '0000%02x%s' % (len(payload)//2, payload)) Does this work? The class is E0 but E0 >> 4 is not in [0x0, 0xA, 0x8] (line 42) -- To view, visit https://gerrit.osmocom.org/c/pysim/+/34846?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: Ibe5650dedc0f7681acf82018a86f83377ba81d30 Gerrit-Change-Number: 34846 Gerrit-PatchSet: 2 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-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Tue, 24 Oct 2023 10:29:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0