pespin has uploaded this change for review.
mgw: Rename and move code freeing endp connection
All the code to free conns in an endpoint is really entanged, which also
makes it inefficient in several places because the list of conns is
iterated twice, or even ~N*2.
* Move code freeing the conn object to its own mgcp_conn_free()
function.
* Rename functions acting on endp to be mgcp_endp_* prefix, and move
them to mgcp_endp.{c,h}.
* Remove old mgcp_conn_free(endp, id) since it's not really needed
anymore.
Change-Id: I9a87a07699dd8aa7856d5036e9b95a4ddf699a15
---
M include/osmocom/mgcp/mgcp_conn.h
M include/osmocom/mgcp/mgcp_endp.h
M src/libosmo-mgcp/mgcp_conn.c
M src/libosmo-mgcp/mgcp_endp.c
M src/libosmo-mgcp/mgcp_protocol.c
M tests/mgcp/mgcp_test.c
6 files changed, 44 insertions(+), 53 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/83/39183/1
diff --git a/include/osmocom/mgcp/mgcp_conn.h b/include/osmocom/mgcp/mgcp_conn.h
index 4f7b705..6219579 100644
--- a/include/osmocom/mgcp/mgcp_conn.h
+++ b/include/osmocom/mgcp/mgcp_conn.h
@@ -239,12 +239,10 @@
struct mgcp_conn *mgcp_conn_alloc(void *ctx, struct mgcp_endpoint *endp,
enum mgcp_conn_type type, char *name);
+void mgcp_conn_free(struct mgcp_conn *conn);
struct mgcp_conn *mgcp_conn_get(struct mgcp_endpoint *endp, const char *id);
struct mgcp_conn_rtp *mgcp_conn_get_rtp(struct mgcp_endpoint *endp,
const char *id);
-void mgcp_conn_free(struct mgcp_endpoint *endp, const char *id);
-void mgcp_conn_free_oldest(struct mgcp_endpoint *endp);
-void mgcp_conn_free_all(struct mgcp_endpoint *endp);
char *mgcp_conn_dump(struct mgcp_conn *conn);
struct mgcp_conn *mgcp_find_dst_conn(struct mgcp_conn *conn);
struct mgcp_conn *mgcp_conn_get_oldest(struct mgcp_endpoint *endp);
diff --git a/include/osmocom/mgcp/mgcp_endp.h b/include/osmocom/mgcp/mgcp_endp.h
index 7bf034b..8e5b4cc 100644
--- a/include/osmocom/mgcp/mgcp_endp.h
+++ b/include/osmocom/mgcp/mgcp_endp.h
@@ -141,6 +141,8 @@
bool mgcp_endp_avail(struct mgcp_endpoint *endp);
void mgcp_endp_add_conn(struct mgcp_endpoint *endp, struct mgcp_conn *conn);
void mgcp_endp_remove_conn(struct mgcp_endpoint *endp, struct mgcp_conn *conn);
+void mgcp_endp_free_conn_oldest(struct mgcp_endpoint *endp);
+void mgcp_endp_free_conn_all(struct mgcp_endpoint *endp);
void mgcp_endp_strip_name(char *epname_stripped, const char *epname,
const struct mgcp_trunk *trunk);
struct mgcp_endpoint *mgcp_endp_find_specific(const char *epname,
diff --git a/src/libosmo-mgcp/mgcp_conn.c b/src/libosmo-mgcp/mgcp_conn.c
index 5eb4897..9c60dfa 100644
--- a/src/libosmo-mgcp/mgcp_conn.c
+++ b/src/libosmo-mgcp/mgcp_conn.c
@@ -146,7 +146,7 @@
{
struct mgcp_conn *conn = data;
LOGPCONN(conn, DLMGCP, LOGL_ERROR, "connection timed out!\n");
- mgcp_conn_free(conn->endp, conn->id);
+ mgcp_conn_free(conn);
}
void mgcp_conn_watchdog_kick(struct mgcp_conn *conn)
@@ -286,20 +286,17 @@
rate_ctr_inc(rate_ctr_group_get_ctr(all_stats, RTP_NUM_CONNECTIONS));
}
-/*! free a connection by its ID.
- * \param[in] endp associated endpoint
- * \param[in] id identification number of the connection */
-void mgcp_conn_free(struct mgcp_endpoint *endp, const char *id)
+/*! free a connection
+ * \param[in] conn the conn to free. May be NULL.
+*/
+void mgcp_conn_free(struct mgcp_conn *conn)
{
- struct mgcp_conn *conn;
-
- conn = mgcp_conn_get(endp, id);
if (!conn)
return;
switch (conn->type) {
case MGCP_CONN_TYPE_RTP:
- aggregate_rtp_conn_stats(endp, &conn->u.rtp);
+ aggregate_rtp_conn_stats(conn->endp, &conn->u.rtp);
mgcp_rtp_conn_cleanup(&conn->u.rtp);
break;
default:
@@ -310,45 +307,11 @@
}
osmo_timer_del(&conn->watchdog);
- mgcp_endp_remove_conn(endp, conn);
+ mgcp_endp_remove_conn(conn->endp, conn);
/* WARN: endp may have be freed after call to mgcp_endp_remove_conn */
talloc_free(conn);
}
-/*! free oldest connection in the list.
- * \param[in] endp associated endpoint */
-void mgcp_conn_free_oldest(struct mgcp_endpoint *endp)
-{
- struct mgcp_conn *conn;
-
- if (llist_empty(&endp->conns))
- return;
-
- conn = llist_last_entry(&endp->conns, struct mgcp_conn, entry);
- if (!conn)
- return;
-
- mgcp_conn_free(endp, conn->id);
-}
-
-/*! free all connections at once.
- * \param[in] endp associated endpoint */
-#if defined(__has_attribute)
-#if __has_attribute(no_sanitize)
-__attribute__((no_sanitize("undefined"))) /* ubsan detects a misaligned load */
-#endif
-#endif
-void mgcp_conn_free_all(struct mgcp_endpoint *endp)
-{
- struct mgcp_conn *conn;
-
- /* Drop all items in the list, might be consecutive! */
- while ((conn = llist_first_entry_or_null(&endp->conns, struct mgcp_conn, entry)))
- mgcp_conn_free(endp, conn->id);
-
- return;
-}
-
/*! dump basic connection information to human readable string.
* \param[in] conn to dump
* \returns human readable string */
diff --git a/src/libosmo-mgcp/mgcp_endp.c b/src/libosmo-mgcp/mgcp_endp.c
index 4c870ec..6973b43 100644
--- a/src/libosmo-mgcp/mgcp_endp.c
+++ b/src/libosmo-mgcp/mgcp_endp.c
@@ -655,6 +655,35 @@
mgcp_endp_release(endp);
}
+/*! free oldest connection in the list.
+ * \param[in] endp associated endpoint */
+void mgcp_endp_free_conn_oldest(struct mgcp_endpoint *endp)
+{
+ struct mgcp_conn *conn;
+
+ if (llist_empty(&endp->conns))
+ return;
+
+ conn = llist_last_entry(&endp->conns, struct mgcp_conn, entry);
+ mgcp_conn_free(conn);
+}
+
+/*! free all connections at once.
+ * \param[in] endp associated endpoint */
+#if defined(__has_attribute)
+#if __has_attribute(no_sanitize)
+__attribute__((no_sanitize("undefined"))) /* ubsan detects a misaligned load */
+#endif
+#endif
+void mgcp_endp_free_conn_all(struct mgcp_endpoint *endp)
+{
+ struct mgcp_conn *conn;
+
+ /* Drop all items in the list, might be consecutive! */
+ while ((conn = llist_first_entry_or_null(&endp->conns, struct mgcp_conn, entry)))
+ mgcp_conn_free(conn);
+}
+
/*! release endpoint, all open connections are closed.
* \param[in] endp endpoint to release */
void mgcp_endp_release(struct mgcp_endpoint *endp)
@@ -665,7 +694,7 @@
* all connections have been removed already. In case
* that there are still connections open (e.g. when
* RSIP is executed), free them all at once. */
- mgcp_conn_free_all(endp);
+ mgcp_endp_free_conn_all(endp);
/* We must only decrement the stat item when the endpoint as actually
* claimed. An endpoint is claimed when a call-id is set */
diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c
index 47fa30b..dbe6760 100644
--- a/src/libosmo-mgcp/mgcp_protocol.c
+++ b/src/libosmo-mgcp/mgcp_protocol.c
@@ -975,7 +975,7 @@
/* There is no more room for a connection, make some
* room by blindly tossing the oldest of the two two
* connections */
- mgcp_conn_free_oldest(endp);
+ mgcp_endp_free_conn_oldest(endp);
} else {
/* There is no more room for a connection, leave
* everything as it is and return with an error */
@@ -1020,7 +1020,6 @@
"CRCX: unable to allocate RTP connection\n");
rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_ALLOC_CONN));
goto error2;
-
}
conn = mgcp_conn_get_rtp(endp, _conn->id);
@@ -1487,7 +1486,7 @@
/* delete connection */
LOGPCONN(conn->conn, DLMGCP, LOGL_DEBUG, "DLCX: deleting conn:%s\n",
mgcp_conn_dump(conn->conn));
- mgcp_conn_free(endp, conn_id);
+ mgcp_conn_free(conn->conn);
LOGPENDP(endp, DLMGCP, LOGL_NOTICE,
"DLCX: connection successfully deleted\n");
diff --git a/tests/mgcp/mgcp_test.c b/tests/mgcp/mgcp_test.c
index de359d5..44927e1 100644
--- a/tests/mgcp/mgcp_test.c
+++ b/tests/mgcp/mgcp_test.c
@@ -1230,7 +1230,7 @@
pl_test_dat[i].expected);
}
- mgcp_conn_free_all(&endp);
+ mgcp_endp_free_conn_all(&endp);
}
talloc_free(trunk);
@@ -1513,7 +1513,7 @@
}
force_monotonic_time_us = -1;
- mgcp_conn_free_all(&endp);
+ mgcp_endp_free_conn_all(&endp);
talloc_free(trunk);
talloc_free(cfg);
}
To view, visit change 39183. To unsubscribe, or for help writing mail filters, visit settings.