Review at  https://gerrit.osmocom.org/6877

rsl: Improve ERROR REPORTing

Let's make sure all useful optional IEs of the RSL ERROR REPort aare present

Change-Id: I5ecb98f8c72f472ac23c1e4e0f606b75e2cf032c
---
M src/common/rsl.c
1 file changed, 81 insertions(+), 44 deletions(-)


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

diff --git a/src/common/rsl.c b/src/common/rsl.c
index bffe69d..df480a6 100644
--- a/src/common/rsl.c
+++ b/src/common/rsl.c
@@ -56,7 +56,9 @@
 
 //#define FAKE_CIPH_MODE_COMPL
 
-static int rsl_tx_error_report(struct gsm_bts_trx *trx, uint8_t cause);
+
+static int rsl_tx_error_report(struct gsm_bts_trx *trx, uint8_t cause, const 
uint8_t *chan_nr,
+                               const uint8_t *link_id, const struct msgb 
*orig_msg);
 
 /* list of RSL SI types that can occur on the SACCH */
 static const unsigned int rsl_sacch_sitypes[] = {
@@ -225,16 +227,36 @@
  */
 
 /* 8.6.4 sending ERROR REPORT */
-static int rsl_tx_error_report(struct gsm_bts_trx *trx, uint8_t cause)
+static int rsl_tx_error_report(struct gsm_bts_trx *trx, uint8_t cause, const 
uint8_t *chan_nr,
+                               const uint8_t *link_id, const struct msgb 
*orig_msg)
 {
+       unsigned int len = sizeof(struct abis_rsl_common_hdr);
        struct msgb *nmsg;
 
        LOGP(DRSL, LOGL_NOTICE, "Tx RSL Error Report: cause = 0x%02x\n", cause);
 
-       nmsg = rsl_msgb_alloc(sizeof(struct abis_rsl_common_hdr));
+       if (orig_msg)
+               len += 2 + 3+msgb_l2len(orig_msg); /* chan_nr + TLV(orig_msg) */
+       if (chan_nr)
+               len += 2;
+       if (link_id)
+               len += 2;
+
+       nmsg = rsl_msgb_alloc(len);
        if (!nmsg)
                return -ENOMEM;
        msgb_tlv_put(nmsg, RSL_IE_CAUSE, 1, &cause);
+       if (orig_msg && msgb_l2len(orig_msg) >= sizeof(struct 
abis_rsl_common_hdr)) {
+               struct abis_rsl_common_hdr *ch = (struct abis_rsl_common_hdr *) 
msgb_l2(orig_msg);
+               msgb_tv_put(nmsg, RSL_IE_MSG_ID, ch->msg_type);
+       }
+       if (chan_nr)
+               msgb_tv_put(nmsg, RSL_IE_CHAN_NR, *chan_nr);
+       if (link_id)
+               msgb_tv_put(nmsg, RSL_IE_LINK_IDENT, *link_id);
+       if (orig_msg)
+               msgb_tlv_put(nmsg, RSL_IE_ERR_MSG, msgb_l2len(orig_msg), 
msgb_l2(orig_msg));
+
        rsl_trx_push_hdr(nmsg, RSL_MT_ERROR_REPORT);
        nmsg->trx = trx;
 
@@ -276,16 +298,16 @@
 
        /* 9.3.30 System Info Type */
        if (!TLVP_PRESENT(&tp, RSL_IE_SYSINFO_TYPE))
-               return rsl_tx_error_report(trx, RSL_ERR_MAND_IE_ERROR);
+               return rsl_tx_error_report(trx, RSL_ERR_MAND_IE_ERROR, NULL, 
NULL, msg);
 
        rsl_si = *TLVP_VAL(&tp, RSL_IE_SYSINFO_TYPE);
        if (OSMO_IN_ARRAY(rsl_si, rsl_sacch_sitypes))
-               return rsl_tx_error_report(trx, RSL_ERR_IE_CONTENT);
+               return rsl_tx_error_report(trx, RSL_ERR_IE_CONTENT, NULL, NULL, 
msg);
 
        osmo_si = osmo_rsl2sitype(rsl_si);
        if (osmo_si == SYSINFO_TYPE_NONE) {
                LOGP(DRSL, LOGL_NOTICE, " Rx RSL SI 0x%02x not supported.\n", 
rsl_si);
-               return rsl_tx_error_report(trx, RSL_ERR_IE_CONTENT);
+               return rsl_tx_error_report(trx, RSL_ERR_IE_CONTENT, NULL, NULL, 
msg);
        }
        /* 9.3.39 Full BCCH Information */
        if (TLVP_PRESENT(&tp, RSL_IE_FULL_BCCH_INFO)) {
@@ -316,13 +338,13 @@
                        if (bts->si2q_index > bts->si2q_count) {
                                LOGP(DRSL, LOGL_ERROR, " Rx RSL SI2quater with 
index %u > count %u\n",
                                     bts->si2q_index, bts->si2q_count);
-                               return rsl_tx_error_report(trx, 
RSL_ERR_IE_CONTENT);
+                               return rsl_tx_error_report(trx, 
RSL_ERR_IE_CONTENT, NULL, NULL, msg);
                        }
 
                        if (bts->si2q_index > SI2Q_MAX_NUM || bts->si2q_count > 
SI2Q_MAX_NUM) {
                                LOGP(DRSL, LOGL_ERROR, " Rx RSL SI2quater with 
impossible parameters: index %u, count %u"
                                     "should be <= %u\n", bts->si2q_index, 
bts->si2q_count, SI2Q_MAX_NUM);
-                               return rsl_tx_error_report(trx, 
RSL_ERR_IE_CONTENT);
+                               return rsl_tx_error_report(trx, 
RSL_ERR_IE_CONTENT, NULL, NULL, msg);
                        }
 
                        memset(GSM_BTS_SI2Q(bts, bts->si2q_index), 
GSM_MACBLOCK_PADDING, sizeof(sysinfo_buf_t));
@@ -432,7 +454,7 @@
 
        if (!TLVP_PRESENT(&tp, RSL_IE_PAGING_GROUP) ||
            !TLVP_PRESENT(&tp, RSL_IE_MS_IDENTITY))
-               return rsl_tx_error_report(trx, RSL_ERR_MAND_IE_ERROR);
+               return rsl_tx_error_report(trx, RSL_ERR_MAND_IE_ERROR, NULL, 
NULL, msg);
 
        paging_group = *TLVP_VAL(&tp, RSL_IE_PAGING_GROUP);
        identity_lv = TLVP_VAL(&tp, RSL_IE_MS_IDENTITY)-1;
@@ -464,7 +486,7 @@
 
        if (!TLVP_PRESENT(&tp, RSL_IE_CB_CMD_TYPE) ||
            !TLVP_PRESENT(&tp, RSL_IE_SMSCB_MSG))
-               return rsl_tx_error_report(trx, RSL_ERR_MAND_IE_ERROR);
+               return rsl_tx_error_report(trx, RSL_ERR_MAND_IE_ERROR, NULL, 
NULL, msg);
 
        cb_cmd_type = (struct rsl_ie_cb_cmd_type *)
                                        TLVP_VAL(&tp, RSL_IE_CB_CMD_TYPE);
@@ -514,16 +536,16 @@
 
        /* 9.3.30 System Info Type */
        if (!TLVP_PRESENT(&tp, RSL_IE_SYSINFO_TYPE))
-               return rsl_tx_error_report(trx, RSL_ERR_MAND_IE_ERROR);
+               return rsl_tx_error_report(trx, RSL_ERR_MAND_IE_ERROR, NULL, 
NULL, msg);
 
        rsl_si = *TLVP_VAL(&tp, RSL_IE_SYSINFO_TYPE);
        if (!OSMO_IN_ARRAY(rsl_si, rsl_sacch_sitypes))
-               return rsl_tx_error_report(trx, RSL_ERR_IE_CONTENT);
+               return rsl_tx_error_report(trx, RSL_ERR_IE_CONTENT, NULL, NULL, 
msg);
 
        osmo_si = osmo_rsl2sitype(rsl_si);
        if (osmo_si == SYSINFO_TYPE_NONE) {
                LOGP(DRSL, LOGL_NOTICE, " Rx SACCH SI 0x%02x not supported.\n", 
rsl_si);
-               return rsl_tx_error_report(trx, RSL_ERR_IE_CONTENT);
+               return rsl_tx_error_report(trx, RSL_ERR_IE_CONTENT, NULL, NULL, 
msg);
        }
        if (TLVP_PRESENT(&tp, RSL_IE_L3_INFO)) {
                uint16_t len = TLVP_LEN(&tp, RSL_IE_L3_INFO);
@@ -550,7 +572,7 @@
        rsl_tlv_parse(&tp, msgb_l3(msg), msgb_l3len(msg));
 
        if (!TLVP_PRESENT(&tp, RSL_IE_FULL_IMM_ASS_INFO))
-               return rsl_tx_error_report(trx, RSL_ERR_MAND_IE_ERROR);
+               return rsl_tx_error_report(trx, RSL_ERR_MAND_IE_ERROR, NULL, 
NULL, msg);
 
        /* cut down msg to the 04.08 RR part */
        msg->l3h = (uint8_t *) TLVP_VAL(&tp, RSL_IE_FULL_IMM_ASS_INFO);
@@ -913,8 +935,10 @@
                uint8_t len = TLVP_LEN(&tp, RSL_IE_ENCR_INFO);
                const uint8_t *val = TLVP_VAL(&tp, RSL_IE_ENCR_INFO);
 
-               if (encr_info2lchan(lchan, val, len) < 0)
-                        return rsl_tx_error_report(msg->trx, 
RSL_ERR_IE_CONTENT);
+               if (encr_info2lchan(lchan, val, len) < 0) {
+                        return rsl_tx_error_report(msg->trx, 
RSL_ERR_IE_CONTENT,
+                                                   &dch->chan_nr, NULL, msg);
+               }
        } else
                memset(&lchan->encr, 0, sizeof(lchan->encr));
 
@@ -955,13 +979,16 @@
                        uint8_t si_len = *cur++;
                        uint8_t osmo_si;
 
-                       if (!OSMO_IN_ARRAY(rsl_si, rsl_sacch_sitypes))
-                               return rsl_tx_error_report(msg->trx, 
RSL_ERR_IE_CONTENT);
+                       if (!OSMO_IN_ARRAY(rsl_si, rsl_sacch_sitypes)) {
+                               return rsl_tx_error_report(msg->trx, 
RSL_ERR_IE_CONTENT,
+                                                          &dch->chan_nr, NULL, 
msg);
+                       }
 
                        osmo_si = osmo_rsl2sitype(rsl_si);
                        if (osmo_si == SYSINFO_TYPE_NONE) {
                                LOGP(DRSL, LOGL_NOTICE, " Rx SACCH SI 0x%02x 
not supported.\n", rsl_si);
-                               return rsl_tx_error_report(msg->trx, 
RSL_ERR_IE_CONTENT);
+                               return rsl_tx_error_report(msg->trx, 
RSL_ERR_IE_CONTENT, &dch->chan_nr,
+                                                          NULL, msg);
                        }
 
                        lapdm_ui_prefix_lchan(lchan, cur, osmo_si, si_len);
@@ -969,7 +996,8 @@
                        cur += si_len;
                        if (cur >= val + tot_len) {
                                LOGP(DRSL, LOGL_ERROR, "Error parsing SACCH 
INFO IE\n");
-                               return rsl_tx_error_report(msg->trx, 
RSL_ERR_IE_CONTENT);
+                               return rsl_tx_error_report(msg->trx, 
RSL_ERR_IE_CONTENT, &dch->chan_nr,
+                                                          NULL, msg);
                        }
                }
        } else {
@@ -980,7 +1008,8 @@
        if (TLVP_PRESENT(&tp, RSL_IE_MR_CONFIG)) {
                if (TLVP_LEN(&tp, RSL_IE_MR_CONFIG) > sizeof(lchan->mr_bts_lv) 
- 1) {
                        LOGP(DRSL, LOGL_ERROR, "Error parsing MultiRate conf 
IE\n");
-                       return rsl_tx_error_report(msg->trx, 
RSL_ERR_IE_CONTENT);
+                       return rsl_tx_error_report(msg->trx, 
RSL_ERR_IE_CONTENT, &dch->chan_nr,
+                                                  NULL, msg);
                }
                memcpy(lchan->mr_bts_lv, TLVP_VAL(&tp, RSL_IE_MR_CONFIG) - 1,
                       TLVP_LEN(&tp, RSL_IE_MR_CONFIG) + 1);
@@ -1037,8 +1066,8 @@
                        rc = 0;
                }
                if (rc)
-                       return rsl_tx_error_report(msg->trx,
-                                                  RSL_ERR_NORMAL_UNSPEC);
+                       return rsl_tx_error_report(msg->trx, 
RSL_ERR_NORMAL_UNSPEC, &dch->chan_nr,
+                                                  NULL, msg);
                return 0;
        }
 
@@ -1190,21 +1219,25 @@
        struct tlv_parsed tp;
        uint8_t link_id;
 
-       if (rsl_tlv_parse(&tp, msgb_l3(msg), msgb_l3len(msg)) < 0)
-               return rsl_tx_error_report(msg->trx, RSL_ERR_IE_CONTENT);
+       if (rsl_tlv_parse(&tp, msgb_l3(msg), msgb_l3len(msg)) < 0) {
+               return rsl_tx_error_report(msg->trx, RSL_ERR_IE_CONTENT, 
&dch->chan_nr, NULL, msg);
+       }
 
        if (!TLVP_PRESENT(&tp, RSL_IE_ENCR_INFO) ||
            !TLVP_PRESENT(&tp, RSL_IE_L3_INFO) ||
-           !TLVP_PRESENT(&tp, RSL_IE_LINK_IDENT))
-               return rsl_tx_error_report(msg->trx, RSL_ERR_MAND_IE_ERROR);
+           !TLVP_PRESENT(&tp, RSL_IE_LINK_IDENT)) {
+               return rsl_tx_error_report(msg->trx, RSL_ERR_MAND_IE_ERROR, 
&dch->chan_nr, NULL, msg);
+       }
 
        /* 9.3.7 Encryption Information */
        if (TLVP_PRESENT(&tp, RSL_IE_ENCR_INFO)) {
                uint8_t len = TLVP_LEN(&tp, RSL_IE_ENCR_INFO);
                const uint8_t *val = TLVP_VAL(&tp, RSL_IE_ENCR_INFO);
 
-               if (encr_info2lchan(lchan, val, len) < 0)
-                        return rsl_tx_error_report(msg->trx, 
RSL_ERR_IE_CONTENT);
+               if (encr_info2lchan(lchan, val, len) < 0) {
+                        return rsl_tx_error_report(msg->trx, 
RSL_ERR_IE_CONTENT, &dch->chan_nr,
+                                                   NULL, msg);
+               }
        }
 
        /* 9.3.2 Link Identifier */
@@ -1329,8 +1362,10 @@
                uint8_t len = TLVP_LEN(&tp, RSL_IE_ENCR_INFO);
                const uint8_t *val = TLVP_VAL(&tp, RSL_IE_ENCR_INFO);
 
-               if (encr_info2lchan(lchan, val, len) < 0)
-                        return rsl_tx_error_report(msg->trx, 
RSL_ERR_IE_CONTENT);
+               if (encr_info2lchan(lchan, val, len) < 0) {
+                        return rsl_tx_error_report(msg->trx, 
RSL_ERR_IE_CONTENT, &dch->chan_nr,
+                                                   NULL, msg);
+               }
        }
 
        /* 9.3.45 Main channel reference */
@@ -1339,7 +1374,8 @@
        if (TLVP_PRESENT(&tp, RSL_IE_MR_CONFIG)) {
                if (TLVP_LEN(&tp, RSL_IE_MR_CONFIG) > sizeof(lchan->mr_bts_lv) 
- 1) {
                        LOGP(DRSL, LOGL_ERROR, "Error parsing MultiRate conf 
IE\n");
-                       return rsl_tx_error_report(msg->trx, 
RSL_ERR_IE_CONTENT);
+                       return rsl_tx_error_report(msg->trx, 
RSL_ERR_IE_CONTENT, &dch->chan_nr,
+                                                  NULL, msg);
                }
                memcpy(lchan->mr_bts_lv, TLVP_VAL(&tp, RSL_IE_MR_CONFIG) - 1,
                        TLVP_LEN(&tp, RSL_IE_MR_CONFIG) + 1);
@@ -1383,6 +1419,7 @@
 /* 8.4.20 SACCH INFO MODify */
 static int rsl_rx_sacch_inf_mod(struct msgb *msg)
 {
+       struct abis_rsl_dchan_hdr *dch = msgb_l2(msg);
        struct gsm_lchan *lchan = msg->lchan;
        struct tlv_parsed tp;
        uint8_t rsl_si, osmo_si;
@@ -1391,22 +1428,22 @@
 
        if (TLVP_PRESENT(&tp, RSL_IE_STARTNG_TIME)) {
                LOGP(DRSL, LOGL_NOTICE, "Starting time not supported\n");
-               return rsl_tx_error_report(msg->trx, RSL_ERR_SERV_OPT_UNIMPL);
+               return rsl_tx_error_report(msg->trx, RSL_ERR_SERV_OPT_UNIMPL, 
&dch->chan_nr, NULL, msg);
        }
 
        /* 9.3.30 System Info Type */
        if (!TLVP_PRESENT(&tp, RSL_IE_SYSINFO_TYPE))
-               return rsl_tx_error_report(msg->trx, RSL_ERR_MAND_IE_ERROR);
+               return rsl_tx_error_report(msg->trx, RSL_ERR_MAND_IE_ERROR, 
&dch->chan_nr, NULL, msg);
 
        rsl_si = *TLVP_VAL(&tp, RSL_IE_SYSINFO_TYPE);
        if (!OSMO_IN_ARRAY(rsl_si, rsl_sacch_sitypes))
-               return rsl_tx_error_report(msg->trx, RSL_ERR_IE_CONTENT);
+               return rsl_tx_error_report(msg->trx, RSL_ERR_IE_CONTENT, 
&dch->chan_nr, NULL, msg);
 
        osmo_si = osmo_rsl2sitype(rsl_si);
        if (osmo_si == SYSINFO_TYPE_NONE) {
                LOGP(DRSL, LOGL_NOTICE, "%s Rx SACCH SI 0x%02x not 
supported.\n",
                        gsm_lchan_name(lchan), rsl_si);
-               return rsl_tx_error_report(msg->trx, RSL_ERR_IE_CONTENT);
+               return rsl_tx_error_report(msg->trx, RSL_ERR_IE_CONTENT, 
&dch->chan_nr, NULL, msg);
        }
        if (TLVP_PRESENT(&tp, RSL_IE_L3_INFO)) {
                uint16_t len = TLVP_LEN(&tp, RSL_IE_L3_INFO);
@@ -2244,7 +2281,7 @@
                                                     RSL_ERR_MAND_IE_ERROR, 
NULL);
                        break;
                default:
-                       rc = rsl_tx_error_report(msg->trx, 
RSL_ERR_MAND_IE_ERROR);
+                       rc = rsl_tx_error_report(msg->trx, 
RSL_ERR_MAND_IE_ERROR, NULL, NULL, msg);
                        break;
                }
                break;
@@ -2258,7 +2295,7 @@
                /* fall-through */
        default:
                /* ERROR REPORT */
-               rc = rsl_tx_error_report(msg->trx, RSL_ERR_MAND_IE_ERROR);
+               rc = rsl_tx_error_report(msg->trx, RSL_ERR_MAND_IE_ERROR, NULL, 
NULL, msg);
        }
 
        msgb_free(msg);
@@ -2276,7 +2313,7 @@
 
        if (msgb_l2len(msg) < sizeof(*rh)) {
                LOGP(DRSL, LOGL_NOTICE, "RSL Radio Link Layer message too 
short\n");
-               rsl_tx_error_report(trx, RSL_ERR_PROTO);
+               rsl_tx_error_report(trx, RSL_ERR_PROTO, &rh->chan_nr, 
&rh->link_id, msg);
                msgb_free(msg);
                return -EIO;
        }
@@ -2470,7 +2507,7 @@
 
        if (msgb_l2len(msg) < sizeof(*cch)) {
                LOGP(DRSL, LOGL_NOTICE, "RSL Common Channel Management message 
too short\n");
-               rsl_tx_error_report(trx, RSL_ERR_PROTO);
+               rsl_tx_error_report(trx, RSL_ERR_PROTO, NULL, NULL, msg);
                msgb_free(msg);
                return -EIO;
        }
@@ -2600,7 +2637,7 @@
 
        if (msgb_l2len(msg) < sizeof(*th)) {
                LOGP(DRSL, LOGL_NOTICE, "RSL TRX message too short\n");
-               rsl_tx_error_report(trx, RSL_ERR_PROTO);
+               rsl_tx_error_report(trx, RSL_ERR_PROTO, NULL, NULL, msg);
                msgb_free(msg);
                return -EIO;
        }
@@ -2629,7 +2666,7 @@
 
        if (msgb_l2len(msg) < sizeof(*dch)) {
                LOGP(DRSL, LOGL_NOTICE, "RSL ip.access message too short\n");
-               rsl_tx_error_report(trx, RSL_ERR_PROTO);
+               rsl_tx_error_report(trx, RSL_ERR_PROTO, NULL, NULL, msg);
                msgb_free(msg);
                return -EIO;
        }
@@ -2687,7 +2724,7 @@
 
        if (msgb_l2len(msg) < sizeof(*rslh)) {
                LOGP(DRSL, LOGL_NOTICE, "RSL message too short\n");
-               rsl_tx_error_report(trx, RSL_ERR_PROTO);
+               rsl_tx_error_report(trx, RSL_ERR_PROTO, NULL, NULL, msg);
                msgb_free(msg);
                return -EIO;
        }
@@ -2713,7 +2750,7 @@
        default:
                LOGP(DRSL, LOGL_NOTICE, "unknown RSL msg_discr 0x%02x\n",
                        rslh->msg_discr);
-               rsl_tx_error_report(trx, RSL_ERR_MSG_DISCR);
+               rsl_tx_error_report(trx, RSL_ERR_MSG_DISCR, NULL, NULL, msg);
                msgb_free(msg);
                ret = -EINVAL;
        }

-- 
To view, visit https://gerrit.osmocom.org/6877
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I5ecb98f8c72f472ac23c1e4e0f606b75e2cf032c
Gerrit-PatchSet: 1
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Owner: Harald Welte <lafo...@gnumonks.org>

Reply via email to