nat: Return the SCCP connection, change order of patching and updating

* Return the SCCP connection. This will be needed to store the
  assigned timeslot in there.
* Update code to work with this change
* This uncovered a bug in the CC handling, at the time the BSC was
  passed it was still a null pointer and the code would have failed.

4 files changed, 44 insertions, 50 deletions

diff --git a/openbsc/include/openbsc/bsc_nat.h b/openbsc/include/openbsc/bsc_nat.h
index a6e4aac3f..f3548426a 100644
--- a/openbsc/include/openbsc/bsc_nat.h
+++ b/openbsc/include/openbsc/bsc_nat.h
@@ -156,9 +156,9 @@ struct bsc_connection *bsc_nat_find_bsc(struct bsc_nat *nat, struct msgb *msg);
  * SCCP patching and handling
  */
 int create_sccp_src_ref(struct bsc_connection *bsc, struct msgb *msg, struct bsc_nat_parsed *parsed);
-int update_sccp_src_ref(struct bsc_connection *bsc, struct msgb *msg, struct bsc_nat_parsed *parsed);
+int update_sccp_src_ref(struct sccp_connections *sccp, struct bsc_nat_parsed *parsed);
 void remove_sccp_src_ref(struct bsc_connection *bsc, struct msgb *msg, struct bsc_nat_parsed *parsed);
-struct bsc_connection *patch_sccp_src_ref_to_bsc(struct msgb *, struct bsc_nat_parsed *, struct bsc_nat *);
-struct bsc_connection *patch_sccp_src_ref_to_msc(struct msgb *, struct bsc_nat_parsed *, struct bsc_nat *);
+struct sccp_connections *patch_sccp_src_ref_to_bsc(struct msgb *, struct bsc_nat_parsed *, struct bsc_nat *);
+struct sccp_connections *patch_sccp_src_ref_to_msc(struct msgb *, struct bsc_nat_parsed *, struct bsc_nat *);
 
 #endif
