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 <nhofm...@sysmocom.de>