pespin has submitted this change. (
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(-)
Approvals:
Jenkins Builder: Verified
pespin: Looks good to me, approved
laforge: Looks good to me, but someone else must approve
osmith: Looks good to me, but someone else must approve
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: merged
Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: I524f6da8e5936135b2153def103d83cec6549f94
Gerrit-Change-Number: 40246
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pes...@sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <lafo...@osmocom.org>
Gerrit-Reviewer: osmith <osm...@sysmocom.de>
Gerrit-Reviewer: pespin <pes...@sysmocom.de>
