aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorPhilipp <pmaier@sysmocom.de>2016-12-16 16:03:28 +0100
committerPhilipp <pmaier@sysmocom.de>2016-12-16 16:03:28 +0100
commitb0a32e58d11f7dd79272a178eef838980b980d41 (patch)
tree119f60e1b023df9a5b9b410942b0404aab7fc6dc
parent74cf9351db7997996b9b0b0886796c3aead712cd (diff)
sndcp: Allow empty SNDCP-XID indications
In some rare cases the modem might send a xid indication that does not contain anything except the version number field. The sgsn ignors such SNDCP-XID indications by stripping the entire field from the response. We found a modem in the wild that started to act problematic when the empty SNDCP-XID was missing in the response. This patch changes the XID negotiation behaviour in a way that if a modem should send empty SNDCP-XID indications, the reply will also contain an empty SNDCP-XID indication. Apart from that the SNDCP-XID version number is now parsed and echoed in the response. This ensures that we always reply with the version number that the modem expects. (The version was 0 in all cases we observed so far)
-rw-r--r--openbsc/include/openbsc/gprs_sndcp_xid.h14
-rw-r--r--openbsc/src/gprs/gprs_llc.c14
-rw-r--r--openbsc/src/gprs/gprs_sndcp.c25
-rw-r--r--openbsc/src/gprs/gprs_sndcp_xid.c54
-rw-r--r--openbsc/tests/sndcp_xid/sndcp_xid_test.c10
5 files changed, 68 insertions, 49 deletions
diff --git a/openbsc/include/openbsc/gprs_sndcp_xid.h b/openbsc/include/openbsc/gprs_sndcp_xid.h
index 02904a7d2..e64bc5237 100644
--- a/openbsc/include/openbsc/gprs_sndcp_xid.h
+++ b/openbsc/include/openbsc/gprs_sndcp_xid.h
@@ -24,7 +24,7 @@
#include <stdint.h>
#include <osmocom/core/linuxlist.h>
-#define CURRENT_SNDCP_VERSION 0 /* See 3GPP TS 44.065, clause 8 */
+#define DEFAULT_SNDCP_VERSION 0 /* See 3GPP TS 44.065, clause 8 */
#define MAX_ENTITIES 32 /* 3GPP TS 44.065 reserves 5 bit
* for compression enitity number */
@@ -197,13 +197,15 @@ enum gprs_sndcp_dcomp_v44_dcomp {
/* Transform a list with compression fields into an SNDCP-XID message (dst) */
int gprs_sndcp_compile_xid(uint8_t *dst, unsigned int dst_maxlen,
- const struct llist_head *comp_fields);
+ const struct llist_head *comp_fields, int version);
/* Transform an SNDCP-XID message (src) into a list of SNDCP-XID fields */
-struct llist_head *gprs_sndcp_parse_xid(const void *ctx,
- const uint8_t * src,
- unsigned int src_len,
- const struct llist_head *comp_fields_req);
+struct llist_head *gprs_sndcp_parse_xid(int *version,
+ const void *ctx,
+ const uint8_t *src,
+ unsigned int src_len,
+ const struct llist_head
+ *comp_fields_req);
/* Find out to which compression class the specified comp-field belongs
* (header compression or data compression?) */
diff --git a/openbsc/src/gprs/gprs_llc.c b/openbsc/src/gprs/gprs_llc.c
index 021502932..eb4abfd76 100644
--- a/openbsc/src/gprs/gprs_llc.c
+++ b/openbsc/src/gprs/gprs_llc.c
@@ -136,7 +136,7 @@ static int gprs_llc_process_xid_conf(uint8_t *bytes, int bytes_len,
struct gprs_llc_lle *lle)
{
/* Note: This function handles the response of a network originated
- * XID-Request. There XID messages reflected by the phone are analyzed
+ * XID-Request. There XID messages reflected by the modem are analyzed
* and processed here. The caller is called by rx_llc_xid(). */
struct llist_head *xid_fields;
@@ -202,7 +202,7 @@ static int gprs_llc_process_xid_ind(uint8_t *bytes_request,
struct gprs_llc_lle *lle)
{
/* Note: This function computes the response that is sent back to the
- * phone when a phone originated XID is received. The function is
+ * modem when a modem originated XID is received. The function is
* called by rx_llc_xid() */
int rc = -EINVAL;
@@ -229,7 +229,7 @@ static int gprs_llc_process_xid_ind(uint8_t *bytes_request,
* for validity. Currently we just blindly
* accept all XID fields by just echoing them.
* There is a remaining risk of malfunction
- * when a phone submits values which defer from
+ * when a modem submits values which defer from
* the default! */
LOGP(DLLC, LOGL_NOTICE,
"Echoing XID-Field: XID: type=%d, data_len=%d, data=%s\n",
@@ -271,7 +271,7 @@ static int gprs_llc_process_xid_ind(uint8_t *bytes_request,
return rc;
}
-/* Dispatch XID indications and responses comming from the Phone */
+/* Dispatch XID indications and responses comming from the modem */
static void rx_llc_xid(struct gprs_llc_lle *lle,
struct gprs_llc_hdr_parsed *gph)
{
@@ -281,7 +281,7 @@ static void rx_llc_xid(struct gprs_llc_lle *lle,
/* FIXME: 8.5.3.3: check if XID is invalid */
if (gph->is_cmd) {
LOGP(DLLC, LOGL_NOTICE,
- "Received XID indication from phone.\n");
+ "Received XID indication from modem.\n");
struct msgb *resp;
uint8_t *xid;
@@ -301,7 +301,7 @@ static void rx_llc_xid(struct gprs_llc_lle *lle,
gprs_llc_tx_xid(lle, resp, 0);
} else {
LOGP(DLLC, LOGL_NOTICE,
- "Received XID confirmation from phone.\n");
+ "Received XID confirmation from modem.\n");
gprs_llc_process_xid_conf(gph->data, gph->data_len, lle);
/* FIXME: if we had sent a XID reset, send
* LLGMM-RESET.conf to GMM */
@@ -331,7 +331,7 @@ int gprs_ll_xid_req(struct gprs_llc_lle *lle,
msg = msgb_alloc_headroom(4096, 1024, "LLC_XID");
xid = msgb_put(msg, xid_bytes_len);
memcpy(xid, xid_bytes, xid_bytes_len);
- LOGP(DLLC, LOGL_NOTICE, "Sending XID request to phone...\n");
+ LOGP(DLLC, LOGL_NOTICE, "Sending XID request to modem...\n");
gprs_llc_tx_xid(lle, msg, 1);
} else {
LOGP(DLLC, LOGL_ERROR,
diff --git a/openbsc/src/gprs/gprs_sndcp.c b/openbsc/src/gprs/gprs_sndcp.c
index 1cbeede09..77c9507ad 100644
--- a/openbsc/src/gprs/gprs_sndcp.c
+++ b/openbsc/src/gprs/gprs_sndcp.c
@@ -960,8 +960,13 @@ static int gprs_llc_gen_sndcp_xid(uint8_t *bytes, int bytes_len, uint8_t nsapi)
llist_add(&v42bis_comp_field.list, &comp_fields);
}
+ /* Do not attempt to compile anything if there is no data in the list */
+ if(llist_empty(&comp_fields))
+ return 0;
+
/* Compile bytestream */
- return gprs_sndcp_compile_xid(bytes, bytes_len, &comp_fields);
+ return gprs_sndcp_compile_xid(bytes, bytes_len, &comp_fields,
+ DEFAULT_SNDCP_VERSION);
}
/* Set of SNDCP-XID bnegotiation (See also: TS 144 065,
@@ -1106,6 +1111,7 @@ int sndcp_sn_xid_ind(struct gprs_llc_xid_field *xid_field_indication,
int rc;
int compclass;
+ int version;
struct llist_head *comp_fields;
struct gprs_sndcp_comp_field *comp_field;
@@ -1115,22 +1121,13 @@ int sndcp_sn_xid_ind(struct gprs_llc_xid_field *xid_field_indication,
OSMO_ASSERT(lle);
/* Parse SNDCP-CID XID-Field */
- comp_fields = gprs_sndcp_parse_xid(lle->llme,
+ comp_fields = gprs_sndcp_parse_xid(&version, lle->llme,
xid_field_indication->data,
xid_field_indication->data_len,
NULL);
if (!comp_fields)
return -EINVAL;
- /* Don't bother with empty indications */
- if (llist_empty(comp_fields)) {
- xid_field_response->data = NULL;
- xid_field_response->data_len = 0;
- DEBUGP(DSNDCP,
- "SNDCP-XID indication did not contain any parameters!\n");
- return 0;
- }
-
/* Handle compression entites */
DEBUGP(DSNDCP, "SNDCP-XID-IND (ms):\n");
gprs_sndcp_dump_comp_fields(comp_fields, LOGL_DEBUG);
@@ -1168,7 +1165,7 @@ int sndcp_sn_xid_ind(struct gprs_llc_xid_field *xid_field_indication,
/* Compile modified SNDCP-XID bytes */
rc = gprs_sndcp_compile_xid(xid_field_response->data,
xid_field_indication->data_len,
- comp_fields);
+ comp_fields, 0);
if (rc > 0)
xid_field_response->data_len = rc;
@@ -1210,7 +1207,7 @@ int sndcp_sn_xid_conf(struct gprs_llc_xid_field *xid_field_conf,
OSMO_ASSERT(xid_field_request);
/* Parse SNDCP-CID XID-Field */
- comp_fields_req = gprs_sndcp_parse_xid(lle->llme,
+ comp_fields_req = gprs_sndcp_parse_xid(NULL, lle->llme,
xid_field_request->data,
xid_field_request->data_len,
NULL);
@@ -1221,7 +1218,7 @@ int sndcp_sn_xid_conf(struct gprs_llc_xid_field *xid_field_conf,
gprs_sndcp_dump_comp_fields(comp_fields_req, LOGL_DEBUG);
/* Parse SNDCP-CID XID-Field */
- comp_fields_conf = gprs_sndcp_parse_xid(lle->llme,
+ comp_fields_conf = gprs_sndcp_parse_xid(NULL, lle->llme,
xid_field_conf->data,
xid_field_conf->data_len,
comp_fields_req);
diff --git a/openbsc/src/gprs/gprs_sndcp_xid.c b/openbsc/src/gprs/gprs_sndcp_xid.c
index bb43eab68..37551e462 100644
--- a/openbsc/src/gprs/gprs_sndcp_xid.c
+++ b/openbsc/src/gprs/gprs_sndcp_xid.c
@@ -549,26 +549,29 @@ static int gprs_sndcp_pack_fields(const struct llist_head *comp_fields,
/* Transform a list with compression fields into an SNDCP-XID message (dst) */
int gprs_sndcp_compile_xid(uint8_t *dst, unsigned int dst_maxlen,
- const struct llist_head *comp_fields)
+ const struct llist_head *comp_fields, int version)
{
int rc;
int byte_counter = 0;
uint8_t comp_bytes[512];
- uint8_t xid_version_number[1] = { CURRENT_SNDCP_VERSION };
+ uint8_t xid_version_number[1];
OSMO_ASSERT(comp_fields);
OSMO_ASSERT(dst);
OSMO_ASSERT(dst_maxlen >= 2 + sizeof(xid_version_number));
- /* Bail if there is no input */
- if (llist_empty(comp_fields))
- return -EINVAL;
+ /* Prepend header with version number */
+ if (version >= 0) {
+ xid_version_number[0] = (uint8_t) (version & 0xff);
+ dst =
+ tlv_put(dst, SNDCP_XID_VERSION_NUMBER,
+ sizeof(xid_version_number), xid_version_number);
+ byte_counter += (sizeof(xid_version_number) + 2);
+ }
- /* Prepend header */
- dst =
- tlv_put(dst, SNDCP_XID_VERSION_NUMBER,
- sizeof(xid_version_number), xid_version_number);
- byte_counter += (sizeof(xid_version_number) + 2);
+ /* Stop if there is no compression fields supplied */
+ if (llist_empty(comp_fields))
+ return byte_counter;
/* Add data compression fields */
rc = gprs_sndcp_pack_fields(comp_fields, comp_bytes,
@@ -1283,11 +1286,10 @@ static int decode_xid_block(struct llist_head *comp_fields, uint8_t tag,
}
/* Transform an SNDCP-XID message (src) into a list of SNDCP-XID fields */
-static int gprs_sndcp_decode_xid(struct llist_head *comp_fields,
+static int gprs_sndcp_decode_xid(int *version, struct llist_head *comp_fields,
const uint8_t *src, unsigned int src_len,
- const struct
- entity_algo_table
- *lt, unsigned int lt_len)
+ const struct entity_algo_table *lt,
+ unsigned int lt_len)
{
int src_pos = 0;
uint8_t tag;
@@ -1297,6 +1299,10 @@ static int gprs_sndcp_decode_xid(struct llist_head *comp_fields,
int rc;
int tlv_count = 0;
+ /* Preset version value as invalid */
+ if(version)
+ *version = -1;
+
/* Valid TLV-Tag and types */
static const struct tlv_definition sndcp_xid_def = {
.def = {
@@ -1327,6 +1333,10 @@ static int gprs_sndcp_decode_xid(struct llist_head *comp_fields,
return -EINVAL;
}
+ /* Decode sndcp xid version number */
+ if (version && tag == SNDCP_XID_VERSION_NUMBER)
+ *version = val[0];
+
/* Decode compression parameters */
if ((tag == SNDCP_XID_PROTOCOL_COMPRESSION)
|| (tag == SNDCP_XID_DATA_COMPRESSION)) {
@@ -1548,7 +1558,8 @@ static int gprs_sndcp_complete_comp_fields(struct llist_head
}
/* Transform an SNDCP-XID message (src) into a list of SNDCP-XID fields */
-struct llist_head *gprs_sndcp_parse_xid(const void *ctx,
+struct llist_head *gprs_sndcp_parse_xid(int *version,
+ const void *ctx,
const uint8_t *src,
unsigned int src_len,
const struct llist_head
@@ -1559,6 +1570,12 @@ struct llist_head *gprs_sndcp_parse_xid(const void *ctx,
struct llist_head *comp_fields;
struct entity_algo_table lt[MAX_ENTITIES * 2];
+ /* In case of a zero length field, just exit */
+ if(src_len == 0)
+ return NULL;
+
+ /* We should go any further if we have a field length greater
+ * zero and a null pointer as buffer! */
OSMO_ASSERT(src);
comp_fields = talloc_zero(ctx, struct llist_head);
@@ -1575,8 +1592,8 @@ struct llist_head *gprs_sndcp_parse_xid(const void *ctx,
}
/* Parse SNDCP-CID XID-Field */
- rc = gprs_sndcp_decode_xid(comp_fields, src, src_len, lt,
- lt_len);
+ rc = gprs_sndcp_decode_xid(version, comp_fields, src, src_len,
+ lt, lt_len);
if (rc < 0) {
talloc_free(comp_fields);
return NULL;
@@ -1591,7 +1608,8 @@ struct llist_head *gprs_sndcp_parse_xid(const void *ctx,
} else {
/* Parse SNDCP-CID XID-Field */
- rc = gprs_sndcp_decode_xid(comp_fields, src, src_len, NULL, 0);
+ rc = gprs_sndcp_decode_xid(version, comp_fields, src, src_len,
+ NULL, 0);
if (rc < 0) {
talloc_free(comp_fields);
return NULL;
diff --git a/openbsc/tests/sndcp_xid/sndcp_xid_test.c b/openbsc/tests/sndcp_xid/sndcp_xid_test.c
index 3a3361970..151dd2bb5 100644
--- a/openbsc/tests/sndcp_xid/sndcp_xid_test.c
+++ b/openbsc/tests/sndcp_xid/sndcp_xid_test.c
@@ -47,13 +47,14 @@ static void test_xid_decode_realworld(const void *ctx)
uint8_t xid_r[512];
/* Parse and show contained comp fields */
- comp_fields = gprs_sndcp_parse_xid(ctx, xid, sizeof(xid), NULL);
+ comp_fields = gprs_sndcp_parse_xid(NULL, ctx, xid, sizeof(xid), NULL);
OSMO_ASSERT(comp_fields);
printf("Decoded:\n");
gprs_sndcp_dump_comp_fields(comp_fields, DSNDCP);
/* Encode comp-fields again */
- rc = gprs_sndcp_compile_xid(xid_r,sizeof(xid_r), comp_fields);
+ rc = gprs_sndcp_compile_xid(xid_r,sizeof(xid_r), comp_fields,
+ DEFAULT_SNDCP_VERSION);
printf("Result length=%i\n",rc);
printf("Encoded: %s\n", osmo_hexdump_nospc(xid, sizeof(xid)));
printf("Rencoded: %s\n", osmo_hexdump_nospc(xid_r, rc));
@@ -226,13 +227,14 @@ static void test_xid_encode_decode(const void *ctx)
gprs_sndcp_dump_comp_fields(&comp_fields, DSNDCP);
/* Encode SNDCP-XID fields */
- rc = gprs_sndcp_compile_xid(xid, xid_len, &comp_fields);
+ rc = gprs_sndcp_compile_xid(xid, xid_len, &comp_fields,
+ DEFAULT_SNDCP_VERSION);
OSMO_ASSERT(rc > 0);
printf("Encoded: %s (%i bytes)\n", osmo_hexdump_nospc(xid, rc), rc);
/* Parse and show contained comp fields */
- comp_fields_dec = gprs_sndcp_parse_xid(ctx, xid, rc, NULL);
+ comp_fields_dec = gprs_sndcp_parse_xid(NULL, ctx, xid, rc, NULL);
OSMO_ASSERT(comp_fields_dec);
printf("Decoded:\n");