This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
fixeria gerrit-no-reply at lists.osmocom.orgfixeria has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/26046 )
Change subject: l1sap: rework handling of DATA.ind on SACCH
......................................................................
l1sap: rework handling of DATA.ind on SACCH
Currently an Uplink SACCH block is being passed to LAPDm first, and
then gets forwareded to the BSC in handle_ms_meas_report(), together
with the Uplink measurements collected so far.
This approach has a serious flaw: handle_ms_meas_report() won't be
called if an Uplink block contains SAPI=3 data (SMS) or was not
decoded at all (len=0) fow whatever reason. Therefore, no RSL
MEASurement RESult message will be sent to the BSC.
Rename handle_ms_meas_report() to lchan_meas_handle_sacch(), and call
it from l1sap_ph_data_ind(). This way perioduc RSL MEASurement RESult
messages will be sent regardless of what happens on Uplink SACCH.
Change-Id: Ifed91f87fd653debc87a09da3fd31ad64a13f330
Fixes: TC_meas_res_speech_{tchf,tchh}_sapi3
Related: SYS#5319
---
M include/osmo-bts/measurement.h
M src/common/l1sap.c
M src/common/measurement.c
M src/common/rsl.c
4 files changed, 84 insertions(+), 74 deletions(-)
Approvals:
Jenkins Builder: Verified
laforge: Looks good to me, approved
pespin: Looks good to me, but someone else must approve
diff --git a/include/osmo-bts/measurement.h b/include/osmo-bts/measurement.h
index f63a05a..ad86d8d 100644
--- a/include/osmo-bts/measurement.h
+++ b/include/osmo-bts/measurement.h
@@ -20,8 +20,6 @@
int is_meas_complete(struct gsm_lchan *lchan, uint32_t fn);
-int handle_ms_meas_report(struct gsm_lchan *lchan,
- const struct gsm48_hdr *gh,
- unsigned int len);
+void lchan_meas_handle_sacch(struct gsm_lchan *lchan, struct msgb *msg);
#endif
diff --git a/src/common/l1sap.c b/src/common/l1sap.c
index f983b62..8dee209 100644
--- a/src/common/l1sap.c
+++ b/src/common/l1sap.c
@@ -33,7 +33,6 @@
#include <osmocom/gsm/l1sap.h>
#include <osmocom/gsm/gsm_utils.h>
#include <osmocom/gsm/rsl.h>
-#include <osmocom/gsm/protocol/gsm_44_004.h>
#include <osmocom/core/gsmtap.h>
#include <osmocom/core/gsmtap_util.h>
#include <osmocom/core/utils.h>
@@ -1470,7 +1469,6 @@
uint8_t tn;
uint32_t fn;
enum osmo_ph_pres_info_type pr_info = data_ind->pdch_presence_info;
- struct gsm_sacch_l1_hdr *l1_hdr;
chan_nr = data_ind->chan_nr;
link_id = data_ind->link_id;
@@ -1529,48 +1527,33 @@
if (bts_internal_flag_get(trx->bts, BTS_INTERNAL_FLAG_MEAS_PAYLOAD_COMB))
process_l1sap_meas_data(lchan, l1sap, PRIM_PH_DATA);
- if (L1SAP_IS_LINK_SACCH(link_id))
+ if (L1SAP_IS_LINK_SACCH(link_id)) {
repeated_ul_sacch_active_decision(lchan, data_ind->ber10k);
- /* bad frame */
- if (len == 0) {
- if (L1SAP_IS_LINK_SACCH(link_id)) {
- /* In case we loose a SACCH block, we must take care
- * that the related measurement report is sent via RSL.
- * This is a fallback method. The report will also
- * lack the measurement report from the MS side. See
- * also rsl.c:lapdm_rll_tx_cb() */
- LOGPGT(DL1P, LOGL_INFO, &g_time, "Lost SACCH block, faking meas reports and ms pwr\n");
- handle_ms_meas_report(lchan, NULL, 0);
-
+ /* Radio Link Timeout counter */
+ if (len == 0) {
+ LOGPGT(DL1P, LOGL_INFO, &g_time, "%s Lost SACCH block\n",
+ gsm_lchan_name(lchan));
radio_link_timeout(lchan, true);
+ } else {
+ radio_link_timeout(lchan, false);
}
- return -EINVAL;
+
+ /* Trigger the measurement reporting/processing logic */
+ lchan_meas_handle_sacch(lchan, msg);
}
+ /* bad frame */
+ if (len == 0)
+ return -EINVAL;
+
/* report first valid received frame to handover process */
if (lchan->ho.active == HANDOVER_WAIT_FRAME)
handover_frame(lchan);
- if (L1SAP_IS_LINK_SACCH(link_id)) {
- radio_link_timeout(lchan, false);
+ if (L1SAP_IS_LINK_SACCH(link_id))
le = &lchan->lapdm_ch.lapdm_acch;
- /* save the SACCH L1 header in the lchan struct for RSL MEAS RES */
- if (len != GSM_MACBLOCK_LEN) {
- LOGPGT(DL1P, LOGL_NOTICE, &g_time, "SACCH with odd len=%u!?!\n", len);
- return -EINVAL;
- }
- /* Some brilliant engineer decided that the ordering of
- * fields on the Um interface is different from the
- * order of fields in RSL. See 3GPP TS 44.004 (section 7.2)
- * vs. 3GPP TS 48.058 (section 9.3.10). */
- l1_hdr = (struct gsm_sacch_l1_hdr*)data;
- lchan->meas.l1_info.ms_pwr = l1_hdr->ms_pwr;
- lchan->meas.l1_info.fpc_epc = l1_hdr->fpc_epc;
- lchan->meas.l1_info.srr_sro = l1_hdr->srr_sro;
- lchan->meas.l1_info.ta = l1_hdr->ta;
- lchan->meas.flags |= LC_UL_M_F_L1_VALID;
- } else
+ else
le = &lchan->lapdm_ch.lapdm_dcch;
if (check_for_first_ciphrd(lchan, data, len))
diff --git a/src/common/measurement.c b/src/common/measurement.c
index a83be34..3aeb901 100644
--- a/src/common/measurement.c
+++ b/src/common/measurement.c
@@ -2,8 +2,11 @@
#include <stdint.h>
#include <errno.h>
-#include <osmocom/gsm/gsm_utils.h>
#include <osmocom/core/utils.h>
+#include <osmocom/core/endian.h>
+
+#include <osmocom/gsm/gsm_utils.h>
+#include <osmocom/gsm/protocol/gsm_44_004.h>
#include <osmo-bts/gsm_data.h>
#include <osmo-bts/logging.h>
@@ -865,24 +868,74 @@
LOGPLCHAN(lchan, DL1P, LOGL_DEBUG, "DL-FACCH repetition: active => inactive\n");
}
-/* Called every time a Measurement Result (TS 08.58 8.4.8) is received from
- * lower layers and has to be forwarded to BSC */
-int handle_ms_meas_report(struct gsm_lchan *lchan,
- const struct gsm48_hdr *gh,
- unsigned int len)
+static bool data_is_rr_meas_rep(const uint8_t *data)
{
+ const struct gsm48_hdr *gh = (void *)(data + 5);
+ const uint8_t *lapdm_hdr = (void *)(data + 2);
+
+ /* LAPDm address field: SAPI=0, C/R=0, EA=1 */
+ if (lapdm_hdr[0] != 0x01)
+ return false;
+ /* LAPDm control field: U, func=UI */
+ if (lapdm_hdr[1] != 0x03)
+ return false;
+ /* Protocol discriminator: RR */
+ if (gh->proto_discr != GSM48_PDISC_RR)
+ return false;
+
+ switch (gh->msg_type) {
+ case GSM48_MT_RR_EXT_MEAS_REP:
+ case GSM48_MT_RR_MEAS_REP:
+ return true;
+ default:
+ return false;
+ }
+}
+
+/* Called every time a SACCH block is received from lower layers */
+void lchan_meas_handle_sacch(struct gsm_lchan *lchan, struct msgb *msg)
+{
+ const struct gsm48_meas_res *mr = NULL;
+ const struct gsm48_hdr *gh = NULL;
int timing_offset, rc;
struct lapdm_entity *le;
- bool dtxu_used;
+ bool dtxu_used = true; /* safe default assumption */
uint8_t ms_pwr;
uint8_t ms_ta;
int8_t ul_rssi;
int16_t ul_ci_cb;
+ if (msgb_l2len(msg) == GSM_MACBLOCK_LEN) {
+ /* Some brilliant engineer decided that the ordering of
+ * fields on the Um interface is different from the
+ * order of fields in RSL. See 3GPP TS 44.004 (section 7.2)
+ * vs. 3GPP TS 48.058 (section 9.3.10). */
+ const struct gsm_sacch_l1_hdr *l1h = msgb_l2(msg);
+ lchan->meas.l1_info.ms_pwr = l1h->ms_pwr;
+ lchan->meas.l1_info.fpc_epc = l1h->fpc_epc;
+ lchan->meas.l1_info.srr_sro = l1h->srr_sro;
+ lchan->meas.l1_info.ta = l1h->ta;
+ lchan->meas.flags |= LC_UL_M_F_L1_VALID;
+
+ /* Check if this is a Measurement Report */
+ if (data_is_rr_meas_rep(msgb_l2(msg))) {
+ /* Skip both L1 SACCH and LAPDm headers */
+ msg->l3h = (void *)(msg->l2h + 2 + 3);
+ gh = msgb_l3(msg);
+ }
+
+ ms_pwr = lchan->meas.l1_info.ms_pwr;
+ ms_ta = lchan->meas.l1_info.ta;
+ } else {
+ lchan->meas.flags &= ~LC_UL_M_F_L1_VALID;
+ ms_pwr = lchan->ms_power_ctrl.current;
+ ms_ta = lchan->ta_ctrl.current;
+ }
+
le = &lchan->lapdm_ch.lapdm_acch;
timing_offset = ms_to_valid(lchan) ? ms_to2rsl(lchan, le) : -1;
- rc = rsl_tx_meas_res(lchan, (const uint8_t *)gh, len, timing_offset);
+ rc = rsl_tx_meas_res(lchan, msgb_l3(msg), msgb_l3len(msg), timing_offset);
if (rc == 0) /* Count successful transmissions */
lchan->meas.res_nr++;
@@ -896,25 +949,9 @@
* feed the Control Loop with the measurements for the same
* period (the previous one), which is stored in lchan->meas(.ul_res):
*/
- if (len == 0) {
- dtxu_used = true;
- ms_ta = lchan->ta_ctrl.current;
- ms_pwr = lchan->ms_power_ctrl.current;
- } else {
- /* if len!=0, it means we were able to parse L1Header in UL SACCH: */
- OSMO_ASSERT(lchan->meas.flags & LC_UL_M_F_L1_VALID);
-
- ms_ta = lchan->meas.l1_info.ta;
- ms_pwr = lchan->meas.l1_info.ms_pwr;
- switch (gh->msg_type) {
- case GSM48_MT_RR_MEAS_REP:
- dtxu_used = (len > sizeof(*gh) + 1) && !!(gh->data[0] & 0x40);
- break;
- case GSM48_MT_RR_EXT_MEAS_REP:
- default:
- dtxu_used = true; /* FIXME: not implemented */
- break;
- }
+ if (gh && gh->msg_type == GSM48_MT_RR_MEAS_REP) {
+ mr = (const struct gsm48_meas_res *)gh->data;
+ dtxu_used = mr->dtx_used;
}
if (dtxu_used) {
@@ -934,8 +971,6 @@
/* Reset state for next iteration */
lchan->tch.dtx.dl_active = false;
lchan->meas.flags &= ~LC_UL_M_F_OSMO_EXT_VALID;
- lchan->meas.flags &= ~LC_UL_M_F_L1_VALID;
lchan->ms_t_offs = -1;
lchan->p_offs = -1;
- return rc;
}
diff --git a/src/common/rsl.c b/src/common/rsl.c
index 2d00005..5d069c2 100644
--- a/src/common/rsl.c
+++ b/src/common/rsl.c
@@ -3566,17 +3566,11 @@
msg->trx = lchan->ts->trx;
msg->lchan = lchan;
- /* check if this is a measurement report from SACCH which needs special
- * processing before forwarding */
+ /* If this is a Measurement Report, then we simply ignore it,
+ * because it has already been processed in l1sap_ph_data_ind(). */
if (rslms_is_meas_rep(msg)) {
- int rc;
-
- LOGPLCHAN(lchan, DRSL, LOGL_INFO, "Handing RLL msg %s from LAPDm to MEAS REP\n",
- rsl_msg_name(rh->msg_type));
-
- rc = handle_ms_meas_report(lchan, (struct gsm48_hdr *)msgb_l3(msg), msgb_l3len(msg));
msgb_free(msg);
- return rc;
+ return 0;
} else if (rslms_is_gprs_susp_req(msg)) {
return handle_gprs_susp_req(msg);
} else {
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/26046
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: Ifed91f87fd653debc87a09da3fd31ad64a13f330
Gerrit-Change-Number: 26046
Gerrit-PatchSet: 4
Gerrit-Owner: fixeria <vyanitskiy at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy at sysmocom.de>
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20211104/0fc393bb/attachment.htm>