[M] Change in osmo-mgw[master]: mgw: Split conn mode parsing and applying into conn - gerrit-log

7 Jan 2025

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/39209?usp=email )
Change subject: mgw: Split conn mode parsing and applying into conn
......................................................................
mgw: Split conn mode parsing and applying into conn
Change-Id: I456843de0be1997802905873c057a83430327f50
---
M include/osmocom/mgcp/mgcp_conn.h
M include/osmocom/mgcp/mgcp_msg.h
M include/osmocom/mgcp/mgcp_protocol.h
M src/libosmo-mgcp/mgcp_conn.c
M src/libosmo-mgcp/mgcp_msg.c
M src/libosmo-mgcp/mgcp_protocol.c
6 files changed, 83 insertions(+), 62 deletions(-)
Approvals:
  Jenkins Builder: Verified
  pespin: Looks good to me, approved
  laforge: Looks good to me, but someone else must approve

diff --git a/include/osmocom/mgcp/mgcp_conn.h b/include/osmocom/mgcp/mgcp_conn.h
index 106544a..26cdffc 100644
--- a/include/osmocom/mgcp/mgcp_conn.h
+++ b/include/osmocom/mgcp/mgcp_conn.h
@@ -24,6 +24,7 @@
 #pragma once
#include <osmocom/mgcp/mgcp.h>
+#include <osmocom/mgcp/mgcp_common.h>
 #include <osmocom/mgcp/mgcp_network.h>
 #include <osmocom/mgcp/osmux.h>
 #include <osmocom/core/linuxlist.h>
@@ -247,6 +248,7 @@
 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);
+int mgcp_conn_set_mode(struct mgcp_conn *conn, enum mgcp_connection_mode mode);
 char *mgcp_conn_dump(struct mgcp_conn *conn);
 struct mgcp_conn *mgcp_find_dst_conn(struct mgcp_conn *conn);
 void mgcp_conn_watchdog_kick(struct mgcp_conn *conn);
diff --git a/include/osmocom/mgcp/mgcp_msg.h b/include/osmocom/mgcp/mgcp_msg.h
index 9572d39..d11fb85 100644
--- a/include/osmocom/mgcp/mgcp_msg.h
+++ b/include/osmocom/mgcp/mgcp_msg.h
@@ -27,6 +27,8 @@
 #include <stdint.h>
 #include <stdbool.h>
+#include <osmocom/mgcp/mgcp_common.h>
+
 struct mgcp_conn;
 struct mgcp_parse_data;
 struct mgcp_endpoint;
@@ -34,8 +36,7 @@
void mgcp_disp_msg(unsigned char *message, unsigned int len, char *preamble);
-int mgcp_parse_conn_mode(const char *msg, struct mgcp_endpoint *endp,
-			 struct mgcp_conn *conn);
+enum mgcp_connection_mode mgcp_parse_conn_mode(const char *msg);
int mgcp_parse_header(struct mgcp_parse_data *pdata, char *data);
diff --git a/include/osmocom/mgcp/mgcp_protocol.h b/include/osmocom/mgcp/mgcp_protocol.h
index df134ab..ddb226b 100644
--- a/include/osmocom/mgcp/mgcp_protocol.h
+++ b/include/osmocom/mgcp/mgcp_protocol.h
@@ -1,5 +1,8 @@
 #pragma once
+#include <osmocom/core/utils.h>
+#include <osmocom/mgcp/mgcp_common.h>
+
 /* Internal structure while parsing a request */
 struct mgcp_parse_data {
    struct mgcp_config *cfg;
@@ -28,3 +31,9 @@
uint32_t mgcp_rtp_packet_duration(const struct mgcp_endpoint *endp,
    			  const struct mgcp_rtp_end *rtp);
+
+extern const struct value_string mgcp_connection_mode_strs[];
+static inline const char *mgcp_cmode_name(enum mgcp_connection_mode mode)
+{
+	return get_value_string(mgcp_connection_mode_strs, mode);
+}
diff --git a/src/libosmo-mgcp/mgcp_conn.c b/src/libosmo-mgcp/mgcp_conn.c
index 403cbda..b4d971e 100644
--- a/src/libosmo-mgcp/mgcp_conn.c
+++ b/src/libosmo-mgcp/mgcp_conn.c
@@ -263,6 +263,37 @@
    talloc_free(conn);
 }
