gtphub: fix User plane decoding, add unit test.

Split decoding return code GTP_RC_PDU in GTP_RC_PDU_C and GTP_RC_PDU_U.
Don't do IEs in GTP_RC_PDU_U.

Add a unit test for User plane data, expected to fail (nonstandard port case).

In gtphub_test.c, tweak logging so that it is easily visible which test
produced which output. Also add the global resolved_sgsn_addr and ggsn_sender,
symmetrically to resolved_ggsn_add and sgsn_sender.

Sponsored-by: On-Waves ehi
diff --git a/openbsc/src/gprs/gtphub.c b/openbsc/src/gprs/gtphub.c
index f24fc04..9a29cc0 100644
--- a/openbsc/src/gprs/gtphub.c
+++ b/openbsc/src/gprs/gtphub.c
@@ -66,7 +66,8 @@
 enum gtp_rc {
 	GTP_RC_UNKNOWN = 0,
 	GTP_RC_TINY = 1,    /* no IEs (like ping/pong) */
-	GTP_RC_PDU = 2,     /* a real packet with IEs */
+	GTP_RC_PDU_C = 2,     /* a real packet with IEs */
+	GTP_RC_PDU_U = 3,     /* a real packet with User data */
 
 	GTP_RC_TOOSHORT = -1,
 	GTP_RC_UNSUPPORTED_VERSION = -2,
@@ -176,7 +177,7 @@
 
 static int gsn_addr_get(struct gsn_addr *gsna, const struct gtp_packet_desc *p, int idx)
 {
-	if (p->rc != GTP_RC_PDU)
+	if (p->rc != GTP_RC_PDU_C)
 		return -1;
 
 	unsigned int len;
@@ -191,7 +192,7 @@
 
 static int gsn_addr_put(const struct gsn_addr *gsna, struct gtp_packet_desc *p, int idx)
 {
-	if (p->rc != GTP_RC_PDU)
+	if (p->rc != GTP_RC_PDU_C)
 		return -1;
 
 	int ie_idx;
@@ -251,7 +252,7 @@
 
 	LOG("GTP v0 TID = %" PRIu64 "\n", pheader->tid);
 	p->header_len = GTP0_HEADER_SIZE;
-	p->rc = GTP_RC_PDU;
+	p->rc = GTP_RC_PDU_C;
 }
 
 /* Validate GTP version 1 data, and update p->rc with the result, as well as
@@ -306,7 +307,7 @@
 		return;
 	}
 
-	p->rc = GTP_RC_PDU;
+	p->rc = GTP_RC_PDU_C;
 	p->header_len = GTP1_HEADER_SIZE_LONG;
 }
 
@@ -449,7 +450,12 @@
 
 	LOG("Valid GTP header (v%d)\n", res->version);
 
-	if (res->rc != GTP_RC_PDU) {
+	if (from_plane_idx == GTPH_PLANE_USER) {
+		res->rc = GTP_RC_PDU_U;
+		return;
+	}
+
+	if (res->rc != GTP_RC_PDU_C) {
 		LOG("no IEs in this GTP packet\n");
 		return;
 	}
@@ -458,6 +464,7 @@
 			 (void*)(data + res->header_len),
 			 res->data_len - res->header_len) != 0) {
 		res->rc = GTP_RC_INVALID_IE;
+		LOGERR("INVALID: cannot decode IEs. Dropping GTP packet.\n");
 		return;
 	}
 
@@ -708,10 +715,6 @@
 						  const char *apn_ni_str);
 int gtphub_ares_init(struct gtphub *hub);
 
-static struct gtphub_peer_port *gtphub_port_find(const struct gtphub_bind *bind,
-						 const struct gsn_addr *addr,
-						 uint16_t port);
-
 static void gtphub_zero(struct gtphub *hub)
 {
 	ZERO_STRUCT(hub);
@@ -1377,7 +1380,9 @@
 				 struct osmo_fd **to_ofd,
 				 struct osmo_sockaddr *to_addr)
 {
-	LOG("<- rx from GGSN %s\n", osmo_sockaddr_to_str(from_addr));
+	LOG("<- rx %s from GGSN %s\n",
+	    gtphub_plane_idx_names[plane_idx],
+	    osmo_sockaddr_to_str(from_addr));
 
 	static struct gtp_packet_desc p;
 	gtp_decode(buf, received, plane_idx, &p);
@@ -1421,7 +1426,8 @@
 	 * this GGSN. If we don't have an entry, the GGSN has nothing to tell
 	 * us about. */
 	if (!ggsn) {
-		LOGERR("Invalid GGSN peer. Dropping packet.\n");
+		LOGERR("Dropping packet: unknown GGSN peer: %s\n",
+		       osmo_sockaddr_to_str(from_addr));
 		return -1;
 	}
 
@@ -1466,6 +1472,9 @@
 	osmo_sockaddr_copy(to_addr, &sgsn->sa);
 
 	*reply_buf = (uint8_t*)p.data;
+
+	LOG("<-- Forward to SGSN: %d bytes to %s\n",
+	    (int)received, osmo_sockaddr_to_str(to_addr));
 	return received;
 }
 
@@ -1511,7 +1520,9 @@
 				 struct osmo_fd **to_ofd,
 				 struct osmo_sockaddr *to_addr)
 {
-	LOG("-> rx from SGSN %s\n", osmo_sockaddr_to_str(from_addr));
+	LOG("-> rx %s from SGSN %s\n",
+	    gtphub_plane_idx_names[plane_idx],
+	    osmo_sockaddr_to_str(from_addr));
 
 	static struct gtp_packet_desc p;
 	gtp_decode(buf, received, plane_idx, &p);
@@ -1574,7 +1585,8 @@
 
 	if (!sgsn) {
 		/* This could theoretically happen for invalid address data or somesuch. */
-		LOGERR("Invalid SGSN peer. Dropping packet.\n");
+		LOGERR("Dropping packet: invalid SGSN peer: %s\n",
+		       osmo_sockaddr_to_str(from_addr));
 		return -1;
 	}
 	LOG("SGSN peer: %s\n", gtphub_port_str(sgsn));
@@ -1635,6 +1647,9 @@
 	osmo_sockaddr_copy(to_addr, &ggsn->sa);
 
 	*reply_buf = (uint8_t*)p.data;
+
+	LOG("--> Forward to GGSN: %d bytes to %s\n",
+	    (int)received, osmo_sockaddr_to_str(to_addr));
 	return received;
 }
 
