Change in osmo-mgw[master]: mgcp_protocol: get rid of policy_cb and change_cb.

dexter gerrit-no-reply at lists.osmocom.org
Wed Aug 4 15:44:16 UTC 2021


dexter has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-mgw/+/25126 )


Change subject: mgcp_protocol: get rid of policy_cb and change_cb.
......................................................................

mgcp_protocol: get rid of policy_cb and change_cb.

The two callback functions policy_cb and change_cb are essentially dead
code. They also make the code more difficult to read and understand.
Lets remove them.

Change-Id: I19f67db1c56473f47338b56114f6bbae8981d067
---
M include/osmocom/mgcp/mgcp.h
M include/osmocom/mgcp/mgcp_protocol.h
M src/libosmo-mgcp/mgcp_protocol.c
M tests/mgcp/mgcp_test.c
4 files changed, 17 insertions(+), 110 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/26/25126/1

diff --git a/include/osmocom/mgcp/mgcp.h b/include/osmocom/mgcp/mgcp.h
index 29963cc..b3f2eb5 100644
--- a/include/osmocom/mgcp/mgcp.h
+++ b/include/osmocom/mgcp/mgcp.h
@@ -62,8 +62,6 @@
 #define MGCP_POLICY_REJECT	5
 #define MGCP_POLICY_DEFER	6
 
-typedef int (*mgcp_change)(struct mgcp_endpoint *endp, int state);
-typedef int (*mgcp_policy)(struct mgcp_endpoint *endp, int state, const char *transaction_id);
 typedef int (*mgcp_reset)(struct mgcp_trunk *cfg);
 typedef int (*mgcp_rqnt)(struct mgcp_endpoint *endp, char tone);
 
@@ -147,8 +145,6 @@
 
 	int force_ptime;
 
-	mgcp_change change_cb;
-	mgcp_policy policy_cb;
 	mgcp_reset reset_cb;
 	mgcp_rqnt rqnt_cb;
 	void *data;
diff --git a/include/osmocom/mgcp/mgcp_protocol.h b/include/osmocom/mgcp/mgcp_protocol.h
index 7ab283d..df134ab 100644
--- a/include/osmocom/mgcp/mgcp_protocol.h
+++ b/include/osmocom/mgcp/mgcp_protocol.h
@@ -16,6 +16,8 @@
 	int pkt_period_max; /* time in ms */
 };
 
+char *mgcp_debug_get_last_endpoint_name(void);
+
 char *get_lco_identifier(const char *options);
 int check_local_cx_options(void *ctx, const char *options);
 
diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c
index e69c00f..1343b61 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>
 
+/* Contains the last successfuly resolved endpoint name. This variable is used
+ * for the unit-tests to verify that the endpoint was correctly resolved. */
+static char debug_last_endpoint_name[MGCP_ENDPOINT_MAXLEN];
+
+/* Called from unit-tests only */
+char *mgcp_debug_get_last_endpoint_name(void)
+{
+	return debug_last_endpoint_name;
+}
+
 /* 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...) \
@@ -328,6 +338,8 @@
 	struct msgb *resp = NULL;
 	char *data;
 
+	debug_last_endpoint_name[0] = '\0';
+
 	/* Count all messages, even incorect ones */
 	rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_GENERAL_RX_MSGS_TOTAL));
 
@@ -395,6 +407,7 @@
 			return create_err_response(NULL, -rq.mgcp_cause, rq.name, pdata.trans);
 		}
 	} else {
+		osmo_strlcpy(debug_last_endpoint_name, rq.endp->name, sizeof(debug_last_endpoint_name));
 		rq.trunk = rq.endp->trunk;
 		rq.mgcp_cause = 0;
 
@@ -1050,32 +1063,8 @@
 		goto error2;
 	}
 
