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

[M] Change in osmo-bts[master]: l1sap: Accept RFC5993 and TS 101.318 HR GSM payload

by pespin

Attention is currently required from: laforge, fixeria, dexter. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/31417 ) Change subject: l1sap: Accept RFC5993 and TS 101.318 HR GSM payload ...................................................................... Patch Set 6: (4 comments) File src/common/l1sap.c: https://gerrit.osmocom.org/c/osmo-bts/+/31417/comment/0a00aaf2_241cdc79 PS6, Line 1274: if (OSMO_UNLIKELY((resp_msg->len != GSM_HR_BYTES + 1) && rfc5993)) { Should this be: "OSMO_UNLIKELY((resp_msg->len != GSM_HR_BYTES + 1) && !ts101318)" You are not interested on whether the BTS supports the other type, instead you are interested in knowing if the received format is not supported? https://gerrit.osmocom.org/c/osmo-bts/+/31417/comment/24136ea7_28b857fc PS6, Line 1279: } else if (OSMO_UNLIKELY((resp_msg->len != GSM_HR_BYTES) && ts101318)) { same here, !rfc5993 https://gerrit.osmocom.org/c/osmo-bts/+/31417/comment/1a8a9154_dfa6e64e PS6, Line 1940: if (lchan->type == GSM_LCHAN_TCH_H && rtp_pl_len == GSM_HR_BYTES please break each condition into one line with uniform formatting. https://gerrit.osmocom.org/c/osmo-bts/+/31417/comment/cbde0555_eca145a5 PS6, Line 1950: } else if (lchan->type == GSM_LCHAN_TCH_H && rtp_pl_len == GSM_HR_BYTES + 1 I see you are using "GSM_HR_BYTES" and "GSM_HR_BYTES + 1" in several places. It would be far cleared if you added defines for those (they can be at the start if this C file for instance). For instance something like: GSM_HR_BYTES_RTP_RFC5993 (GSM_HR_BYTES) GSM_HR_BYTES_RTP_TS101318 (GSM_HR_BYTES + 1) BTW, you are checking twice that "lchan->type == GSM_LCHAN_TCH_H", make levels of checks. -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/31417 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I453562da412fde5b928bd2b588129c58ec8e2a7e Gerrit-Change-Number: 31417 Gerrit-PatchSet: 6 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Wed, 03 May 2023 12:07:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

2 years, 1 month

1
0
0 0

[S] Change in libosmo-abis[master]: e1d: get rid of strange file descriptor registered check

by pespin

Attention is currently required from: tnt, dexter. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-abis/+/32569 ) Change subject: e1d: get rid of strange file descriptor registered check ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/libosmo-abis/+/32569 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-abis Gerrit-Branch: master Gerrit-Change-Id: I92c426271dc5455427471030fcf0749566e4c2ee Gerrit-Change-Number: 32569 Gerrit-PatchSet: 2 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: tnt <tnt(a)246tNt.com> Gerrit-Attention: tnt <tnt(a)246tNt.com> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Wed, 03 May 2023 11:56:01 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 1 month

1
0
0 0

[S] Change in libosmo-abis[master]: e1d: get rid of strange file descriptor registered check

by pespin

Attention is currently required from: tnt, dexter. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-abis/+/32569 ) Change subject: e1d: get rid of strange file descriptor registered check ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: For now it's fine. Ideally at some point the code in e1d should be improved to make sure that the assumption: "bfd->fd != -1" == "osmo_fd_registered(bfd)" -- To view, visit https://gerrit.osmocom.org/c/libosmo-abis/+/32569 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-abis Gerrit-Branch: master Gerrit-Change-Id: I92c426271dc5455427471030fcf0749566e4c2ee Gerrit-Change-Number: 32569 Gerrit-PatchSet: 2 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: tnt <tnt(a)246tNt.com> Gerrit-Attention: tnt <tnt(a)246tNt.com> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Wed, 03 May 2023 11:55:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

2 years, 1 month

1
0
0 0

[S] Change in libosmocore[master]: select: speed up osmo_fd_is_registered

by pespin

Attention is currently required from: dexter. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/32579 ) Change subject: select: speed up osmo_fd_is_registered ...................................................................... Patch Set 1: Code-Review-2 (1 comment) Patchset: PS1: the iteartion is done precisely to avoid that kind of hacky checks whith assumptions regarding the content of the list items which may not be true. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/32579 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I1ce894da39e3f2329c88666a5dda594b8bce4228 Gerrit-Change-Number: 32579 Gerrit-PatchSet: 1 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Wed, 03 May 2023 11:54:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 1 month

1
0
0 0

