diff mbox

[5/6] osmux: Enforce Osmux only global and per BSC configuration

Message ID 1443950574-75194-5-git-send-email-holger@freyther.de
State Accepted
Headers show

Commit Message

Holger Freyther Oct. 4, 2015, 9:22 a.m. UTC 
From: Holger Hans Peter Freyther <holger@moiji-mobile.com>

Extend the osmux only setting from the MGCP MGW to the NAT. This
is applied when an endpoint is allocated and/or when the allocation
is confirmed by the remote system.

Not tested. The impact should only be when the new option is
being used.

Fixes: OW#1492
---
 openbsc/src/osmo-bsc_nat/bsc_mgcp_utils.c | 32 ++++++++++++++++++++++++++++---
 openbsc/src/osmo-bsc_nat/bsc_nat_vty.c    | 22 ++++++++++++++-------
 2 files changed, 44 insertions(+), 10 deletions(-)

Comments

Neels Hofmeyr Oct. 5, 2015, 10:37 a.m. UTC | #1 
On Sun, Oct 04, 2015 at 11:22:53AM +0200, Holger Hans Peter Freyther wrote:
> +	/* If we require osmux and it is not disabled.. fail */
> +	if (nat_osmux_only(bsc->nat->mgcp_cfg, bsc->cfg) &&
> +		endp->osmux.state == OSMUX_STATE_DISABLED) {
[...]

Comment: If we require osmux and it IS disabled, fail

That's the only thing I spotted in your patch series.

~Neels
Holger Freyther Oct. 5, 2015, 2:22 p.m. UTC | #2 
> On 05 Oct 2015, at 12:37, Neels Hofmeyr <nhofmeyr@sysmocom.de> wrote:
> 
> On Sun, Oct 04, 2015 at 11:22:53AM +0200, Holger Hans Peter Freyther wrote:
>> +	/* If we require osmux and it is not disabled.. fail */
>> +	if (nat_osmux_only(bsc->nat->mgcp_cfg, bsc->cfg) &&
>> +		endp->osmux.state == OSMUX_STATE_DISABLED) {
> [...]
> 
> Comment: If we require osmux and it IS disabled, fail
> 
> That's the only thing I spotted in your patch series.

thanks!
diff mbox

Patch

diff --git a/openbsc/src/osmo-bsc_nat/bsc_mgcp_utils.c b/openbsc/src/osmo-bsc_nat/bsc_mgcp_utils.c
index f19cb4c..aad59d4 100644
--- a/openbsc/src/osmo-bsc_nat/bsc_mgcp_utils.c
+++ b/openbsc/src/osmo-bsc_nat/bsc_mgcp_utils.c
@@ -499,6 +499,15 @@  struct nat_sccp_connection *bsc_mgcp_find_con(struct bsc_nat *nat, int endpoint)
 	return NULL;
 }
 
+static int nat_osmux_only(struct mgcp_config *mgcp_cfg, struct bsc_config *bsc_cfg)
+{
+	if (mgcp_cfg->osmux == OSMUX_USAGE_ONLY)
+		return 1;
+	if (bsc_cfg->osmux == OSMUX_USAGE_ONLY)
+		return 1;
+	return 0;
+}
+
 static int bsc_mgcp_policy_cb(struct mgcp_trunk_config *tcfg, int endpoint, int state, const char *transaction_id)
 {
 	struct bsc_nat *nat;
@@ -544,9 +553,16 @@  static int bsc_mgcp_policy_cb(struct mgcp_trunk_config *tcfg, int endpoint, int
 	}
 
 	/* Allocate a Osmux circuit ID */
-	if (state == MGCP_ENDP_CRCX &&
-	    nat->mgcp_cfg->osmux && sccp->bsc->cfg->osmux)
-		osmux_cid = osmux_get_cid();
+	if (state == MGCP_ENDP_CRCX) {
+		if (nat->mgcp_cfg->osmux && sccp->bsc->cfg->osmux) {
+			osmux_cid = osmux_get_cid();
+			if (osmux_cid < 0 && nat_osmux_only(nat->mgcp_cfg, sccp->bsc->cfg)) {
+				LOGP(DMGCP, LOGL_ERROR,
+					"Rejecting usage of endpoint\n");
+				return MGCP_POLICY_REJECT;
+			}
+		}
+	}
 
 	/* we need to generate a new and patched message */
 	bsc_msg = bsc_mgcp_rewrite((char *) nat->mgcp_msg, nat->mgcp_length,
@@ -734,6 +750,16 @@  void bsc_mgcp_forward(struct bsc_connection *bsc, struct msgb *msg)
 	if (endp->osmux.state == OSMUX_STATE_ACTIVATING)
 		bsc_mgcp_osmux_confirm(endp, (const char *) msg->l2h);
 
+	/* If we require osmux and it is not disabled.. fail */
+	if (nat_osmux_only(bsc->nat->mgcp_cfg, bsc->cfg) &&
+		endp->osmux.state == OSMUX_STATE_DISABLED) {
+		LOGP(DMGCP, LOGL_ERROR,
+			"Failed to activate osmux endpoint 0x%x\n",
+			ENDPOINT_NUMBER(endp));
+		free_chan_downstream(endp, bsc_endp, bsc);
+		return;
+	}
+
 	/* free some stuff */
 	talloc_free(bsc_endp->transaction_id);
 	bsc_endp->transaction_id = NULL;
diff --git a/openbsc/src/osmo-bsc_nat/bsc_nat_vty.c b/openbsc/src/osmo-bsc_nat/bsc_nat_vty.c
index 981168c..cd8293c 100644
--- a/openbsc/src/osmo-bsc_nat/bsc_nat_vty.c
+++ b/openbsc/src/osmo-bsc_nat/bsc_nat_vty.c
@@ -162,8 +162,14 @@  static void config_write_bsc_single(struct vty *vty, struct bsc_config *bsc)
 	if (bsc->paging_group != -1)
 		vty_out(vty, "  paging group %d%s", bsc->paging_group, VTY_NEWLINE);
 	vty_out(vty, "  paging forbidden %d%s", bsc->forbid_paging, VTY_NEWLINE);
-	if (bsc->osmux)
+	switch (bsc->osmux) {
+	case OSMUX_USAGE_ON:
 		vty_out(vty, "  osmux on%s", VTY_NEWLINE);
+		break;
+	case OSMUX_USAGE_ONLY:
+		vty_out(vty, "  osmux only%s", VTY_NEWLINE);
+		break;
+	}
 }
 
 static int config_write_bsc(struct vty *vty)
@@ -1124,18 +1130,20 @@  DEFUN(show_ussd_connection,
 #define OSMUX_STR "RTP multiplexing\n"
 DEFUN(cfg_bsc_osmux,
       cfg_bsc_osmux_cmd,
-      "osmux (on|off)",
-       OSMUX_STR "Enable OSMUX\n" "Disable OSMUX\n")
+      "osmux (on|off|only)",
+       OSMUX_STR "Enable OSMUX\n" "Disable OSMUX\n" "Only OSMUX\n")
 {
 	struct bsc_config *conf = vty->index;
 	int old = conf->osmux;
 
 	if (strcmp(argv[0], "on") == 0)
-		conf->osmux = 1;
+		conf->osmux = OSMUX_USAGE_ON;
 	else if (strcmp(argv[0], "off") == 0)
-		conf->osmux = 0;
+		conf->osmux = OSMUX_USAGE_OFF;
+	else if (strcmp(argv[0], "only") == 0)
+		conf->osmux = OSMUX_USAGE_ONLY;
 
-	if (old == 0 && conf->osmux == 1 && !conf->nat->mgcp_cfg->osmux_init) {
+	if (old == 0 && conf->osmux > 0 && !conf->nat->mgcp_cfg->osmux_init) {
 		LOGP(DMGCP, LOGL_NOTICE, "Setting up OSMUX socket\n");
 		if (osmux_init(OSMUX_ROLE_BSC_NAT, conf->nat->mgcp_cfg) < 0) {
 			LOGP(DMGCP, LOGL_ERROR, "Cannot init OSMUX\n");
@@ -1143,7 +1151,7 @@  DEFUN(cfg_bsc_osmux,
 				VTY_NEWLINE);
 			return CMD_WARNING;
 		}
-	} else if (old == 1 && conf->osmux == 0) {
+	} else if (old > 0 && conf->osmux == 0) {
 		LOGP(DMGCP, LOGL_NOTICE, "Disabling OSMUX socket\n");
 		/* Don't stop the socket, we may already have ongoing voice
 		 * flows already using Osmux. This just switch indicates that