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

include/osmocom/mgcp/mgcp_conn.h

@@ -239,12 +239,10 @@ static inline bool mgcp_conn_rtp_is_iuup(const struct mgcp_conn_rtp *conn)
 
 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);

include/osmocom/mgcp/mgcp_endp.h

@@ -141,6 +141,8 @@ struct mgcp_endpoint *mgcp_endp_by_name(int *cause, const char *epname,
 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,

src/libosmo-mgcp/mgcp_conn.c

@@ -146,7 +146,7 @@ void mgcp_conn_watchdog_cb(void *data)
 {
 	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 @@ static void aggregate_rtp_conn_stats(struct mgcp_endpoint *endp, struct mgcp_con
 	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 @@ void mgcp_conn_free(struct mgcp_endpoint *endp, const char *id)
 	}
 
 	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 */

src/libosmo-mgcp/mgcp_endp.c

@@ -655,6 +655,35 @@ void mgcp_endp_remove_conn(struct mgcp_endpoint *endp, struct mgcp_conn *conn)
 		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 @@ void mgcp_endp_release(struct mgcp_endpoint *endp)
 	 * 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 */

src/libosmo-mgcp/mgcp_protocol.c

@@ -975,7 +975,7 @@ mgcp_header_done:
 			/* 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 @@ mgcp_header_done:
 			 "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 @@ static struct msgb *handle_delete_con(struct mgcp_request_data *rq)
 	/* 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");
 

tests/mgcp/mgcp_test.c

@@ -1230,7 +1230,7 @@ static void test_packet_loss_calc(void)
 			     pl_test_dat[i].expected);
 		}
 
-		mgcp_conn_free_all(&endp);
+		mgcp_endp_free_conn_all(&endp);
 	}
 
 	talloc_free(trunk);
@@ -1513,7 +1513,7 @@ static void test_packet_error_detection(int patch_ssrc, int patch_ts)
 	}
 
 	force_monotonic_time_us = -1;
-	mgcp_conn_free_all(&endp);
+	mgcp_endp_free_conn_all(&endp);
 	talloc_free(trunk);
 	talloc_free(cfg);
 }