[S] Change in osmocom-bb[master]: layer23: modem: sm: Properly inform SM layer of ownsership transfer

by pespin

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmocom-bb/+/32588 ) Change subject: layer23: modem: sm: Properly inform SM layer of ownsership transfer ...................................................................... layer23: modem: sm: Properly inform SM layer of ownsership transfer Change-Id: I95ed63fe6442369ab3afb75cbd9908b87addbb3e --- M src/host/layer23/src/modem/sm.c 1 file changed, 10 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmocom-bb refs/changes/88/32588/1 diff --git a/src/host/layer23/src/modem/sm.c b/src/host/layer23/src/modem/sm.c index 2a404d4..56b223f 100644 --- a/src/host/layer23/src/modem/sm.c +++ b/src/host/layer23/src/modem/sm.c @@ -162,6 +162,7 @@ case OSMO_PRIM(OSMO_GPRS_SNDCP_SNSM_ACTIVATE, PRIM_OP_INDICATION): LOGP(DSM, LOGL_INFO, "%s(): Rx %s\n", __func__, pdu_name); rc = osmo_gprs_sndcp_prim_dispatch_snsm(sndcp_prim); + rc = 1; /* Tell SM that we take ownership of the prim. */ break; default: LOGP(DSM, LOGL_ERROR, "%s(): Unexpected Rx %s\n", __func__, pdu_name); -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/32588 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: I95ed63fe6442369ab3afb75cbd9908b87addbb3e Gerrit-Change-Number: 32588 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 1 month

1
0
0 0

[S] Change in osmocom-bb[master]: layer23: modem: grr: Log ignored CCCH ImmAss

