Change in ...osmo-pcu[master]: gprs_bssgp_pcu: explicit allocate & initialize bssgp_nsi instance

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/.

laforge gerrit-no-reply at lists.osmocom.org
Thu Jun 13 15:35:53 UTC 2019


laforge has submitted this change and it was merged. ( https://gerrit.osmocom.org/c/osmo-pcu/+/14171 )

Change subject: gprs_bssgp_pcu: explicit allocate & initialize bssgp_nsi instance
......................................................................

gprs_bssgp_pcu: explicit allocate & initialize bssgp_nsi instance

The instance bssgp_nsi is a global instance to be used by all
NS related functions. Previous the PCU allocated and initialized
the bssgp_nsi instance when (re-)connecting and freeing on disconnect.
The problem of the implicit initialisation is gprs_ns_vty_init(bssgp_nsi).
All vty init functions must be called before the configuration is read,
otherwise a previous vty written configuration is invalid.
Furthermore the vty modifications to the `ns` object were lost when the PCU has to
reconnect to the SGSN.

Fixes: OS#4024
Change-Id: I2aa53ea54e9352577f6280ad7b9d1d9da9f57eaf
---
M src/gprs_bssgp_pcu.cpp
M src/pcu_main.cpp
M tests/emu/pcu_emu.cpp
M tests/tbf/TbfTest.cpp
4 files changed, 49 insertions(+), 17 deletions(-)

Approvals:
  Jenkins Builder: Verified
  fixeria: Looks good to me, but someone else must approve
  laforge: Looks good to me, approved



diff --git a/src/gprs_bssgp_pcu.cpp b/src/gprs_bssgp_pcu.cpp
index 50f10db..22abc6b 100644
--- a/src/gprs_bssgp_pcu.cpp
+++ b/src/gprs_bssgp_pcu.cpp
@@ -875,11 +875,6 @@
 {
 	struct gprs_nsvc *nsvc2;
 
-	if (!bssgp_nsi) {
-		LOGP(DBSSGP, LOGL_ERROR, "NS instance does not exist\n");
-		return -EINVAL;
-	}
-
 	if (nsvc != the_pcu.nsvc) {
 		LOGP(DBSSGP, LOGL_ERROR, "NSVC is invalid\n");
 		return -EBADF;
@@ -913,12 +908,6 @@
 
 	the_pcu.bts = bts;
 
-	bssgp_nsi = gprs_ns_instantiate(&gprs_bssgp_ns_cb, tall_pcu_ctx);
-	if (!bssgp_nsi) {
-		LOGP(DBSSGP, LOGL_ERROR, "Failed to create NS instance\n");
-		return NULL;
-	}
-	gprs_ns_vty_init(bssgp_nsi);
 	/* don't specify remote IP/port if SNS dialect is in use; Doing so would
 	 * issue a connect() on the socket, which prevents us to dynamically communicate
 	 * with any number of IP-SNS endpoints on the SGSN side */
@@ -930,8 +919,7 @@
 	rc = gprs_ns_nsip_listen(bssgp_nsi);
 	if (rc < 0) {
 		LOGP(DBSSGP, LOGL_ERROR, "Failed to create socket\n");
-		gprs_ns_destroy(bssgp_nsi);
-		bssgp_nsi = NULL;
+		gprs_ns_close(bssgp_nsi);
 		return NULL;
 	}
 
@@ -945,8 +933,7 @@
 		the_pcu.nsvc = gprs_ns_nsip_connect(bssgp_nsi, &dest, nsei, nsvci);
 	if (!the_pcu.nsvc) {
 		LOGP(DBSSGP, LOGL_ERROR, "Failed to create NSVCt\n");
-		gprs_ns_destroy(bssgp_nsi);
-		bssgp_nsi = NULL;
+		gprs_ns_close(bssgp_nsi);
 		return NULL;
 	}
 
@@ -954,8 +941,7 @@
 	if (!the_pcu.bctx) {
 		LOGP(DBSSGP, LOGL_ERROR, "Failed to create BSSGP context\n");
 		the_pcu.nsvc = NULL;
-		gprs_ns_destroy(bssgp_nsi);
-		bssgp_nsi = NULL;
+		gprs_ns_close(bssgp_nsi);
 		return NULL;
 	}
 	the_pcu.bctx->ra_id.mcc = spoof_mcc ? : mcc;
diff --git a/src/pcu_main.cpp b/src/pcu_main.cpp
index 1003e3c..fa075cd 100644
--- a/src/pcu_main.cpp
+++ b/src/pcu_main.cpp
@@ -33,6 +33,8 @@
 #include <bts.h>
 #include <gprs_coding_scheme.h>
 #include <osmocom/pcu/pcuif_proto.h>
+#include "gprs_bssgp_pcu.h"
+
 extern "C" {
 #include "pcu_vty.h"
 #include <osmocom/gprs/gprs_bssgp.h>
@@ -291,6 +293,13 @@
 	else
 		fprintf(stderr, "Failed to initialize GSMTAP for %s\n", gsmtap_addr);
 
+	bssgp_nsi = gprs_ns_instantiate(&gprs_bssgp_ns_cb, tall_pcu_ctx);
+	if (!bssgp_nsi) {
+		LOGP(DBSSGP, LOGL_ERROR, "Failed to create NS instance\n");
+		exit(1);
+	}
+	gprs_ns_vty_init(bssgp_nsi);
+
 	rc = vty_read_config_file(config_file, NULL);
 	if (rc < 0 && config_given) {
 		fprintf(stderr, "Failed to parse the config file: '%s'\n",
diff --git a/tests/emu/pcu_emu.cpp b/tests/emu/pcu_emu.cpp
index 354a328..e0190bf 100644
--- a/tests/emu/pcu_emu.cpp
+++ b/tests/emu/pcu_emu.cpp
@@ -113,6 +113,13 @@
 
 	msgb_talloc_ctx_init(tall_pcu_ctx, 0);
 	osmo_init_logging2(tall_pcu_ctx, &gprs_log_info);
+
+	bssgp_nsi = gprs_ns_instantiate(&gprs_bssgp_ns_cb, tall_pcu_ctx);
+	if (!bssgp_nsi) {
+		LOGP(DBSSGP, LOGL_ERROR, "Failed to create NS instance\n");
+		abort();
+	}
+
 	vty_init(&pcu_vty_info);
 	pcu_vty_init(&gprs_log_info);
 
diff --git a/tests/tbf/TbfTest.cpp b/tests/tbf/TbfTest.cpp
index 151e7fa..675cf89 100644
--- a/tests/tbf/TbfTest.cpp
+++ b/tests/tbf/TbfTest.cpp
@@ -447,6 +447,12 @@
 
 	printf("=== start %s ===\n", __func__);
 
+	bssgp_nsi = gprs_ns_instantiate(&gprs_bssgp_ns_cb, tall_pcu_ctx);
+	if (!bssgp_nsi) {
+		LOGP(DBSSGP, LOGL_ERROR, "Failed to create NS instance\n");
+		abort();
+	}
+
 	bts = the_bts.bts_data();
 	setup_bts(&the_bts, ts_no);
 	gprs_bssgp_create_and_connect(bts, 33001, 0, 33001, 1234, 1234, 1234, 1, 1, false, 0, 0, 0);
@@ -485,6 +491,12 @@
 
 	uint8_t buf[19];
 
+	bssgp_nsi = gprs_ns_instantiate(&gprs_bssgp_ns_cb, tall_pcu_ctx);
+	if (!bssgp_nsi) {
+		LOGP(DBSSGP, LOGL_ERROR, "Failed to create NS instance\n");
+		abort();
+	}
+
 	printf("=== start %s ===\n", __func__);
 
 	bts = the_bts.bts_data();
@@ -2171,6 +2183,12 @@
 
 	printf("=== start %s ===\n", __func__);
 
+	bssgp_nsi = gprs_ns_instantiate(&gprs_bssgp_ns_cb, tall_pcu_ctx);
+	if (!bssgp_nsi) {
+		LOGP(DBSSGP, LOGL_ERROR, "Failed to create NS instance\n");
+		abort();
+	}
+
 	bts = the_bts.bts_data();
 	setup_bts(&the_bts, ts_no);
 
@@ -2227,6 +2245,12 @@
 
 	printf("=== start %s ===\n", __func__);
 
+	bssgp_nsi = gprs_ns_instantiate(&gprs_bssgp_ns_cb, tall_pcu_ctx);
+	if (!bssgp_nsi) {
+		LOGP(DBSSGP, LOGL_ERROR, "Failed to create NS instance\n");
+		abort();
+	}
+
 	bts = the_bts.bts_data();
 	setup_bts(&the_bts, ts_no);
 
@@ -2264,6 +2288,12 @@
 
 	printf("=== start %s ===\n", __func__);
 
+	bssgp_nsi = gprs_ns_instantiate(&gprs_bssgp_ns_cb, tall_pcu_ctx);
+	if (!bssgp_nsi) {
+		LOGP(DBSSGP, LOGL_ERROR, "Failed to create NS instance\n");
+		abort();
+	}
+
 	bts = the_bts.bts_data();
 	setup_bts(&the_bts, ts_no);
 

-- 
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/14171
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I2aa53ea54e9352577f6280ad7b9d1d9da9f57eaf
Gerrit-Change-Number: 14171
Gerrit-PatchSet: 2
Gerrit-Owner: lynxis lazus <lynxis at fe80.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <axilirator at gmail.com>
Gerrit-Reviewer: laforge <laforge at gnumonks.org>
Gerrit-Reviewer: lynxis lazus <lynxis at fe80.eu>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190613/b3a1b7b7/attachment.htm>


More information about the gerrit-log mailing list