From e726d4fad21c9b88e0cf3aecb7d7cb0facda5e48 Mon Sep 17 00:00:00 2001 From: Philipp Maier Date: Wed, 1 Nov 2017 10:41:34 +0100 Subject: [PATCH] cosmetic: make dummy packet handling more explicit The way how osmo-mgw decides when to send a dummy packet and when not is not very obvious. use more explicit if statements, and define constants. Also add comments that explain how it works. Change-Id: Ie7ee9409baec50a09fb357d655b5253434fae924 --- include/osmocom/mgcp/mgcp.h | 10 ++++++++++ src/libosmo-mgcp/mgcp_protocol.c | 33 +++++++++++++++++++++++++------- src/libosmo-mgcp/mgcp_vty.c | 2 +- 3 files changed, 37 insertions(+), 8 deletions(-) diff --git a/include/osmocom/mgcp/mgcp.h b/include/osmocom/mgcp/mgcp.h index 83505a2fd..59147f0a2 100644 --- a/include/osmocom/mgcp/mgcp.h +++ b/include/osmocom/mgcp/mgcp.h @@ -98,7 +98,17 @@ struct mgcp_port_range { int last_port; }; +/* There are up to three modes in which the keep-alive dummy packet can be + * sent. The beviour is controlled viw the keepalive_interval member of the + * trunk config. If that member is set to 0 (MGCP_KEEPALIVE_NEVER) no dummy- + * packet is sent at all and the timer that sends regular dummy packets + * is no longer scheduled. If the keepalive_interval is set to -1, only + * one dummy packet is sent when an CRCX or an MDCX is performed. No timer + * is scheduled. For all vales greater 0, the a timer is scheduled and the + * value is used as interval. See also mgcp_keepalive_timer_cb(), + * handle_modify_con(), and handle_create_con() */ #define MGCP_KEEPALIVE_ONCE (-1) +#define MGCP_KEEPALIVE_NEVER 0 struct mgcp_trunk_config { struct llist_head entry; diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c index 9c92c65b8..482679035 100644 --- a/src/libosmo-mgcp/mgcp_protocol.c +++ b/src/libosmo-mgcp/mgcp_protocol.c @@ -666,10 +666,11 @@ mgcp_header_done: if (p->cfg->change_cb) p->cfg->change_cb(tcfg, ENDPOINT_NUMBER(endp), MGCP_ENDP_CRCX); + /* Send dummy packet, see also comments in mgcp_keepalive_timer_cb() */ + OSMO_ASSERT(tcfg->keepalive_interval >= MGCP_KEEPALIVE_ONCE); if (conn->conn->mode & MGCP_CONN_RECV_ONLY - && tcfg->keepalive_interval != 0) { + && tcfg->keepalive_interval != MGCP_KEEPALIVE_NEVER) send_dummy(endp, conn); - } LOGP(DLMGCP, LOGL_NOTICE, "CRCX: endpoint:%x connection successfully created\n", @@ -815,8 +816,10 @@ mgcp_header_done: p->cfg->change_cb(endp->tcfg, ENDPOINT_NUMBER(endp), MGCP_ENDP_MDCX); - if (conn->conn->mode & MGCP_CONN_RECV_ONLY && - endp->tcfg->keepalive_interval != 0) + /* Send dummy packet, see also comments in mgcp_keepalive_timer_cb() */ + OSMO_ASSERT(endp->tcfg->keepalive_interval >= MGCP_KEEPALIVE_ONCE); + if (conn->conn->mode & MGCP_CONN_RECV_ONLY + && endp->tcfg->keepalive_interval != MGCP_KEEPALIVE_NEVER) send_dummy(endp, conn); if (silent) @@ -1051,10 +1054,25 @@ static void mgcp_keepalive_timer_cb(void *_tcfg) struct mgcp_conn *conn; int i; - LOGP(DLMGCP, LOGL_DEBUG, "Triggered trunk %d keepalive timer.\n", + LOGP(DLMGCP, LOGL_DEBUG, "triggered trunk %d keepalive timer\n", tcfg->trunk_nr); - if (tcfg->keepalive_interval <= 0) + /* Do not accept invalid configuration values + * valid is MGCP_KEEPALIVE_NEVER, MGCP_KEEPALIVE_ONCE and + * values greater 0 */ + OSMO_ASSERT(tcfg->keepalive_interval >= MGCP_KEEPALIVE_ONCE); + + /* The dummy packet functionality has been disabled, we will exit + * immediately, no further timer is scheduled, which means we will no + * longer send dummy packets even when we did before */ + if (tcfg->keepalive_interval == MGCP_KEEPALIVE_NEVER) + return; + + /* In cases where only one dummy packet is sent, we do not need + * the timer since the functions that handle the CRCX and MDCX are + * triggering the sending of the dummy packet. So we behave like in + * the MGCP_KEEPALIVE_NEVER case */ + if (tcfg->keepalive_interval == MGCP_KEEPALIVE_ONCE) return; /* Send walk over all endpoints and send out dummy packets through @@ -1067,7 +1085,8 @@ static void mgcp_keepalive_timer_cb(void *_tcfg) } } - LOGP(DLMGCP, LOGL_DEBUG, "Rescheduling trunk %d keepalive timer.\n", + /* Schedule the keepalive timer for the next round */ + LOGP(DLMGCP, LOGL_DEBUG, "rescheduling trunk %d keepalive timer\n", tcfg->trunk_nr); osmo_timer_schedule(&tcfg->keepalive_timer, tcfg->keepalive_interval, 0); diff --git a/src/libosmo-mgcp/mgcp_vty.c b/src/libosmo-mgcp/mgcp_vty.c index e8ad818e8..7107bcc56 100644 --- a/src/libosmo-mgcp/mgcp_vty.c +++ b/src/libosmo-mgcp/mgcp_vty.c @@ -569,7 +569,7 @@ DEFUN(cfg_mgcp_no_rtp_keepalive, cfg_mgcp_no_rtp_keepalive_cmd, "no rtp keep-alive", NO_STR RTP_STR RTP_KEEPALIVE_STR) { - mgcp_trunk_set_keepalive(&g_cfg->trunk, 0); + mgcp_trunk_set_keepalive(&g_cfg->trunk, MGCP_KEEPALIVE_NEVER); return CMD_SUCCESS; }