fixeria has uploaded this change for review. ( 
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

Change-Id: Ifed91f87fd653debc87a09da3fd31ad64a13f330
---
M include/osmo-bts/measurement.h
M src/common/l1sap.c
M src/common/measurement.c
M src/common/rsl.c
4 files changed, 83 insertions(+), 72 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/46/26046/1

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..10b8401 100644
--- a/src/common/l1sap.c
+++ b/src/common/l1sap.c
@@ -1470,7 +1470,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 +1528,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..e4d950e 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,12 +868,35 @@
                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;
@@ -879,10 +905,37 @@
        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: 1
Gerrit-Owner: fixeria <[email protected]>
Gerrit-MessageType: newchange

Reply via email to