<p>Pau Espin Pedrol <strong>merged</strong> this change.</p><p><a href="https://gerrit.osmocom.org/13774">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Harald Welte: Looks good to me, approved
Jenkins Builder: Verified
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">osmux: Cleanup of CID alloc pool APIs<br><br>* Cleanup naming to make it far more clear<br>* Drop 2 variables holding CID values (allocated_cid, cid), in favour of<br>1 value holdinf the value and one bool stating whether the value is<br>used.<br>* Change conn_osmux_allocate_cid to allow allocating either next<br>available CID or a specific one, in preparation for forthcoming patches.<br><br>This commit can be merged straight away because anyway osmux cannot be<br>enabled in current status (blocked by vty config) and<br>(conn_)osmux_allocate_cid was/is not called anywhere. However, it helps<br>improving code base for future re-introduction of Osmux as it is<br>envisioned.<br><br>Change-Id: I737a248ac6c74add8e917fe2e2f36779d0f1d685<br>---<br>M include/osmocom/mgcp/mgcp_internal.h<br>M include/osmocom/mgcp/osmux.h<br>M src/libosmo-mgcp/mgcp_conn.c<br>M src/libosmo-mgcp/mgcp_osmux.c<br>M src/libosmo-mgcp/mgcp_vty.c<br>M tests/mgcp/mgcp_test.c<br>6 files changed, 80 insertions(+), 39 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/include/osmocom/mgcp/mgcp_internal.h b/include/osmocom/mgcp/mgcp_internal.h</span><br><span>index 3defc4c..24e28b4 100644</span><br><span>--- a/include/osmocom/mgcp/mgcp_internal.h</span><br><span>+++ b/include/osmocom/mgcp/mgcp_internal.h</span><br><span>@@ -188,9 +188,9 @@</span><br><span> struct {</span><br><span> /* Osmux state: disabled, activating, active */</span><br><span> enum osmux_state state;</span><br><span style="color: hsl(0, 100%, 40%);">- /* Allocated Osmux circuit ID for this endpoint */</span><br><span style="color: hsl(0, 100%, 40%);">- int allocated_cid;</span><br><span style="color: hsl(0, 100%, 40%);">- /* Used Osmux circuit ID for this endpoint */</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Is cid holding valid data? is it allocated from pool? */</span><br><span style="color: hsl(120, 100%, 40%);">+ bool cid_allocated;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Allocated Osmux circuit ID for this conn */</span><br><span> uint8_t cid;</span><br><span> /* handle to batch messages */</span><br><span> struct osmux_in_handle *in;</span><br><span>diff --git a/include/osmocom/mgcp/osmux.h b/include/osmocom/mgcp/osmux.h</span><br><span>index 685be9c..c080c85 100644</span><br><span>--- a/include/osmocom/mgcp/osmux.h</span><br><span>+++ b/include/osmocom/mgcp/osmux.h</span><br><span>@@ -15,13 +15,16 @@</span><br><span> int osmux_enable_conn(struct mgcp_endpoint *endp, struct mgcp_conn_rtp *conn,</span><br><span> struct in_addr *addr, uint16_t port);</span><br><span> void osmux_disable_conn(struct mgcp_conn_rtp *conn);</span><br><span style="color: hsl(0, 100%, 40%);">-void osmux_allocate_cid(struct mgcp_conn_rtp *conn);</span><br><span style="color: hsl(0, 100%, 40%);">-void osmux_release_cid(struct mgcp_conn_rtp *conn);</span><br><span style="color: hsl(120, 100%, 40%);">+int conn_osmux_allocate_cid(struct mgcp_conn_rtp *conn, int osmux_cid);</span><br><span style="color: hsl(120, 100%, 40%);">+void conn_osmux_release_cid(struct mgcp_conn_rtp *conn);</span><br><span> int osmux_xfrm_to_osmux(char *buf, int buf_len, struct mgcp_conn_rtp *conn);</span><br><span> int osmux_send_dummy(struct mgcp_endpoint *endp, struct mgcp_conn_rtp *conn);</span><br><span style="color: hsl(0, 100%, 40%);">-int osmux_get_cid(void);</span><br><span style="color: hsl(0, 100%, 40%);">-void osmux_put_cid(uint8_t osmux_cid);</span><br><span style="color: hsl(0, 100%, 40%);">-int osmux_used_cid(void);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+void osmux_cid_pool_get(uint8_t osmux_cid);</span><br><span style="color: hsl(120, 100%, 40%);">+int osmux_cid_pool_get_next(void);</span><br><span style="color: hsl(120, 100%, 40%);">+void osmux_cid_pool_put(uint8_t osmux_cid);</span><br><span style="color: hsl(120, 100%, 40%);">+bool osmux_cid_pool_allocated(uint8_t osmux_cid);</span><br><span style="color: hsl(120, 100%, 40%);">+int osmux_cid_pool_count_used(void);</span><br><span> </span><br><span> enum osmux_state {</span><br><span> OSMUX_STATE_DISABLED = 0, /* Osmux not being currently used by endp */</span><br><span>diff --git a/src/libosmo-mgcp/mgcp_conn.c b/src/libosmo-mgcp/mgcp_conn.c</span><br><span>index bfaa111..af5426f 100644</span><br><span>--- a/src/libosmo-mgcp/mgcp_conn.c</span><br><span>+++ b/src/libosmo-mgcp/mgcp_conn.c</span><br><span>@@ -90,7 +90,8 @@</span><br><span> static unsigned int rate_ctr_index = 0;</span><br><span> </span><br><span> conn_rtp->type = MGCP_RTP_DEFAULT;</span><br><span style="color: hsl(0, 100%, 40%);">- conn_rtp->osmux.allocated_cid = -1;</span><br><span style="color: hsl(120, 100%, 40%);">+ conn_rtp->osmux.cid_allocated = false;</span><br><span style="color: hsl(120, 100%, 40%);">+ conn_rtp->osmux.cid = 0;</span><br><span> </span><br><span> /* backpointer to the generic part of the connection */</span><br><span> conn->u.rtp.conn = conn;</span><br><span>@@ -120,7 +121,6 @@</span><br><span> static void mgcp_rtp_conn_cleanup(struct mgcp_conn_rtp *conn_rtp)</span><br><span> {</span><br><span> osmux_disable_conn(conn_rtp);</span><br><span style="color: hsl(0, 100%, 40%);">- osmux_release_cid(conn_rtp);</span><br><span> mgcp_free_rtp_port(&conn_rtp->end);</span><br><span> rate_ctr_group_free(conn_rtp->rate_ctr_group);</span><br><span> }</span><br><span>diff --git a/src/libosmo-mgcp/mgcp_osmux.c b/src/libosmo-mgcp/mgcp_osmux.c</span><br><span>index a2c138d..3bd765e 100644</span><br><span>--- a/src/libosmo-mgcp/mgcp_osmux.c</span><br><span>+++ b/src/libosmo-mgcp/mgcp_osmux.c</span><br><span>@@ -614,31 +614,46 @@</span><br><span> </span><br><span> osmux_xfrm_input_close_circuit(conn->osmux.in, conn->osmux.cid);</span><br><span> conn->osmux.state = OSMUX_STATE_DISABLED;</span><br><span style="color: hsl(0, 100%, 40%);">- conn->osmux.cid = -1;</span><br><span style="color: hsl(120, 100%, 40%);">+ conn_osmux_release_cid(conn);</span><br><span> osmux_handle_put(conn->osmux.in);</span><br><span> }</span><br><span> </span><br><span> /*! relase OSXMUX cid, that had been allocated to this connection.</span><br><span> * \param[in] conn connection with OSMUX cid to release */</span><br><span style="color: hsl(0, 100%, 40%);">-void osmux_release_cid(struct mgcp_conn_rtp *conn)</span><br><span style="color: hsl(120, 100%, 40%);">+void conn_osmux_release_cid(struct mgcp_conn_rtp *conn)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- if (!conn)</span><br><span style="color: hsl(0, 100%, 40%);">- return;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- if (conn->osmux.state != OSMUX_STATE_ENABLED)</span><br><span style="color: hsl(0, 100%, 40%);">- return;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- if (conn->osmux.allocated_cid >= 0)</span><br><span style="color: hsl(0, 100%, 40%);">- osmux_put_cid(conn->osmux.allocated_cid);</span><br><span style="color: hsl(0, 100%, 40%);">- conn->osmux.allocated_cid = -1;</span><br><span style="color: hsl(120, 100%, 40%);">+ if (conn->osmux.cid_allocated)</span><br><span style="color: hsl(120, 100%, 40%);">+ osmux_cid_pool_put(conn->osmux.cid);</span><br><span style="color: hsl(120, 100%, 40%);">+ conn->osmux.cid = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ conn->osmux.cid_allocated = false;</span><br><span> }</span><br><span> </span><br><span> /*! allocate OSXMUX cid to connection.</span><br><span style="color: hsl(0, 100%, 40%);">- * \param[in] conn connection for which we allocate the OSMUX cid*/</span><br><span style="color: hsl(0, 100%, 40%);">-void osmux_allocate_cid(struct mgcp_conn_rtp *conn)</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param[in] conn connection for which we allocate the OSMUX cid</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param[in] osmux_cid OSMUX cid to allocate. -1 Means take next available one.</span><br><span style="color: hsl(120, 100%, 40%);">+ * \returns Allocated OSMUX cid, -1 on error (no free cids avail, or selected one is already taken).</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+int conn_osmux_allocate_cid(struct mgcp_conn_rtp *conn, int osmux_cid)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- osmux_release_cid(conn);</span><br><span style="color: hsl(0, 100%, 40%);">- conn->osmux.allocated_cid = osmux_get_cid();</span><br><span style="color: hsl(120, 100%, 40%);">+ if (osmux_cid != -1 && osmux_cid_pool_allocated((uint8_t) osmux_cid)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DLMGCP, LOGL_INFO, "Conn %s: Osmux CID %d already allocated!\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ conn->conn->id, osmux_cid);</span><br><span style="color: hsl(120, 100%, 40%);">+ return -1;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (osmux_cid == -1) {</span><br><span style="color: hsl(120, 100%, 40%);">+ osmux_cid = osmux_cid_pool_get_next();</span><br><span style="color: hsl(120, 100%, 40%);">+ if (osmux_cid == -1) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DLMGCP, LOGL_INFO, "Conn %s: no available Osmux CID to allocate!\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ conn->conn->id);</span><br><span style="color: hsl(120, 100%, 40%);">+ return -1;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ } else</span><br><span style="color: hsl(120, 100%, 40%);">+ osmux_cid_pool_get(osmux_cid);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ conn->osmux.cid = (uint8_t) osmux_cid;</span><br><span style="color: hsl(120, 100%, 40%);">+ conn->osmux.cid_allocated = true;</span><br><span style="color: hsl(120, 100%, 40%);">+ return osmux_cid;</span><br><span> }</span><br><span> </span><br><span> /*! send RTP dummy packet to OSMUX connection port.</span><br><span>@@ -682,7 +697,7 @@</span><br><span> </span><br><span> /*! count the number of taken OSMUX cids.</span><br><span> * \returns number of OSMUX cids in use */</span><br><span style="color: hsl(0, 100%, 40%);">-int osmux_used_cid(void)</span><br><span style="color: hsl(120, 100%, 40%);">+int osmux_cid_pool_count_used(void)</span><br><span> {</span><br><span> int i, j, used = 0;</span><br><span> </span><br><span>@@ -698,7 +713,7 @@</span><br><span> </span><br><span> /*! take a free OSMUX cid.</span><br><span> * \returns OSMUX cid */</span><br><span style="color: hsl(0, 100%, 40%);">-int osmux_get_cid(void)</span><br><span style="color: hsl(120, 100%, 40%);">+int osmux_cid_pool_get_next(void)</span><br><span> {</span><br><span> int i, j;</span><br><span> </span><br><span>@@ -718,10 +733,24 @@</span><br><span> return -1;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+/*! take a specific OSMUX cid.</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param[in] osmux_cid OSMUX cid */</span><br><span style="color: hsl(120, 100%, 40%);">+void osmux_cid_pool_get(uint8_t osmux_cid)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DLMGCP, LOGL_DEBUG, "Allocating Osmux CID %u from pool\n", osmux_cid);</span><br><span style="color: hsl(120, 100%, 40%);">+ osmux_cid_bitmap[osmux_cid / 8] |= (1 << (osmux_cid % 8));</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /*! put back a no longer used OSMUX cid.</span><br><span> * \param[in] osmux_cid OSMUX cid */</span><br><span style="color: hsl(0, 100%, 40%);">-void osmux_put_cid(uint8_t osmux_cid)</span><br><span style="color: hsl(120, 100%, 40%);">+void osmux_cid_pool_put(uint8_t osmux_cid)</span><br><span> {</span><br><span> LOGP(DLMGCP, LOGL_DEBUG, "Osmux CID %u is back to the pool\n", osmux_cid);</span><br><span> osmux_cid_bitmap[osmux_cid / 8] &= ~(1 << (osmux_cid % 8));</span><br><span> }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+/*! check if OSMUX cid is already taken */</span><br><span style="color: hsl(120, 100%, 40%);">+bool osmux_cid_pool_allocated(uint8_t osmux_cid)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ return !!(osmux_cid_bitmap[osmux_cid / 8] & (1 << (osmux_cid % 8)));</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span>diff --git a/src/libosmo-mgcp/mgcp_vty.c b/src/libosmo-mgcp/mgcp_vty.c</span><br><span>index a47376b..25d7a67 100644</span><br><span>--- a/src/libosmo-mgcp/mgcp_vty.c</span><br><span>+++ b/src/libosmo-mgcp/mgcp_vty.c</span><br><span>@@ -297,7 +297,7 @@</span><br><span> dump_trunk(vty, trunk, show_stats);</span><br><span> </span><br><span> if (g_cfg->osmux)</span><br><span style="color: hsl(0, 100%, 40%);">- vty_out(vty, "Osmux used CID: %d%s", osmux_used_cid(),</span><br><span style="color: hsl(120, 100%, 40%);">+ vty_out(vty, "Osmux used CID: %d%s", osmux_cid_pool_count_used(),</span><br><span> VTY_NEWLINE);</span><br><span> </span><br><span> return CMD_SUCCESS;</span><br><span>diff --git a/tests/mgcp/mgcp_test.c b/tests/mgcp/mgcp_test.c</span><br><span>index be45598..c4931b2 100644</span><br><span>--- a/tests/mgcp/mgcp_test.c</span><br><span>+++ b/tests/mgcp/mgcp_test.c</span><br><span>@@ -1554,25 +1554,34 @@</span><br><span> {</span><br><span> int id, i;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- OSMO_ASSERT(osmux_used_cid() == 0);</span><br><span style="color: hsl(0, 100%, 40%);">- id = osmux_get_cid();</span><br><span style="color: hsl(120, 100%, 40%);">+ OSMO_ASSERT(osmux_cid_pool_count_used() == 0);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ id = osmux_cid_pool_get_next();</span><br><span> OSMO_ASSERT(id == 0);</span><br><span style="color: hsl(0, 100%, 40%);">- OSMO_ASSERT(osmux_used_cid() == 1);</span><br><span style="color: hsl(0, 100%, 40%);">- osmux_put_cid(id);</span><br><span style="color: hsl(0, 100%, 40%);">- OSMO_ASSERT(osmux_used_cid() == 0);</span><br><span style="color: hsl(120, 100%, 40%);">+ OSMO_ASSERT(osmux_cid_pool_count_used() == 1);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ osmux_cid_pool_get(30);</span><br><span style="color: hsl(120, 100%, 40%);">+ OSMO_ASSERT(osmux_cid_pool_count_used() == 2);</span><br><span style="color: hsl(120, 100%, 40%);">+ osmux_cid_pool_get(30);</span><br><span style="color: hsl(120, 100%, 40%);">+ OSMO_ASSERT(osmux_cid_pool_count_used() == 2);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ osmux_cid_pool_put(id);</span><br><span style="color: hsl(120, 100%, 40%);">+ OSMO_ASSERT(osmux_cid_pool_count_used() == 1);</span><br><span style="color: hsl(120, 100%, 40%);">+ osmux_cid_pool_put(30);</span><br><span style="color: hsl(120, 100%, 40%);">+ OSMO_ASSERT(osmux_cid_pool_count_used() == 0);</span><br><span> </span><br><span> for (i = 0; i < 256; ++i) {</span><br><span style="color: hsl(0, 100%, 40%);">- id = osmux_get_cid();</span><br><span style="color: hsl(120, 100%, 40%);">+ id = osmux_cid_pool_get_next();</span><br><span> OSMO_ASSERT(id == i);</span><br><span style="color: hsl(0, 100%, 40%);">- OSMO_ASSERT(osmux_used_cid() == i + 1);</span><br><span style="color: hsl(120, 100%, 40%);">+ OSMO_ASSERT(osmux_cid_pool_count_used() == i + 1);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- id = osmux_get_cid();</span><br><span style="color: hsl(120, 100%, 40%);">+ id = osmux_cid_pool_get_next();</span><br><span> OSMO_ASSERT(id == -1);</span><br><span> </span><br><span> for (i = 0; i < 256; ++i)</span><br><span style="color: hsl(0, 100%, 40%);">- osmux_put_cid(i);</span><br><span style="color: hsl(0, 100%, 40%);">- OSMO_ASSERT(osmux_used_cid() == 0);</span><br><span style="color: hsl(120, 100%, 40%);">+ osmux_cid_pool_put(i);</span><br><span style="color: hsl(120, 100%, 40%);">+ OSMO_ASSERT(osmux_cid_pool_count_used() == 0);</span><br><span> }</span><br><span> </span><br><span> static const struct log_info_cat log_categories[] = {</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/13774">change 13774</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/13774"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-mgw </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: I737a248ac6c74add8e917fe2e2f36779d0f1d685 </div>
<div style="display:none"> Gerrit-Change-Number: 13774 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: Pau Espin Pedrol <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Harald Welte <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder (1000002) </div>
<div style="display:none"> Gerrit-Reviewer: Pau Espin Pedrol <pespin@sysmocom.de> </div>