+/*! Parse connection mode.
+ *  \param[in] conn Connection whose mode is being set
+ *  \param[in] mode Mode to set
+ *  \returns 0 on success, -1 on error */
+int mgcp_conn_set_mode(struct mgcp_conn *conn, enum mgcp_connection_mode mode)
+{
+	OSMO_ASSERT(conn);
+	if (mode == MGCP_CONN_NONE) {
+		LOGPCONN(conn, DLMGCP, LOGL_ERROR,
+			 "missing connection mode\n");
+		return -1;
+	}
+	conn->mode = mode;
+	LOGPCONN(conn, DLMGCP, LOGL_DEBUG, "connection mode '%s' %d\n",
+		 mgcp_cmode_name(mode), conn->mode);
+
+	/* Special handling for RTP connections */
+	if (conn->type == MGCP_CONN_TYPE_RTP) {
+		struct mgcp_conn_rtp *conn_rtp = mgcp_conn_get_conn_rtp(conn);
+		conn_rtp->end.output_enabled = !!(conn->mode & MGCP_CONN_SEND_ONLY);
+		LOGPCONN(conn, DLMGCP, LOGL_DEBUG, "output_enabled %u\n",
+			 conn_rtp->end.output_enabled);
+	}
+
+	/* The VTY might change the connection mode at any time, so we have
+	 * to hold a copy of the original connection mode */
+	conn->mode_orig = conn->mode;
+
+	return 0;
+}
+
 /*! dump basic connection information to human readable string.
  *  \param[in] conn to dump
  *  \returns human readable string */
diff --git a/src/libosmo-mgcp/mgcp_msg.c b/src/libosmo-mgcp/mgcp_msg.c
index 2360a0b..f8ef50b 100644
--- a/src/libosmo-mgcp/mgcp_msg.c
+++ b/src/libosmo-mgcp/mgcp_msg.c
@@ -76,57 +76,24 @@
/*! Parse connection mode.
  *  \param[in] mode as string (recvonly, sendrecv, sendonly confecho or loopback)
- *  \param[in] endp pointer to endpoint (only used for log output)
- *  \param[out] associated connection to be modified accordingly
- *  \returns 0 on success, -1 on error */
-int mgcp_parse_conn_mode(const char *mode, struct mgcp_endpoint *endp,
-			 struct mgcp_conn *conn)
+ *  \returns MGCP_CONN_* on success, MGCP_CONN_NONE on error */
+enum mgcp_connection_mode mgcp_parse_conn_mode(const char *mode)
 {
-	int ret = 0;
-	if (!mode) {
-		LOGPCONN(conn, DLMGCP, LOGL_ERROR,
-			 "missing connection mode\n");
-		return -1;
-	}
-	if (!conn)
-		return -1;
-	if (!endp)
-		return -1;
+	if (!mode)
+		return MGCP_CONN_NONE;
if (strcasecmp(mode, "recvonly") == 0)
-		conn->mode = MGCP_CONN_RECV_ONLY;
-	else if (strcasecmp(mode, "sendrecv") == 0)
-		conn->mode = MGCP_CONN_RECV_SEND;
-	else if (strcasecmp(mode, "sendonly") == 0)
-		conn->mode = MGCP_CONN_SEND_ONLY;
-	else if (strcasecmp(mode, "confecho") == 0)
-		conn->mode = MGCP_CONN_CONFECHO;
-	else if (strcasecmp(mode, "loopback") == 0)
-		conn->mode = MGCP_CONN_LOOPBACK;
-	else {
-		LOGPCONN(conn, DLMGCP, LOGL_ERROR,
-			 "unknown connection mode: '%s'\n", mode);
-		ret = -1;
-	}
-
-	LOGPENDP(endp, DLMGCP, LOGL_DEBUG, "conn:%s\n", mgcp_conn_dump(conn));
-	LOGPCONN(conn, DLMGCP, LOGL_DEBUG, "connection mode '%s' %d\n",
-		 mode, conn->mode);
-
-	/* Special handling for RTP connections */
-	if (conn->type == MGCP_CONN_TYPE_RTP) {
-		struct mgcp_conn_rtp *conn_rtp = mgcp_conn_get_conn_rtp(conn);
-		conn_rtp->end.output_enabled = !!(conn->mode & MGCP_CONN_SEND_ONLY);
-		LOGPCONN(conn, DLMGCP, LOGL_DEBUG, "output_enabled %u\n",
-			 conn_rtp->end.output_enabled);
-	}
-
-	/* The VTY might change the connection mode at any time, so we have
-	 * to hold a copy of the original connection mode */
-	conn->mode_orig = conn->mode;
-
-	return ret;
+		return MGCP_CONN_RECV_ONLY;
+	if (strcasecmp(mode, "sendrecv") == 0)
+		return MGCP_CONN_RECV_SEND;
+	if (strcasecmp(mode, "sendonly") == 0)
+		return MGCP_CONN_SEND_ONLY;
+	if (strcasecmp(mode, "confecho") == 0)
+		return MGCP_CONN_CONFECHO;
+	if (strcasecmp(mode, "loopback") == 0)
+		return MGCP_CONN_LOOPBACK;
+	return MGCP_CONN_NONE;
 }
