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

Change in libosmocore[master]: iuup: Rework API to support RFCI IDs != RFCI index

by pespin

pespin has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/28182 ) Change subject: iuup: Rework API to support RFCI IDs != RFCI index ...................................................................... iuup: Rework API to support RFCI IDs != RFCI index The initially merged IuUP API and implementation assumed that RFCI with ID was always in the position of its ID inside the list of RFCIs. This was the case for messages sent by ip.access nano3g as well as our own osmocom implementation. However it was noticed that other nodes from other vendors actually use other order, as allowed by the IuUP message format. Hence, we need to break the assumption and provide explicit ID information in the list. NOTICE: This commit breaks API and ABI compatibility with older versions of libosmogsm, but not with any previous release of libosmocore since the API is only available in master so far (it was added in 9fe1f9fb0b3197cdecaa55017d3afd7355e59c36). Similary, it's only user (osmo-mgw) only uses the API in master, so there's no API breakage with older releases. Related: SYS#5969 Change-Id: Ib21cee2e30bf83dff4e167f79541796007af9845 --- M TODO-RELEASE M include/osmocom/gsm/iuup.h M src/gsm/iuup.c M tests/iuup/iuup_test.c 4 files changed, 76 insertions(+), 48 deletions(-) Approvals: laforge: Looks good to me, approved osmith: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/TODO-RELEASE b/TODO-RELEASE index 0ae28c0..9f9f9d9 100644 --- a/TODO-RELEASE +++ b/TODO-RELEASE @@ -11,3 +11,4 @@ libosmogsm add struct member Add codec_list_bss_supported to gsm0808_handover_request_ack (more_items flag ensures ABI compat) libosmogb ABI BREAKAGE Add reset_ack_notification function pointer to struct bssgp_bvc_fsm_ops libosmogsm add API Add rach_tx_integer_raw2val() in gsm_utils.h +libosmogsm add API iuup.h diff --git a/include/osmocom/gsm/iuup.h b/include/osmocom/gsm/iuup.h index 1ea2b2e..db3d5e0 100644 --- a/include/osmocom/gsm/iuup.h +++ b/include/osmocom/gsm/iuup.h @@ -42,6 +42,14 @@ uint32_t t_ms; /* time in ms */ uint32_t n_max; /* max number of repetitions */ }; +struct osmo_iuup_rfci { + uint8_t used:1, + spare1:1, + id:6; + uint8_t spare2:4, + IPTI:4; /* values range 0-15, 4 bits */; + uint16_t subflow_sizes[IUUP_MAX_SUBFLOWS]; +}; struct osmo_iuup_rnl_config { /* transparent (true) or SMpSDU (false): */ bool transparent; @@ -56,9 +64,8 @@ uint16_t supported_versions_mask; uint8_t num_rfci; uint8_t num_subflows; - uint16_t subflow_sizes[IUUP_MAX_RFCIS][IUUP_MAX_SUBFLOWS]; bool IPTIs_present; - uint8_t IPTIs[IUUP_MAX_RFCIS]; /* values range 0-15, 4 bits */ + struct osmo_iuup_rfci rfci[IUUP_MAX_RFCIS]; /* TODO: Indication of delivery of erroneous SDUs*/ struct osmo_iuup_rnl_config_timer t_init; @@ -84,9 +91,8 @@ uint8_t data_pdu_type; uint8_t num_rfci; uint8_t num_subflows; - uint16_t subflow_sizes[IUUP_MAX_RFCIS][IUUP_MAX_SUBFLOWS]; bool IPTIs_present; - uint8_t IPTIs[IUUP_MAX_RFCIS]; /* values range 0-15, 4 bits */ + struct osmo_iuup_rfci rfci[IUUP_MAX_RFCIS]; } initialization; struct { } rate_control; diff --git a/src/gsm/iuup.c b/src/gsm/iuup.c index eb595ce..edcea3b 100644 --- a/src/gsm/iuup.c +++ b/src/gsm/iuup.c @@ -286,13 +286,10 @@ struct iuup_ctrl_init_rfci_hdr *ihdr_rfci; struct iuup_ctrl_init_tail *itail; unsigned int i, j; - uint8_t num_subflows, num_rfci; uint16_t payload_crc; + uint8_t rfci_cnt; struct msgb *msg; - num_subflows = iui->config.num_subflows; - num_rfci = iui->config.num_rfci; - itp = osmo_iuup_tnl_prim_alloc(iui, OSMO_IUUP_TNL_UNITDATA, PRIM_OP_REQUEST, IUUP_MSGB_SIZE); msg = itp->oph.msg; @@ -307,42 +304,68 @@ ihdr = (struct iuup_ctrl_init_hdr *)msgb_put(msg, sizeof(*ihdr)); ihdr->chain_ind = 0; /* this frame is the last frame for the procedure. TODO: support several */ - ihdr->num_subflows_per_rfci = num_subflows; + ihdr->num_subflows_per_rfci = iui->config.num_subflows; ihdr->ti = iui->config.IPTIs_present ? 1 : 0; ihdr->spare = 0; /* RFCI + subflow size part: */ - for (i = 0; i < num_rfci; i++) { - bool last = (i+1 == num_rfci); - uint8_t len_size = 1; - for (j = 0; j < num_subflows; j++) { - if (iui->config.subflow_sizes[i][j] > UINT8_MAX) + rfci_cnt = 0; + for (i = 0; i < ARRAY_SIZE(iui->config.rfci); i++) { + bool last; + uint8_t len_size; + struct osmo_iuup_rfci *rfci = &iui->config.rfci[i]; + if (!rfci->used) + continue; + rfci_cnt++; + last = (rfci_cnt == iui->config.num_rfci); + + len_size = 1; + for (j = 0; j < iui->config.num_subflows; j++) { + if (rfci->subflow_sizes[j] > UINT8_MAX) len_size = 2; } - ihdr_rfci = (struct iuup_ctrl_init_rfci_hdr *)msgb_put(msg, sizeof(*ihdr_rfci) + len_size * num_subflows); - ihdr_rfci->rfci = i; + + ihdr_rfci = (struct iuup_ctrl_init_rfci_hdr *)msgb_put(msg, sizeof(*ihdr_rfci) + len_size * iui->config.num_subflows); + ihdr_rfci->rfci = rfci->id; ihdr_rfci->li = len_size - 1; ihdr_rfci->lri = last; if (len_size == 2) { uint16_t *buf = (uint16_t *)&ihdr_rfci->subflow_length[0]; - for (j = 0; j < num_subflows; j++) - osmo_store16be(iui->config.subflow_sizes[i][j], buf++); + for (j = 0; j < iui->config.num_subflows; j++) + osmo_store16be(rfci->subflow_sizes[j], buf++); } else { - for (j = 0; j < num_subflows; j++) - ihdr_rfci->subflow_length[j] = iui->config.subflow_sizes[i][j]; + for (j = 0; j < iui->config.num_subflows; j++) + ihdr_rfci->subflow_length[j] = rfci->subflow_sizes[j]; } + /* early loop termination: */ + if (last) + break; + } + /* Sanity check: */ + if (rfci_cnt != iui->config.num_rfci) { + LOGP(DLIUUP, LOGL_ERROR, "rfci_cnt %u != num_rfci %u\n", + rfci_cnt, iui->config.num_rfci); + msgb_free(msg); + return NULL; } if (iui->config.IPTIs_present) { - uint8_t num_bytes = (num_rfci + 1) / 2; + uint8_t num_bytes = (iui->config.num_rfci + 1) / 2; uint8_t *buf = msgb_put(msg, num_bytes); - for (i = 0; i < num_bytes - 1; i++) - buf[i] = iui->config.IPTIs[i*2] << 4 | - (iui->config.IPTIs[i*2 + 1] & 0x0f); - buf[i] = iui->config.IPTIs[i*2] << 4; - if (!(num_rfci & 0x01)) /* is even: */ - buf[i] |= (iui->config.IPTIs[i*2 + 1] & 0x0f); - + rfci_cnt = 0; + for (i = 0; i < ARRAY_SIZE(iui->config.rfci); i++) { + struct osmo_iuup_rfci *rfci = &iui->config.rfci[i]; + if (!rfci->used) + continue; + if (!(rfci_cnt & 0x01)) /* is even: */ + buf[rfci_cnt / 2] = (((uint8_t)rfci->IPTI) << 4); + else + buf[rfci_cnt / 2] |= (rfci->IPTI & 0x0F); + rfci_cnt++; + /* early loop termination: */ + if (rfci_cnt == iui->config.num_rfci) + break; + } } itail = (struct iuup_ctrl_init_tail *)msgb_put(msg, sizeof(*itail)); @@ -368,12 +391,8 @@ irp->u.status.u.initialization.data_pdu_type = iui->config.data_pdu_type; irp->u.status.u.initialization.num_subflows = iui->config.num_subflows; irp->u.status.u.initialization.num_rfci = iui->config.num_rfci; - memcpy(irp->u.status.u.initialization.subflow_sizes, iui->config.subflow_sizes, - IUUP_MAX_RFCIS * IUUP_MAX_SUBFLOWS * sizeof(iui->config.subflow_sizes[0][0])); irp->u.status.u.initialization.IPTIs_present = iui->config.IPTIs_present; - if (iui->config.IPTIs_present) - memcpy(irp->u.status.u.initialization.IPTIs, iui->config.IPTIs, - IUUP_MAX_RFCIS * sizeof(iui->config.IPTIs[0])); + memcpy(irp->u.status.u.initialization.rfci, iui->config.rfci, sizeof(iui->config.rfci)); return irp; } @@ -530,24 +549,27 @@ ihdr_rfci = (struct iuup_ctrl_init_rfci_hdr *)ihdr->rfci_data; do { + struct osmo_iuup_rfci *rfci = &iui->config.rfci[num_rfci]; uint8_t l_size_bytes = ihdr_rfci->li + 1; is_last = ihdr_rfci->lri; - if (ihdr_rfci->rfci != num_rfci) { - LOGPFSML(iui->fi, LOGL_NOTICE, "Initialization: Unexpected RFCI %u at position %u received\n", - ihdr_rfci->rfci, num_rfci); + if (num_rfci >= IUUP_MAX_RFCIS) { + LOGPFSML(iui->fi, LOGL_NOTICE, "Initialization: Too many RFCIs received (%u)\n", + num_rfci); err_cause = IUUP_ERR_CAUSE_UNEXPECTED_RFCI; goto send_nack; } + rfci->used = 1; + rfci->id = ihdr_rfci->rfci; if (l_size_bytes == 2) { uint16_t *subflow_size = (uint16_t *)ihdr_rfci->subflow_length; for (i = 0; i < ihdr->num_subflows_per_rfci; i++) { - iui->config.subflow_sizes[ihdr_rfci->rfci][i] = osmo_load16be(subflow_size); + rfci->subflow_sizes[i] = osmo_load16be(subflow_size); subflow_size++; } } else { uint8_t *subflow_size = ihdr_rfci->subflow_length; for (i = 0; i < ihdr->num_subflows_per_rfci; i++) { - iui->config.subflow_sizes[ihdr_rfci->rfci][i] = *subflow_size; + rfci->subflow_sizes[i] = *subflow_size; subflow_size++; } } @@ -561,19 +583,18 @@ uint8_t num_bytes = (num_rfci + 1) / 2; iui->config.IPTIs_present = true; for (i = 0; i < num_bytes - 1; i++) { - iui->config.IPTIs[i*2] = *buf >> 4; - iui->config.IPTIs[i*2 + 1] = *buf & 0x0f; + iui->config.rfci[i*2].IPTI = *buf >> 4; + iui->config.rfci[i*2 + 1].IPTI = *buf & 0x0f; buf++; } - iui->config.IPTIs[i*2] = *buf >> 4; + iui->config.rfci[i*2].IPTI = *buf >> 4; if (!(num_rfci & 0x01)) /* is even: */ - iui->config.IPTIs[i*2 + 1] = *buf & 0x0f; + iui->config.rfci[i*2 + 1].IPTI = *buf & 0x0f; buf++; itail = (struct iuup_ctrl_init_tail *)buf; } else { itail = (struct iuup_ctrl_init_tail *)ihdr_rfci; } - if (itail->data_pdu_type > 1) { LOGPFSML(iui->fi, LOGL_NOTICE, "Initialization: Unexpected Data PDU Type %u received\n", itail->data_pdu_type); err_cause = IUUP_ERR_CAUSE_UNEXPECTED_VALUE; diff --git a/tests/iuup/iuup_test.c b/tests/iuup/iuup_test.c index a58481d..6e3d8ca 100644 --- a/tests/iuup/iuup_test.c +++ b/tests/iuup/iuup_test.c @@ -19,14 +19,14 @@ .supported_versions_mask = 0x0001, .num_rfci = 3, .num_subflows = 3, - .subflow_sizes = { - {81, 103, 60}, - {39, 0, 0}, - {0, 0, 0}, + .IPTIs_present = true, + .rfci = { + {.used = 1, .id = 0, .IPTI = 1, .subflow_sizes = {81, 103, 60} }, + {.used = 1, .id = 1, .IPTI = 7, .subflow_sizes = {39, 0, 0} }, + {.used = 1, .id = 2, .IPTI = 1, .subflow_sizes = {0, 0, 0} }, }, /* .delivery_err_sdu = All set to 0 (YES) by default, */ .IPTIs_present = true, - .IPTIs = {1, 7, 1}, .t_init = { .t_ms = IUUP_TIMER_INIT_T_DEFAULT, .n_max = IUUP_TIMER_INIT_N_DEFAULT }, .t_ta = { .t_ms = IUUP_TIMER_TA_T_DEFAULT, .n_max = IUUP_TIMER_TA_N_DEFAULT }, .t_rc = { .t_ms = IUUP_TIMER_RC_T_DEFAULT, .n_max = IUUP_TIMER_RC_N_DEFAULT }, -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/28182 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ib21cee2e30bf83dff4e167f79541796007af9845 Gerrit-Change-Number: 28182 Gerrit-PatchSet: 4 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

