diff mbox

Change in libosmo-sccp[master]: Add selector for ANSI or ITU variant

Message ID gerrit.1463521025093.Ia17eef8c9b7d8e1092c587f469b4a68aa9702651@gerrit.osmocom.org
State New
Headers show

Commit Message

gerrit-no-reply@lists.osmocom.org May 17, 2016, 9:37 p.m. UTC 
From Arran Cudbard-bell <a.cudbardb@freeradius.org>:

Arran Cudbard-bell has uploaded a new change for review.

  https://gerrit.osmocom.org/73

Change subject: Add selector for ANSI or ITU variant
......................................................................

Add selector for ANSI or ITU variant

Change-Id: Ia17eef8c9b7d8e1092c587f469b4a68aa9702651
---
M include/sccp/sccp.h
M include/sccp/sccp_types.h
M src/sccp.c
3 files changed, 124 insertions(+), 46 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmo-sccp refs/changes/73/73/1

Comments

gerrit-no-reply@lists.osmocom.org May 17, 2016, 9:38 p.m. UTC | #1 
From Arran Cudbard-bell <a.cudbardb@freeradius.org>:

Arran Cudbard-bell has posted comments on this change.

Change subject: Add selector for ANSI or ITU variant
......................................................................


Patch Set 1:

This is for initial review.  Wouldn't expect this to be merged as is (needs tests for a start).

More to get feedback on whether ANSI support is desired.  In the project we're doing currently it is.  I have similar patches for the MTP3 layer in osmobsc, but there needs to be more discussion around that.
gerrit-no-reply@lists.osmocom.org May 17, 2016, 9:53 p.m. UTC | #2 
From Holger Freyther <holger@freyther.de>:

Holger Freyther has posted comments on this change.

Change subject: Add selector for ANSI or ITU variant
......................................................................


Patch Set 1:

> This is for initial review.  Wouldn't expect this to be merged as
 > is (needs tests for a start).
 > 
 > More to get feedback on whether ANSI support is desired.  In the
 > project we're doing currently it is.  I have similar patches for
 > the MTP3 layer in osmobsc, but there needs to be more discussion
 > around that.

Yes, ANSI (and Japan, China?) would be nice to have (for being complete). I don't know much about the differences of ANSI SCCP.

In general:
* It should not break ITU support
* It should not make code more ugly/slower (not saying the patch does)
* ANSI code should have test cases by itself (and preferable make traces available, e.g. to the wireshark Sample Captures).
gerrit-no-reply@lists.osmocom.org May 17, 2016, 9:58 p.m. UTC | #3 
From Arran Cudbard-bell <a.cudbardb@freeradius.org>:

Arran Cudbard-bell has posted comments on this change.

Change subject: Add selector for ANSI or ITU variant
......................................................................


Patch Set 1:

Yes, oops.  Should have run the test suite *first*.  Is the general method of dealing with the variations OK? It seemed cleaner than lots of if/else if statements, and the performance hit should be minimal.
gerrit-no-reply@lists.osmocom.org May 18, 2016, 3:56 a.m. UTC | #4 
Hello Jenkins Builder,

I'd like you to reexamine a change.  Please visit

    https://gerrit.osmocom.org/73