diff --git a/openbsc/src/nat/bsc_nat.c b/openbsc/src/nat/bsc_nat.c
index 9bd301f78..338b15c40 100644
--- a/openbsc/src/nat/bsc_nat.c
+++ b/openbsc/src/nat/bsc_nat.c
@@ -168,7 +168,8 @@ static void bsc_write(struct bsc_connection *bsc, const u_int8_t *data, unsigned
 
 static int forward_sccp_to_bts(struct msgb *msg)
 {
-	struct bsc_connection *bsc = NULL;
+	struct sccp_connections *con;
+	struct bsc_connection *bsc;
 	struct bsc_nat_parsed *parsed;
 
 	/* filter, drop, patch the message? */
@@ -191,12 +192,12 @@ static int forward_sccp_to_bts(struct msgb *msg)
 		case SCCP_MSG_TYPE_RLSD:
 		case SCCP_MSG_TYPE_CREF:
 		case SCCP_MSG_TYPE_DT1:
-			bsc = patch_sccp_src_ref_to_bsc(msg, parsed, nat);
+			con = patch_sccp_src_ref_to_bsc(msg, parsed, nat);
 			break;
 		case SCCP_MSG_TYPE_CC:
-			if (update_sccp_src_ref(bsc, msg, parsed) != 0)
+			con = patch_sccp_src_ref_to_bsc(msg, parsed, nat);
+			if (!con || update_sccp_src_ref(con, parsed) != 0)
 				goto exit;
-			bsc = patch_sccp_src_ref_to_bsc(msg, parsed, nat);
 			break;
 		case SCCP_MSG_TYPE_RLC:
 			LOGP(DNAT, LOGL_ERROR, "Unexpected release complete from MSC.\n");
@@ -210,14 +211,14 @@ static int forward_sccp_to_bts(struct msgb *msg)
 	}
 
 	talloc_free(parsed);
-	if (!bsc)
+	if (!con)
 		return -1;
-	if (!bsc->authenticated) {
+	if (!con->bsc->authenticated) {
 		LOGP(DNAT, LOGL_ERROR, "Selected BSC not authenticated.\n");
 		return -1;
 	}
 
-	bsc_write(bsc, msg->data, msg->len);
+	bsc_write(con->bsc, msg->data, msg->len);
 	return 0;
 
 send_to_all:
@@ -365,7 +366,7 @@ static void ipaccess_auth_bsc(struct tlv_parsed *tvp, struct bsc_connection *bsc
 
 static int forward_sccp_to_msc(struct bsc_connection *bsc, struct msgb *msg)
 {
-	struct bsc_connection *found_bsc = NULL;
+	struct sccp_connections *con;
 	struct bsc_nat_parsed *parsed;
 
 	if (!bsc->authenticated) {
@@ -391,29 +392,34 @@ static int forward_sccp_to_msc(struct bsc_connection *bsc, struct msgb *msg)
 		case SCCP_MSG_TYPE_CR:
 			if (create_sccp_src_ref(bsc, msg, parsed) != 0)
 				goto exit2;
-			found_bsc = patch_sccp_src_ref_to_msc(msg, parsed, nat);
+			con = patch_sccp_src_ref_to_msc(msg, parsed, nat);
 			break;
 		case SCCP_MSG_TYPE_RLSD:
 		case SCCP_MSG_TYPE_CREF:
 		case SCCP_MSG_TYPE_DT1:
 		case SCCP_MSG_TYPE_CC:
-			found_bsc = patch_sccp_src_ref_to_msc(msg, parsed, nat);
+			con = patch_sccp_src_ref_to_msc(msg, parsed, nat);
 			break;
 		case SCCP_MSG_TYPE_RLC:
-			found_bsc = patch_sccp_src_ref_to_msc(msg, parsed, nat);
+			con = patch_sccp_src_ref_to_msc(msg, parsed, nat);
 			remove_sccp_src_ref(bsc, msg, parsed);
 			break;
 		case SCCP_MSG_TYPE_UDT:
 			/* simply forward everything */
+			con = NULL;
 			break;
 		default:
 			LOGP(DNAT, LOGL_ERROR, "Not forwarding to msc sccp type: 0x%x\n", parsed->sccp_type);
+			con = NULL;
 			goto exit2;
 			break;
 		}
+	} else {
+		LOGP(DNAT, LOGL_ERROR, "Not forwarding unknown stream id: 0x%x\n", parsed->ipa_proto);
+		goto exit2;
 	}
 
-	if (found_bsc != bsc) {
+	if (con && con->bsc != bsc) {
 		LOGP(DNAT, LOGL_ERROR, "Found the wrong entry.\n");
 		goto exit2;
 	}
diff --git a/openbsc/src/nat/bsc_sccp.c b/openbsc/src/nat/bsc_sccp.c
index fe85a5dd6..2a9d8411f 100644
--- a/openbsc/src/nat/bsc_sccp.c
+++ b/openbsc/src/nat/bsc_sccp.c
@@ -108,32 +108,19 @@ int create_sccp_src_ref(struct bsc_connection *bsc, struct msgb *msg, struct bsc
 	return 0;
 }
 
-int update_sccp_src_ref(struct bsc_connection *bsc, struct msgb *msg, struct bsc_nat_parsed *parsed)
+int update_sccp_src_ref(struct sccp_connections *sccp, struct bsc_nat_parsed *parsed)
 {
-	struct sccp_connections *conn;
-
 	if (!parsed->dest_local_ref || !parsed->src_local_ref) {
 		LOGP(DNAT, LOGL_ERROR, "CC MSG should contain both local and dest address.\n");
 		return -1;
 	}
 
-	llist_for_each_entry(conn, &bsc->nat->sccp_connections, list_entry) {
-		if (conn->bsc != bsc)
-			continue;
-
-		if (memcmp(parsed->dest_local_ref,
-			   &conn->patched_ref, sizeof(conn->patched_ref)) != 0)
-			continue;
-
-		conn->remote_ref = *parsed->src_local_ref;
-		LOGP(DNAT, LOGL_DEBUG, "Updating 0x%x to remote 0x%x on %p\n",
-		     sccp_src_ref_to_int(&conn->patched_ref),
-		     sccp_src_ref_to_int(&conn->remote_ref), bsc);
-		return 0;
-	}
+	sccp->remote_ref = *parsed->src_local_ref;
+	LOGP(DNAT, LOGL_DEBUG, "Updating 0x%x to remote 0x%x on %p\n",
+	     sccp_src_ref_to_int(&sccp->patched_ref),
+	     sccp_src_ref_to_int(&sccp->remote_ref), sccp->bsc);
 
-	LOGP(DNAT, LOGL_ERROR, "Referenced connection not found on BSC: %p\n", bsc);
-	return -1;
+	return 0;
 }
 
 void remove_sccp_src_ref(struct bsc_connection *bsc, struct msgb *msg, struct bsc_nat_parsed *parsed)
@@ -165,9 +152,9 @@ void remove_sccp_src_ref(struct bsc_connection *bsc, struct msgb *msg, struct bs
  * an address that was assigned by the MUX, we need to update the
  * dest reference to the real network.
  */
-struct bsc_connection *patch_sccp_src_ref_to_bsc(struct msgb *msg,
-						 struct bsc_nat_parsed *parsed,
-						 struct bsc_nat *nat)
+struct sccp_connections *patch_sccp_src_ref_to_bsc(struct msgb *msg,
+						   struct bsc_nat_parsed *parsed,
+						   struct bsc_nat *nat)
 {
 	struct sccp_connections *conn;
 
@@ -183,7 +170,7 @@ struct bsc_connection *patch_sccp_src_ref_to_bsc(struct msgb *msg,
 
 		/* Change the dest address to the real one */
 		*parsed->dest_local_ref = conn->real_ref;
-		return conn->bsc;
+		return conn;
 	}
 
 	return NULL;
@@ -197,9 +184,9 @@ struct bsc_connection *patch_sccp_src_ref_to_bsc(struct msgb *msg,
  * in all other cases we need to work by the destination local
  * reference..
  */
-struct bsc_connection *patch_sccp_src_ref_to_msc(struct msgb *msg,
-						 struct bsc_nat_parsed *parsed,
-						 struct bsc_nat *nat)
+struct sccp_connections *patch_sccp_src_ref_to_msc(struct msgb *msg,
+						   struct bsc_nat_parsed *parsed,
+						   struct bsc_nat *nat)
 {
 	struct sccp_connections *conn;
 
@@ -207,11 +194,11 @@ struct bsc_connection *patch_sccp_src_ref_to_msc(struct msgb *msg,
 		if (parsed->src_local_ref) {
 			if (equal(parsed->src_local_ref, &conn->real_ref)) {
 				*parsed->src_local_ref = conn->patched_ref;
-				return conn->bsc;
+				return conn;
 			}
 		} else if (parsed->dest_local_ref) {
 			if (equal(parsed->dest_local_ref, &conn->remote_ref))
-				return conn->bsc;
+				return conn;
 		} else {
 			LOGP(DNAT, LOGL_ERROR, "Header has neither loc/dst ref.\n");
 			return NULL;
diff --git a/openbsc/tests/bsc-nat/bsc_nat_test.c b/openbsc/tests/bsc-nat/bsc_nat_test.c
index 196f187d8..3a1096001 100644
--- a/openbsc/tests/bsc-nat/bsc_nat_test.c
+++ b/openbsc/tests/bsc-nat/bsc_nat_test.c
@@ -189,7 +189,7 @@ static void copy_to_msg(struct msgb *msg, const u_int8_t *data, unsigned int len
 }
 
 #define VERIFY(con_found, con, msg, ver, str) \
-	if (con_found != con) { \
+	if (!con_found || con_found->bsc != con) { \
 		fprintf(stderr, "Failed to find the con: %p\n", con_found); \
 		abort(); \
 	} \
@@ -203,7 +203,8 @@ static void test_contrack()
 {
 	int rc;
 	struct bsc_nat *nat;
-	struct bsc_connection *con, *con_found;
+	struct bsc_connection *con;
+	struct sccp_connections *con_found;
 	struct bsc_nat_parsed *parsed;
 	struct msgb *msg;
 
@@ -226,7 +227,7 @@ static void test_contrack()
 		abort();
 	}
 	con_found = patch_sccp_src_ref_to_msc(msg, parsed, nat);
-	if (con_found != con) {
+	if (!con_found || con_found->bsc != con) {
 		fprintf(stderr, "Failed to find the con: %p\n", con_found);
 		abort();
 	}
@@ -239,12 +240,12 @@ static void test_contrack()
 	/* 2.) get the cc */
 	copy_to_msg(msg, msc_cc, sizeof(msc_cc));
 	parsed = bsc_nat_parse(msg);
-	if (update_sccp_src_ref(con, msg, parsed) != 0) {
+	con_found = patch_sccp_src_ref_to_bsc(msg, parsed, nat);
+	VERIFY(con_found, con, msg, msc_cc_patched, "MSC CC");
+	if (update_sccp_src_ref(con_found, parsed) != 0) {
 		fprintf(stderr, "Failed to update the SCCP con.\n");
 		abort();
 	}
-	con_found = patch_sccp_src_ref_to_bsc(msg, parsed, nat);
-	VERIFY(con_found, con, msg, msc_cc_patched, "MSC CC");
 
 	/* 3.) send some data */
 	copy_to_msg(msg, bsc_dtap, sizeof(bsc_dtap));
@@ -268,7 +269,7 @@ static void test_contrack()
 	copy_to_msg(msg, bsc_rlc, sizeof(bsc_rlc));
 	parsed = bsc_nat_parse(msg);
 	con_found = patch_sccp_src_ref_to_msc(msg, parsed, nat);
-	if (con_found != con) {
+	if (!con_found || con_found->bsc != con) {
 		fprintf(stderr, "Failed to find the con: %p\n", con_found);
 		abort();
 	}