3 years

1
0
0 0

Change in pysim[master]: ts_102_221: fix SFI generation

by dexter

dexter has uploaded this change for review. ( https://gerrit.osmocom.org/c/pysim/+/28184 ) Change subject: ts_102_221: fix SFI generation ...................................................................... ts_102_221: fix SFI generation The generation of the SFI does not work. The result is always a zero length TLV IE. Change-Id: Iaa38d2be4719f12c1d7b30a8befe278f1ed78ac1 --- M pySim/ts_102_221.py 1 file changed, 4 insertions(+), 3 deletions(-) git pull ssh://gerrit.osmocom.org:29418/pysim refs/changes/84/28184/1 diff --git a/pySim/ts_102_221.py b/pySim/ts_102_221.py index bf63f55..e7aff97 100644 --- a/pySim/ts_102_221.py +++ b/pySim/ts_102_221.py @@ -190,10 +190,11 @@ # of the TLV value field. In this case, bits b3 to b1 shall be set to 0 class Shift3RAdapter(Adapter): def _decode(self, obj, context, path): - return obj >> 3 + return int.from_bytes(obj, 'big') >> 3 def _encode(self, obj, context, path): - return obj << 3 - _construct = COptional(Shift3RAdapter(Byte)) + val = int(obj) << 3 + return val.to_bytes(1, 'big') + _construct = COptional(Shift3RAdapter(Bytes(1))) # ETSI TS 102 221 11.1.1.4.9 class LifeCycleStatusInteger(BER_TLV_IE, tag=0x8A): -- To view, visit https://gerrit.osmocom.org/c/pysim/+/28184 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: Iaa38d2be4719f12c1d7b30a8befe278f1ed78ac1 Gerrit-Change-Number: 28184 Gerrit-PatchSet: 1 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: newchange

3 years

1
0
0 0

Change in pysim[master]: ts_102_221: The BTLV IEs FILE SIZE and TOTAL FILE SIZE have a min length

by dexter

dexter has uploaded this change for review. ( https://gerrit.osmocom.org/c/pysim/+/28185 ) Change subject: ts_102_221: The BTLV IEs FILE SIZE and TOTAL FILE SIZE have a min length ...................................................................... ts_102_221: The BTLV IEs FILE SIZE and TOTAL FILE SIZE have a min length The TLV IEs FILE SIZE and TOTAL FILE SIZE have a minimum length of 2 byte. Even when the length is in the single digit range two bytes must be used. See also: ETSI TS 102 221, section 11.1.1.4.1 and 11.1.1.4.2 Change-Id: Ief113ce8fe3bcae2c9fb2ff4138df9ccf98d26ff --- M pySim/construct.py M pySim/ts_102_221.py 2 files changed, 6 insertions(+), 3 deletions(-) git pull ssh://gerrit.osmocom.org:29418/pysim refs/changes/85/28185/1 diff --git a/pySim/construct.py b/pySim/construct.py index fcbadd8..a43cd29 100644 --- a/pySim/construct.py +++ b/pySim/construct.py @@ -208,10 +208,11 @@ class GreedyInteger(Construct): """A variable-length integer implementation, think of combining GrredyBytes with BytesInteger.""" - def __init__(self, signed=False, swapped=False): + def __init__(self, signed=False, swapped=False, minlen=0): super().__init__() self.signed = signed self.swapped = swapped + self.minlen = minlen def _parse(self, stream, context, path): data = stream_read_entire(stream, path) @@ -239,6 +240,8 @@ if not isinstance(obj, integertypes): raise IntegerError(f"value {obj} is not an integer", path=path) length = self.__bytes_required(obj) + if length < self.minlen: + length = length + (self.minlen - length) try: data = integer2bytes(obj, length, self.signed) except ValueError as e: diff --git a/pySim/ts_102_221.py b/pySim/ts_102_221.py index e7aff97..08e836c 100644 --- a/pySim/ts_102_221.py +++ b/pySim/ts_102_221.py @@ -80,11 +80,11 @@ # ETSI TS 102 221 11.1.1.4.2 class FileSize(BER_TLV_IE, tag=0x80): - _construct = GreedyInteger() + _construct = GreedyInteger(minlen=2) # ETSI TS 102 221 11.1.1.4.2 class TotalFileSize(BER_TLV_IE, tag=0x81): - _construct = GreedyInteger() + _construct = GreedyInteger(minlen=2) # ETSI TS 102 221 11.1.1.4.3 class FileDescriptor(BER_TLV_IE, tag=0x82): -- To view, visit https://gerrit.osmocom.org/c/pysim/+/28185 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: Ief113ce8fe3bcae2c9fb2ff4138df9ccf98d26ff Gerrit-Change-Number: 28185 Gerrit-PatchSet: 1 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: newchange

3 years

1
0
0 0

Change in libosmocore[master]: coding: do not reset codec ID on receipt of DTX frames

by fixeria

fixeria has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/28175 ) Change subject: coding: do not reset codec ID on receipt of DTX frames ...................................................................... coding: do not reset codec ID on receipt of DTX frames Parsing of CMI/CMC/CMR from AMR's special DTX frames is currently not implemented. It's better to keep the old stored value rather than resetting it to 0 every time we receive such a frame. Add TODO comments for each DTX frame type. Change-Id: Ic4edbb8ab873fe0bdd69a8710803628bc4f447d0 Related: OS#5570 --- M src/coding/gsm0503_coding.c 1 file changed, 28 insertions(+), 23 deletions(-) Approvals: laforge: Looks good to me, approved pespin: Looks good to me, but someone else must approve osmith: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/src/coding/gsm0503_coding.c b/src/coding/gsm0503_coding.c index 715a361..2b59b7b 100644 --- a/src/coding/gsm0503_coding.c +++ b/src/coding/gsm0503_coding.c @@ -2157,7 +2157,7 @@ { sbit_t iB[912], cB[456], h; ubit_t d[244], p[6], conv[250]; - int i, j, k, best = 0, rv, len, steal = 0, id = 0; + int i, j, k, best = 0, rv, len, steal = 0, id = -1; ubit_t cBd[456]; *n_errors = 0; *n_bits_total = 0; static ubit_t sid_first_dummy[64] = { 0 }; @@ -2202,6 +2202,7 @@ * (AFS_SID_UPDATE_CN). */ if (dtx_prev != AFS_SID_UPDATE) break; + /* TODO: parse CMI _and_ CMC/CMR (16 + 16 bit) */ *dtx = AFS_SID_UPDATE_CN; extract_afs_sid_update(sid_update_enc, cB); @@ -2217,19 +2218,19 @@ tch_amr_sid_update_append(conv, 1, (codec_mode_req) ? codec[*ft] - : codec[id]); + : codec[id > 0 ? id : 0]); tch_amr_reassemble(tch_data, conv, 39); len = 5; goto out; - case AFS_SID_FIRST: + case AFS_SID_FIRST: /* TODO: parse CMI or CMC/CMR (16 bit) */ tch_amr_sid_update_append(sid_first_dummy, 0, (codec_mode_req) ? codec[*ft] - : codec[id]); + : codec[id > 0 ? id : 0]); tch_amr_reassemble(tch_data, conv, 39); len = 5; goto out; - case AFS_SID_UPDATE: - case AFS_ONSET: + case AFS_SID_UPDATE: /* TODO: parse CMI _and_ CMC/CMR (16 + 16 bit) */ + case AFS_ONSET: /* TODO: parse CMI or CMC/CMR (N * 16 - M bit) */ len = 0; goto out; default: @@ -2399,10 +2400,12 @@ out: /* Change codec request / indication, if frame is valid */ - if (codec_mode_req) - *cmr = id; - else - *ft = id; + if (id != -1) { + if (codec_mode_req) + *cmr = id; + else + *ft = id; + } return len; } @@ -2621,7 +2624,7 @@ { sbit_t iB[912], cB[456], h; ubit_t d[244], p[6], conv[135]; - int i, j, k, best = 0, rv, len, steal = 0, id = 0; + int i, j, k, best = 0, rv, len, steal = 0, id = -1; ubit_t cBd[456]; static ubit_t sid_first_dummy[64] = { 0 }; @@ -2684,7 +2687,7 @@ /* TODO: detect and handle AHS_SID_UPDATE + AHS_SID_UPDATE_INH */ switch (*dtx) { - case AHS_SID_UPDATE: + case AHS_SID_UPDATE: /* TODO: parse CMI _and_ CMC/CMR (16 + 16 bit) */ /* cB[] contains 16 bits of coded in-band data and 212 bits containing * the identification marker. We need to unmap/deinterleave 114 odd * bits from the last two blocks, 114 even bits from the first two @@ -2717,21 +2720,21 @@ tch_amr_sid_update_append(conv, 1, (codec_mode_req) ? codec[*ft] - : codec[id]); + : codec[id > 0 ? id : 0]); tch_amr_reassemble(tch_data, conv, 39); len = 5; goto out; - case AHS_SID_FIRST_P2: + case AHS_SID_FIRST_P2: /* TODO: parse CMI or CMC/CMR (N * 16 - M bit) */ tch_amr_sid_update_append(sid_first_dummy, 0, (codec_mode_req) ? codec[*ft] - : codec[id]); + : codec[id > 0 ? id : 0]); tch_amr_reassemble(tch_data, sid_first_dummy, 39); len = 5; goto out; - case AHS_ONSET: - case AHS_SID_FIRST_INH: - case AHS_SID_UPDATE_INH: - case AHS_SID_FIRST_P1: + case AHS_ONSET: /* TODO: parse CMI or CMC/CMR (N * 16 - M bit) */ + case AHS_SID_FIRST_INH: /* TODO: parse CMI or CMC/CMR (16 bit) */ + case AHS_SID_UPDATE_INH: /* TODO: parse CMI or CMC/CMR (16 bit) */ + case AHS_SID_FIRST_P1: /* TODO: parse CMI or CMC/CMR (16 bit) */ len = 0; goto out; default: @@ -2885,10 +2888,12 @@ out: /* Change codec request / indication, if frame is valid */ - if (codec_mode_req) - *cmr = id; - else - *ft = id; + if (id != -1) { + if (codec_mode_req) + *cmr = id; + else + *ft = id; + } return len; } -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/28175 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ic4edbb8ab873fe0bdd69a8710803628bc4f447d0 Gerrit-Change-Number: 28175 Gerrit-PatchSet: 2 Gerrit-Owner: fixeria <vyanitskiy(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-MessageType: merged

3 years

1
0
0 0

Change in libosmocore[master]: coding: fix decoding of AHS_SID_UPDATE frames (BER ~50%)

by fixeria

fixeria has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/28166 ) Change subject: coding: fix decoding of AHS_SID_UPDATE frames (BER ~50%) ...................................................................... coding: fix decoding of AHS_SID_UPDATE frames (BER ~50%) As was demonstrated in [1], there is a TCH/AHS specific problem in libosmocoding causing unexpected BER ~50% in decoded AHS_SID_UPDATE frames. The reason is that A[H]S_SID_UPDATE employs quite tricky interleaving algorithm, which is different from the algorithm used by normal TCH/AHS speech frames or A[F]S_SID_UPDATE frames. An AHS_SID_UPDATE frame consists of two halves (228 bits each): +---------+--------------------|---------+--------------------+ | in-band | SID marker | in-band | coded data | +---------+--------------------|---------+--------------------+ | 16 bits | 212 bits | 16 bits | 212 bits | The first half contains coded in-band signalling data (16 bits) and the identification marker (212 bits), which allows to detect that it's an AHS_SID_UPDATE. This half is carried by even bits of the first two bursts and odd bits of the last two bursts. The other half also contains the in-band data (16 bits), while the remaining 212 bits contain encoded SID_UPDATE (212 bits). This half is carried by even bits of the last two bursts and odd bits of the first two bursts. Current implementation does not use odd bits of the first two bursts at all, so buffer cB[] in gsm0503_tch_ahs_decode_dtx() contains only 114 out of 228 bits. This patch changes the logic, so that gsm0503_tch_ahs_decode_dtx() would not split AHS_SID_UPDATE onto two frames anymore like its TCH/AFS equivalent does, but attempt to deinterleave the second half and attempt to decode the payload immediately. Change-Id: I8686d895e96fa0e606c1898b6574cc80a8f46983 Related: [1] I434157e2091a306c039123cea08d84bd8533c937 Related: SYS#5853 --- M src/coding/gsm0503_coding.c M tests/dtx/dtx_gsm0503_test.ok 2 files changed, 27 insertions(+), 15 deletions(-) Approvals: laforge: Looks good to me, approved osmith: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/src/coding/gsm0503_coding.c b/src/coding/gsm0503_coding.c index e81bf02..715a361 100644 --- a/src/coding/gsm0503_coding.c +++ b/src/coding/gsm0503_coding.c @@ -2676,25 +2676,38 @@ /* Determine the DTX frame type (SID_UPDATE, ONSET etc...) */ if (dtx) { - const enum gsm0503_amr_dtx_frames dtx_prev = *dtx; + int n_bits_total_sid; + int n_errors_sid; osmo_sbit2ubit(cBd, cB, 456); *dtx = gsm0503_detect_ahs_dtx_frame(n_errors, n_bits_total, cBd); + /* TODO: detect and handle AHS_SID_UPDATE + AHS_SID_UPDATE_INH */ switch (*dtx) { - case AMR_OTHER: - /* NOTE: The AHS_SID_UPDATE frame is splitted into - * two half rate frames. If the id marker frame - * (AHS_SID_UPDATE) is detected the following frame - * contains the actual comfort noised data part of - * (AHS_SID_UPDATE_CN). */ - if (dtx_prev != AHS_SID_UPDATE) - break; + case AHS_SID_UPDATE: + /* cB[] contains 16 bits of coded in-band data and 212 bits containing + * the identification marker. We need to unmap/deinterleave 114 odd + * bits from the last two blocks, 114 even bits from the first two + * blocks and combine them together. */ + gsm0503_tch_burst_unmap(&iB[0 * 114], &bursts[2 * 116], NULL, 0); + gsm0503_tch_burst_unmap(&iB[1 * 114], &bursts[3 * 116], NULL, 0); + gsm0503_tch_burst_unmap(&iB[2 * 114], &bursts[0 * 116], NULL, 1); + gsm0503_tch_burst_unmap(&iB[3 * 114], &bursts[1 * 116], NULL, 1); + gsm0503_tch_hr_deinterleave(cB, iB); + + /* cB[] is expected to contain 16 bits of coded in-band data and + * 212 bits containing the coded data (53 bits coded at 1/4 rate). */ *dtx = AHS_SID_UPDATE_CN; osmo_conv_decode_ber(&gsm0503_tch_axs_sid_update, - cB + 16, conv, n_errors, - n_bits_total); + cB + 16, conv, &n_errors_sid, + &n_bits_total_sid); + /* gsm0503_detect_ahs_dtx_frame() calculates BER for the marker, + * osmo_conv_decode_ber() calculates BER for the coded data. */ + if (n_errors != NULL) + *n_errors += n_errors_sid; + if (n_bits_total != NULL) + *n_bits_total += n_bits_total_sid; rv = osmo_crc16gen_check_bits(&gsm0503_amr_crc14, conv, 35, conv + 35); if (rv != 0) { @@ -2715,7 +2728,6 @@ tch_amr_reassemble(tch_data, sid_first_dummy, 39); len = 5; goto out; - case AHS_SID_UPDATE: case AHS_ONSET: case AHS_SID_FIRST_INH: case AHS_SID_UPDATE_INH: diff --git a/tests/dtx/dtx_gsm0503_test.ok b/tests/dtx/dtx_gsm0503_test.ok index 750b440..60f3884 100644 --- a/tests/dtx/dtx_gsm0503_test.ok +++ b/tests/dtx/dtx_gsm0503_test.ok @@ -17,10 +17,10 @@ Running test_gsm0503_tch_afhs_decode_dtx(at offset=0): testing detection/decoding of AHS_SID_UPDATE ==> gsm0503_tch_ahs_decode_dtx() yields 'AMR_OTHER (audio)' (rc=-1, BER 109/212) Running test_gsm0503_tch_afhs_decode_dtx(at offset=232): testing detection/decoding of AHS_SID_UPDATE - ==> gsm0503_tch_ahs_decode_dtx() yields 'AHS_SID_UPDATE (marker)' (rc=0, BER 0/212) -Running test_gsm0503_tch_afhs_decode_dtx(at offset=464): testing detection/decoding of AHS_SID_UPDATE - ==> gsm0503_tch_ahs_decode_dtx() yields 'AHS_SID_UPDATE_CN (audio)' (rc=5, BER 106/212) + ==> gsm0503_tch_ahs_decode_dtx() yields 'AHS_SID_UPDATE_CN (audio)' (rc=5, BER 0/424) ====> tch_data[] = { 6003ccb270 } +Running test_gsm0503_tch_afhs_decode_dtx(at offset=464): testing detection/decoding of AHS_SID_UPDATE + ==> gsm0503_tch_ahs_decode_dtx() yields 'AMR_OTHER (audio)' (rc=-1, BER 111/212) Running test_gsm0503_tch_afhs_decode_dtx(at offset=0): testing tagging of FACCH/F ==> gsm0503_tch_afs_decode_dtx() yields 'AMR_OTHER (audio)' (rc=-1, BER 456/456) Running test_gsm0503_tch_afhs_decode_dtx(at offset=0): testing tagging of FACCH/H -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/28166 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I8686d895e96fa0e606c1898b6574cc80a8f46983 Gerrit-Change-Number: 28166 Gerrit-PatchSet: 5 Gerrit-Owner: fixeria <vyanitskiy(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-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

