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/.
lynxis lazus gerrit-no-reply at lists.osmocom.orglynxis lazus has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/24923 )
Change subject: WIP: gprs_ns2: start use (ref) counting
......................................................................
WIP: gprs_ns2: start use (ref) counting
Also avoid recursive free() rounds within the ns2 code
Change-Id: I03cb2419926c639d4bd357a33ce8008c50cd3bee
---
M src/gb/gprs_ns2.c
M src/gb/gprs_ns2_internal.h
M src/gb/gprs_ns2_sns.c
M tests/gb/gprs_ns2_test.c
M tests/gb/gprs_ns2_test.ok
5 files changed, 137 insertions(+), 17 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/23/24923/1
diff --git a/src/gb/gprs_ns2.c b/src/gb/gprs_ns2.c
index 3eb59e5..09286ec 100644
--- a/src/gb/gprs_ns2.c
+++ b/src/gb/gprs_ns2.c
@@ -236,6 +236,8 @@
{ 0, NULL }
};
+static void ns2_free_nse(struct gprs_ns2_nse *nse);
+
/*! string-format a given NS-VC into a user-supplied buffer.
* \param[in] buf user-allocated output buffer
* \param[in] buf_len size of user-allocated output buffer in bytes
@@ -631,9 +633,9 @@
* \param[in] nsvc NS-VC to destroy */
void gprs_ns2_free_nsvc(struct gprs_ns2_vc *nsvc)
{
- if (!nsvc)
+ if (!nsvc || nsvc->freed)
return;
-
+ nsvc->freed = true;
ns2_prim_status_ind(nsvc->nse, nsvc, 0, GPRS_NS2_AFF_CAUSE_VC_FAILURE);
llist_del(&nsvc->list);
@@ -661,14 +663,18 @@
*/
void gprs_ns2_free_nsvcs(struct gprs_ns2_nse *nse)
{
- struct gprs_ns2_vc *nsvc, *tmp;
+ struct gprs_ns2_vc *nsvc;
- if (!nse)
+ if (!nse || nse->freed)
return;
- llist_for_each_entry_safe(nsvc, tmp, &nse->nsvc, list) {
+ ns2_get(nse, NSE_USE_LIST);
+loop:
+ llist_for_each_entry(nsvc, &nse->nsvc, list) {
gprs_ns2_free_nsvc(nsvc);
+ goto loop;
}
+ ns2_put(nse, NSE_USE_LIST);
}
/*! Allocate a message buffer for use with the NS2 stack. */
@@ -778,6 +784,30 @@
return NULL;
}
+static int nse_use_cb(struct osmo_use_count_entry *e, int32_t old_use_count, const char *file, int line)
+{
+ struct gprs_ns2_nse *nse = e->use_count->talloc_object;
+ struct osmo_use_count_entry *e2;
+
+ int32_t other_count = 0;
+ int32_t core_count = 0;
+ if (!e->use)
+ return -EINVAL;
+ if (e->count < 0)
+ return -ERANGE;
+
+ llist_for_each_entry(e2, &nse->use_count.use_counts, entry) {
+ if (strcmp(e2->use, NSE_USE_CORE) == 0)
+ core_count += e2->count;
+ else
+ other_count += e2->count;
+ }
+
+ if (other_count <= 0 && core_count <= 0)
+ ns2_free_nse(nse);
+ return 0;
+}
+
/*! Create a NS Entity within given NS instance.
* \param[in] nsi NS instance in which to create NS Entity
* \param[in] nsei NS Entity Identifier of to-be-created NSE
@@ -813,6 +843,11 @@
nse->mtu = 0;
llist_add_tail(&nse->list, &nsi->nse);
INIT_LLIST_HEAD(&nse->nsvc);
+ nse->use_count = (struct osmo_use_count){
+ .talloc_object = nse,
+ .use_cb = nse_use_cb,
+ };
+ ns2_get(nse, NSE_USE_CORE);
return nse;
}
@@ -871,26 +906,35 @@
return nse->nsei;
}
-/*! Destroy given NS Entity.
- * \param[in] nse NS Entity to destroy */
-void gprs_ns2_free_nse(struct gprs_ns2_nse *nse)
+static void ns2_free_nse(struct gprs_ns2_nse *nse)
{
- if (!nse)
+ struct gprs_ns2_vc *nsvc, *nsvc2;
+ if (!nse || nse->freed)
return;
+ nse->freed = true;
nse->alive = false;
if (nse->bss_sns_fi) {
osmo_fsm_inst_term(nse->bss_sns_fi, OSMO_FSM_TERM_REQUEST, NULL);
nse->bss_sns_fi = NULL;
}
- gprs_ns2_free_nsvcs(nse);
- ns2_prim_status_ind(nse, NULL, 0, GPRS_NS2_AFF_CAUSE_FAILURE);
+ llist_for_each_entry_safe(nsvc, nsvc2, &nse->nsvc, list) {
+ gprs_ns2_free_nsvc(nsvc);
+ }
+ ns2_prim_status_ind(nse, NULL, 0, GPRS_NS2_AFF_CAUSE_FAILURE);
llist_del(&nse->list);
talloc_free(nse);
}
+/*! Destroy given NS Entity.
+ * \param[in] nse NS Entity to destroy */
+void gprs_ns2_free_nse(struct gprs_ns2_nse *nse)
+{
+ ns2_put(nse, NSE_USE_CORE);
+}
+
void gprs_ns2_free_nses(struct gprs_ns2_inst *nsi)
{
struct gprs_ns2_nse *nse, *ntmp;
@@ -1240,11 +1284,14 @@
{
struct gprs_ns2_vc *nsvc, *tmp;
int rc = 0;
+
+ ns2_get(nse, NSE_USE_LIST);
llist_for_each_entry_safe(nsvc, tmp, &nse->nsvc, list) {
rc = cb(nsvc, cb_data);
if (rc < 0)
return rc;
}
+ ns2_put(nse, NSE_USE_LIST);
return 0;
}
@@ -1450,20 +1497,26 @@
void gprs_ns2_free_bind(struct gprs_ns2_vc_bind *bind)
{
struct gprs_ns2_vc *nsvc, *tmp;
- struct gprs_ns2_nse *nse;
- if (!bind)
+ struct gprs_ns2_nse *nse, *nse2;
+ if (!bind || bind->freed)
return;
- llist_for_each_entry_safe(nsvc, tmp, &bind->nsvc, blist) {
- gprs_ns2_free_nsvc(nsvc);
- }
+ bind->freed = true;
if (gprs_ns2_is_ip_bind(bind)) {
- llist_for_each_entry(nse, &bind->nsi->nse, list) {
+ llist_for_each_entry_safe(nse, nse2, &bind->nsi->nse, list) {
gprs_ns2_sns_del_bind(nse, bind);
}
}
+
+ /* because freeing a single nsvc might free all other nsvcs */
+loop:
+ llist_for_each_entry(nsvc, &bind->nsvc, blist) {
+ gprs_ns2_free_nsvc(nsvc);
+ goto loop;
+ }
+
if (bind->driver->free_bind)
bind->driver->free_bind(bind);
diff --git a/src/gb/gprs_ns2_internal.h b/src/gb/gprs_ns2_internal.h
index 70e212a..65cfda6 100644
--- a/src/gb/gprs_ns2_internal.h
+++ b/src/gb/gprs_ns2_internal.h
@@ -6,6 +6,7 @@
#include <stdint.h>
#include <osmocom/core/logging.h>
+#include <osmocom/core/use_count.h>
#include <osmocom/gprs/protocol/gsm_08_16.h>
#include <osmocom/gprs/gprs_ns2.h>
@@ -198,6 +199,11 @@
/*! are we implementing the SGSN role? */
bool ip_sns_role_sgsn;
+
+ /*! recursive anchor */
+ bool freed;
+
+ struct osmo_use_count use_count;
};
/*! Structure representing a single NS-VC */
@@ -242,6 +248,9 @@
enum gprs_ns2_vc_mode mode;
struct osmo_fsm_inst *fi;
+
+ /*! recursive anchor */
+ bool freed;
};
/*! Structure repesenting a bind instance. E.g. IPv4 listen port. */
@@ -286,6 +295,9 @@
uint8_t sns_data_weight;
struct osmo_stat_item_group *statg;
+
+ /*! recursive anchor */
+ bool freed;
};
struct gprs_ns2_vc_driver {
@@ -294,6 +306,15 @@
void (*free_bind)(struct gprs_ns2_vc_bind *driver);
};
+#define ns2_get(ns2_obj, use) \
+ OSMO_ASSERT(osmo_use_count_get_put(&(ns2_obj)->use_count, use, 1) == 0)
+#define ns2_put(ns2_obj, use) \
+ OSMO_ASSERT(osmo_use_count_get_put(&(ns2_obj)->use_count, use, -1) == 0)
+#define NSE_USE_CORE "ns2core"
+#define NSE_USE_LIST "ns2list"
+#define NSE_USE_SNS "ns2sns"
+
+
enum ns2_cs ns2_create_vc(struct gprs_ns2_vc_bind *bind,
struct msgb *msg,
const struct osmo_sockaddr *remote,
diff --git a/src/gb/gprs_ns2_sns.c b/src/gb/gprs_ns2_sns.c
index 642b47c..b4a8d7f 100644
--- a/src/gb/gprs_ns2_sns.c
+++ b/src/gb/gprs_ns2_sns.c
@@ -658,6 +658,7 @@
OSMO_ASSERT(false);
}
+ ns2_get(nse, NSE_USE_LIST);
llist_for_each_entry_safe(nsvc, tmp, &nse->nsvc, list) {
remote = gprs_ns2_ip_vc_remote(nsvc);
/* all nsvc in NSE should be IP/UDP nsvc */
@@ -668,6 +669,7 @@
LOGPFSML(fi, LOGL_INFO, "DELETE NS-VC %s\n", gprs_ns2_ll_str(nsvc));
gprs_ns2_free_nsvc(nsvc);
}
+ ns2_put(nse, NSE_USE_LIST);
return 0;
}
@@ -1482,6 +1484,7 @@
struct ns2_sns_bind *sbind;
struct gprs_ns2_vc *nsvc, *nsvc2;
+ ns2_get(nse, NSE_USE_SNS);
switch (event) {
case GPRS_SNS_EV_REQ_ADD_BIND:
sbind = data;
@@ -1528,6 +1531,7 @@
talloc_free(sbind);
break;
}
+ ns2_put(nse, NSE_USE_SNS);
}
/* validate the bss configuration (sns endpoint and binds)
diff --git a/tests/gb/gprs_ns2_test.c b/tests/gb/gprs_ns2_test.c
index 515d908..2322735 100644
--- a/tests/gb/gprs_ns2_test.c
+++ b/tests/gb/gprs_ns2_test.c
@@ -642,6 +642,43 @@
printf("--- Finish force unconfigured test\n");
}
+void test_use_count(void *ctx)
+{
+ struct gprs_ns2_inst *nsi;
+ struct gprs_ns2_vc_bind *bind[2];
+ struct gprs_ns2_vc_bind *loopbind;
+ struct gprs_ns2_nse *nse;
+ struct gprs_ns2_vc *nsvc[2];
+ struct gprs_ns2_vc *loop[2];
+
+ struct msgb *msg, *other;
+ char idbuf[32];
+ int i;
+
+ printf("--- Testing nse use count\n");
+ osmo_wqueue_clear(unitdata);
+ printf("---- Create Binds\n");
+ nsi = gprs_ns2_instantiate(ctx, ns_prim_cb, NULL);
+ bind[0] = dummy_bind(nsi, "bblock1");
+ bind[1] = dummy_bind(nsi, "bblock2");
+ loopbind = loopback_bind(nsi, "loopback");
+
+ // free SNS NSE with active NSVCs
+ // free SNS NSE with active NSVCs on a SNS/NSE event
+ // free a dynamic SNS NSE
+
+ nse = gprs_ns2_create_nse(nsi, 1004, GPRS_NS2_LL_UDP, GPRS_NS2_DIALECT_STATIC_RESETBLOCK);
+ OSMO_ASSERT(nse);
+ for (i=0; i<2; i++) {
+ printf("---- Create NSVC[%d]\n", i);
+ snprintf(idbuf, sizeof(idbuf), "NSE%05u-dummy-%i", nse->nsei, i);
+ nsvc[i] = ns2_vc_alloc(bind[i], nse, false, GPRS_NS2_VC_MODE_BLOCKRESET, idbuf);
+ OSMO_ASSERT(nsvc[i]);
+ ns2_vc_fsm_start(nsvc[i]);
+ }
+ gprs_ns2_free_nse(nse);
+}
+
int main(int argc, char **argv)
{
void *ctx = talloc_named_const(NULL, 0, "gprs_ns2_test");
@@ -663,6 +700,7 @@
test_unitdata_weights(ctx);
test_unconfigured(ctx);
test_mtu(ctx);
+ test_use_count(ctx);
printf("===== NS2 protocol test END\n\n");
talloc_free(ctx);
diff --git a/tests/gb/gprs_ns2_test.ok b/tests/gb/gprs_ns2_test.ok
index 8bae5b9..f4af60c 100644
--- a/tests/gb/gprs_ns2_test.ok
+++ b/tests/gb/gprs_ns2_test.ok
@@ -47,5 +47,9 @@
---- Send a small UNITDATA to NSVC[0]
---- Check if got mtu reported
--- Finish unitdata test
+--- Testing nse use count
+---- Create Binds
+---- Create NSVC[0]
+---- Create NSVC[1]
===== NS2 protocol test END
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/24923
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I03cb2419926c639d4bd357a33ce8008c50cd3bee
Gerrit-Change-Number: 24923
Gerrit-PatchSet: 1
Gerrit-Owner: lynxis lazus <lynxis at fe80.eu>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210712/9248253b/attachment.htm>