to look at the new patch set (#2).

Change subject: Add selector for ANSI or ITU variant
......................................................................

Add selector for ANSI or ITU variant

Change-Id: Ia17eef8c9b7d8e1092c587f469b4a68aa9702651
---
M include/sccp/sccp.h
M include/sccp/sccp_types.h
M src/sccp.c
M tests/sccp/sccp_test.c
4 files changed, 124 insertions(+), 46 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmo-sccp refs/changes/73/73/2
gerrit-no-reply@lists.osmocom.org May 18, 2016, 4 a.m. UTC | #5 
Hello Jenkins Builder,

I'd like you to reexamine a change.  Please visit

    https://gerrit.osmocom.org/73

to look at the new patch set (#3).

Change subject: Add selector for ANSI or ITU variant
......................................................................

Add selector for ANSI or ITU variant

Change-Id: Ia17eef8c9b7d8e1092c587f469b4a68aa9702651
---
M include/sccp/sccp.h
M include/sccp/sccp_types.h
M src/sccp.c
M tests/sccp/sccp_test.c
4 files changed, 126 insertions(+), 48 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmo-sccp refs/changes/73/73/3
diff mbox

Patch

diff --git a/include/sccp/sccp.h b/include/sccp/sccp.h
index 36b424f..665422e 100644
--- a/include/sccp/sccp.h
+++ b/include/sccp/sccp.h
@@ -46,11 +46,24 @@ 
 	SCCP_CONNECTION_STATE_SETUP_ERROR,
 };
 
+struct sccp_variant {
+	uint8_t ai_nb;			/* National or reserved bit */
+	uint8_t ai_gti;			/* GTI mask */
+	uint8_t ai_pc_ind;		/* Point code indicator mask */
+	uint8_t ai_ssn_ind;		/* SSN indicator mask */
+	uint8_t ai_route;		/* Route type mask */
+	uint8_t pc_len;			/* Point code length */
+	uint8_t pc_first;		/* whether the pointcode comes before the SSN */
+};
+
 struct sockaddr_sccp {
-	sa_family_t	sccp_family;		/* AF_SCCP in the future??? */
+	sa_family_t	sccp_family;	/* AF_SCCP in the future??? */
+
+	int use_ssn;
 	uint8_t	sccp_ssn;		/* subssystem number for routing */
 
-	/* TODO fill in address indicator... if that is ever needed */
+	int nb;				/* National bit (8) */
+	int route_on_ssn;		/* Force routing on SSN instead of GTI */
 
 	/* optional gti information */
 	uint8_t *gti;
@@ -60,7 +73,7 @@ 
 	uint8_t gti_ind;
 
 	int use_poi;
-	uint8_t poi[2];
+	uint8_t poi[3];			/* Allows ITU 14bit and ANSI 24bit */
 
 	/* not sure about these */
 	/* uint8_t    sccp_class; */
@@ -70,9 +83,8 @@ 
  * parsed structure of an address
  */
 struct sccp_address {
-	struct sccp_called_party_address    address;
 	uint8_t			    ssn;
-	uint8_t			    poi[2];
+	uint8_t			    poi[3];	/* Allows ITU 14bit and ANSI 24bit */
 
 	uint8_t			    *gti_data;
 	int			    gti_len;
@@ -103,6 +115,8 @@ 
 	int incoming;
 };
 
+extern struct sccp_variant sccp_variant[];
+
 /**
  * system functionality to implement on top of any other transport layer:
  *   call sccp_system_incoming for incoming data (from the network)
@@ -122,7 +136,7 @@ 
 int sccp_connection_free(struct sccp_connection *connection);
 
 /**
- * internal.. 
+ * internal..
  */
 int sccp_connection_force_free(struct sccp_connection *conn);
 
@@ -157,6 +171,7 @@ 
 int sccp_set_read(const struct sockaddr_sccp *sock,
 		  int (*read_cb)(struct msgb *msgb, unsigned int, void *user_data),
 		  void *user_data);
+void sccp_set_variant(int variant);
 
 /* generic sock addresses */
 extern const struct sockaddr_sccp sccp_ssn_bssap;
diff --git a/include/sccp/sccp_types.h b/include/sccp/sccp_types.h
index 986de0d..cd60c25 100644
--- a/include/sccp/sccp_types.h
+++ b/include/sccp/sccp_types.h
@@ -26,6 +26,12 @@ 
 
 #include <osmocom/core/endian.h>
 
+/* Which variant of SCCP we're using */
+enum {
+	SCCP_VARIANT_ITU,
+	SCCP_VARIANT_ANSI
+};
+
 /* Table 1/Q.713 - SCCP message types */
 enum sccp_message_types {
 	SCCP_MSG_TYPE_CR	= 1,
diff --git a/src/sccp.c b/src/sccp.c
index e6c538d..a3cda90 100644
--- a/src/sccp.c
+++ b/src/sccp.c
@@ -46,16 +46,40 @@ 
 	.sccp_ssn	= SCCP_SSN_BSSAP,
 };
 
+struct sccp_variant sccp_variant[] = {
+	[SCCP_VARIANT_ITU] = {
+		.ai_nb = 0x80,
+		.ai_gti = 0x3c,
+		.ai_pc_ind = 0x01,
+		.ai_ssn_ind = 0x02,
+		.ai_route = 0x40,
+		.pc_len = 2,
+		.pc_first = 1
+	},
+	[SCCP_VARIANT_ANSI] = {
+		.ai_nb = 0x80,
+		.ai_gti = 0x3c,
+		.ai_pc_ind = 0x02,
+		.ai_ssn_ind = 0x01,
+		.ai_route = 0x40,
+		.pc_len = 3,
+		.pc_first = 0,
+	}
+};
+
 struct sccp_system {
 	/* layer3 -> layer2 */
 	void (*write_data)(struct sccp_connection *conn, struct msgb *data,
 			   void *gctx, void *ctx);
 	void *write_context;
+
+	int variant;
 };
 
 
 static struct sccp_system sccp_system = {
 	.write_data = NULL,
+	.variant = SCCP_VARIANT_ITU
 };
 
 struct sccp_data_callback {
@@ -105,11 +129,13 @@ 
  */
 static int copy_address(struct sccp_address *addr, uint8_t offset, struct msgb *msgb)
 {
-	struct sccp_called_party_address *party;
-
 	int room = msgb_l2len(msgb) - offset;
-	uint8_t read = 0;
+
+	uint8_t *data;
+	uint8_t read = 1;
 	uint8_t length;
+	uint8_t ai;
+	uint8_t pc_len = sccp_variant[sccp_system.variant].pc_len;
 
 	if (room <= 0) {
 		LOGP(DSCCP, LOGL_ERROR, "Not enough room for an address: %u\n", room);
@@ -122,36 +148,48 @@ 
 		return -1;
 	}
 
+	data = msgb->l2h + offset + 1;
+	ai = data[0];
+	data++;
 
-	party = (struct sccp_called_party_address *)(msgb->l2h + offset + 1);
-	if (party->point_code_indicator) {
-		if (length <= read + 2) {
-		    LOGP(DSCCP, LOGL_ERROR, "POI does not fit %u\n", length);
-		    return -1;
-		}
+#define PARSE_POI \
+	do { \
+		if (ai & sccp_variant[sccp_system.variant].ai_pc_ind) { \
+			if (length <= read + pc_len) { \
+				LOGP(DSCCP, LOGL_ERROR, "POI does not fit %u\n", length); \
+				return -1; \
+			} \
+			memcpy(&addr->poi, &data[read], pc_len); \
+			read += pc_len; \
+		} \
+	} while (0)
 
+#define PARSE_SSN \
+	do { \
+		if (ai & sccp_variant[sccp_system.variant].ai_ssn_ind) { \
+			if (length <= read + 1) { \
+				LOGP(DSCCP, LOGL_ERROR, "SSN does not fit %u\n", length); \
+				return -1; \
+			} \
+			addr->ssn = data[read]; \
+			read += 1; \
+		} \
+	} while (0)
 
-		memcpy(&addr->poi, &party->data[read], 2);
-		read += 2;
-	}
-
-	if (party->ssn_indicator) {
-		if (length <= read + 1) {
-		    LOGP(DSCCP, LOGL_ERROR, "SSN does not fit %u\n", length);
-		    return -1;
-		}
-
-		addr->ssn = party->data[read];
-		read += 1;
+	if (sccp_variant[sccp_system.variant].pc_first) {
+		PARSE_POI;
+		PARSE_SSN;
+	} else {
+		PARSE_SSN;
+		PARSE_POI;
 	}
 
 	/* copy the GTI over */
-	if (party->global_title_indicator) {
-		addr->gti_len = length - read - 1;
-		addr->gti_data = &party->data[read];
+	if (ai & sccp_variant[sccp_system.variant].ai_gti) {
+		addr->gti_len = length - read;
+		addr->gti_data = &data[read];
 	}
 
-	addr->address = *party;
 	return 0;
 }
 
@@ -173,7 +211,6 @@ 
 
 		uint8_t length = msgb->l2h[offset + read + 1];
 		read += 2 + length;
-
 
 		if (room <= read) {
 			LOGP(DSCCP, LOGL_ERROR,
@@ -486,28 +523,43 @@ 
 int sccp_create_sccp_addr(struct msgb *msg, const struct sockaddr_sccp *sock)
 {
 	uint8_t *len, *ai, *gti;
+	uint8_t *poi;
+	uint8_t pc_len = sccp_variant[sccp_system.variant].pc_len;
 
 	len = msgb_put(msg, 1);
 	ai = msgb_put(msg, 1);
 
+	if (sock->gti) ai[0] = (sock->gti_ind & 0x0f) << 2;
+	if (sock->route_on_ssn || !sock->gti) ai[0] |= sccp_variant[sccp_system.variant].ai_route;
 
-	if (sock->gti)
-		ai[0] = 0 << 6 | (sock->gti_ind & 0x0f) << 2 | 1 << 1;
-	else
-		ai[0] = 1 << 6 | 1 << 1;
+	/* National/reserved bit */
+	if (sock->nb) ai[0] |= sccp_variant[sccp_system.variant].ai_nb;
 
-	/* store a point code */
-	if (sock->use_poi) {
-		uint8_t *poi;
+	/* Pointcode ind */
+	if (sock->use_poi) ai[0] |= sccp_variant[sccp_system.variant].ai_pc_ind;
 
-		ai[0] |= 0x01;
-		poi = msgb_put(msg, 2);
-		poi[0] = sock->poi[0];
-		poi[1] = sock->poi[1];
+	/* SSN ind */
+	ai[0] |= sccp_variant[sccp_system.variant].ai_ssn_ind;
+
+#define ADD_POI \
+	do { \
+		if (sock->use_poi) { \
+			poi = msgb_put(msg, pc_len); \
+			if (!poi) return -1; \
+			memcpy(poi, &sock->poi[0], pc_len); \
+		} \
+	} while (0)
+
+#define ADD_SSN \
+	msgb_v_put(msg, sock->sccp_ssn)
+
+	if (sccp_variant[sccp_system.variant].pc_first) {
+		ADD_POI;
+		ADD_SSN;
+	} else {
+		ADD_SSN;
+		ADD_POI;
 	}
-
-	/* copy the SSN */
-	msgb_v_put(msg, sock->sccp_ssn);
 
 	/* copy the gti if it is present */
 	gti = msgb_put(msg, sock->gti_len);
@@ -1225,6 +1277,11 @@ 
 	return 0;
 }
 
+void sccp_set_variant(int variant)
+{
+	sccp_system.variant = variant;
+}
+
 /* oh my god a real SCCP packet. need to dispatch it now */
 int sccp_system_incoming(struct msgb *msgb)
 {