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

@@ -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;

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 &&
+	/* Send dummy packet, see also comments in mgcp_keepalive_timer_cb() */
-	    endp->tcfg->keepalive_interval != 0)
+	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);

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;
 }