Change in ...osmo-ggsn[master]: ggsn: Use gtp_delete_context_req2() everywhere

Harald Welte gerrit-no-reply at lists.osmocom.org
Mon Jun 3 11:18:16 UTC 2019


Harald Welte has submitted this change and it was merged. ( https://gerrit.osmocom.org/c/osmo-ggsn/+/14011 )

Change subject: ggsn: Use gtp_delete_context_req2() everywhere
......................................................................

ggsn: Use gtp_delete_context_req2() everywhere

Replace calls to gtp_delete_context_req() with
gtp_delete_context_req2().

Related: OS#2741
Change-Id: Iecc8c5ac45207e7e20129559c4ac7f3c67dfb36a
---
M ggsn/ggsn.c
M gtp/gtp.c
M sgsnemu/sgsnemu.c
3 files changed, 40 insertions(+), 7 deletions(-)

Approvals:
  Jenkins Builder: Verified
  Harald Welte: Looks good to me, approved



diff --git a/ggsn/ggsn.c b/ggsn/ggsn.c
index 9b45109..c559923 100644
--- a/ggsn/ggsn.c
+++ b/ggsn/ggsn.c
@@ -107,7 +107,12 @@
 		if (!pdp)
 			continue;
 		LOGPPDP(LOGL_DEBUG, pdp, "Sending DELETE PDP CTX due to shutdown\n");
-		gtp_delete_context_req(pdp->gsn, pdp, NULL, 1);
+		gtp_delete_context_req2(pdp->gsn, pdp, NULL, 1);
+		/* We have nothing more to do with pdp ctx, free it. Upon cb_delete_context
+		   called during this call we'll clean up ggsn related stuff attached to this
+		   pdp context. After this call, ippool member is cleared so
+		   data is no longer valid and should not be accessed anymore. */
+		gtp_freepdp_teardown(pdp->gsn, pdp);
 	}
 }
 
@@ -980,6 +985,32 @@
 	}
 }
 
+/* libgtp callback for confirmations */
+static int cb_conf(int type, int cause, struct pdp_t *pdp, void *cbp)
+{
+	int rc = 0;
+
+	if (cause == EOF)
+		LOGP(DGGSN, LOGL_NOTICE, "libgtp EOF (type=%u, pdp=%p, cbp=%p)\n",
+			type, pdp, cbp);
+
+	switch (type) {
+	case GTP_DELETE_PDP_REQ:
+		/* Remark: We actually never reach this path nowadays because
+		   only place where we call gtp_delete_context_req2() is during
+		   apn_stop()->pool_close_all_pdp() path, and in that case we
+		   free all pdp contexts immediatelly without waiting for
+		   confirmation since we want to tear down the whole APN
+		   anyways. As a result, DeleteCtxResponse will never reach here
+		   since it will be dropped at some point in lower layers in the
+		   Rx path. This code is nevertheless left here in order to ease
+		   future developent and avoid possible future memleaks once more
+		   scenarios where GGSN sends a DeleteCtxRequest are introduced. */
+		   if (pdp)
+			rc = pdp_freepdp(pdp);
+	}
+	return rc;
+}
 
 /* Start a given GGSN */
 int ggsn_start(struct ggsn_ctx *ggsn)
@@ -1027,6 +1058,7 @@
 	gtp_set_cb_data_ind(ggsn->gsn, encaps_tun);
 	gtp_set_cb_delete_context(ggsn->gsn, delete_context);
 	gtp_set_cb_create_context_ind(ggsn->gsn, create_context_ind);
+	gtp_set_cb_conf(ggsn->gsn, cb_conf);
 
 	LOGPGGSN(LOGL_NOTICE, ggsn, "Successfully started\n");
 	ggsn->started = true;
diff --git a/gtp/gtp.c b/gtp/gtp.c
index 9ae208a..2b14026 100644
--- a/gtp/gtp.c
+++ b/gtp/gtp.c
@@ -2434,7 +2434,9 @@
 	return 0;
 }
 
-/* API: Send Delete PDP Context Request. PDP CTX shall be free'd by user at cb_conf(GTP_DELETE_PDP_RSP) */
+/* API: Send Delete PDP Context Request. PDP CTX shall be free'd by user at any
+   point in time later than this function through a call to pdp_freepdp(pdp), but
+   it must be freed no later than during cb_conf(GTP_DELETE_PDP_REQ, pdp) */
 int gtp_delete_context_req2(struct gsn_t *gsn, struct pdp_t *pdp, void *cbp,
 			   int teardown)
 {
@@ -2643,7 +2645,7 @@
 	if (pdp_getgtp1(&pdp, get_tei(pack))) {
 		gsn->err_unknownpdp++;
 		GTP_LOGPKG(LOGL_NOTICE, peer, pack, len,
-			    "Unknown PDP context: %u (expected if gtp_delete_context_req is used)\n",
+			    "Unknown PDP context: %u (expected if gtp_delete_context_req is used or pdp ctx was freed manually before response)\n",
 			     get_tei(pack));
 		if (gsn->cb_conf)
 			gsn->cb_conf(type, EOF, NULL, cbp);
diff --git a/sgsnemu/sgsnemu.c b/sgsnemu/sgsnemu.c
index a2220f0..225dc59 100644
--- a/sgsnemu/sgsnemu.c
+++ b/sgsnemu/sgsnemu.c
@@ -1474,6 +1474,8 @@
 {
 	printf("Received delete PDP context response. Cause value: %d\n",
 	       cause);
+	if (pdp)
+		pdp_freepdp(pdp);
 	return 0;
 }
 
@@ -1508,8 +1510,6 @@
 	case GTP_CREATE_PDP_REQ:
 		return create_pdp_conf(pdp, cbp, cause);
 	case GTP_DELETE_PDP_REQ:
-		if (cause != 128)
-			return 0;	/* Request not accepted. We don't care */
 		return delete_pdp_conf(pdp, cause);
 	default:
 		return 0;
@@ -1756,8 +1756,7 @@
 			for (n = 0; n < options.contexts; n++) {
 				/* Delete context */
 				printf("Disconnecting PDP context #%d\n", n);
-				gtp_delete_context_req(gsn, iparr[n].pdp, NULL,
-						       1);
+				gtp_delete_context_req2(gsn, iparr[n].pdp, NULL, 1);
 				if ((options.pinghost.s_addr != 0)
 				    && ntransmitted)
 					ping_finish();

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

Gerrit-Project: osmo-ggsn
Gerrit-Branch: master
Gerrit-Change-Id: Iecc8c5ac45207e7e20129559c4ac7f3c67dfb36a
Gerrit-Change-Number: 14011
Gerrit-PatchSet: 5
Gerrit-Owner: osmith <osmith at sysmocom.de>
Gerrit-Assignee: pespin <pespin at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <axilirator at gmail.com>
Gerrit-Reviewer: osmith <osmith at sysmocom.de>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190603/6612d9ed/attachment.html>


More information about the gerrit-log mailing list