From 3f4d6458904e8db42fcebff8b811f5bf90514a1b Mon Sep 17 00:00:00 2001 From: Neels Janosch Hofmeyr Date: Wed, 15 Feb 2023 02:49:59 +0100 Subject: [PATCH] Deprecate 'sccp cr max-payload-len', remove SCCP CR limit code The SCCP CR payload length limit is now implemented in libosmo-sigtran v1.7.0. The limit no longer needs to be enforced in osmo-hnbgw. This reverts commit 2c91bd66a1dc4740282c9f8221f80da0d688a293, except for keeping the cfg option, marked deprecated, and not doing anything. Fixes: OS#5906 Related: SYS#5968 Related: OS#5579 Depends: I174b2ce06a31daa5a129c8a39099fe8962092df8 (osmo-ttcn3-hacks) Change-Id: I18dece84b33bbefce8617fbb0b2d79a7e5adb263 --- TODO-RELEASE | 2 + include/osmocom/hnbgw/context_map.h | 8 ---- include/osmocom/hnbgw/hnbgw.h | 3 -- include/osmocom/hnbgw/hnbgw_rua.h | 7 --- src/osmo-hnbgw/context_map.c | 15 ------ src/osmo-hnbgw/hnbgw.c | 9 ---- src/osmo-hnbgw/hnbgw_cn.c | 5 -- src/osmo-hnbgw/hnbgw_rua.c | 74 +++++------------------------ src/osmo-hnbgw/hnbgw_vty.c | 14 +++--- 9 files changed, 22 insertions(+), 115 deletions(-) diff --git a/TODO-RELEASE b/TODO-RELEASE index d0852fc..84cc2ea 100644 --- a/TODO-RELEASE +++ b/TODO-RELEASE @@ -7,3 +7,5 @@ # If any interfaces have been added since the last public release: c:r:a + 1. # If any interfaces have been removed or changed since the last public release: c:r:0. #library what description / commit summary line +osmo-hnbgw cfg config deprecated: 'sccp cr max-payload-len <0-999999>' +libosmo-sigtran >=1.7.0 Ensure SCCP CR max payload length of 130 bytes is enforced. diff --git a/include/osmocom/hnbgw/context_map.h b/include/osmocom/hnbgw/context_map.h index fe09da0..a8d1f97 100644 --- a/include/osmocom/hnbgw/context_map.h +++ b/include/osmocom/hnbgw/context_map.h @@ -2,9 +2,6 @@ #include #include -#include - -struct msgb; #define LOG_MAP(HNB_CTX_MAP, SUBSYS, LEVEL, FMT, ARGS...) \ LOGHNB((HNB_CTX_MAP) ? (HNB_CTX_MAP)->hnb_ctx : NULL, \ @@ -47,9 +44,6 @@ struct hnbgw_context_map { * User SAP conn. Useful to avoid leaking SCCP connections: guarantee that an OSMO_SCU_PRIM_N_DISCONNECT gets * sent, even when RUA fails to gracefully disconnect. */ bool scu_conn_active; - /* Pending data to be sent: when we send an "empty" SCCP CR first, the initial RANAP message will be sent in a - * separate DT once the CR is confirmed. This caches the initial RANAP message. */ - struct msgb *cached_msg; enum hnbgw_context_map_state state; @@ -80,8 +74,6 @@ context_map_alloc_by_hnb(struct hnb_context *hnb, uint32_t rua_ctx_id, struct hnbgw_context_map * context_map_by_cn(struct hnbgw_cnlink *cn, uint32_t scu_conn_id); -int context_map_send_cached_msg(struct hnbgw_context_map *map); - void context_map_deactivate(struct hnbgw_context_map *map); int context_map_init(struct hnb_gw *gw); diff --git a/include/osmocom/hnbgw/hnbgw.h b/include/osmocom/hnbgw/hnbgw.h index ed18ca4..f1b107e 100644 --- a/include/osmocom/hnbgw/hnbgw.h +++ b/include/osmocom/hnbgw/hnbgw.h @@ -136,7 +136,6 @@ struct hnb_gw { bool hnbap_allow_tmsi; /*! print hnb-id (true) or MCC-MNC-LAC-RAC-SAC (false) in logs */ bool log_prefix_hnb_id; - unsigned int max_sccp_cr_payload_len; struct mgcp_client_conf *mgcp_client; struct { char *local_addr; @@ -193,8 +192,6 @@ void hnb_context_release_ue_state(struct hnb_context *ctx); void hnbgw_vty_init(struct hnb_gw *gw, void *tall_ctx); int hnbgw_vty_go_parent(struct vty *vty); -bool hnbgw_requires_empty_sccp_cr(struct hnb_gw *gw, unsigned int ranap_msg_len); - /* Return true when the user configured GTP mapping to be enabled, by configuring a PFCP link to a UPF. * Return false when the user configured to skip GTP mapping and RANAP PS RAB Requests/Responses should be passed thru * 1:1. diff --git a/include/osmocom/hnbgw/hnbgw_rua.h b/include/osmocom/hnbgw/hnbgw_rua.h index fa33d6b..4b65115 100644 --- a/include/osmocom/hnbgw/hnbgw_rua.h +++ b/include/osmocom/hnbgw/hnbgw_rua.h @@ -2,7 +2,6 @@ #include #include -#include int hnbgw_rua_rx(struct hnb_context *hnb, struct msgb *msg); int hnbgw_rua_init(void); @@ -12,9 +11,3 @@ int rua_tx_dt(struct hnb_context *hnb, int is_ps, uint32_t context_id, const uint8_t *data, unsigned int len); int rua_tx_disc(struct hnb_context *hnb, int is_ps, uint32_t context_id, const RUA_Cause_t *cause, const uint8_t *data, unsigned int len); - -int rua_to_scu(struct hnb_context *hnb, - RUA_CN_DomainIndicator_t cN_DomainIndicator, - enum osmo_scu_prim_type type, - uint32_t context_id, uint32_t cause, - const uint8_t *data, unsigned int len); diff --git a/src/osmo-hnbgw/context_map.c b/src/osmo-hnbgw/context_map.c index 31ba032..39b9b50 100644 --- a/src/osmo-hnbgw/context_map.c +++ b/src/osmo-hnbgw/context_map.c @@ -29,7 +29,6 @@ #include #include -#include #include #include #include @@ -151,20 +150,6 @@ context_map_by_cn(struct hnbgw_cnlink *cn, uint32_t scu_conn_id) return NULL; } -int context_map_send_cached_msg(struct hnbgw_context_map *map) -{ - int rc; - if (!map || !map->cached_msg) - return 0; - rc = rua_to_scu(map->hnb_ctx, - map->is_ps ? RUA_CN_DomainIndicator_ps_domain : RUA_CN_DomainIndicator_cs_domain, - OSMO_SCU_PRIM_N_DATA, map->rua_ctx_id, 0, - msgb_data(map->cached_msg), msgb_length(map->cached_msg)); - msgb_free(map->cached_msg); - map->cached_msg = NULL; - return rc; -} - void context_map_deactivate(struct hnbgw_context_map *map) { LOG_MAP(map, DMAIN, LOGL_INFO, "Deactivating\n"); diff --git a/src/osmo-hnbgw/hnbgw.c b/src/osmo-hnbgw/hnbgw.c index 3a6879e..da6df43 100644 --- a/src/osmo-hnbgw/hnbgw.c +++ b/src/osmo-hnbgw/hnbgw.c @@ -94,10 +94,6 @@ static struct hnb_gw *hnb_gw_create(void *ctx) gw->config.iuh_local_port = IUH_DEFAULT_SCTP_PORT; gw->config.log_prefix_hnb_id = true; - /* No limit by default, always include the initial RANAP message in the SCCP CR towards the CN. - * 999999 is the maximum value in hnbgw_vty.c */ - gw->config.max_sccp_cr_payload_len = 999999; - gw->next_ue_ctx_id = 23; INIT_LLIST_HEAD(&gw->hnb_list); INIT_LLIST_HEAD(&gw->ue_list); @@ -431,11 +427,6 @@ void hnb_context_release(struct hnb_context *ctx) talloc_free(ctx); } -bool hnbgw_requires_empty_sccp_cr(struct hnb_gw *gw, unsigned int ranap_msg_len) -{ - return ranap_msg_len > gw->config.max_sccp_cr_payload_len; -} - /*! call-back when the listen FD has something to read */ static int accept_cb(struct osmo_stream_srv_link *srv, int fd) { diff --git a/src/osmo-hnbgw/hnbgw_cn.c b/src/osmo-hnbgw/hnbgw_cn.c index c4189e8..4c0e685 100644 --- a/src/osmo-hnbgw/hnbgw_cn.c +++ b/src/osmo-hnbgw/hnbgw_cn.c @@ -355,11 +355,6 @@ static int handle_cn_conn_conf(struct hnbgw_cnlink *cnlink, /* SCCP connection is confirmed. Mark conn as active, i.e. requires a DISCONNECT to clean up the SCCP * connection. */ map->scu_conn_active = true; - - /* If our initial SCCP CR was sent without data payload, then the initial RANAP message is cached and waiting to - * be sent as soon as the SCCP connection is confirmed. See if that is the case, send cached data. */ - context_map_send_cached_msg(map); - return 0; } diff --git a/src/osmo-hnbgw/hnbgw_rua.c b/src/osmo-hnbgw/hnbgw_rua.c index 2757073..07b78b3 100644 --- a/src/osmo-hnbgw/hnbgw_rua.c +++ b/src/osmo-hnbgw/hnbgw_rua.c @@ -179,11 +179,11 @@ int rua_tx_disc(struct hnb_context *hnb, int is_ps, uint32_t context_id, /* forward a RUA message to the SCCP User API to SCCP */ -int rua_to_scu(struct hnb_context *hnb, - RUA_CN_DomainIndicator_t cN_DomainIndicator, - enum osmo_scu_prim_type type, - uint32_t context_id, uint32_t cause, - const uint8_t *data, unsigned int len) +static int rua_to_scu(struct hnb_context *hnb, + RUA_CN_DomainIndicator_t cN_DomainIndicator, + enum osmo_scu_prim_type type, + uint32_t context_id, uint32_t cause, + const uint8_t *data, unsigned int len) { struct msgb *msg; struct osmo_scu_prim *prim; @@ -227,9 +227,9 @@ int rua_to_scu(struct hnb_context *hnb, default: map = context_map_alloc_by_hnb(hnb, context_id, is_ps, cn); OSMO_ASSERT(map); - LOGHNB(hnb, DRUA, LOGL_DEBUG, "rua_to_scu() %s to %s, rua_ctx_id %u scu_conn_id %u data-len %u\n", + LOGHNB(hnb, DRUA, LOGL_DEBUG, "rua_to_scu() %s to %s, rua_ctx_id %u scu_conn_id %u\n", cn_domain_indicator_to_str(cN_DomainIndicator), osmo_sccp_addr_dump(remote_addr), - map->rua_ctx_id, map->scu_conn_id, len); + map->rua_ctx_id, map->scu_conn_id); } /* add primitive header */ @@ -388,69 +388,21 @@ static int rua_rx_init_connect(struct msgb *msg, ANY_t *in) struct hnb_context *hnb = msg->dst; uint32_t context_id; int rc; - const uint8_t *data; - unsigned int data_len; rc = rua_decode_connecties(&ies, in); if (rc < 0) return rc; context_id = asn1bitstr_to_u24(&ies.context_ID); - data = ies.ranaP_Message.buf; - data_len = ies.ranaP_Message.size; - LOGHNB(hnb, DRUA, LOGL_DEBUG, "RUA %s Connect.req(ctx=0x%x, %s, RANAP.size=%u)\n", - cn_domain_indicator_to_str(ies.cN_DomainIndicator), context_id, - ies.establishment_Cause == RUA_Establishment_Cause_emergency_call ? "emergency" : "normal", - data_len); - - if (hnbgw_requires_empty_sccp_cr(hnb->gw, data_len)) { - /* Do not include data in the SCCP CR, to avoid hitting a message size limit at the remote end that may - * lead to rejection. */ - bool is_ps; - struct osmo_sccp_addr *remote_addr; - struct hnbgw_context_map *map; - - switch (ies.cN_DomainIndicator) { - case RUA_CN_DomainIndicator_cs_domain: - remote_addr = &hnb->gw->sccp.iucs_remote_addr; - is_ps = false; - break; - case RUA_CN_DomainIndicator_ps_domain: - remote_addr = &hnb->gw->sccp.iups_remote_addr; - is_ps = true; - break; - default: - LOGHNB(hnb, DRUA, LOGL_ERROR, "Unsupported Domain %ld\n", ies.cN_DomainIndicator); - rua_free_connecties(&ies); - return -1; - } - - if (!hnb->gw->sccp.cnlink) { - LOGHNB(hnb, DRUA, LOGL_NOTICE, "CN=NULL, discarding message\n"); - rua_free_connecties(&ies); - return 0; - } - - map = context_map_alloc_by_hnb(hnb, context_id, is_ps, hnb->gw->sccp.cnlink); - OSMO_ASSERT(map); - OSMO_ASSERT(map->is_ps == is_ps); - LOGHNB(hnb, DRUA, LOGL_DEBUG, "rua_rx_init_connect() %s to %s, rua_ctx_id %u scu_conn_id %u;" - " Sending SCCP CR without payload, caching %u octets\n", - cn_domain_indicator_to_str(ies.cN_DomainIndicator), osmo_sccp_addr_dump(remote_addr), - map->rua_ctx_id, map->scu_conn_id, data_len); - - map->cached_msg = msgb_alloc_c(map, data_len, "map.cached_msg"); - OSMO_ASSERT(map->cached_msg); - memcpy(msgb_put(map->cached_msg, data_len), data, data_len); - - /* Data is cached for after CR is confirmed, send SCCP CR but omit payload. */ - data = NULL; - data_len = 0; - } + LOGHNB(hnb, DRUA, LOGL_DEBUG, "RUA %s Connect.req(ctx=0x%x, %s)\n", + cn_domain_indicator_to_str(ies.cN_DomainIndicator), context_id, + ies.establishment_Cause == RUA_Establishment_Cause_emergency_call ? "emergency" : "normal"); rc = rua_to_scu(hnb, ies.cN_DomainIndicator, OSMO_SCU_PRIM_N_CONNECT, - context_id, 0, data, data_len); + context_id, 0, ies.ranaP_Message.buf, + ies.ranaP_Message.size); + rua_free_connecties(&ies); return rc; diff --git a/src/osmo-hnbgw/hnbgw_vty.c b/src/osmo-hnbgw/hnbgw_vty.c index 5c2c42d..abddc79 100644 --- a/src/osmo-hnbgw/hnbgw_vty.c +++ b/src/osmo-hnbgw/hnbgw_vty.c @@ -332,15 +332,17 @@ DEFUN(cfg_hnbgw_log_prefix, cfg_hnbgw_log_prefix_cmd, return CMD_SUCCESS; } -DEFUN(cfg_hnbgw_max_sccp_cr_payload_len, cfg_hnbgw_max_sccp_cr_payload_len_cmd, +DEFUN_DEPRECATED(cfg_hnbgw_max_sccp_cr_payload_len, cfg_hnbgw_max_sccp_cr_payload_len_cmd, "sccp cr max-payload-len <0-999999>", "Configure SCCP behavior\n" "Configure SCCP Connection Request\n" - "Set an upper bound for payload data length included directly in the CR. If an initial RUA message has a" - " RANAP payload larger than this value (octets), send an SCCP CR without data, followed by an SCCP DT." - " This may be necessary if the remote component has a size limit on valid SCCP CR messages.\n") + "DEPRECATED: The maximum SCCP CR PDU length of 130 is now enforced in libosmo-sccp v1.7.0. This config item no" + " longer has any effect.\n" + "ignored\n") { - g_hnb_gw->config.max_sccp_cr_payload_len = atoi(argv[0]); + vty_out(vty, "%% deprecated, ignored: remove this from your config file: 'sccp cr max-payload-len N'%s", + VTY_NEWLINE); + /* Still return success to not break osmo-hnbgw startup for users with old config files. */ return CMD_SUCCESS; } @@ -414,8 +416,6 @@ static int config_write_hnbgw(struct vty *vty) vty_out(vty, "hnbgw%s", VTY_NEWLINE); vty_out(vty, " log-prefix %s%s", g_hnb_gw->config.log_prefix_hnb_id ? "hnb-id" : "umts-cell-id", VTY_NEWLINE); - if (g_hnb_gw->config.max_sccp_cr_payload_len != 999999) - vty_out(vty, " sccp cr max-payload-len %u%s", g_hnb_gw->config.max_sccp_cr_payload_len, VTY_NEWLINE); osmo_tdef_vty_groups_write(vty, " "); return CMD_SUCCESS; }