This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
dexter gerrit-no-reply at lists.osmocom.orgdexter 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>