Merge remote-tracking branch 'daniel/lapdm-desync'

This should fix long delays when sending a long SMS on an encrypted
channel. It is the most minimal change to the polling mode. One
option we discussed internally is to queue a higher level representation
and generate the actual LAPDm frames later.

8 files changed, 300 insertions, 12 deletions

diff --git a/TODO-RELEASE b/TODO-RELEASE
new file mode 100644
index 00000000..2a1f9788
--- /dev/null
+++ b/TODO-RELEASE
@@ -0,0 +1,2 @@
+#library	what		description / commit summary line
+libosmogsm	abi-change	Prevent LAPD tx_queue from filling up in polling mode
diff --git a/include/osmocom/gsm/lapd_core.h b/include/osmocom/gsm/lapd_core.h
index c2fdc622..f88fb112 100644
--- a/include/osmocom/gsm/lapd_core.h
+++ b/include/osmocom/gsm/lapd_core.h
@@ -125,6 +125,7 @@ struct lapd_datalink {
 	int (*send_dlsap)(struct osmo_dlsap_prim *dp,
 	        struct lapd_msg_ctx *lctx);
 	int (*send_ph_data_req)(struct lapd_msg_ctx *lctx, struct msgb *msg);
+	int (*update_pending_frames)(struct lapd_msg_ctx *lctx);
 	struct {
 		/*! \brief filled-in once we set the lapd_mode above */
 		struct lapd_cr_ent loc2rem;
diff --git a/include/osmocom/gsm/lapdm.h b/include/osmocom/gsm/lapdm.h
index a6e4ad72..93934068 100644
--- a/include/osmocom/gsm/lapdm.h
+++ b/include/osmocom/gsm/lapdm.h
@@ -81,6 +81,8 @@ struct lapdm_channel {
 const char *get_rsl_name(int value);
 extern const char *lapdm_state_names[];
 
+struct lapdm_datalink *lapdm_datalink_for_sapi(struct lapdm_entity *le, uint8_t sapi);
+
 /* initialize a LAPDm entity */
 void lapdm_entity_init(struct lapdm_entity *le, enum lapdm_mode mode, int t200);
 void lapdm_channel_init(struct lapdm_channel *lc, enum lapdm_mode mode);
diff --git a/src/gsm/lapd_core.c b/src/gsm/lapd_core.c
index ef1d22a7..8c5fe9c6 100644
--- a/src/gsm/lapd_core.c
+++ b/src/gsm/lapd_core.c
@@ -1607,6 +1607,12 @@ static int lapd_rx_i(struct msgb *msg, struct lapd_msg_ctx *lctx)
 		if (!dl->own_busy) {
 			/* NOTE: V(R) is already set above */
 			rc = lapd_send_i(lctx, __LINE__);
+
+			/* if update_pending_iframe returns 0 it updated
+			 * the lapd header of an iframe in the tx queue */
+			if (rc && dl->update_pending_frames)
+				rc = dl->update_pending_frames(lctx);
+
 			if (rc) {
 				LOGP(DLLAPD, LOGL_INFO, "we are not busy and "
 					"have no pending data, send RR\n");
diff --git a/src/gsm/lapdm.c b/src/gsm/lapdm.c
index 41f4be71..698f8502 100644
--- a/src/gsm/lapdm.c
+++ b/src/gsm/lapdm.c
@@ -114,6 +114,7 @@ enum lapdm_format {
 static int lapdm_send_ph_data_req(struct lapd_msg_ctx *lctx, struct msgb *msg);
 static int send_rslms_dlsap(struct osmo_dlsap_prim *dp,
 	struct lapd_msg_ctx *lctx);
+static int update_pending_frames(struct lapd_msg_ctx *lctx);
 
 static void lapdm_dl_init(struct lapdm_datalink *dl,
 			  struct lapdm_entity *entity, int t200)
@@ -124,6 +125,7 @@ static void lapdm_dl_init(struct lapdm_datalink *dl,
 	dl->dl.reestablish = 0; /* GSM uses no reestablish */
 	dl->dl.send_ph_data_req = lapdm_send_ph_data_req;
 	dl->dl.send_dlsap = send_rslms_dlsap;
+	dl->dl.update_pending_frames = update_pending_frames;
 	dl->dl.n200_est_rel = N200_EST_REL;
 	dl->dl.n200 = N200;
 	dl->dl.t203_sec = 0; dl->dl.t203_usec = 0;
@@ -181,7 +183,7 @@ void lapdm_channel_exit(struct lapdm_channel *lc)
 	lapdm_entity_exit(&lc->lapdm_dcch);
 }
 
-static struct lapdm_datalink *datalink_for_sapi(struct lapdm_entity *le, uint8_t sapi)
+struct lapdm_datalink *lapdm_datalink_for_sapi(struct lapdm_entity *le, uint8_t sapi)
 {
 	switch (sapi) {
 	case LAPDm_SAPI_NORMAL:
@@ -488,6 +490,25 @@ static int lapdm_send_ph_data_req(struct lapd_msg_ctx *lctx, struct msgb *msg)
 			23);
 }
 
+static int update_pending_frames(struct lapd_msg_ctx *lctx)
+{
+	struct lapd_datalink *dl = lctx->dl;
+	struct msgb *msg;
+	int rc = -1;
+
+	llist_for_each_entry(msg, &dl->tx_queue, list) {
+		if (LAPDm_CTRL_is_I(msg->l2h[1])) {
+			msg->l2h[1] = LAPDm_CTRL_I(dl->v_recv, LAPDm_CTRL_I_Ns(msg->l2h[1]),
+					LAPDm_CTRL_PF_BIT(msg->l2h[1]));
+			rc = 0;
+		} else if (LAPDm_CTRL_is_S(msg->l2h[1])) {
+			LOGP(DLLAPD, LOGL_ERROR, "Supervisory frame in queue, this shouldn't happen\n");
+		}
+	}
+
+	return rc;
+}
+
 /* input into layer2 (from layer 1) */
 static int l2_ph_data_ind(struct msgb *msg, struct lapdm_entity *le,
 	uint8_t chan_nr, uint8_t link_id)
@@ -543,7 +564,7 @@ static int l2_ph_data_ind(struct msgb *msg, struct lapdm_entity *le,
 		}
 	}
 
-	mctx.dl = datalink_for_sapi(le, sapi);
+	mctx.dl = lapdm_datalink_for_sapi(le, sapi);
 	/* G.2.1 No action on frames containing an unallocated SAPI. */
 	if (!mctx.dl) {
 		LOGP(DLLAPD, LOGL_NOTICE, "Received frame for unsupported "
@@ -1071,7 +1092,7 @@ static int rslms_rx_rll(struct msgb *msg, struct lapdm_channel *lc)
 	/* G.2.1 No action shall be taken on frames containing an unallocated
 	 * SAPI.
 	 */
-	dl = datalink_for_sapi(le, sapi);
+	dl = lapdm_datalink_for_sapi(le, sapi);
 	if (!dl) {
 		LOGP(DLLAPD, LOGL_ERROR, "No instance for SAPI %d!\n", sapi);
 		msgb_free(msg);
diff --git a/src/gsm/libosmogsm.map b/src/gsm/libosmogsm.map
index 9d15d668..3c5025df 100644
--- a/src/gsm/libosmogsm.map
+++ b/src/gsm/libosmogsm.map
@@ -172,6 +172,7 @@ lapdm_channel_set_flags;
 lapdm_channel_set_l1;
 lapdm_channel_set_l3;
 lapdm_channel_set_mode;
+lapdm_datalink_for_sapi;
 lapdm_entity_exit;
 lapdm_entity_init;
 lapdm_entity_reset;
diff --git a/tests/lapd/lapd_test.c b/tests/lapd/lapd_test.c
index 6b5cfd36..18ea1dc3 100644
--- a/tests/lapd/lapd_test.c
+++ b/tests/lapd/lapd_test.c
@@ -1,6 +1,7 @@
 /*
  * (C) 2011 by Holger Hans Peter Freyther
  * (C) 2011 by On-Waves
+ * (C) 2014 by Daniel Willmann <dwillmann@sysmocom.de>
  * All Rights Reserved
  *
  * This program is free software; you can redistribute it and/or modify
@@ -178,22 +179,66 @@ static int send(struct msgb *in_msg, struct lapdm_channel *chan)
 	return 0;
 }
 
-static int send_sabm(struct lapdm_channel *chan, int second_ms)
+/* Receive from L1 */
+static int send_buf(const uint8_t *buf, size_t len, struct lapdm_channel *chan)
 {
 	struct osmo_phsap_prim pp;
 	struct msgb *msg;
 	int rc;
 
+	msg = msgb_from_array(buf, len);
+	msg->l2h = msg->l3h;
+	msg->l3h = NULL;
+	osmo_prim_init(&pp.oph, SAP_GSM_PH, PRIM_PH_DATA,
+			PRIM_OP_INDICATION, msg);
+
+	/* LAPDm requires those... */
+	pp.u.data.chan_nr = 0;
+	pp.u.data.link_id = 0;
+        /* feed into the LAPDm code of libosmogsm */
+        rc = lapdm_phsap_up(&pp.oph, &chan->lapdm_dcch);
+	OSMO_ASSERT(rc == 0 || rc == -EBUSY);
+	return 0;
+}
+
+/* Receive from L3 */
+static int enqueue_buf(const uint8_t *buf, size_t len, int sapi, struct lapdm_channel *chan)
+{
+	struct osmo_dlsap_prim dp;
+	struct msgb *msg;
+	int rc;
+	struct lapdm_datalink *dl;
+
+	dl = lapdm_datalink_for_sapi(&chan->lapdm_dcch, sapi);
+	OSMO_ASSERT(dl);
+
+	msg = msgb_from_array(buf, len);
+	osmo_prim_init(&dp.oph, 0, PRIM_DL_DATA, PRIM_OP_REQUEST, msg);
+
+	rc = lapd_recv_dlsap(&dp, &dl->dl.lctx);
+	OSMO_ASSERT(rc == 0 || rc == -EBUSY);
+	return 0;
+}
+
+static int send_sabm(struct lapdm_channel *chan, int sapi, const uint8_t *data, size_t len)
+{
+	struct osmo_phsap_prim pp;
+	struct msgb *msg;
+	int rc;
+
+	OSMO_ASSERT(sapi == 0 || sapi == 3);
+
 	msg = msgb_alloc_headroom(128, 64, "PH-DATA.ind");
 	osmo_prim_init(&pp.oph, SAP_GSM_PH, PRIM_PH_DATA,
 			PRIM_OP_INDICATION, msg);
 	/* copy over actual MAC block */
-	msg->l2h = msgb_put(msg, 23);
-	msg->l2h[0] = 0x01;
+	msg->l2h = msgb_put(msg, 3 + len);
+	msg->l2h[0] = 0x01 | (sapi << 2);
 	msg->l2h[1] = 0x3f;
-	msg->l2h[2] = 0x01 | (sizeof(cm) << 2);
-	memcpy(msg->l2h + 3, cm, sizeof(cm));
-	msg->l2h[3] += second_ms; /* alter message, for second mobile */
+	msg->l2h[2] = 0x01 | (len << 2);
+
+	if (len > 0)
+		memcpy(msg->l2h + 3, data, len);
 
 	/* LAPDm requires those... */
 	pp.u.data.chan_nr = 0;
@@ -430,6 +475,7 @@ static void test_lapdm_contention_resolution()
 	int rc;
 	struct lapdm_polling_state test_state;
 	struct osmo_phsap_prim pp;
+	uint8_t *cm2;
 
 	/* Configure LAPDm on both sides */
 	struct lapdm_channel bts_to_ms_channel;
@@ -445,18 +491,22 @@ static void test_lapdm_contention_resolution()
 	lapdm_channel_set_l3(&bts_to_ms_channel, bts_to_ms_tx_cb, &test_state);
 
 	/* Send SABM MS 1, we must get UA */
-	send_sabm(&bts_to_ms_channel, 0);
+	send_sabm(&bts_to_ms_channel, 0, cm, sizeof(cm));
 	rc = dequeue_prim(&bts_to_ms_channel.lapdm_dcch, &pp, "DCCH");
 	CHECK_RC(rc);
 	OSMO_ASSERT(memcmp(pp.oph.msg->l2h, ua, ARRAY_SIZE(ua)) == 0);
 
 	/* Send SABM MS 2, we must get nothing, due to collision */
-	send_sabm(&bts_to_ms_channel, 1);
+	cm2 = malloc(sizeof(cm));
+	memcpy(cm2, cm, sizeof(cm));
+	cm2[0] += 1;
+	send_sabm(&bts_to_ms_channel, 0, cm2, sizeof(cm2));
+	free(cm2);
 	rc = dequeue_prim(&bts_to_ms_channel.lapdm_dcch, &pp, "DCCH");
 	OSMO_ASSERT(rc == -ENODEV);
 
 	/* Send SABM MS 1 again, we must get UA gain */
-	send_sabm(&bts_to_ms_channel, 0);
+	send_sabm(&bts_to_ms_channel, 0, cm, sizeof(cm));
 	rc = dequeue_prim(&bts_to_ms_channel.lapdm_dcch, &pp, "DCCH");
 	CHECK_RC(rc);
 	OSMO_ASSERT(memcmp(pp.oph.msg->l2h, ua, ARRAY_SIZE(ua)) == 0);
@@ -555,6 +605,152 @@ static void test_lapdm_establishment()
 	lapdm_establish(est_req_sacch_sapi3, sizeof(est_req_sacch_sapi3));
 }
 
+
+const uint8_t cm_chg[] = {
+	0x01, 0x00, 0x49, 0x06, 0x16, 0x03, 0x33,
+	0x59, 0xa6, 0x20, 0x0a, 0x20, 0x04, 0x04,
+	0x2f, 0x65, 0x23, 0x02, 0x00, 0x24, 0x04
+};
+
+const uint8_t cm_chg_ack[] = {
+	0x01, 0x21, 0x01
+};
+
+const uint8_t gprs_susp[] = {
+	0x01, 0x02, 0x35, 0x06, 0x34, 0xe3, 0xd4,
+	0xd2, 0x6f, 0x09, 0xf1, 0x07, 0x00, 0x01, 0x00, 0x02
+};
+
+const uint8_t cipher_cmd[] = {
+	0x06, 0x35, 0x01
+};
+
+/* The cipher command we send to the MS after updating our N(R) */
+const uint8_t cipher_cmd_out[] = {
+	0x03, 0x40, 0x0d, 0x06, 0x35, 0x01
+};
+
+uint8_t cipher_compl[] = {
+	0x01, 0x24, 0x09, 0x06, 0x32
+};
+
+uint8_t cipher_compl_ack[] = {
+	0x01, 0x61, 0x01
+};
+
+static const uint8_t ua_sms[] = {
+	0x0d, 0x73, 0x01
+};
+
+const uint8_t cp_data_1[] = {
+	0x0d, 0x00, 0x53, 0x59, 0x01, 0x5c, 0x00,
+	0x4c, 0x00, 0x06, 0x91, 0x86, 0x77, 0x07,
+	0x00, 0xf9, 0x51, 0x11, 0x76, 0x05, 0x81, 0x29, 0x32
+};
+
+const uint8_t cp_data_1_ack[] = {
+	0x0d, 0x21, 0x01
+};
+
+static int bts_to_ms_dummy_tx_cb(struct msgb *in_msg, struct lapdm_entity *le, void *_ctx)
+{
+	printf("%s: MS->BTS(us) message %d\n", __func__, msgb_length(in_msg));
+	msgb_free(in_msg);
+
+	return 0;
+}
+
+static void dump_queue(struct llist_head *head)
+{
+	struct msgb* msg;
+
+	printf("\nDumping queue:\n");
+	llist_for_each_entry(msg, head, list)
+		printf("%s\n", msgb_hexdump(msg));
+	printf("\n");
+}
+
+static void test_lapdm_desync()
+{
+	printf("I test if desync problems exist in LAPDm\n");
+
+	int rc;
+	struct osmo_phsap_prim pp;
+
+	/* Configure LAPDm on both sides */
+	struct lapdm_channel bts_to_ms_channel;
+	memset(&bts_to_ms_channel, 0, sizeof(bts_to_ms_channel));
+
+	/* BTS to MS in polling mode */
+	lapdm_channel_init(&bts_to_ms_channel, LAPDM_MODE_BTS);
+	lapdm_channel_set_flags(&bts_to_ms_channel, LAPDM_ENT_F_POLLING_ONLY);
+	lapdm_channel_set_l1(&bts_to_ms_channel, NULL, NULL);
+	lapdm_channel_set_l3(&bts_to_ms_channel, bts_to_ms_dummy_tx_cb, NULL);
+	struct lapdm_datalink *dl = lapdm_datalink_for_sapi(&bts_to_ms_channel.lapdm_dcch, 0);
+	dl->mctx.dl = dl;
+	dl->dl.lctx.dl = &dl->dl;
+
+	/* Send SABM MS 1, we must get UA */
+	printf("\nEstablishing SAPI=0\n");
+	send_sabm(&bts_to_ms_channel, 0, cm, sizeof(cm));
+
+	dump_queue(&dl->dl.tx_queue);
+
+	rc = dequeue_prim(&bts_to_ms_channel.lapdm_dcch, &pp, "DCCH");
+	CHECK_RC(rc);
+	OSMO_ASSERT(memcmp(pp.oph.msg->l2h, ua, ARRAY_SIZE(ua)) == 0);
+
+	printf("\nSending Classmark Change\n");
+	send_buf(cm_chg, sizeof(cm_chg), &bts_to_ms_channel);
+	dump_queue(&dl->dl.tx_queue);
+
+	rc = dequeue_prim(&bts_to_ms_channel.lapdm_dcch, &pp, "DCCH");
+	CHECK_RC(rc);
+	OSMO_ASSERT(memcmp(pp.oph.msg->l2h, cm_chg_ack, ARRAY_SIZE(cm_chg_ack)) == 0);
+
+	printf("\nEnqueueing Ciphering Mode Command\n");
+	enqueue_buf(cipher_cmd, sizeof(cipher_cmd), 0, &bts_to_ms_channel);
+	dump_queue(&dl->dl.tx_queue);
+
+	printf("\nSending GPRS Suspend Request\n");
+	send_buf(gprs_susp, sizeof(gprs_susp), &bts_to_ms_channel);
+	dump_queue(&dl->dl.tx_queue);
+
+	rc = dequeue_prim(&bts_to_ms_channel.lapdm_dcch, &pp, "DCCH");
+	CHECK_RC(rc);
+	OSMO_ASSERT(memcmp(pp.oph.msg->l2h, cipher_cmd_out, ARRAY_SIZE(cipher_cmd_out)) == 0);
+
+	printf("\nSending Cipher Mode Complete\n");
+	send_buf(cipher_compl, sizeof(cipher_compl), &bts_to_ms_channel);
+	dump_queue(&dl->dl.tx_queue);
+
+	rc = dequeue_prim(&bts_to_ms_channel.lapdm_dcch, &pp, "DCCH");
+	CHECK_RC(rc);
+	OSMO_ASSERT(memcmp(pp.oph.msg->l2h, cipher_compl_ack, ARRAY_SIZE(cipher_compl_ack)) == 0);
+
+	printf("\nEstablishing SAPI=3\n");
+	send_sabm(&bts_to_ms_channel, 3, NULL, 0);
+	dump_queue(&dl->dl.tx_queue);
+
+	rc = dequeue_prim(&bts_to_ms_channel.lapdm_dcch, &pp, "DCCH");
+	CHECK_RC(rc);
+	OSMO_ASSERT(memcmp(pp.oph.msg->l2h, ua_sms, ARRAY_SIZE(ua_sms)) == 0);
+
+	printf("\nSending CP-DATA\n");
+	send_buf(cp_data_1, sizeof(cp_data_1), &bts_to_ms_channel);
+	dump_queue(&dl->dl.tx_queue);
+
+	rc = dequeue_prim(&bts_to_ms_channel.lapdm_dcch, &pp, "DCCH");
+	CHECK_RC(rc);
+	OSMO_ASSERT(memcmp(pp.oph.msg->l2h, cp_data_1_ack, ARRAY_SIZE(cp_data_1_ack)) == 0);
+
+	/* clean up */
+	lapdm_channel_exit(&bts_to_ms_channel);
+
+	/* Check if exit is idempotent */
+	lapdm_channel_exit(&bts_to_ms_channel);
+}
+
 int main(int argc, char **argv)
 {
 	osmo_init_logging(&info);
@@ -567,6 +763,7 @@ int main(int argc, char **argv)
 	test_lapdm_early_release();
 	test_lapdm_contention_resolution();
 	test_lapdm_establishment();
+	test_lapdm_desync();
 
 	printf("Success.\n");
 
diff --git a/tests/lapd/lapd_test.ok b/tests/lapd/lapd_test.ok
index 9fb58e0c..e188e27e 100644
--- a/tests/lapd/lapd_test.ok
+++ b/tests/lapd/lapd_test.ok
@@ -38,4 +38,62 @@ Message: [L2]> 0f 3f 01 [L3]> 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b
 Testing SAPI3/SACCH
 Took message from ACCH queue: L2 header size 5, L3 size 18, SAP 0x1000000, 0/0, Link 0x43
 Message: [L2]> 00 00 0f 3f 01 [L3]> 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 
+I test if desync problems exist in LAPDm
+
+Establishing SAPI=0
+bts_to_ms_dummy_tx_cb: MS->BTS(us) message 25
+
+Dumping queue:
+00 00 17 [L2]> 01 73 41 [L3]> 05 24 31 03 50 18 93 08 29 47 80 00 00 00 00 80 
+
+Took message from DCCH queue: L2 header size 3, L3 size 20, SAP 0x1000000, 0/0, Link 0x00
+Message: [L2]> 01 73 41 [L3]> 05 24 31 03 50 18 93 08 29 47 80 00 00 00 00 80 2b 2b 2b 2b 
+
+Sending Classmark Change
+bts_to_ms_dummy_tx_cb: MS->BTS(us) message 27
+
+Dumping queue:
+00 00 17 [L2]> 01 21 01 
+
+Took message from DCCH queue: L2 header size 23, L3 size 0, SAP 0x1000000, 0/0, Link 0x00
+Message: [L2]> 01 21 01 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 
+
+Enqueueing Ciphering Mode Command
+
+Dumping queue:
+00 00 17 [L2]> 03 20 0d [L3]> 06 35 01 
+
+
+Sending GPRS Suspend Request
+bts_to_ms_dummy_tx_cb: MS->BTS(us) message 22
+
+Dumping queue:
+00 00 17 [L2]> 03 40 0d [L3]> 06 35 01 
+
+Took message from DCCH queue: L2 header size 3, L3 size 20, SAP 0x1000000, 0/0, Link 0x00
+Message: [L2]> 03 40 0d [L3]> 06 35 01 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 
+
+Sending Cipher Mode Complete
+bts_to_ms_dummy_tx_cb: MS->BTS(us) message 11
+
+Dumping queue:
+00 00 17 [L2]> 01 61 01 
+
+Took message from DCCH queue: L2 header size 23, L3 size 0, SAP 0x1000000, 0/0, Link 0x00
+Message: [L2]> 01 61 01 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 
+
+Establishing SAPI=3
+bts_to_ms_dummy_tx_cb: MS->BTS(us) message 6
+
+Dumping queue:
+
+Took message from DCCH queue: L2 header size 3, L3 size 20, SAP 0x1000000, 0/0, Link 0x03
+Message: [L2]> 0d 73 01 [L3]> 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 
+
+Sending CP-DATA
+
+Dumping queue:
+
+Took message from DCCH queue: L2 header size 23, L3 size 0, SAP 0x1000000, 0/0, Link 0x03
+Message: [L2]> 0d 21 01 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 
 Success.