<p>Vadim Yanitskiy has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/13708">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">common/oml.c: refactor Get Attribute Response message generation<br><br>Instead of allocating two transitional buffers (one static,<br>another dynamic), we can use the existing message buffer.<br><br>Both handle_attrs_bts() and handle_attrs_trx() can put (append)<br>the reported attributes, and push (prepend) unreported ones<br>as per 3GPP TS 52.021, 9.4.64 "Get Attribute Response Info".<br><br>Change-Id: I349447a43bce360f59e0c6b435906c65167d158b<br>---<br>M src/common/oml.c<br>1 file changed, 50 insertions(+), 40 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/08/13708/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/common/oml.c b/src/common/oml.c</span><br><span>index de7a0e7..caff595 100644</span><br><span>--- a/src/common/oml.c</span><br><span>+++ b/src/common/oml.c</span><br><span>@@ -191,64 +191,75 @@</span><br><span>    return len + out_offset;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-static inline int handle_attrs_trx(uint8_t *out, const struct gsm_bts_trx *trx, const uint8_t *attr, uint16_t attr_len)</span><br><span style="color: hsl(120, 100%, 40%);">+static inline int handle_attrs_trx(struct msgb *msg, const struct gsm_bts_trx *trx,</span><br><span style="color: hsl(120, 100%, 40%);">+                            const uint8_t *attr, uint16_t attr_len)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- uint16_t i, attr_out_index = 1; /* byte 0 is reserved for unsupported attributes counter */</span><br><span style="color: hsl(0, 100%, 40%);">-     struct msgb *attr_buf = oml_msgb_alloc();</span><br><span style="color: hsl(120, 100%, 40%);">+     uint8_t num_unreported = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+   uint8_t *buf;</span><br><span style="color: hsl(120, 100%, 40%);">+ int i;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-      if (!attr_buf)</span><br><span style="color: hsl(0, 100%, 40%);">-          return -NM_NACK_CANT_PERFORM;</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!trx) {</span><br><span style="color: hsl(120, 100%, 40%);">+           LOGP(DOML, LOGL_ERROR, "O&M Get Attributes for unknown TRX\n");</span><br><span style="color: hsl(120, 100%, 40%);">+         return -NM_NACK_TRXNR_UNKN;</span><br><span style="color: hsl(120, 100%, 40%);">+   }</span><br><span> </span><br><span>        for (i = 0; i < attr_len; i++) {</span><br><span style="color: hsl(0, 100%, 40%);">-             bool processed = false;</span><br><span>              switch (attr[i]) {</span><br><span>           case NM_ATT_SW_CONFIG:</span><br><span style="color: hsl(0, 100%, 40%);">-                  if (trx) {</span><br><span style="color: hsl(0, 100%, 40%);">-                              add_trx_attr(attr_buf, trx);</span><br><span style="color: hsl(0, 100%, 40%);">-                            processed = true;</span><br><span style="color: hsl(0, 100%, 40%);">-                       } else</span><br><span style="color: hsl(0, 100%, 40%);">-                          LOGP(DOML, LOGL_ERROR, "O&M Get Attributes [%u], %s is unhandled due to missing TRX.\n",</span><br><span style="color: hsl(0, 100%, 40%);">-                               i, get_value_string(abis_nm_att_names, attr[i]));</span><br><span style="color: hsl(120, 100%, 40%);">+                        add_trx_attr(msg, trx);</span><br><span>                      break;</span><br><span>               default:</span><br><span>                     LOGP(DOML, LOGL_ERROR, "O&M Get Attributes [%u], %s is unsupported by TRX.\n", i,</span><br><span>                           get_value_string(abis_nm_att_names, attr[i]));</span><br><span style="color: hsl(0, 100%, 40%);">-             }</span><br><span style="color: hsl(0, 100%, 40%);">-               /* assemble values of supported attributes and list of unsupported ones */</span><br><span style="color: hsl(0, 100%, 40%);">-              if (!processed) {</span><br><span style="color: hsl(0, 100%, 40%);">-                       out[attr_out_index] = attr[i];</span><br><span style="color: hsl(0, 100%, 40%);">-                  attr_out_index++;</span><br><span style="color: hsl(120, 100%, 40%);">+                     /* Push this tag to the list of unreported attributes */</span><br><span style="color: hsl(120, 100%, 40%);">+                      buf = msgb_push(msg, 1);</span><br><span style="color: hsl(120, 100%, 40%);">+                      *buf = attr[i];</span><br><span style="color: hsl(120, 100%, 40%);">+                       num_unreported++;</span><br><span>            }</span><br><span>    }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-   return cleanup_attr_msg(out, attr_out_index, attr_buf);</span><br><span style="color: hsl(120, 100%, 40%);">+       /* Push the amount of unreported attributes */</span><br><span style="color: hsl(120, 100%, 40%);">+        buf = msgb_push(msg, 1);</span><br><span style="color: hsl(120, 100%, 40%);">+      *buf = num_unreported;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+      return 0;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-static inline int handle_attrs_bts(uint8_t *out, const struct gsm_bts *bts, const uint8_t *attr, uint16_t attr_len)</span><br><span style="color: hsl(120, 100%, 40%);">+static inline int handle_attrs_bts(struct msgb *msg, const struct gsm_bts *bts,</span><br><span style="color: hsl(120, 100%, 40%);">+                                   const uint8_t *attr, uint16_t attr_len)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- uint16_t i, attr_out_index = 1; /* byte 0 is reserved for unsupported attributes counter */</span><br><span style="color: hsl(0, 100%, 40%);">-     struct msgb *attr_buf = oml_msgb_alloc();</span><br><span style="color: hsl(120, 100%, 40%);">+     uint8_t num_unreported = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+   uint8_t *buf;</span><br><span style="color: hsl(120, 100%, 40%);">+ int i;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-      if (!attr_buf)</span><br><span style="color: hsl(0, 100%, 40%);">-          return -NM_NACK_CANT_PERFORM;</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!bts) {</span><br><span style="color: hsl(120, 100%, 40%);">+           LOGP(DOML, LOGL_ERROR, "O&M Get Attributes for unknown BTS\n");</span><br><span style="color: hsl(120, 100%, 40%);">+         return -NM_NACK_BTSNR_UNKN;</span><br><span style="color: hsl(120, 100%, 40%);">+   }</span><br><span> </span><br><span>        for (i = 0; i < attr_len; i++) {</span><br><span>          switch (attr[i]) {</span><br><span>           case NM_ATT_SW_CONFIG:</span><br><span style="color: hsl(0, 100%, 40%);">-                  add_bts_attrs(attr_buf, bts);</span><br><span style="color: hsl(120, 100%, 40%);">+                 add_bts_attrs(msg, bts);</span><br><span>                     break;</span><br><span>               case NM_ATT_MANUF_ID:</span><br><span style="color: hsl(0, 100%, 40%);">-                   add_bts_feat(attr_buf, bts);</span><br><span style="color: hsl(120, 100%, 40%);">+                  add_bts_feat(msg, bts);</span><br><span>                      break;</span><br><span>               default:</span><br><span>                     LOGP(DOML, LOGL_ERROR, "O&M Get Attributes [%u], %s is unsupported by BTS.\n", i,</span><br><span>                           get_value_string(abis_nm_att_names, attr[i]));</span><br><span style="color: hsl(0, 100%, 40%);">-                     out[attr_out_index] = attr[i]; /* assemble values of supported attributes and list of unsupported ones */</span><br><span style="color: hsl(0, 100%, 40%);">-                       attr_out_index++;</span><br><span style="color: hsl(120, 100%, 40%);">+                     /* Push this tag to the list of unreported attributes */</span><br><span style="color: hsl(120, 100%, 40%);">+                      buf = msgb_push(msg, 1);</span><br><span style="color: hsl(120, 100%, 40%);">+                      *buf = attr[i];</span><br><span style="color: hsl(120, 100%, 40%);">+                       num_unreported++;</span><br><span>            }</span><br><span>    }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-   return cleanup_attr_msg(out, attr_out_index, attr_buf);</span><br><span style="color: hsl(120, 100%, 40%);">+       /* Push the amount of unreported attributes */</span><br><span style="color: hsl(120, 100%, 40%);">+        buf = msgb_push(msg, 1);</span><br><span style="color: hsl(120, 100%, 40%);">+      *buf = num_unreported;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+      return 0;</span><br><span> }</span><br><span> </span><br><span> /* send 3GPP TS 52.021 §8.11.2 Get Attribute Response */</span><br><span>@@ -256,8 +267,7 @@</span><br><span>                            const uint8_t *attr, uint16_t attr_len)</span><br><span> {</span><br><span>     struct msgb *nmsg = oml_msgb_alloc();</span><br><span style="color: hsl(0, 100%, 40%);">-   uint8_t resp[MAX_VERSION_LENGTH * attr_len * 2]; /* heuristic for Attribute Response Info space requirements */</span><br><span style="color: hsl(0, 100%, 40%);">- int len;</span><br><span style="color: hsl(120, 100%, 40%);">+      int rc;</span><br><span> </span><br><span>  LOGP(DOML, LOGL_INFO, "%s Tx Get Attribute Response\n",</span><br><span>         get_value_string(abis_nm_obj_class_names, mo->obj_class));</span><br><span>@@ -267,28 +277,28 @@</span><br><span> </span><br><span>       switch (mo->obj_class) {</span><br><span>  case NM_OC_BTS:</span><br><span style="color: hsl(0, 100%, 40%);">-         len = handle_attrs_bts(resp, mo->bts, attr, attr_len);</span><br><span style="color: hsl(120, 100%, 40%);">+             rc = handle_attrs_bts(nmsg, mo->bts, attr, attr_len);</span><br><span>             break;</span><br><span>       case NM_OC_BASEB_TRANSC:</span><br><span style="color: hsl(0, 100%, 40%);">-                len = handle_attrs_trx(resp, gsm_bts_trx_num(mo->bts, mo->obj_inst.trx_nr), attr, attr_len);</span><br><span style="color: hsl(120, 100%, 40%);">+            rc = handle_attrs_trx(nmsg, gsm_bts_trx_num(mo->bts, mo->obj_inst.trx_nr), attr, attr_len);</span><br><span>            break;</span><br><span>       default:</span><br><span>             LOGP(DOML, LOGL_ERROR, "Unsupported MO class %s in Get Attribute Response\n",</span><br><span>                   get_value_string(abis_nm_obj_class_names, mo->obj_class));</span><br><span style="color: hsl(0, 100%, 40%);">-              len = -NM_NACK_OBJCLASS_NOTSUPP;</span><br><span style="color: hsl(120, 100%, 40%);">+              rc = -NM_NACK_OBJCLASS_NOTSUPP;</span><br><span>      }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-   if (len < 0) {</span><br><span style="color: hsl(0, 100%, 40%);">-               LOGP(DOML, LOGL_ERROR, "Tx Get Attribute Response FAILED with %d\n", len);</span><br><span style="color: hsl(120, 100%, 40%);">+  if (rc < 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+              LOGP(DOML, LOGL_ERROR, "Tx Get Attribute Response FAILED with rc=%d\n", rc);</span><br><span>               msgb_free(nmsg);</span><br><span style="color: hsl(0, 100%, 40%);">-                return len;</span><br><span style="color: hsl(120, 100%, 40%);">+           return rc;</span><br><span>   }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-   /* §9.4.64 Get Attribute Response Info */</span><br><span style="color: hsl(0, 100%, 40%);">-      msgb_tl16v_put(nmsg, NM_ATT_GET_ARI, len, resp);</span><br><span style="color: hsl(120, 100%, 40%);">+      /* Push Get Attribute Response Info TL (actually TV where V is L) */</span><br><span style="color: hsl(120, 100%, 40%);">+  msgb_tv16_push(nmsg, NM_ATT_GET_ARI, msgb_length(nmsg));</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-    len = oml_mo_send_msg(mo, nmsg, NM_MT_GET_ATTR_RESP);</span><br><span style="color: hsl(0, 100%, 40%);">-   return (len < 0) ? -NM_NACK_CANT_PERFORM : len;</span><br><span style="color: hsl(120, 100%, 40%);">+    rc = oml_mo_send_msg(mo, nmsg, NM_MT_GET_ATTR_RESP);</span><br><span style="color: hsl(120, 100%, 40%);">+  return (rc < 0) ? -NM_NACK_CANT_PERFORM : rc;</span><br><span> }</span><br><span> </span><br><span> /* 8.8.1 sending State Changed Event Report */</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/13708">change 13708</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/13708"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: osmo-bts </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>
<div style="display:none"> Gerrit-Change-Id: I349447a43bce360f59e0c6b435906c65167d158b </div>
<div style="display:none"> Gerrit-Change-Number: 13708 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Vadim Yanitskiy <axilirator@gmail.com> </div>