Harald Welte has submitted this change and it was merged. ( https://gerrit.osmocom.org/13708 )
Change subject: common/oml.c: refactor Get Attribute Response message generation ...................................................................... common/oml.c: refactor Get Attribute Response message generation Instead of allocating two transitional buffers (one static, another dynamic), we can use the existing message buffer. Both handle_attrs_bts() and handle_attrs_trx() can put (append) the reported attributes, and push (prepend) non-reported ones as per 3GPP TS 52.021, 9.4.64 "Get Attribute Response Info". Change-Id: I349447a43bce360f59e0c6b435906c65167d158b --- M src/common/oml.c 1 file changed, 54 insertions(+), 68 deletions(-) Approvals: Jenkins Builder: Verified Harald Welte: Looks good to me, approved diff --git a/src/common/oml.c b/src/common/oml.c index 62dea7b..d1db8f4 100644 --- a/src/common/oml.c +++ b/src/common/oml.c @@ -166,92 +166,79 @@ true); } -/* The number of attributes in §9.4.26 List of Required Attributes is 2 bytes, - but the Count of not-reported attributes from §9.4.64 is 1 byte */ -static inline uint8_t pack_num_unreported_attr(uint16_t attrs) +/* Handle a list of attributes requested by the BSC, compose + * TRX-specific Get Attribute Response IE as per 9.4.64. */ +static inline int handle_attrs_trx(struct msgb *out_msg, const struct gsm_bts_trx *trx, + const uint8_t *attr, uint16_t attr_len) { - if (attrs > 255) { - LOGP(DOML, LOGL_ERROR, "O&M Get Attributes, Count of not-reported attributes is too big: %u\n", - attrs); - return 255; + uint8_t num_unsupported = 0; + uint8_t *buf; + int i; + + if (!trx) { + LOGP(DOML, LOGL_ERROR, "O&M Get Attributes for unknown TRX\n"); + return -NM_NACK_TRXNR_UNKN; } - return attrs; /* Return number of unhandled attributes */ -} - -/* copy all the attributes accumulated in msg to out and return the total length of out buffer */ -static inline int cleanup_attr_msg(uint8_t *out, int out_offset, struct msgb *msg) -{ - int len = 0; - - out[0] = pack_num_unreported_attr(out_offset - 1); - - if (msg) { - memcpy(out + out_offset, msgb_data(msg), msg->len); - len = msg->len; - msgb_free(msg); - } - - return len + out_offset; -} - -static inline int handle_attrs_trx(uint8_t *out, const struct gsm_bts_trx *trx, const uint8_t *attr, uint16_t attr_len) -{ - uint16_t i, attr_out_index = 1; /* byte 0 is reserved for unsupported attributes counter */ - struct msgb *attr_buf = oml_msgb_alloc(); - - if (!attr_buf) - return -NM_NACK_CANT_PERFORM; for (i = 0; i < attr_len; i++) { - bool processed = false; switch (attr[i]) { case NM_ATT_SW_CONFIG: - if (trx) { - add_trx_attr(attr_buf, trx); - processed = true; - } else - LOGP(DOML, LOGL_ERROR, "O&M Get Attributes [%u], %s is unhandled due to missing TRX.\n", - i, get_value_string(abis_nm_att_names, attr[i])); + add_trx_attr(out_msg, trx); break; default: LOGP(DOML, LOGL_ERROR, "O&M Get Attributes [%u], %s is unsupported by TRX.\n", i, get_value_string(abis_nm_att_names, attr[i])); - } - /* assemble values of supported attributes and list of unsupported ones */ - if (!processed) { - out[attr_out_index] = attr[i]; - attr_out_index++; + /* Push this tag to the list of unsupported attributes */ + buf = msgb_push(out_msg, 1); + *buf = attr[i]; + num_unsupported++; } } - return cleanup_attr_msg(out, attr_out_index, attr_buf); + /* Push the amount of unsupported attributes */ + buf = msgb_push(out_msg, 1); + *buf = num_unsupported; + + return 0; } -static inline int handle_attrs_bts(uint8_t *out, const struct gsm_bts *bts, const uint8_t *attr, uint16_t attr_len) +/* Handle a list of attributes requested by the BSC, compose + * BTS-specific Get Attribute Response IE as per 9.4.64. */ +static inline int handle_attrs_bts(struct msgb *out_msg, const struct gsm_bts *bts, + const uint8_t *attr, uint16_t attr_len) { - uint16_t i, attr_out_index = 1; /* byte 0 is reserved for unsupported attributes counter */ - struct msgb *attr_buf = oml_msgb_alloc(); + uint8_t num_unsupported = 0; + uint8_t *buf; + int i; - if (!attr_buf) - return -NM_NACK_CANT_PERFORM; + if (!bts) { + LOGP(DOML, LOGL_ERROR, "O&M Get Attributes for unknown BTS\n"); + return -NM_NACK_BTSNR_UNKN; + } for (i = 0; i < attr_len; i++) { switch (attr[i]) { case NM_ATT_SW_CONFIG: - add_bts_attrs(attr_buf, bts); + add_bts_attrs(out_msg, bts); break; case NM_ATT_MANUF_ID: - add_bts_feat(attr_buf, bts); + add_bts_feat(out_msg, bts); break; default: LOGP(DOML, LOGL_ERROR, "O&M Get Attributes [%u], %s is unsupported by BTS.\n", i, get_value_string(abis_nm_att_names, attr[i])); - out[attr_out_index] = attr[i]; /* assemble values of supported attributes and list of unsupported ones */ - attr_out_index++; + /* Push this tag to the list of unsupported attributes */ + buf = msgb_push(out_msg, 1); + *buf = attr[i]; + num_unsupported++; } } - return cleanup_attr_msg(out, attr_out_index, attr_buf); + /* Push the amount of unsupported attributes */ + buf = msgb_push(out_msg, 1); + *buf = num_unsupported; + + return 0; } /* send 3GPP TS 52.021 §8.11.2 Get Attribute Response */ @@ -259,8 +246,7 @@ const uint8_t *attr, uint16_t attr_len) { struct msgb *nmsg = oml_msgb_alloc(); - uint8_t resp[MAX_VERSION_LENGTH * attr_len * 2]; /* heuristic for Attribute Response Info space requirements */ - int len; + int rc; LOGP(DOML, LOGL_INFO, "%s Tx Get Attribute Response\n", get_value_string(abis_nm_obj_class_names, mo->obj_class)); @@ -270,28 +256,28 @@ switch (mo->obj_class) { case NM_OC_BTS: - len = handle_attrs_bts(resp, mo->bts, attr, attr_len); + rc = handle_attrs_bts(nmsg, mo->bts, attr, attr_len); break; case NM_OC_BASEB_TRANSC: - len = handle_attrs_trx(resp, gsm_bts_trx_num(mo->bts, mo->obj_inst.trx_nr), attr, attr_len); + rc = handle_attrs_trx(nmsg, gsm_bts_trx_num(mo->bts, mo->obj_inst.trx_nr), attr, attr_len); break; default: LOGP(DOML, LOGL_ERROR, "Unsupported MO class %s in Get Attribute Response\n", get_value_string(abis_nm_obj_class_names, mo->obj_class)); - len = -NM_NACK_OBJCLASS_NOTSUPP; + rc = -NM_NACK_OBJCLASS_NOTSUPP; } - if (len < 0) { - LOGP(DOML, LOGL_ERROR, "Tx Get Attribute Response FAILED with %d\n", len); + if (rc < 0) { + LOGP(DOML, LOGL_ERROR, "Tx Get Attribute Response FAILED with rc=%d\n", rc); msgb_free(nmsg); - return len; + return rc; } - /* §9.4.64 Get Attribute Response Info */ - msgb_tl16v_put(nmsg, NM_ATT_GET_ARI, len, resp); + /* Push Get Attribute Response Info TL (actually TV where V is L) */ + msgb_tv16_push(nmsg, NM_ATT_GET_ARI, msgb_length(nmsg)); - len = oml_mo_send_msg(mo, nmsg, NM_MT_GET_ATTR_RESP); - return (len < 0) ? -NM_NACK_CANT_PERFORM : len; + rc = oml_mo_send_msg(mo, nmsg, NM_MT_GET_ATTR_RESP); + return (rc < 0) ? -NM_NACK_CANT_PERFORM : rc; } /* 8.8.1 sending State Changed Event Report */ -- To view, visit https://gerrit.osmocom.org/13708 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I349447a43bce360f59e0c6b435906c65167d158b Gerrit-Change-Number: 13708 Gerrit-PatchSet: 3 Gerrit-Owner: Vadim Yanitskiy <axilira...@gmail.com> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Jenkins Builder (1000002) Gerrit-Reviewer: Pau Espin Pedrol <pes...@sysmocom.de> Gerrit-Reviewer: Vadim Yanitskiy <axilira...@gmail.com>