<p>Harald Welte <strong>merged</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/osmo-ggsn/+/14011">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Jenkins Builder: Verified
Harald Welte: Looks good to me, approved
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">ggsn: Use gtp_delete_context_req2() everywhere<br><br>Replace calls to gtp_delete_context_req() with<br>gtp_delete_context_req2().<br><br>Related: OS#2741<br>Change-Id: Iecc8c5ac45207e7e20129559c4ac7f3c67dfb36a<br>---<br>M ggsn/ggsn.c<br>M gtp/gtp.c<br>M sgsnemu/sgsnemu.c<br>3 files changed, 40 insertions(+), 7 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/ggsn/ggsn.c b/ggsn/ggsn.c</span><br><span>index 9b45109..c559923 100644</span><br><span>--- a/ggsn/ggsn.c</span><br><span>+++ b/ggsn/ggsn.c</span><br><span>@@ -107,7 +107,12 @@</span><br><span> if (!pdp)</span><br><span> continue;</span><br><span> LOGPPDP(LOGL_DEBUG, pdp, "Sending DELETE PDP CTX due to shutdown\n");</span><br><span style="color: hsl(0, 100%, 40%);">- gtp_delete_context_req(pdp->gsn, pdp, NULL, 1);</span><br><span style="color: hsl(120, 100%, 40%);">+ gtp_delete_context_req2(pdp->gsn, pdp, NULL, 1);</span><br><span style="color: hsl(120, 100%, 40%);">+ /* We have nothing more to do with pdp ctx, free it. Upon cb_delete_context</span><br><span style="color: hsl(120, 100%, 40%);">+ called during this call we'll clean up ggsn related stuff attached to this</span><br><span style="color: hsl(120, 100%, 40%);">+ pdp context. After this call, ippool member is cleared so</span><br><span style="color: hsl(120, 100%, 40%);">+ data is no longer valid and should not be accessed anymore. */</span><br><span style="color: hsl(120, 100%, 40%);">+ gtp_freepdp_teardown(pdp->gsn, pdp);</span><br><span> }</span><br><span> }</span><br><span> </span><br><span>@@ -980,6 +985,32 @@</span><br><span> }</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+/* libgtp callback for confirmations */</span><br><span style="color: hsl(120, 100%, 40%);">+static int cb_conf(int type, int cause, struct pdp_t *pdp, void *cbp)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ int rc = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (cause == EOF)</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DGGSN, LOGL_NOTICE, "libgtp EOF (type=%u, pdp=%p, cbp=%p)\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ type, pdp, cbp);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ switch (type) {</span><br><span style="color: hsl(120, 100%, 40%);">+ case GTP_DELETE_PDP_REQ:</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Remark: We actually never reach this path nowadays because</span><br><span style="color: hsl(120, 100%, 40%);">+ only place where we call gtp_delete_context_req2() is during</span><br><span style="color: hsl(120, 100%, 40%);">+ apn_stop()->pool_close_all_pdp() path, and in that case we</span><br><span style="color: hsl(120, 100%, 40%);">+ free all pdp contexts immediatelly without waiting for</span><br><span style="color: hsl(120, 100%, 40%);">+ confirmation since we want to tear down the whole APN</span><br><span style="color: hsl(120, 100%, 40%);">+ anyways. As a result, DeleteCtxResponse will never reach here</span><br><span style="color: hsl(120, 100%, 40%);">+ since it will be dropped at some point in lower layers in the</span><br><span style="color: hsl(120, 100%, 40%);">+ Rx path. This code is nevertheless left here in order to ease</span><br><span style="color: hsl(120, 100%, 40%);">+ future developent and avoid possible future memleaks once more</span><br><span style="color: hsl(120, 100%, 40%);">+ scenarios where GGSN sends a DeleteCtxRequest are introduced. */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (pdp)</span><br><span style="color: hsl(120, 100%, 40%);">+ rc = pdp_freepdp(pdp);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ return rc;</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span> </span><br><span> /* Start a given GGSN */</span><br><span> int ggsn_start(struct ggsn_ctx *ggsn)</span><br><span>@@ -1027,6 +1058,7 @@</span><br><span> gtp_set_cb_data_ind(ggsn->gsn, encaps_tun);</span><br><span> gtp_set_cb_delete_context(ggsn->gsn, delete_context);</span><br><span> gtp_set_cb_create_context_ind(ggsn->gsn, create_context_ind);</span><br><span style="color: hsl(120, 100%, 40%);">+ gtp_set_cb_conf(ggsn->gsn, cb_conf);</span><br><span> </span><br><span> LOGPGGSN(LOGL_NOTICE, ggsn, "Successfully started\n");</span><br><span> ggsn->started = true;</span><br><span>diff --git a/gtp/gtp.c b/gtp/gtp.c</span><br><span>index 9ae208a..2b14026 100644</span><br><span>--- a/gtp/gtp.c</span><br><span>+++ b/gtp/gtp.c</span><br><span>@@ -2434,7 +2434,9 @@</span><br><span> return 0;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-/* API: Send Delete PDP Context Request. PDP CTX shall be free'd by user at cb_conf(GTP_DELETE_PDP_RSP) */</span><br><span style="color: hsl(120, 100%, 40%);">+/* API: Send Delete PDP Context Request. PDP CTX shall be free'd by user at any</span><br><span style="color: hsl(120, 100%, 40%);">+ point in time later than this function through a call to pdp_freepdp(pdp), but</span><br><span style="color: hsl(120, 100%, 40%);">+ it must be freed no later than during cb_conf(GTP_DELETE_PDP_REQ, pdp) */</span><br><span> int gtp_delete_context_req2(struct gsn_t *gsn, struct pdp_t *pdp, void *cbp,</span><br><span> int teardown)</span><br><span> {</span><br><span>@@ -2643,7 +2645,7 @@</span><br><span> if (pdp_getgtp1(&pdp, get_tei(pack))) {</span><br><span> gsn->err_unknownpdp++;</span><br><span> GTP_LOGPKG(LOGL_NOTICE, peer, pack, len,</span><br><span style="color: hsl(0, 100%, 40%);">- "Unknown PDP context: %u (expected if gtp_delete_context_req is used)\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ "Unknown PDP context: %u (expected if gtp_delete_context_req is used or pdp ctx was freed manually before response)\n",</span><br><span> get_tei(pack));</span><br><span> if (gsn->cb_conf)</span><br><span> gsn->cb_conf(type, EOF, NULL, cbp);</span><br><span>diff --git a/sgsnemu/sgsnemu.c b/sgsnemu/sgsnemu.c</span><br><span>index a2220f0..225dc59 100644</span><br><span>--- a/sgsnemu/sgsnemu.c</span><br><span>+++ b/sgsnemu/sgsnemu.c</span><br><span>@@ -1474,6 +1474,8 @@</span><br><span> {</span><br><span> printf("Received delete PDP context response. Cause value: %d\n",</span><br><span> cause);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (pdp)</span><br><span style="color: hsl(120, 100%, 40%);">+ pdp_freepdp(pdp);</span><br><span> return 0;</span><br><span> }</span><br><span> </span><br><span>@@ -1508,8 +1510,6 @@</span><br><span> case GTP_CREATE_PDP_REQ:</span><br><span> return create_pdp_conf(pdp, cbp, cause);</span><br><span> case GTP_DELETE_PDP_REQ:</span><br><span style="color: hsl(0, 100%, 40%);">- if (cause != 128)</span><br><span style="color: hsl(0, 100%, 40%);">- return 0; /* Request not accepted. We don't care */</span><br><span> return delete_pdp_conf(pdp, cause);</span><br><span> default:</span><br><span> return 0;</span><br><span>@@ -1756,8 +1756,7 @@</span><br><span> for (n = 0; n < options.contexts; n++) {</span><br><span> /* Delete context */</span><br><span> printf("Disconnecting PDP context #%d\n", n);</span><br><span style="color: hsl(0, 100%, 40%);">- gtp_delete_context_req(gsn, iparr[n].pdp, NULL,</span><br><span style="color: hsl(0, 100%, 40%);">- 1);</span><br><span style="color: hsl(120, 100%, 40%);">+ gtp_delete_context_req2(gsn, iparr[n].pdp, NULL, 1);</span><br><span> if ((options.pinghost.s_addr != 0)</span><br><span> && ntransmitted)</span><br><span> ping_finish();</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-ggsn/+/14011">change 14011</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-ggsn/+/14011"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-ggsn </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: Iecc8c5ac45207e7e20129559c4ac7f3c67dfb36a </div>
<div style="display:none"> Gerrit-Change-Number: 14011 </div>
<div style="display:none"> Gerrit-PatchSet: 5 </div>
<div style="display:none"> Gerrit-Owner: osmith <osmith@sysmocom.de> </div>
<div style="display:none"> Gerrit-Assignee: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Harald Welte <laforge@gnumonks.org> </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: osmith <osmith@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>