by pespin

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmocom-bb/+/32589 ) Change subject: layer23: modem: grr: Log ignored CCCH ImmAss ...................................................................... layer23: modem: grr: Log ignored CCCH ImmAss There seems to be some bug when using virtphy where sometimes the received T2 and/or T3 in the ImmASs is not matching what we sent. This helps in showing the problem and not failing silently. Change-Id: Iaecd2616733d84f35a825916fe888142800b426b --- M src/host/layer23/src/modem/grr.c 1 file changed, 30 insertions(+), 3 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmocom-bb refs/changes/89/32589/1 diff --git a/src/host/layer23/src/modem/grr.c b/src/host/layer23/src/modem/grr.c index b6abae5..04a8b2a 100644 --- a/src/host/layer23/src/modem/grr.c +++ b/src/host/layer23/src/modem/grr.c @@ -238,13 +238,27 @@ struct osmo_gprs_rlcmac_prim *rlcmac_prim; /* Discard CS channel assignment */ - if ((ia->page_mode >> 4) == 0) + if ((ia->page_mode >> 4) == 0) { + LOGP(DRR, LOGL_INFO, + "%s(): Discard CS channel assignment\n", + __func__); return 0; + } - if (rr->state != GSM48_RR_ST_CONN_PEND) + if (rr->state != GSM48_RR_ST_CONN_PEND) { + LOGP(DRR, LOGL_INFO, + "%s(): rr_state != GSM48_RR_ST_CONN_PEND\n", + __func__); return 0; - if (!grr_match_req_ref(ms, &ia->req_ref)) + } + if (!grr_match_req_ref(ms, &ia->req_ref)) { + LOGP(DRR, LOGL_INFO, + "%s(): no req_ref match (RA=0x%02x, T1=%u, T3=%u, T2=%u)\n", __func__, + ia->req_ref.ra, ia->req_ref.t1, + ia->req_ref.t3_high << 3 | ia->req_ref.t3_low, + ia->req_ref.t2); return 0; + } if (rsl_dec_chan_nr(ia->chan_desc.chan_nr, &ch_type, &ch_subch, &ch_ts) != 0) { LOGP(DRR, LOGL_ERROR, -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/32589 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: Iaecd2616733d84f35a825916fe888142800b426b Gerrit-Change-Number: 32589 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 1 month

1
0
0 0

[S] Change in osmocom-bb[master]: layer23: modem: sndcp: Forward LL-XID.req SNDCP->LLC

by pespin

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmocom-bb/+/32587 ) Change subject: layer23: modem: sndcp: Forward LL-XID.req SNDCP->LLC ...................................................................... layer23: modem: sndcp: Forward LL-XID.req SNDCP->LLC Change-Id: Icbc033cd0659bba75bb280a3d46e76edd2e20226 --- M src/host/layer23/src/modem/sndcp.c 1 file changed, 10 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmocom-bb refs/changes/87/32587/1 diff --git a/src/host/layer23/src/modem/sndcp.c b/src/host/layer23/src/modem/sndcp.c index 00c21bc..fd6aada 100644 --- a/src/host/layer23/src/modem/sndcp.c +++ b/src/host/layer23/src/modem/sndcp.c @@ -117,6 +117,7 @@ switch (OSMO_PRIM_HDR(&llc_prim->oph)) { case OSMO_PRIM(OSMO_GPRS_LLC_LL_UNITDATA, PRIM_OP_REQUEST): + case OSMO_PRIM(OSMO_GPRS_LLC_LL_XID, PRIM_OP_REQUEST): LOGP(DSNDCP, LOGL_DEBUG, "%s(): Rx %s TLLI=0x%08x SAPI=%s L3=[%s]\n", __func__, pdu_name, llc_prim->ll.tlli, osmo_gprs_llc_sapi_name(llc_prim->ll.sapi), -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/32587 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: Icbc033cd0659bba75bb280a3d46e76edd2e20226 Gerrit-Change-Number: 32587 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 1 month

1
0
0 0

[S] Change in osmocom-bb[master]: layer23: modem: gmm: Activate PDP Context of APN after GPRS attach

by pespin

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmocom-bb/+/32590 ) Change subject: layer23: modem: gmm: Activate PDP Context of APN after GPRS attach ...................................................................... layer23: modem: gmm: Activate PDP Context of APN after GPRS attach Change-Id: Icb667a6a3359267d2eb20e11a40eb27a8c3f7e2d --- M src/host/layer23/src/modem/gmm.c 1 file changed, 16 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmocom-bb refs/changes/90/32590/1 diff --git a/src/host/layer23/src/modem/gmm.c b/src/host/layer23/src/modem/gmm.c index 62c6d5e..f3a936a 100644 --- a/src/host/layer23/src/modem/gmm.c +++ b/src/host/layer23/src/modem/gmm.c @@ -44,12 +44,14 @@ #include <osmocom/bb/common/apn.h> #include <osmocom/bb/common/ms.h> #include <osmocom/bb/modem/gmm.h> +#include <osmocom/bb/modem/sm.h> #include <osmocom/bb/modem/modem.h> static int modem_gmm_prim_up_cb(struct osmo_gprs_gmm_prim *gmm_prim, void *user_data) { const char *pdu_name = osmo_gprs_gmm_prim_name(gmm_prim); struct osmocom_ms *ms = user_data; + struct osmobb_apn *apn; int rc = 0; switch (gmm_prim->oph.sap) { @@ -61,6 +63,11 @@ __func__, pdu_name, gmm_prim->gmmreg.attach_cnf.acc.allocated_ptmsi); ms->subscr.ptmsi = gmm_prim->gmmreg.attach_cnf.acc.allocated_ptmsi; app_data.modem_state = MODEM_ST_ATTACHED; + /* Activate APN if not yet already: */ + apn = llist_first_entry_or_null(&ms->gprs.apn_list, struct osmobb_apn, list); + if (!apn || apn->cfg.shutdown) + break; + modem_sm_smreg_pdp_act_req(ms, apn); } else { uint8_t cause = gmm_prim->gmmreg.attach_cnf.rej.cause; LOGP(DGMM, LOGL_ERROR, "%s(): Rx %s: Attach rejected, cause=%u (%s)\n", -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/32590 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: Icb667a6a3359267d2eb20e11a40eb27a8c3f7e2d Gerrit-Change-Number: 32590 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 1 month

1
0
0 0

[M] Change in pysim[master]: pySim-shell: fix compatibility problem with cmd2 >= 2.0.0 (Settable)

by osmith

Attention is currently required from: fixeria, dexter. osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/pysim/+/32532 ) Change subject: pySim-shell: fix compatibility problem with cmd2 >= 2.0.0 (Settable) ...................................................................... Patch Set 12: (1 comment) File pySim-shell.py: https://gerrit.osmocom.org/c/pysim/+/32532/comment/2ba819e5_7f79db6b PS12, Line 162: # pylint: disable=E1121 > I have tested this also with too-many-function-args but it seems not to be recognized by pylint. […] We currently use a rather old pylint version (the one in debian buster), so apparently it doesn't disable the check for the whole block as it says in the documentation. Adding it at the end of each line, and adding a \ for multiline statements makes it work with the current version in the meantime. ``` self.add_settable(cmd2.Settable('numeric_path', bool, 'Print File IDs instead of names', self, \ onchange_cb=self._onchange_numeric_path)) # pylint: disable=too-many-function-args self.add_settable(cmd2.Settable('conserve_write', bool, 'Read and compare before write', self, \ onchange_cb=self._onchange_conserve_write)) # pylint: disable=too-many-function-args self.add_settable(cmd2.Settable('json_pretty_print', bool, 'Pretty-Print JSON output', self)) # pylint: disable=too-many-function-args self.add_settable(cmd2.Settable('apdu_trace', bool, 'Trace and display APDUs exchanged with card', self, \ onchange_cb=self._onchange_apdu_trace)) # pylint: disable=too-many-function-args ``` as diff: ``` diff --git a/pySim-shell.py b/pySim-shell.py index 1e390d2..f265f5d 100755 --- a/pySim-shell.py +++ b/pySim-shell.py @@ -159,14 +159,13 @@ class PysimApp(cmd2.Cmd): self.add_settable(cmd2.Settable('apdu_trace', bool, 'Trace and display APDUs exchanged with card', onchange_cb=self._onchange_apdu_trace)) else: - # pylint: disable=E1121 - self.add_settable(cmd2.Settable('numeric_path', bool, 'Print File IDs instead of names', self, - onchange_cb=self._onchange_numeric_path)) - self.add_settable(cmd2.Settable('conserve_write', bool, 'Read and compare before write', self, - onchange_cb=self._onchange_conserve_write)) - self.add_settable(cmd2.Settable('json_pretty_print', bool, 'Pretty-Print JSON output', self)) - self.add_settable(cmd2.Settable('apdu_trace', bool, 'Trace and display APDUs exchanged with card', self, - onchange_cb=self._onchange_apdu_trace)) + self.add_settable(cmd2.Settable('numeric_path', bool, 'Print File IDs instead of names', self, \ + onchange_cb=self._onchange_numeric_path)) # pylint: disable=too-many-function-args + self.add_settable(cmd2.Settable('conserve_write', bool, 'Read and compare before write', self, \ + onchange_cb=self._onchange_conserve_write)) # pylint: disable=too-many-function-args + self.add_settable(cmd2.Settable('json_pretty_print', bool, 'Pretty-Print JSON output', self)) # pylint: disable=too-many-function-args + self.add_settable(cmd2.Settable('apdu_trace', bool, 'Trace and display APDUs exchanged with card', self, \ + onchange_cb=self._onchange_apdu_trace)) # pylint: disable=too-many-function-args self.equip(card, rs) def equip(self, card, rs): ``` -- To view, visit https://gerrit.osmocom.org/c/pysim/+/32532 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I38efe4702277ee092a5542d7d659df08cb0adeff Gerrit-Change-Number: 32532 Gerrit-PatchSet: 12 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: osmith <osmith(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Wed, 03 May 2023 11:48:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: comment

2 years, 1 month

1
0
0 0

[S] Change in libosmo-gprs[master]: sndcp: Allow cb users taking ownership of primitives

by pespin

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-gprs/+/32586 ) Change subject: sndcp: Allow cb users taking ownership of primitives ...................................................................... sndcp: Allow cb users taking ownership of primitives Change-Id: Ie3ddc835d8393e934d3f357108c81f639247c485 --- M src/sndcp/sndcp_prim.c 1 file changed, 17 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-gprs refs/changes/86/32586/1 diff --git a/src/sndcp/sndcp_prim.c b/src/sndcp/sndcp_prim.c index b9b5041..9281784 100644 --- a/src/sndcp/sndcp_prim.c +++ b/src/sndcp/sndcp_prim.c @@ -304,7 +304,10 @@ rc = g_sndcp_ctx->sndcp_up_cb(sndcp_prim, g_sndcp_ctx->sndcp_up_cb_user_data); else rc = sndcp_up_cb_dummy(sndcp_prim, g_sndcp_ctx->sndcp_up_cb_user_data); - msgb_free(sndcp_prim->oph.msg); + if (rc != 1) + msgb_free(sndcp_prim->oph.msg); + else + rc = 0; return rc; } @@ -443,7 +446,10 @@ rc = g_sndcp_ctx->sndcp_down_cb(llc_prim, g_sndcp_ctx->sndcp_down_cb_user_data); else rc = sndcp_down_cb_dummy(llc_prim, g_sndcp_ctx->sndcp_down_cb_user_data); - msgb_free(llc_prim->oph.msg); + if (rc != 1) + msgb_free(llc_prim->oph.msg); + else + rc = 0; return rc; } -- To view, visit https://gerrit.osmocom.org/c/libosmo-gprs/+/32586 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-gprs Gerrit-Branch: master Gerrit-Change-Id: Ie3ddc835d8393e934d3f357108c81f639247c485 Gerrit-Change-Number: 32586 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 1 month

1
0
0 0

2025

2024

2023

2022

gerrit-log May 2023