Change in osmo-mgw[master]: osmux: Cleanup of CID alloc pool APIs

Pau Espin Pedrol gerrit-no-reply at lists.osmocom.org
Thu Apr 25 21:38:38 UTC 2019


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 <pespin at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: Pau Espin Pedrol <pespin at sysmocom.de>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190425/ca8b54be/attachment.html>


More information about the gerrit-log mailing list