3 years

1
0
0 0

Change in libosmocore[master]: reader: more meaningful null pointer check in get_sw

by laforge

laforge has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/28180 ) Change subject: reader: more meaningful null pointer check in get_sw ...................................................................... reader: more meaningful null pointer check in get_sw At the moment msgb_apdu_de(resp) is used to check if the msgb that is handed over to get_sw is properly populated with data. However, since msgb_apdu_de() is just adding an offset, which cannot be 0 to ->l2h the returned value also can never be NULL. This means that we cannot use msgb_apdu_de() to detect if resp contains a nullpointer. Lets check if ->l2h is not NULL instead. This will make sure that ->l2h is populated. Change-Id: I32bc56c9264c01911a4f4b4f911b09e955205010 Related: OS#5560 --- M src/sim/reader.c 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Jenkins Builder: Verified fixeria: Looks good to me, but someone else must approve laforge: Looks good to me, approved diff --git a/src/sim/reader.c b/src/sim/reader.c index b41b730..982b2ee 100644 --- a/src/sim/reader.c +++ b/src/sim/reader.c @@ -40,7 +40,7 @@ { int ret; - if (!msgb_apdu_de(resp) || msgb_apdu_le(resp) < 2) + if (!resp->l2h || msgb_apdu_le(resp) < 2) return -EIO; ret = msgb_get_u16(resp); -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/28180 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I32bc56c9264c01911a4f4b4f911b09e955205010 Gerrit-Change-Number: 28180 Gerrit-PatchSet: 1 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

