Review at https://gerrit.osmocom.org/4617
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
---
M include/osmocom/mgcp/mgcp.h
M src/libosmo-mgcp/mgcp_protocol.c
M src/libosmo-mgcp/mgcp_vty.c
3 files changed, 41 insertions(+), 10 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/17/4617/1
diff --git a/include/osmocom/mgcp/mgcp.h b/include/osmocom/mgcp/mgcp.h
index 83505a2..59147f0 100644
--- a/include/osmocom/mgcp/mgcp.h
+++ b/include/osmocom/mgcp/mgcp.h
@@ -98,7 +98,17 @@
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 9c92c65..ba8ae4f 100644
--- a/src/libosmo-mgcp/mgcp_protocol.c
+++ b/src/libosmo-mgcp/mgcp_protocol.c
@@ -666,9 +666,11 @@
if (p->cfg->change_cb)
p->cfg->change_cb(tcfg, ENDPOINT_NUMBER(endp), MGCP_ENDP_CRCX);
- if (conn->conn->mode & MGCP_CONN_RECV_ONLY
- && tcfg->keepalive_interval != 0) {
- send_dummy(endp, conn);
+ /* Send dummy packet, see also commens in mgcp_keepalive_timer_cb() */
+ OSMO_ASSERT(tcfg->keepalive_interval >= -1);
+ if (conn->conn->mode & MGCP_CONN_RECV_ONLY) {
+ if (tcfg->keepalive_interval != MGCP_KEEPALIVE_NEVER)
+ send_dummy(endp, conn);
}
LOGP(DLMGCP, LOGL_NOTICE,
@@ -815,9 +817,12 @@
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(endp, conn);
+ /* Send dummy packet, see also commens in mgcp_keepalive_timer_cb() */
+ OSMO_ASSERT(endp->tcfg->keepalive_interval >= -1);
+ if (conn->conn->mode & MGCP_CONN_RECV_ONLY) {
+ if (endp->tcfg->keepalive_interval != MGCP_KEEPALIVE_NEVER)
+ send_dummy(endp, conn);
+ }
if (silent)
goto out_silent;
@@ -1051,10 +1056,25 @@
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 >= -1)
+
+ /* 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 +1087,8 @@
}
}
- 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 a1299b2..1709f52 100644
--- a/src/libosmo-mgcp/mgcp_vty.c
+++ b/src/libosmo-mgcp/mgcp_vty.c
@@ -569,7 +569,7 @@
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;
}
--
To view, visit https://gerrit.osmocom.org/4617
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie7ee9409baec50a09fb357d655b5253434fae924
Gerrit-PatchSet: 1
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Owner: dexter <[email protected]>
