Change in osmo-ggsn[master]: gtp: Rework parsing logic of UpdatePdpCtxResponse

[pespin]

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-ggsn/+/23860 )


Change subject: gtp: Rework parsing logic of UpdatePdpCtxResponse
......................................................................

gtp: Rework parsing logic of UpdatePdpCtxResponse

The previous order of parsing lead to non-optimal information gathering
when pushing events to upper layers.

This patch rearranges parsing of packet data to always gather as much
info as possible for the benefit of the upper layer. This way it can
gather information such as the cause, which is important in the case of
"Non-existent", since user should then drop the context.

First we want to parse the recovery state, but delay cb to upper layers
until we tried to gather the pdp ctx (meaning all except that pdp ctx
should be freed).
Second, we want to parse the cause, in order to know if there's an
associated pdp ctx we can gather from TEID.
Third, once we know if we should expect a meaningul TEID, parse it.

Related: SYS#5435
Change-Id: Idd10b494e8fbac8703c49ecd8f9bbe4246e51c57
---
M gtp/gtp.c
1 file changed, 30 insertions(+), 19 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-ggsn refs/changes/60/23860/1

diff --git a/gtp/gtp.c b/gtp/gtp.c
index 701aa36..1995d10 100644
--- a/gtp/gtp.c
+++ b/gtp/gtp.c
@@ -2324,26 +2324,20 @@
 static int gtp_update_pdp_conf(struct gsn_t *gsn, uint8_t version,
 			struct sockaddr_in *peer, void *pack, unsigned len)
 {
-	struct pdp_t *pdp;
+	struct pdp_t *pdp = NULL;
 	union gtpie_member *ie[GTPIE_SIZE];
-	uint8_t cause, recovery;
+	uint8_t cause = EOF;
+	uint8_t recovery;
+	int rc = 0;
 	void *cbp = NULL;
 	uint8_t type = 0;
+	bool trigger_recovery = false;
 	int hlen = get_hlen(pack);
 
 	/* Remove packet from queue */
 	if (gtp_conf(gsn, 0, peer, pack, len, &type, &cbp))
 		return EOF;
 
-	/* Find the context in question */
-	if (gtp_pdp_getgtp1(gsn, &pdp, get_tei(pack))) {
-		gsn->err_unknownpdp++;
-		GTP_LOGPKG(LOGL_ERROR, peer, pack, len,
-			    "Unknown PDP context: %u\n", get_tei(pack));
-		pdp = NULL;
-		goto err_out;
-	}
-
 	/* Decode information elements */
 	if (gtpie_decaps(ie, version, pack + hlen, len - hlen)) {
 		gsn->invalid++;
@@ -2352,19 +2346,34 @@
 		goto err_out;
 	}
 
+	/* Extract recovery (optional) */
+	if (!gtpie_gettv1(ie, GTPIE_RECOVERY, 0, &recovery))
+		trigger_recovery = true;
+
 	/* Extract cause value (mandatory) */
 	if (gtpie_gettv1(ie, GTPIE_CAUSE, 0, &cause)) {
 		goto err_missing;
 	}
 
-	/* Extract recovery (optional) */
-	if (!gtpie_gettv1(ie, GTPIE_RECOVERY, 0, &recovery)) {
-		emit_cb_recovery(gsn, peer, pdp, recovery);
+	/*  3GPP TS 29.060 sec 8.2: "Receiving node shall send back to the source
+	 *  of the message, a response with the appropriate cause value (either
+	 *  "Non-existent" or "Context not found"). The Tunnel Endpoint
+	 *  Identifier used in the response message shall be set to all zeroes."
+	 *  Hence, TEID=0 in this scenario, it makes no sense to infer PDP ctx
+	 *  from it. User is responsible to infer it from cbp */
+	if (cause != GTPCAUSE_NON_EXIST || cause != GTPCAUSE_CONTEXT_NOT_FOUND) {
+		/* Find the context in question */
+		if (gtp_pdp_getgtp1(gsn, &pdp, get_tei(pack))) {
+			gsn->err_unknownpdp++;
+			GTP_LOGPKG(LOGL_ERROR, peer, pack, len,
+				    "Unknown PDP context: %u\n", get_tei(pack));
+			goto err_out;
+		}
 	}
 
 	/* Check all conditional information elements */
 	/* TODO: This does not handle GGSN-initiated update responses */
-	if (GTPCAUSE_ACC_REQ == cause) {
+	if (cause == GTPCAUSE_ACC_REQ) {
 		if (version == 0) {
 			if (gtpie_gettv0(ie, GTPIE_QOS_PROFILE0, 0,
 					 &pdp->qos_neg0,
@@ -2416,18 +2425,20 @@
 		}
 	}
 
+generic_ret:
+	if (trigger_recovery)
+		emit_cb_recovery(gsn, peer, pdp, recovery);
 	if (gsn->cb_conf)
 		gsn->cb_conf(type, cause, pdp, cbp);
-	return 0;	/* Succes */
+	return rc;	/* Succes */
 
 err_missing:
 	gsn->missing++;
 	GTP_LOGPKG(LOGL_ERROR, peer, pack, len,
 		    "Missing information field\n");
 err_out:
-	if (gsn->cb_conf)
-		gsn->cb_conf(type, EOF, pdp, cbp);
-	return EOF;
+	rc = EOF;
+	goto generic_ret;
 }
 
 /* API: Deprecated. Send Delete PDP Context Request And free pdp ctx. */

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

Gerrit-Project: osmo-ggsn
Gerrit-Branch: master
Gerrit-Change-Id: Idd10b494e8fbac8703c49ecd8f9bbe4246e51c57
Gerrit-Change-Number: 23860
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin at sysmocom.de>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210422/d0015081/attachment.htm>