Change in osmo-mgw[master]: mgcp_protocol: add support for wildcarded DLCX

dexter gerrit-no-reply at lists.osmocom.org
Thu Jul 22 13:33:58 UTC 2021


dexter has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/24973 )

Change subject: mgcp_protocol: add support for wildcarded DLCX
......................................................................

mgcp_protocol: add support for wildcarded DLCX

The request handler handle_delete_con currently rejects wildcarded DLCX
requests even though a wildcarded DLCX would be a valuable tool to
remove lingering connections from the trunk in case osmo-bsc has to be
restarted.

Change-Id: I5c2de6b2b61ee64ba9c0618fd20e8fc2fe6a5ed3
Related: SYS#5535
---
M include/osmocom/mgcp/mgcp_ratectr.h
M src/libosmo-mgcp/mgcp_protocol.c
M src/libosmo-mgcp/mgcp_ratectr.c
3 files changed, 53 insertions(+), 19 deletions(-)

Approvals:
  daniel: Looks good to me, but someone else must approve
  pespin: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/include/osmocom/mgcp/mgcp_ratectr.h b/include/osmocom/mgcp/mgcp_ratectr.h
index 78c687b..4c01059 100644
--- a/include/osmocom/mgcp/mgcp_ratectr.h
+++ b/include/osmocom/mgcp/mgcp_ratectr.h
@@ -53,7 +53,6 @@
 /* Trunk-global MCGP DLCX related rate counters */
 enum {
 	MGCP_DLCX_SUCCESS,
-	MGCP_DLCX_FAIL_WILDCARD,
 	MGCP_DLCX_FAIL_NO_CONN,
 	MGCP_DLCX_FAIL_INVALID_CALLID,
 	MGCP_DLCX_FAIL_INVALID_CONNID,
diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c
index 3af87d0..aef41e0 100644
--- a/src/libosmo-mgcp/mgcp_protocol.c
+++ b/src/libosmo-mgcp/mgcp_protocol.c
@@ -46,6 +46,16 @@
 #include <osmocom/mgcp/mgcp_codec.h>
 #include <osmocom/mgcp/mgcp_conn.h>
 
+/* 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...) \
+do { \
+	if (endp) \
+		LOGPENDP(endp, cat, level, fmt, ## args); \
+	else \
+		LOGPTRUNK(trunk, cat, level, fmt, ## args); \
+} while (0)
+
 /* Request data passed to the request handler */
 struct mgcp_request_data {
 	/* request name (e.g. "MDCX") */
@@ -102,7 +112,7 @@
 	{ .name = "DLCX",
 	  .handle_request = handle_delete_con,
 	  .debug_name = "DeleteConnection",
-	  .require_endp = true },
+	  .require_endp = false },
 	{ .name = "MDCX",
 	  .handle_request = handle_modify_con,
 	  .debug_name = "ModifiyConnection",
@@ -1351,26 +1361,21 @@
 	char stats[1048];
 	const char *conn_id = NULL;
 	struct mgcp_conn_rtp *conn = NULL;
+	unsigned int i;
 
-	LOGPENDP(endp, DLMGCP, LOGL_NOTICE,
-		 "DLCX: deleting connection ...\n");
+	/* NOTE: In this handler we can not take it for granted that the endp
+	 * pointer will be populated, however a trunk is always guaranteed. */
 
-	if (!mgcp_endp_avail(endp)) {
+	LOGPEPTR(endp, trunk, DLMGCP, LOGL_NOTICE, "DLCX: deleting connection(s) ...\n");
+
+	if (endp && !mgcp_endp_avail(endp)) {
 		rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_AVAIL));
 		LOGPENDP(endp, DLMGCP, LOGL_ERROR,
 			 "DLCX: selected endpoint not available!\n");
 		return create_err_response(NULL, 501, "DLCX", pdata->trans);
 	}
 
-	/* Prohibit wildcarded requests */
-	if (endp->wildcarded_req) {
-		LOGPENDP(endp, DLMGCP, LOGL_ERROR,
-			 "DLCX: wildcarded endpoint names not supported.\n");
-		rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_WILDCARD));
-		return create_err_response(endp, 507, "DLCX", pdata->trans);
-	}
-
-	if (llist_count(&endp->conns) <= 0) {
+	if (endp && !rq->wildcarded && llist_empty(&endp->conns)) {
 		LOGPENDP(endp, DLMGCP, LOGL_ERROR,
 			 "DLCX: endpoint is not holding a connection.\n");
 		rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_NO_CONN));
