<p>laforge <strong>merged</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/osmo-pcu/+/14171">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Jenkins Builder: Verified
fixeria: Looks good to me, but someone else must approve
laforge: Looks good to me, approved
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">gprs_bssgp_pcu: explicit allocate & initialize bssgp_nsi instance<br><br>The instance bssgp_nsi is a global instance to be used by all<br>NS related functions. Previous the PCU allocated and initialized<br>the bssgp_nsi instance when (re-)connecting and freeing on disconnect.<br>The problem of the implicit initialisation is gprs_ns_vty_init(bssgp_nsi).<br>All vty init functions must be called before the configuration is read,<br>otherwise a previous vty written configuration is invalid.<br>Furthermore the vty modifications to the `ns` object were lost when the PCU has to<br>reconnect to the SGSN.<br><br>Fixes: OS#4024<br>Change-Id: I2aa53ea54e9352577f6280ad7b9d1d9da9f57eaf<br>---<br>M src/gprs_bssgp_pcu.cpp<br>M src/pcu_main.cpp<br>M tests/emu/pcu_emu.cpp<br>M tests/tbf/TbfTest.cpp<br>4 files changed, 49 insertions(+), 17 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/gprs_bssgp_pcu.cpp b/src/gprs_bssgp_pcu.cpp</span><br><span>index 50f10db..22abc6b 100644</span><br><span>--- a/src/gprs_bssgp_pcu.cpp</span><br><span>+++ b/src/gprs_bssgp_pcu.cpp</span><br><span>@@ -875,11 +875,6 @@</span><br><span> {</span><br><span> struct gprs_nsvc *nsvc2;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (!bssgp_nsi) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DBSSGP, LOGL_ERROR, "NS instance does not exist\n");</span><br><span style="color: hsl(0, 100%, 40%);">- return -EINVAL;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> if (nsvc != the_pcu.nsvc) {</span><br><span> LOGP(DBSSGP, LOGL_ERROR, "NSVC is invalid\n");</span><br><span> return -EBADF;</span><br><span>@@ -913,12 +908,6 @@</span><br><span> </span><br><span> the_pcu.bts = bts;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- bssgp_nsi = gprs_ns_instantiate(&gprs_bssgp_ns_cb, tall_pcu_ctx);</span><br><span style="color: hsl(0, 100%, 40%);">- if (!bssgp_nsi) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DBSSGP, LOGL_ERROR, "Failed to create NS instance\n");</span><br><span style="color: hsl(0, 100%, 40%);">- return NULL;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">- gprs_ns_vty_init(bssgp_nsi);</span><br><span> /* don't specify remote IP/port if SNS dialect is in use; Doing so would</span><br><span> * issue a connect() on the socket, which prevents us to dynamically communicate</span><br><span> * with any number of IP-SNS endpoints on the SGSN side */</span><br><span>@@ -930,8 +919,7 @@</span><br><span> rc = gprs_ns_nsip_listen(bssgp_nsi);</span><br><span> if (rc < 0) {</span><br><span> LOGP(DBSSGP, LOGL_ERROR, "Failed to create socket\n");</span><br><span style="color: hsl(0, 100%, 40%);">- gprs_ns_destroy(bssgp_nsi);</span><br><span style="color: hsl(0, 100%, 40%);">- bssgp_nsi = NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+ gprs_ns_close(bssgp_nsi);</span><br><span> return NULL;</span><br><span> }</span><br><span> </span><br><span>@@ -945,8 +933,7 @@</span><br><span> the_pcu.nsvc = gprs_ns_nsip_connect(bssgp_nsi, &dest, nsei, nsvci);</span><br><span> if (!the_pcu.nsvc) {</span><br><span> LOGP(DBSSGP, LOGL_ERROR, "Failed to create NSVCt\n");</span><br><span style="color: hsl(0, 100%, 40%);">- gprs_ns_destroy(bssgp_nsi);</span><br><span style="color: hsl(0, 100%, 40%);">- bssgp_nsi = NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+ gprs_ns_close(bssgp_nsi);</span><br><span> return NULL;</span><br><span> }</span><br><span> </span><br><span>@@ -954,8 +941,7 @@</span><br><span> if (!the_pcu.bctx) {</span><br><span> LOGP(DBSSGP, LOGL_ERROR, "Failed to create BSSGP context\n");</span><br><span> the_pcu.nsvc = NULL;</span><br><span style="color: hsl(0, 100%, 40%);">- gprs_ns_destroy(bssgp_nsi);</span><br><span style="color: hsl(0, 100%, 40%);">- bssgp_nsi = NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+ gprs_ns_close(bssgp_nsi);</span><br><span> return NULL;</span><br><span> }</span><br><span> the_pcu.bctx->ra_id.mcc = spoof_mcc ? : mcc;</span><br><span>diff --git a/src/pcu_main.cpp b/src/pcu_main.cpp</span><br><span>index 1003e3c..fa075cd 100644</span><br><span>--- a/src/pcu_main.cpp</span><br><span>+++ b/src/pcu_main.cpp</span><br><span>@@ -33,6 +33,8 @@</span><br><span> #include <bts.h></span><br><span> #include <gprs_coding_scheme.h></span><br><span> #include <osmocom/pcu/pcuif_proto.h></span><br><span style="color: hsl(120, 100%, 40%);">+#include "gprs_bssgp_pcu.h"</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> extern "C" {</span><br><span> #include "pcu_vty.h"</span><br><span> #include <osmocom/gprs/gprs_bssgp.h></span><br><span>@@ -291,6 +293,13 @@</span><br><span> else</span><br><span> fprintf(stderr, "Failed to initialize GSMTAP for %s\n", gsmtap_addr);</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ bssgp_nsi = gprs_ns_instantiate(&gprs_bssgp_ns_cb, tall_pcu_ctx);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!bssgp_nsi) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DBSSGP, LOGL_ERROR, "Failed to create NS instance\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ exit(1);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ gprs_ns_vty_init(bssgp_nsi);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> rc = vty_read_config_file(config_file, NULL);</span><br><span> if (rc < 0 && config_given) {</span><br><span> fprintf(stderr, "Failed to parse the config file: '%s'\n",</span><br><span>diff --git a/tests/emu/pcu_emu.cpp b/tests/emu/pcu_emu.cpp</span><br><span>index 354a328..e0190bf 100644</span><br><span>--- a/tests/emu/pcu_emu.cpp</span><br><span>+++ b/tests/emu/pcu_emu.cpp</span><br><span>@@ -113,6 +113,13 @@</span><br><span> </span><br><span> msgb_talloc_ctx_init(tall_pcu_ctx, 0);</span><br><span> osmo_init_logging2(tall_pcu_ctx, &gprs_log_info);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ bssgp_nsi = gprs_ns_instantiate(&gprs_bssgp_ns_cb, tall_pcu_ctx);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!bssgp_nsi) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DBSSGP, LOGL_ERROR, "Failed to create NS instance\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ abort();</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> vty_init(&pcu_vty_info);</span><br><span> pcu_vty_init(&gprs_log_info);</span><br><span> </span><br><span>diff --git a/tests/tbf/TbfTest.cpp b/tests/tbf/TbfTest.cpp</span><br><span>index 151e7fa..675cf89 100644</span><br><span>--- a/tests/tbf/TbfTest.cpp</span><br><span>+++ b/tests/tbf/TbfTest.cpp</span><br><span>@@ -447,6 +447,12 @@</span><br><span> </span><br><span> printf("=== start %s ===\n", __func__);</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ bssgp_nsi = gprs_ns_instantiate(&gprs_bssgp_ns_cb, tall_pcu_ctx);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!bssgp_nsi) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DBSSGP, LOGL_ERROR, "Failed to create NS instance\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ abort();</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> bts = the_bts.bts_data();</span><br><span> setup_bts(&the_bts, ts_no);</span><br><span> gprs_bssgp_create_and_connect(bts, 33001, 0, 33001, 1234, 1234, 1234, 1, 1, false, 0, 0, 0);</span><br><span>@@ -485,6 +491,12 @@</span><br><span> </span><br><span> uint8_t buf[19];</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ bssgp_nsi = gprs_ns_instantiate(&gprs_bssgp_ns_cb, tall_pcu_ctx);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!bssgp_nsi) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DBSSGP, LOGL_ERROR, "Failed to create NS instance\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ abort();</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> printf("=== start %s ===\n", __func__);</span><br><span> </span><br><span> bts = the_bts.bts_data();</span><br><span>@@ -2171,6 +2183,12 @@</span><br><span> </span><br><span> printf("=== start %s ===\n", __func__);</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ bssgp_nsi = gprs_ns_instantiate(&gprs_bssgp_ns_cb, tall_pcu_ctx);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!bssgp_nsi) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DBSSGP, LOGL_ERROR, "Failed to create NS instance\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ abort();</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> bts = the_bts.bts_data();</span><br><span> setup_bts(&the_bts, ts_no);</span><br><span> </span><br><span>@@ -2227,6 +2245,12 @@</span><br><span> </span><br><span> printf("=== start %s ===\n", __func__);</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ bssgp_nsi = gprs_ns_instantiate(&gprs_bssgp_ns_cb, tall_pcu_ctx);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!bssgp_nsi) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DBSSGP, LOGL_ERROR, "Failed to create NS instance\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ abort();</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> bts = the_bts.bts_data();</span><br><span> setup_bts(&the_bts, ts_no);</span><br><span> </span><br><span>@@ -2264,6 +2288,12 @@</span><br><span> </span><br><span> printf("=== start %s ===\n", __func__);</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ bssgp_nsi = gprs_ns_instantiate(&gprs_bssgp_ns_cb, tall_pcu_ctx);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!bssgp_nsi) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DBSSGP, LOGL_ERROR, "Failed to create NS instance\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ abort();</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> bts = the_bts.bts_data();</span><br><span> setup_bts(&the_bts, ts_no);</span><br><span> </span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-pcu/+/14171">change 14171</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/osmo-pcu/+/14171"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-pcu </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I2aa53ea54e9352577f6280ad7b9d1d9da9f57eaf </div>
<div style="display:none"> Gerrit-Change-Number: 14171 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: lynxis lazus <lynxis@fe80.eu> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: fixeria <axilirator@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-Reviewer: lynxis lazus <lynxis@fe80.eu> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>