-	/* policy CB */
-	if (pdata->cfg->policy_cb) {
-		int rc;
-		rc = pdata->cfg->policy_cb(endp, MGCP_ENDP_CRCX, pdata->trans);
-		switch (rc) {
-		case MGCP_POLICY_REJECT:
-			LOGPCONN(_conn, DLMGCP, LOGL_NOTICE,
-				 "CRCX: CRCX rejected by policy\n");
-			mgcp_endp_release(endp);
-			rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_REJECTED_BY_POLICY));
-			return create_err_response(endp, 400, "CRCX", pdata->trans);
-			break;
-		case MGCP_POLICY_DEFER:
-			/* stop processing */
-			return NULL;
-			break;
-		case MGCP_POLICY_CONT:
-			/* just continue */
-			break;
-		}
-	}
-
 	LOGPCONN(conn->conn, DLMGCP, LOGL_DEBUG,
 		 "CRCX: Creating connection: port: %u\n", conn->end.local_port);
-	if (pdata->cfg->change_cb)
-		pdata->cfg->change_cb(endp, MGCP_ENDP_CRCX);
 
 	/* Send dummy packet, see also comments in mgcp_keepalive_timer_cb() */
 	OSMO_ASSERT(trunk->keepalive_interval >= MGCP_KEEPALIVE_ONCE);
@@ -1290,40 +1279,11 @@
 		goto error3;
 	}
 
-
-	/* policy CB */
-	if (pdata->cfg->policy_cb) {
-		int rc;
-		rc = pdata->cfg->policy_cb(endp, MGCP_ENDP_MDCX, pdata->trans);
-		switch (rc) {
-		case MGCP_POLICY_REJECT:
-			LOGPCONN(conn->conn, DLMGCP, LOGL_NOTICE,
-				 "MDCX: rejected by policy\n");
-			rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_REJECTED_BY_POLICY));
-			if (silent)
-				goto out_silent;
-			return create_err_response(endp, 400, "MDCX", pdata->trans);
-			break;
-		case MGCP_POLICY_DEFER:
-			/* stop processing */
-			LOGPCONN(conn->conn, DLMGCP, LOGL_DEBUG,
-				 "MDCX: deferred by policy\n");
-			rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_DEFERRED_BY_POLICY));
-			return NULL;
-			break;
-		case MGCP_POLICY_CONT:
-			/* just continue */
-			break;
-		}
-	}
-
 	mgcp_rtp_end_config(endp, 1, &conn->end);
 
 	/* modify */
 	LOGPCONN(conn->conn, DLMGCP, LOGL_DEBUG,
 		 "MDCX: modified conn:%s\n", mgcp_conn_dump(conn->conn));
-	if (pdata->cfg->change_cb)
-		pdata->cfg->change_cb(endp, MGCP_ENDP_MDCX);
 
 	/* Send dummy packet, see also comments in mgcp_keepalive_timer_cb() */
 	OSMO_ASSERT(trunk->keepalive_interval >= MGCP_KEEPALIVE_ONCE);
@@ -1431,29 +1391,6 @@
 		}
 	}
 
-	/* policy CB */
-	if (pdata->cfg->policy_cb) {
-		int rc;
-		rc = pdata->cfg->policy_cb(endp, MGCP_ENDP_DLCX, pdata->trans);
-		switch (rc) {
-		case MGCP_POLICY_REJECT:
-			LOGPENDP(endp, DLMGCP, LOGL_NOTICE, "DLCX: rejected by policy\n");
-			rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_REJECTED_BY_POLICY));
-			if (silent)
-				goto out_silent;
-			return create_err_response(endp, 400, "DLCX", pdata->trans);
-			break;
-		case MGCP_POLICY_DEFER:
-			/* stop processing */
-			rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_DEFERRED_BY_POLICY));
-			return NULL;
-			break;
-		case MGCP_POLICY_CONT:
-			/* just continue */
-			break;
-		}
-	}
-
 	/* 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) */
@@ -1515,9 +1452,6 @@
 		LOGPENDP(endp, DLMGCP, LOGL_DEBUG, "DLCX: endpoint released\n");
 	}
 