@@ -1383,6 +1388,15 @@
 
 		switch (toupper(line[0])) {
 		case 'C':
+			/* If we have no endpoint, but a call id in the request,
+			   then this request cannot be handled */
+			if (!endp) {
+				LOGPTRUNK(trunk, DLMGCP, LOGL_NOTICE,
+					  "cannot handle requests with call-id (C) without endpoint -- abort!");
+				rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_UNHANDLED_PARAM));
+				return create_err_response(NULL, 539, "DLCX", pdata->trans);
+			}
+
 			if (mgcp_verify_call_id(endp, line + 3) != 0) {
 				error_code = 516;
 				rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_INVALID_CALLID));
@@ -1390,6 +1404,15 @@
 			}
 			break;
 		case 'I':
+			/* If we have no endpoint, but a connection id in the request,
+			   then this request cannot be handled */
+			if (!endp) {
+				LOGPTRUNK(trunk, DLMGCP, LOGL_NOTICE,
+					  "cannot handle requests with conn-id (I) without endpoint -- abort!");
+				rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_UNHANDLED_PARAM));
+				return create_err_response(NULL, 539, "DLCX", pdata->trans);
+			}
+
 			conn_id = (const char *)line + 3;
 			if ((error_code = mgcp_verify_ci(endp, conn_id))) {
 				rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_INVALID_CONNID));
@@ -1400,8 +1423,7 @@
 			silent = strcasecmp("noanswer", line + 3) == 0;
 			break;
 		default:
-			LOGPENDP(endp, DLMGCP, LOGL_NOTICE,
-				 "DLCX: Unhandled MGCP option: '%c'/%d\n",
+			LOGPEPTR(endp, trunk, DLMGCP, LOGL_NOTICE, "DLCX: Unhandled MGCP option: '%c'/%d\n",
 				 line[0], line[0]);
 			rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_UNHANDLED_PARAM));
 			return create_err_response(NULL, 539, "DLCX", pdata->trans);
@@ -1432,9 +1454,23 @@
 		}
 	}
 
+	/* Handle wildcarded DLCX that refers to the whole trunk. This means
+	 * that we walk over all endpoints on the trunk in order to drop all
+	 * connections on the trunk. (see also RFC3435 Annex F.7) */
+	if (rq->wildcarded) {
+		int num_conns = 0;
+		for (i = 0; i < trunk->number_endpoints; i++) {
+			num_conns += llist_count(&trunk->endpoints[i]->conns);
+			mgcp_endp_release(trunk->endpoints[i]);
+		}
+		rate_ctr_add(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_SUCCESS), num_conns);
+		return create_ok_response(NULL, 200, "DLCX", pdata->trans);
+	}
+
 	/* When no connection id is supplied, we will interpret this as a
-	 * wildcarded DLCX and drop all connections at once. (See also
-	 * RFC3435 Section F.7) */
+	 * wildcarded DLCX that refers to the selected endpoint. This means
+	 * that we drop all connections on that specific endpoint at once.
+	 * (See also RFC3435 Section F.7) */
 	if (!conn_id) {
 		int num_conns = llist_count(&endp->conns);
 		LOGPENDP(endp, DLMGCP, LOGL_NOTICE,
diff --git a/src/libosmo-mgcp/mgcp_ratectr.c b/src/libosmo-mgcp/mgcp_ratectr.c
index 1f8b233..3c3b5db 100644
--- a/src/libosmo-mgcp/mgcp_ratectr.c
+++ b/src/libosmo-mgcp/mgcp_ratectr.c
@@ -106,7 +106,6 @@
 
 static const struct rate_ctr_desc mgcp_dlcx_ctr_desc[] = {
 	[MGCP_DLCX_SUCCESS] = { "dlcx:success", "DLCX command processed successfully." },
-	[MGCP_DLCX_FAIL_WILDCARD] = { "dlcx:wildcard", "wildcard names in DLCX commands are unsupported." },
 	[MGCP_DLCX_FAIL_NO_CONN] = { "dlcx:no_conn", "endpoint specified in DLCX command has no active connections." },
 	[MGCP_DLCX_FAIL_INVALID_CALLID] =
 	    { "dlcx:callid", "CallId specified in DLCX command mismatches endpoint's CallId ." },

-- 
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/24973
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I5c2de6b2b61ee64ba9c0618fd20e8fc2fe6a5ed3
Gerrit-Change-Number: 24973
Gerrit-PatchSet: 4
Gerrit-Owner: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann at sysmocom.de>
Gerrit-Reviewer: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210722/5d190ff4/attachment.htm>


More information about the gerrit-log mailing list