@@ -1979,7 +1994,13 @@
 	 * entirely new peer for the new address. More addresses may be added
 	 * to this peer later, but not via this function. */
 	struct gtphub_peer *peer = gtphub_peer_new(hub, bind);
-	return gtphub_peer_add_addr(peer, addr);
+
+	a = gtphub_peer_add_addr(peer, addr);
+	
+	LOG("New peer address: %s\n",
+	    gsn_addr_to_str(&a->addr));
+
+	return a;
 }
 
 static struct gtphub_peer_port *gtphub_addr_add_port(struct gtphub_peer_addr *a,
@@ -1999,7 +2020,7 @@
 
 	llist_add(&pp->entry, &a->ports);
 
-	LOG("New peer: %s port %d\n",
+	LOG("New peer port: %s port %d\n",
 	    gsn_addr_to_str(&a->addr),
 	    (int)port);
 
diff --git a/openbsc/tests/gtphub/gtphub_test.c b/openbsc/tests/gtphub/gtphub_test.c
index 0b3aeb5..fc73570 100644
--- a/openbsc/tests/gtphub/gtphub_test.c
+++ b/openbsc/tests/gtphub/gtphub_test.c
@@ -47,6 +47,11 @@
 		ret; \
 	}
 
+/* Convenience makro, note: only within this C file. */
+#define LOG(label) \
+	{ LOGP(DGTPHUB, LOGL_NOTICE, "\n\n" label "\n"); \
+	  printf(label "\n"); }
+
 void gtphub_init(struct gtphub *hub);
 
 void *osmo_gtphub_ctx;
@@ -345,6 +350,15 @@
 	return 1;
 }
 
+struct osmo_sockaddr resolved_sgsn_addr;
+static int resolve_to_sgsn(const char *addr, uint16_t port)
+{
+	LVL2_ASSERT(osmo_sockaddr_init_udp(&resolved_sgsn_addr,
+					   addr, port)
+		    == 0);
+	return 1;
+}
+
 struct osmo_sockaddr sgsn_sender;
 static int send_from_sgsn(const char *addr, uint16_t port)
 {
@@ -354,6 +368,14 @@
 	return 1;
 }
 