-	if (pdata->cfg->change_cb)
-		pdata->cfg->change_cb(endp, MGCP_ENDP_DLCX);
-
 	rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_SUCCESS));
 	if (silent)
 		goto out_silent;
diff --git a/tests/mgcp/mgcp_test.c b/tests/mgcp/mgcp_test.c
index 26fcc2a..1c0d3cc 100644
--- a/tests/mgcp/mgcp_test.c
+++ b/tests/mgcp/mgcp_test.c
@@ -614,28 +614,6 @@
 	return msg;
 }
 
-static char last_endpoint[MGCP_ENDPOINT_MAXLEN];
-
-static int mgcp_test_policy_cb(struct mgcp_endpoint *endp,
-			       int state, const char *transaction_id)
-{
-	unsigned int i;
-	struct mgcp_trunk *trunk;
-
-	fprintf(stderr, "Policy CB got state %d on endpoint %s\n",
-		state, endp->name);
-
-	trunk = endp->trunk;
-	last_endpoint[0] = '\0';
-	for (i = 0; i < trunk->number_endpoints; i++) {
-		if (strcmp(endp->name, trunk->endpoints[i]->name) == 0)
-			osmo_strlcpy(last_endpoint, trunk->endpoints[i]->name,
-				     sizeof(last_endpoint));
-	}
-
-	return MGCP_POLICY_CONT;
-}
-
 static int dummy_packets = 0;
 /* override and forward */
 ssize_t sendto(int sockfd, const void *buf, size_t len, int flags,
@@ -792,13 +770,13 @@
 	struct mgcp_conn_rtp *conn = NULL;
 	char last_conn_id[256];
 	int rc;
+	char *last_endpoint = mgcp_debug_get_last_endpoint_name();
 
 	cfg = mgcp_config_alloc();
 	trunk = mgcp_trunk_by_num(cfg, MGCP_TRUNK_VIRTUAL, MGCP_VIRT_TRUNK_ID);
 
 	trunk->v.vty_number_endpoints = 64;
         mgcp_trunk_equip(trunk);
-	cfg->policy_cb = mgcp_test_policy_cb;
 
 	memset(last_conn_id, 0, sizeof(last_conn_id));
 
@@ -810,7 +788,6 @@
 		printf("\n================================================\n");
 		printf("Testing %s\n", t->name);
 
-		last_endpoint[0] = '\0';
 		dummy_packets = 0;
 
 		osmo_talloc_replace_string(cfg, &trunk->audio_fmtp_extra,
@@ -1411,6 +1388,7 @@
 	struct in_addr addr;
 	struct mgcp_conn_rtp *conn = NULL;
 	char conn_id[256];
+	char *last_endpoint = mgcp_debug_get_last_endpoint_name();
 
 	printf("Testing multiple payload types\n");
 
@@ -1418,7 +1396,6 @@
 	trunk = mgcp_trunk_by_num(cfg, MGCP_TRUNK_VIRTUAL, MGCP_VIRT_TRUNK_ID);
 	trunk->v.vty_number_endpoints = 64;
         mgcp_trunk_equip(trunk);
-	cfg->policy_cb = mgcp_test_policy_cb;
 
 	/* Allocate endpoint 1 at mgw with two codecs */
 	last_endpoint[0] = '\0';
@@ -1620,8 +1597,6 @@
 	trunk->audio_send_name = 0;
         mgcp_trunk_equip(trunk);
 
-	cfg->policy_cb = mgcp_test_policy_cb;
-
 	inp = create_msg(CRCX, NULL);
 	msg = mgcp_handle_message(cfg, inp);
 

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

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I19f67db1c56473f47338b56114f6bbae8981d067
Gerrit-Change-Number: 25126
Gerrit-PatchSet: 1
Gerrit-Owner: dexter <pmaier at sysmocom.de>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210804/4455c786/attachment.htm>


More information about the gerrit-log mailing list