/*! Analyze and parse the the hader of an MGCP messeage string.
diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c
index 7df2544..6f94f39 100644
--- a/src/libosmo-mgcp/mgcp_protocol.c
+++ b/src/libosmo-mgcp/mgcp_protocol.c
@@ -59,6 +59,16 @@
    return debug_last_endpoint_name;
 }
+const struct value_string mgcp_connection_mode_strs[] = {
+	{ MGCP_CONN_NONE, "none" },
+	{ MGCP_CONN_RECV_SEND, "sendrecv" },
+	{ MGCP_CONN_SEND_ONLY, "sendonly" },
+	{ MGCP_CONN_RECV_ONLY, "recvonly" },
+	{ MGCP_CONN_CONFECHO, "confecho" },
+	{ MGCP_CONN_LOOPBACK, "loopback" },
+	{ 0, NULL }
+};
+
 /* A combination of LOGPENDP and LOGPTRUNK that automatically falls back to
  * LOGPTRUNK when the endp parameter is NULL */
 #define LOGPEPTR(endp, trunk, cat, level, fmt, args...) \
@@ -870,7 +880,7 @@
    int error_code = 400;
    const char *local_options = NULL;
    const char *callid = NULL;
-	const char *mode = NULL;
+	enum mgcp_connection_mode mode = MGCP_CONN_NONE;
    char *line;
    int have_sdp = 0, remote_osmux_cid = -2;
    struct mgcp_conn_rtp *conn = NULL;
@@ -922,7 +932,7 @@
    		return create_err_response(rq->trunk, NULL, 523, "CRCX", pdata->trans);
    		break;
    	case 'M':
-			mode = (const char *)line + 3;
+			mode = mgcp_parse_conn_mode((const char *)line + 3);
    		break;
    	case 'X':
    		if (strncasecmp("Osmux: ", line + 2, strlen("Osmux: ")) == 0) {
@@ -959,9 +969,9 @@
    	return create_err_response(endp, endp, 516, "CRCX", pdata->trans);
    }
-	if (!mode) {
+	if (mode == MGCP_CONN_NONE) {
    	LOGPENDP(endp, DLMGCP, LOGL_ERROR,
-			 "CRCX: insufficient parameters, missing mode\n");
+			 "CRCX: insufficient parameters, invalid mode\n");
    	rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_INVALID_MODE));
    	return create_err_response(endp, endp, 517, "CRCX", pdata->trans);
    }
@@ -1023,15 +1033,15 @@
    	goto error2;
    }
-	conn = mgcp_conn_get_conn_rtp(_conn);
-	OSMO_ASSERT(conn);
-
-	if (mgcp_parse_conn_mode(mode, endp, _conn) != 0) {
+	if (mgcp_conn_set_mode(_conn, mode) < 0) {
    	error_code = 517;
    	rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_INVALID_MODE));
    	goto error2;
    }
+	conn = mgcp_conn_get_conn_rtp(_conn);
+	OSMO_ASSERT(conn);
+
    /* If X-Osmux (remote CID) was received (-1 is wilcard), alloc next avail CID as local CID */
    if (remote_osmux_cid >= -1) {
    	if (osmux_init_conn(conn) < 0) {
@@ -1145,7 +1155,7 @@
    int have_sdp = 0;
    char *line;
    const char *local_options = NULL;
-	const char *mode = NULL;
+	enum mgcp_connection_mode mode = MGCP_CONN_NONE;
    struct mgcp_conn_rtp *conn = NULL;
    const char *conn_id = NULL;
    int remote_osmux_cid = -2;
@@ -1204,7 +1214,7 @@
    		local_options = (const char *)line + 3;
    		break;
    	case 'M':
-			mode = (const char *)line + 3;
+			mode = mgcp_parse_conn_mode((const char *)line + 3);
    		break;
    	case 'X':
    		if (strncasecmp("Osmux: ", line + 2, strlen("Osmux: ")) == 0) {
@@ -1246,14 +1256,15 @@
mgcp_conn_watchdog_kick(conn->conn);
-	if (mode) {
-		if (mgcp_parse_conn_mode(mode, endp, conn->conn) != 0) {
+	if (mode != MGCP_CONN_NONE) {
+		if (mgcp_conn_set_mode(conn->conn, mode) < 0) {
    		rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_INVALID_MODE));
    		error_code = 517;
    		goto error3;
    	}
-	} else
+	} else {
    	conn->conn->mode = conn->conn->mode_orig;
+	}
/* Set local connection options, if present */
    if (local_options) {
-- 
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/39209?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email

Gerrit-MessageType: merged
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I456843de0be1997802905873c057a83430327f50
Gerrit-Change-Number: 39209
Gerrit-PatchSet: 2
Gerrit-Owner: pespin pespin@sysmocom.de
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge laforge@osmocom.org
Gerrit-Reviewer: pespin pespin@sysmocom.de



    