+struct osmo_sockaddr ggsn_sender;
+static int send_from_ggsn(const char *addr, uint16_t port)
+{
+	LVL2_ASSERT(osmo_sockaddr_init_udp(&ggsn_sender,
+					   addr, port)
+		    == 0);
+	return 1;
+}
 
 
 /* override, requires '-Wl,--wrap=gtphub_resolve_ggsn_addr' */
@@ -374,7 +396,8 @@
 	struct gtphub_peer_port *pp;
 	pp = gtphub_port_have(hub, &hub->to_ggsns[GTPH_PLANE_CTRL],
 			      &resolved_gsna, resolved_port);
-	printf("Wrap: returning GGSN addr from imsi %s ni %s: %s\n",
+	printf("- __wrap_gtphub_resolve_ggsn_addr():\n"
+	       "  returning GGSN addr from imsi %s ni %s: %s\n",
 	       imsi_str, apn_ni_str, gtphub_port_str(pp));
 
 	if (imsi_str) {
@@ -518,7 +541,9 @@
 	hub->restart_counter = 0x23;
 	now = 345;
 	LVL2_ASSERT(send_from_sgsn("192.168.42.23", 423));
-	LVL2_ASSERT(resolve_to_ggsn("192.168.43.34", 434));
+	LVL2_ASSERT(resolve_to_ggsn("192.168.43.34", 2123));
+	LVL2_ASSERT(send_from_ggsn("192.168.43.34", 321));
+	LVL2_ASSERT(resolve_to_sgsn("192.168.42.23", 2123));
 
 	return 1;
 }
@@ -526,6 +551,7 @@
 
 static void test_echo(void)
 {
+	LOG("test_echo");
 	OSMO_ASSERT(setup_test_hub());
 
 	now = 123;
@@ -585,15 +611,12 @@
 		"0e23"	/* Recovery with restart counter */
 		;
 
-	struct osmo_sockaddr orig_ggsn_addr;
-	OSMO_ASSERT(osmo_sockaddr_init_udp(&orig_ggsn_addr,
-					   "192.168.24.32", 321) == 0);
 	struct osmo_fd *sgsn_ofd = NULL;
-	send = gtphub_from_ggsns_handle_buf(hub, GTPH_PLANE_CTRL, &orig_ggsn_addr,
+	send = gtphub_from_ggsns_handle_buf(hub, GTPH_PLANE_CTRL, &ggsn_sender,
 					    buf, msg(gtp_ping_from_ggsn), now,
 					    &reply_buf, &sgsn_ofd, &to_addr);
 	OSMO_ASSERT(send > 0);
-	OSMO_ASSERT(same_addr(&to_addr, &orig_ggsn_addr));
+	OSMO_ASSERT(same_addr(&to_addr, &ggsn_sender));
 	OSMO_ASSERT(reply_is(gtp_pong_to_ggsn));
 
 	struct gtphub_peer_port *ggsn_port =
@@ -769,6 +792,8 @@
 						       "0004""7f000101", /* gtphub's address towards SGSNs (Ctrl) */
 						       "0004""7f000102" /* gtphub's address towards SGSNs (User) */
 						      );
+	/* The response should go back to whichever port the request came from
+	 * (unmapped by sequence nr) */
 	LVL2_ASSERT(msg_from_ggsn_c(&resolved_ggsn_addr,
 				    &sgsn_sender,
 				    gtp_resp_from_ggsn,
@@ -779,6 +804,7 @@
 
 static void test_create_pdp_ctx(void)
 {
+	LOG("test_create_pdp_ctx");
 	OSMO_ASSERT(setup_test_hub());
 
 	OSMO_ASSERT(create_pdp_ctx());
@@ -806,6 +832,83 @@
 	gtphub_gc(hub, now + EXPIRE_ALL);
 }
 
+static void test_user_data(void)
+{
+	LOG("test_user_data");
+
+	OSMO_ASSERT(setup_test_hub());
+
+	OSMO_ASSERT(create_pdp_ctx());
+
+	LOG("- user data starts");
+	/* Now expect default port numbers for User. */
+	resolve_to_ggsn("192.168.43.34", 2152);
+	resolve_to_sgsn("192.168.42.23", 2152);
+
+	const char *u_from_ggsn =
+		"32" 	/* 0b001'1 0010: version 1, protocol GTP, with seq nr. */ \
+		"ff"	/* type 255: G-PDU */
+		"0058"	/* length: 88 + 8 octets == 96 */
+		"00000001" /* mapped User TEI for SGSN from create_pdp_ctx() */
+		"0070"	/* seq */
+		"0000"	/* No extensions */
+		/* User data (ICMP packet), 96 - 12 = 84 octets  */
+		"45000054daee40004001f7890a172a010a172a02080060d23f590071e3f8"
+		"4156000000007241010000000000101112131415161718191a1b1c1d1e1f"
+		"202122232425262728292a2b2c2d2e2f3031323334353637"
+		;
+	const char *u_to_sgsn =
+		"32" 	/* 0b001'1 0010: version 1, protocol GTP, with seq nr. */ \
+		"ff"	/* type 255: G-PDU */
+		"0058"	/* length: 88 + 8 octets == 96 */
+		"00000123" /* unmapped User TEI */
+		"6d31"	/* new mapped seq */
+		"0000"
+		"45000054daee40004001f7890a172a010a172a02080060d23f590071e3f8"
+		"4156000000007241010000000000101112131415161718191a1b1c1d1e1f"
+		"202122232425262728292a2b2c2d2e2f3031323334353637"
+		;
+
+	/* This depends on create_pdp_ctx() sending resolved_sgsn_addr as GSN
+	 * Address IEs in the GGSN's Create PDP Ctx Response. */
+	OSMO_ASSERT(msg_from_ggsn_u(&ggsn_sender,
+				    &resolved_sgsn_addr,
+				    u_from_ggsn,
+				    u_to_sgsn));
+
+	const char *u_from_sgsn =
+		"32" 	/* 0b001'1 0010: version 1, protocol GTP, with seq nr. */ \
+		"ff"	/* type 255: G-PDU */
+		"0058"	/* length: 88 + 8 octets == 96 */
+		"00000002" /* mapped User TEI for GGSN from create_pdp_ctx() */
+		"6d31"	/* mapped seq */
+		"0000"	/* No extensions */
+		/* User data (ICMP packet), 96 - 12 = 84 octets  */
+		"45000054daee40004001f7890a172a010a172a02080060d23f590071e3f8"
+		"4156000000007241010000000000101112131415161718191a1b1c1d1e1f"
+		"202122232425262728292a2b2c2d2e2f3031323334353637"
+		;
+	const char *u_to_ggsn =
+		"32" 	/* 0b001'1 0010: version 1, protocol GTP, with seq nr. */ \
+		"ff"	/* type 255: G-PDU */
+		"0058"	/* length: 88 + 8 octets == 96 */
+		"00000567" /* unmapped User TEI */
+		"0070"	/* unmapped seq */
+		"0000"
+		"45000054daee40004001f7890a172a010a172a02080060d23f590071e3f8"
+		"4156000000007241010000000000101112131415161718191a1b1c1d1e1f"
+		"202122232425262728292a2b2c2d2e2f3031323334353637"
+		;
+
+	OSMO_ASSERT(msg_from_sgsn_u(&resolved_sgsn_addr,
+				    &ggsn_sender,
+				    u_from_sgsn,
+				    u_to_ggsn));
+
+	gtphub_gc(hub, now + EXPIRE_ALL);
+}
+
+
 static struct log_info_cat gtphub_categories[] = {
 	[DGTPHUB] = {
 		.name = "DGTPHUB",
@@ -829,6 +932,7 @@
 	test_expiry();
 	test_echo();
 	test_create_pdp_ctx();
+	test_user_data();
 	printf("Done\n");
 
 	talloc_report_full(osmo_gtphub_ctx, stderr);
diff --git a/openbsc/tests/gtphub/gtphub_test.ok b/openbsc/tests/gtphub/gtphub_test.ok
index eac6f0c..24b9aad 100644
--- a/openbsc/tests/gtphub/gtphub_test.ok
+++ b/openbsc/tests/gtphub/gtphub_test.ok
@@ -1,2 +1,9 @@
-Wrap: returning GGSN addr from imsi 240010123456789 ni internet: 192.168.43.34 port 434
+test_echo
+test_create_pdp_ctx
+- __wrap_gtphub_resolve_ggsn_addr():
+  returning GGSN addr from imsi 240010123456789 ni internet: 192.168.43.34 port 2123
+test_user_data
+- __wrap_gtphub_resolve_ggsn_addr():
+  returning GGSN addr from imsi 240010123456789 ni internet: 192.168.43.34 port 2123
+- user data starts
 Done