3 years

1
0
0 0

Change in libosmocore[master]: reader: more meaningful null pointer check in get_sw

by laforge

Attention is currently required from: neels, daniel, dexter. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/28180 ) Change subject: reader: more meaningful null pointer check in get_sw ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/28180 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I32bc56c9264c01911a4f4b4f911b09e955205010 Gerrit-Change-Number: 28180 Gerrit-PatchSet: 1 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Wed, 25 May 2022 13:34:10 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

3 years

1
0
0 0

Change in libosmocore[master]: coding: separate gsm0503_tch_a[fh]s_decode_inband()

by laforge

Attention is currently required from: fixeria. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/28177 ) Change subject: coding: separate gsm0503_tch_a[fh]s_decode_inband() ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/28177 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I0106de7a7f87517006e323299b2dc08457d1c6cf Gerrit-Change-Number: 28177 Gerrit-PatchSet: 2 Gerrit-Owner: fixeria <vyanitskiy(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-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Wed, 25 May 2022 13:33:40 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

3 years

1
0
0 0

Change in libosmocore[master]: coding: add gsm0503_detect_a[fh]s_dtx_frame2()

by laforge

Attention is currently required from: fixeria, pespin. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/28176 ) Change subject: coding: add gsm0503_detect_a[fh]s_dtx_frame2() ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/28176 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ibcac395f800bb64150c97fcdaca3523ecfc5fcee Gerrit-Change-Number: 28176 Gerrit-PatchSet: 2 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Wed, 25 May 2022 13:33:11 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

3 years

1
0
0 0

Change in libosmocore[master]: coding: do not reset codec ID on receipt of DTX frames

by laforge

Attention is currently required from: fixeria. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/28175 ) Change subject: coding: do not reset codec ID on receipt of DTX frames ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/28175 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ic4edbb8ab873fe0bdd69a8710803628bc4f447d0 Gerrit-Change-Number: 28175 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(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-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Wed, 25 May 2022 13:32:00 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

3 years

1
0
0 0

2025

2024

2023

2022

gerrit-log May 2022