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