[M] Change in osmo-hnbgw[master]: Fix lifecycle of struct hnbgw_sccp_user

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/40246?usp=email ) Change subject: Fix lifecycle of struct hnbgw_sccp_user ...................................................................... Fix lifecycle of struct hnbgw_sccp_user There's one struct hnbgw_sccp_user per cs7 instance configured in the hnbgw process. Multiple cnlinks configured on the same cs7 instance will be referencing/using the same struct hnbgw_sccp_user pointer. Hence, it makes no sense to allocate the struct using a talloc context of a cnlink as a parent, nor to keep any kind of specific 1-1 reference to it. Instead, we need to keep it alive in a ref-count basis, and unbind it from libosmo-sigtran sccp SAP and destroy it once no more cnlinks use it. Change-Id: I524f6da8e5936135b2153def103d83cec6549f94 --- M include/osmocom/hnbgw/hnbgw_cn.h M include/osmocom/hnbgw/hnbgw_sccp.h M src/osmo-hnbgw/cnlink.c M src/osmo-hnbgw/hnbgw_cn.c M src/osmo-hnbgw/hnbgw_sccp.c 5 files changed, 110 insertions(+), 42 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hnbgw refs/changes/46/40246/1 diff --git a/include/osmocom/hnbgw/hnbgw_cn.h b/include/osmocom/hnbgw/hnbgw_cn.h index 78b82b7..8f7919f 100644 --- a/include/osmocom/hnbgw/hnbgw_cn.h +++ b/include/osmocom/hnbgw/hnbgw_cn.h @@ -14,6 +14,7 @@ void hnbgw_cnpool_apply_cfg(struct hnbgw_cnpool *cnpool); void hnbgw_cnpool_cnlinks_start_or_restart(struct hnbgw_cnpool *cnpool); int hnbgw_cnlink_start_or_restart(struct hnbgw_cnlink *cnlink); +void hnbgw_cnlink_drop_sccp(struct hnbgw_cnlink *cnlink); char *cnlink_sccp_addr_to_str(struct hnbgw_cnlink *cnlink, const struct osmo_sccp_addr *addr); diff --git a/include/osmocom/hnbgw/hnbgw_sccp.h b/include/osmocom/hnbgw/hnbgw_sccp.h index 7cf67a5..fbb84f4 100644 --- a/include/osmocom/hnbgw/hnbgw_sccp.h +++ b/include/osmocom/hnbgw/hnbgw_sccp.h @@ -4,6 +4,7 @@ #include <osmocom/core/hashtable.h> #include <osmocom/core/linuxlist.h> #include <osmocom/core/prim.h> +#include <osmocom/core/use_count.h> #include <osmocom/sigtran/sccp_sap.h> @@ -27,6 +28,9 @@ /* osmo_sccp API state for above local address on above ss7 instance. */ struct osmo_sccp_user *sccp_user; + /* Ref count of users of this struct, ie.referencing it in cnlink->hnbgw_sccp_user */ + struct osmo_use_count use_count; + /* Fast access to the hnbgw_context_map responsible for a given SCCP conn_id of the ss7->sccp instance. * hlist_node: hnbgw_context_map->hnbgw_sccp_user_entry. */ DECLARE_HASHTABLE(hnbgw_context_map_by_conn_id, 6); @@ -35,4 +39,10 @@ #define LOG_HSU(HSU, SUBSYS, LEVEL, FMT, ARGS...) \ LOGP(SUBSYS, LEVEL, "(%s) " FMT, (HSU) ? (HSU)->name : "null", ##ARGS) -struct hnbgw_sccp_user *hnbgw_sccp_user_alloc(const struct hnbgw_cnlink *cnlink, int ss7_inst_id); +#define HSU_USE_CNLINK "cnlink" +#define hnbgw_sccp_user_get(hsu, use) \ + OSMO_ASSERT(osmo_use_count_get_put(&(hsu)->use_count, use, 1) == 0) +#define hnbgw_sccp_user_put(hsu, use) \ + OSMO_ASSERT(osmo_use_count_get_put(&(hsu)->use_count, use, -1) == 0) + +struct hnbgw_sccp_user *hnbgw_sccp_user_alloc(int ss7_inst_id); diff --git a/src/osmo-hnbgw/cnlink.c b/src/osmo-hnbgw/cnlink.c index fc67fd3..137012a 100644 --- a/src/osmo-hnbgw/cnlink.c +++ b/src/osmo-hnbgw/cnlink.c @@ -98,11 +98,28 @@ return cnlink; } +void hnbgw_cnlink_drop_sccp(struct hnbgw_cnlink *cnlink) +{ + struct hnbgw_context_map *map, *map2; + struct hnbgw_sccp_user *hsu; + + llist_for_each_entry_safe(map, map2, &cnlink->map_list, hnbgw_cnlink_entry) { + map_sccp_dispatch(map, MAP_SCCP_EV_USER_ABORT, NULL); + } + + OSMO_ASSERT(cnlink->hnbgw_sccp_user); + hsu = cnlink->hnbgw_sccp_user; + cnlink->hnbgw_sccp_user = NULL; + hnbgw_sccp_user_put(hsu, HSU_USE_CNLINK); +} + void cnlink_term_and_free(struct hnbgw_cnlink *cnlink) { if (!cnlink) return; osmo_fsm_inst_term(cnlink->fi, OSMO_FSM_TERM_REQUEST, NULL); + if (cnlink->hnbgw_sccp_user) + hnbgw_cnlink_drop_sccp(cnlink); talloc_free(cnlink); } diff --git a/src/osmo-hnbgw/hnbgw_cn.c b/src/osmo-hnbgw/hnbgw_cn.c index 7b5c575..d0a11fd 100644 --- a/src/osmo-hnbgw/hnbgw_cn.c +++ b/src/osmo-hnbgw/hnbgw_cn.c @@ -134,17 +134,6 @@ return changed; } -static void hnbgw_cnlink_drop_sccp(struct hnbgw_cnlink *cnlink) -{ - struct hnbgw_context_map *map, *map2; - - llist_for_each_entry_safe(map, map2, &cnlink->map_list, hnbgw_cnlink_entry) { - map_sccp_dispatch(map, MAP_SCCP_EV_USER_ABORT, NULL); - } - - cnlink->hnbgw_sccp_user = NULL; -} - static void hnbgw_cnlink_log_self(struct hnbgw_cnlink *cnlink) { struct osmo_ss7_instance *ss7 = cnlink->hnbgw_sccp_user->ss7; @@ -210,9 +199,10 @@ llist_for_each_entry(hsu, &g_hnbgw->sccp.users, entry) { if (hsu->ss7 != ss7) continue; - cnlink->hnbgw_sccp_user = hsu; LOG_CNLINK(cnlink, DCN, LOGL_DEBUG, "using existing SCCP instance %s on cs7 instance %u\n", - hsu->name, osmo_ss7_instance_get_id(ss7)); + hsu->name, osmo_ss7_instance_get_id(ss7)); + cnlink->hnbgw_sccp_user = hsu; + hnbgw_sccp_user_get(cnlink->hnbgw_sccp_user, HSU_USE_CNLINK); hnbgw_cnlink_log_self(cnlink); return 0; } @@ -222,7 +212,8 @@ /* No SCCP instance yet for this ss7. Create it. If no address name is given that resolves to a * particular cs7 instance above, use 'cs7 instance 0'. */ - cnlink->hnbgw_sccp_user = hnbgw_sccp_user_alloc(cnlink, ss7 ? osmo_ss7_instance_get_id(ss7) : 0); + cnlink->hnbgw_sccp_user = hnbgw_sccp_user_alloc(ss7 ? osmo_ss7_instance_get_id(ss7) : 0); + hnbgw_sccp_user_get(cnlink->hnbgw_sccp_user, HSU_USE_CNLINK); hnbgw_cnlink_log_self(cnlink); return 0; } diff --git a/src/osmo-hnbgw/hnbgw_sccp.c b/src/osmo-hnbgw/hnbgw_sccp.c index 67e0a53..ba02a5e 100644 --- a/src/osmo-hnbgw/hnbgw_sccp.c +++ b/src/osmo-hnbgw/hnbgw_sccp.c @@ -332,18 +332,70 @@ return rc; } - -struct hnbgw_sccp_user *hnbgw_sccp_user_alloc(const struct hnbgw_cnlink *cnlink, int ss7_id) +static int hnbgw_sccp_user_use_cb(struct osmo_use_count_entry *e, int32_t old_use_count, const char *file, int line) { - struct osmo_ss7_instance *ss7 = NULL; + struct hnbgw_sccp_user *hsu = e->use_count->talloc_object; + int32_t total; + int level; + + if (!e->use) + return -EINVAL; + + total = osmo_use_count_total(&hsu->use_count); + + if (total == 0 + || (total == 1 && old_use_count == 0 && e->count == 1)) + level = LOGL_INFO; + else + level = LOGL_DEBUG; + + LOGPSRC(DCN, level, file, line, + "%s: %s %s: now used by %s\n", + hsu->name, + (e->count - old_use_count) > 0 ? "+" : "-", + e->use, + osmo_use_count_to_str_c(OTC_SELECT, &hsu->use_count)); + + if (e->count < 0) + return -ERANGE; + + if (total == 0) + talloc_free(hsu); + return 0; +} + +static int hnbgw_sccp_user_talloc_destructor(struct hnbgw_sccp_user *hsu) +{ + if (hsu->sccp_user) { + osmo_sccp_user_unbind(hsu->sccp_user); + hsu->sccp_user = NULL; + } + llist_del(&hsu->entry); + return 0; +} + +struct hnbgw_sccp_user *hnbgw_sccp_user_alloc(int ss7_id) +{ struct osmo_sccp_instance *sccp; - struct osmo_sccp_user *sccp_user; uint32_t local_pc; struct hnbgw_sccp_user *hsu; + hsu = talloc_zero(g_hnbgw, struct hnbgw_sccp_user); + OSMO_ASSERT(hsu); + *hsu = (struct hnbgw_sccp_user){ + .name = talloc_asprintf(hsu, "cs7-%u-sccp-OsmoHNBGW", ss7_id), + .use_count = { + .talloc_object = hsu, + .use_cb = hnbgw_sccp_user_use_cb, + }, + }; + hash_init(hsu->hnbgw_context_map_by_conn_id); + llist_add_tail(&hsu->entry, &g_hnbgw->sccp.users); + talloc_set_destructor(hsu, hnbgw_sccp_user_talloc_destructor); + sccp = osmo_sccp_simple_client_on_ss7_id(g_hnbgw, ss7_id, - cnlink->name, + hsu->name, DEFAULT_PC_HNBGW, OSMO_SS7_ASP_PROT_M3UA, 0, @@ -351,40 +403,37 @@ -1, "localhost"); if (!sccp) { - LOG_CNLINK(cnlink, DCN, LOGL_ERROR, "Failed to configure SCCP on 'cs7 instance %u'\n", - ss7_id); - return NULL; + LOG_HSU(hsu, DCN, LOGL_ERROR, "Failed to configure SCCP on 'cs7 instance %u'\n", + ss7_id); + goto free_hsu_ret; } - ss7 = osmo_sccp_get_ss7(sccp); - LOG_CNLINK(cnlink, DCN, LOGL_NOTICE, "created SCCP instance on cs7 instance %u\n", osmo_ss7_instance_get_id(ss7)); + hsu->ss7 = osmo_sccp_get_ss7(sccp); + LOG_HSU(hsu, DCN, LOGL_NOTICE, "created SCCP instance on cs7 instance %u\n", osmo_ss7_instance_get_id(hsu->ss7)); /* Bind the SCCP user, using the cs7 instance's default point-code if one is configured, or osmo-hnbgw's default * local PC. */ - local_pc = osmo_ss7_instance_get_primary_pc(ss7); + local_pc = osmo_ss7_instance_get_primary_pc(hsu->ss7); if (!osmo_ss7_pc_is_valid(local_pc)) local_pc = DEFAULT_PC_HNBGW; - LOG_CNLINK(cnlink, DCN, LOGL_DEBUG, "binding OsmoHNBGW user to cs7 instance %u, local PC %u = %s\n", - osmo_ss7_instance_get_id(ss7), local_pc, osmo_ss7_pointcode_print(ss7, local_pc)); + LOG_HSU(hsu, DCN, LOGL_DEBUG, "binding OsmoHNBGW user to cs7 instance %u, local PC %u = %s\n", + osmo_ss7_instance_get_id(hsu->ss7), local_pc, osmo_ss7_pointcode_print(hsu->ss7, local_pc)); - sccp_user = osmo_sccp_user_bind_pc(sccp, "OsmoHNBGW", sccp_sap_up, OSMO_SCCP_SSN_RANAP, local_pc); - if (!sccp_user) { - LOGP(DCN, LOGL_ERROR, "Failed to init SCCP User\n"); - return NULL; + char *sccp_user_name = talloc_asprintf(hsu, "%s-RANAP", hsu->name); + hsu->sccp_user = osmo_sccp_user_bind_pc(sccp, sccp_user_name, sccp_sap_up, OSMO_SCCP_SSN_RANAP, local_pc); + talloc_free(sccp_user_name); + if (!hsu->sccp_user) { + LOG_HSU(hsu, DCN, LOGL_ERROR, "Failed to init SCCP User\n"); + goto free_hsu_ret; } - hsu = talloc_zero(cnlink, struct hnbgw_sccp_user); - *hsu = (struct hnbgw_sccp_user){ - .name = talloc_asprintf(hsu, "cs7-%u.sccp", osmo_ss7_instance_get_id(ss7)), - .ss7 = ss7, - .sccp_user = sccp_user, - }; osmo_sccp_make_addr_pc_ssn(&hsu->local_addr, local_pc, OSMO_SCCP_SSN_RANAP); - hash_init(hsu->hnbgw_context_map_by_conn_id); - osmo_sccp_user_set_priv(sccp_user, hsu); - - llist_add_tail(&hsu->entry, &g_hnbgw->sccp.users); + osmo_sccp_user_set_priv(hsu->sccp_user, hsu); return hsu; + +free_hsu_ret: + talloc_free(hsu); + return NULL; } -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/40246?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: I524f6da8e5936135b2153def103d83cec6549f94 Gerrit-Change-Number: 40246 Gerrit-PatchSet: 1 Gerrit-Owner: pespin &lt;pespin(a)sysmocom.de&gt;