[PATCH] osmo-bsc[master]: osmo-bsc: SCCP addrs: default only if unset, reject invalid

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/.

Neels Hofmeyr gerrit-no-reply at lists.osmocom.org
Wed Nov 8 02:30:51 UTC 2017


Review at  https://gerrit.osmocom.org/4726

osmo-bsc: SCCP addrs: default only if unset, reject invalid

So far, if the user entered an invalid SCCP address in the config, the
osmo_bsc_sigtran_init() code simply replaced that with the default, i.e.
running with a completely different address than the user may intend.

Use the default SCCP addresses only when they are unset by the user.

Default MSC addr: set directly, do not detour via cs7 instance PC. The default
MSC SCCP addr is just a point code + SSN, deriving it from the cs7 instance
first is a confusing step. Just set the PC and SSN, and done.

Using default addresses does not constitute an "auto configuration": if we set
up a cs7 instance automatically, we do not want to have to create a second one
automatically, to prevent "auto-confusion", and want to bail instead. But for
each MSC on its own, using default SCCP addresses makes sense and is orthogonal
to automatic cs7 instance creation. Hence drop the auto config semantics from
the default SCCP address parts.

Always validate the SCCP addresses we will end up using, and bail immediately
if they are erratic. i.e. don't overwrite a non-empty invalid SCCP address with
defaults, but straight bail.

Beneficial side effects:
- Fix some grammar ultra confusion in log messages.
- Add context: log the MSC number the logging refers to.
- Drop code dup: since we're always logging the used SCCP addresses, might as
  well log those once, unconditionally, in the end.

Change-Id: Iadbc2e9740457e1b389b7e7ad9c94274e7d8cb11
---
M src/osmo-bsc/osmo_bsc_sigtran.c
1 file changed, 25 insertions(+), 32 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/26/4726/1

diff --git a/src/osmo-bsc/osmo_bsc_sigtran.c b/src/osmo-bsc/osmo_bsc_sigtran.c
index 2ba777e..253f1e8 100644
--- a/src/osmo-bsc/osmo_bsc_sigtran.c
+++ b/src/osmo-bsc/osmo_bsc_sigtran.c
@@ -509,46 +509,39 @@
 		if (!msc->a.sccp)
 			return -EINVAL;
 
-		/* Check if the sccp-address fullfills minimum requirements (SSN+PC is present,
-		 * automatically recover addresses if the addresses are not set up properly) */
-		if (!osmo_sccp_check_addr(&msc->a.bsc_addr, OSMO_SCCP_ADDR_T_SSN | OSMO_SCCP_ADDR_T_PC)) {
-			if (fail_on_next_invalid_cfg)
-				goto fail_auto_cofiguration;
-			free_attempt_used = true;
-
-			LOGP(DMSC, LOGL_NOTICE,
-			     "A-interface: invalid or missing local (BSC) SCCP address (a.bsc_addr=%s)\n",
-			     osmo_sccp_addr_name(osmo_ss7_instance_find(msc->a.cs7_instance), &msc->a.bsc_addr));
+		/* If unset, use default local SCCP address */
+		if (!msc->a.bsc_addr.presence)
 			osmo_sccp_local_addr_by_instance(&msc->a.bsc_addr, msc->a.sccp, SCCP_SSN_BSSAP);
-			LOGP(DMSC, LOGL_NOTICE,
-			     "A-interface: using automatically generated local (BSC) SCCP address (a.bsc_addr=%s)\n",
+
+		if (!osmo_sccp_check_addr(&msc->a.bsc_addr, OSMO_SCCP_ADDR_T_SSN | OSMO_SCCP_ADDR_T_PC)) {
+			LOGP(DMSC, LOGL_ERROR,
+			     "(%s) A-interface: invalid local (BSC) SCCP address: %s\n",
+			     msc_name,
 			     osmo_sccp_addr_name(osmo_ss7_instance_find(msc->a.cs7_instance), &msc->a.bsc_addr));
-		} else {
-			LOGP(DMSC, LOGL_NOTICE,
-			     "A-interface: using local (BSC) automatically SCCP address (a.msc_addr=%s)\n",
-			     osmo_sccp_addr_name(osmo_ss7_instance_find(msc->a.cs7_instance), &msc->a.bsc_addr));
+			return -EINVAL;
 		}
+
+		/* If unset, use default SCCP address for the MSC */
+		if (!msc->a.msc_addr.presence)
+			osmo_sccp_make_addr_pc_ssn(&msc->a.msc_addr,
+						   osmo_ss7_pointcode_parse(NULL, MSC_DEFAULT_PC),
+						   SCCP_SSN_BSSAP);
 
 		if (!osmo_sccp_check_addr(&msc->a.msc_addr, OSMO_SCCP_ADDR_T_SSN | OSMO_SCCP_ADDR_T_PC)) {
-			if (fail_on_next_invalid_cfg)
-				goto fail_auto_cofiguration;
-			free_attempt_used = true;
-
-			LOGP(DMSC, LOGL_NOTICE,
-			     "A-interface: invalid or missing remote (MSC) SCCP address for the MSC (a.msc_addr=%s)\n",
+			LOGP(DMSC, LOGL_ERROR,
+			     "(%s) A-interface: invalid remote (MSC) SCCP address: %s\n",
+			     msc_name,
 			     osmo_sccp_addr_name(osmo_ss7_instance_find(msc->a.cs7_instance), &msc->a.msc_addr));
-			osmo_sccp_local_addr_by_instance(&msc->a.msc_addr, msc->a.sccp, SCCP_SSN_BSSAP);
-			msc->a.msc_addr.pc = osmo_ss7_pointcode_parse(NULL, MSC_DEFAULT_PC);
-			LOGP(DMSC, LOGL_NOTICE,
-			     "A-interface: using automatically generated remote (MSC) SCCP address (a.msc_addr=%s)\n",
-			     osmo_sccp_addr_name(osmo_ss7_instance_find(msc->a.cs7_instance), &msc->a.msc_addr));
-			free_attempt_used = true;
-		} else {
-			LOGP(DMSC, LOGL_NOTICE,
-			     "A-interface: using remote (MSC) automatically SCCP address (a.msc_addr=%s)\n",
-			     osmo_sccp_addr_name(osmo_ss7_instance_find(msc->a.cs7_instance), &msc->a.msc_addr));
+			return -EINVAL;
 		}
 
+		LOGP(DMSC, LOGL_NOTICE, "(%s) A-interface: local (BSC) SCCP address: %s\n",
+		     msc_name,
+		     osmo_sccp_addr_name(osmo_ss7_instance_find(msc->a.cs7_instance), &msc->a.bsc_addr));
+		LOGP(DMSC, LOGL_NOTICE, "(%s) A-interface: remote (MSC) SCCP address: %s\n",
+		     msc_name,
+		     osmo_sccp_addr_name(osmo_ss7_instance_find(msc->a.cs7_instance), &msc->a.msc_addr));
+
 		/* Bind SCCP user */
 		msc->a.sccp_user = osmo_sccp_user_bind(msc->a.sccp, msc_name, sccp_sap_up, msc->a.bsc_addr.ssn);
 		if (!msc->a.sccp_user)

-- 
To view, visit https://gerrit.osmocom.org/4726
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Iadbc2e9740457e1b389b7e7ad9c94274e7d8cb11
Gerrit-PatchSet: 1
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de>



More information about the gerrit-log mailing list