tbf: Use a hysteresis when discarding DL LLC frames

Currently single LLC blocks are discarded when the PDU lifetime
expires. If an IP packet has been fragmented either on the IP or on
the LLC layer and is therefore distributed over several LLC frames,
the kept fragments are transmitted and then discarded by the MS
because of the missing PDU. This can cause massive IP packet loss
when there are many fragmented packets (e.g. when trying 'ping
-s1800' or if the GGSN chops downlink IP packets into several SNDCP
packets).

On the other hand, discarding too many packets might disturb the
congestion handling of TCP. Dropping plain TCP ACKs might also hinder
flow control and congestion avoidance.

This commit adds a hysteresis algorithm to the LLC discard loop. If
an LLC message's age reaches the high water mark, further message's
with an age above the low water mark are discarded, too. This is
aborted, if a GMM, a non-UI, or a small message is detected. In
these cases, that message is kept.

The following VTY commands are added (pcu config node):

- queue hysteresis <1-65535>   set the difference between high
                               (lifetime) and low watermark in
                               centiseconds
- no queue hysteresis          disable this feature (default)

Since the SGSN will most probably send all fragments of a single
N-PDU without much delay between them, a value slightly above the
average transmission delay jitter between SGSN and PCU is probably a
sensible value to discard all fragments of a single IP packet.

This is an experimental feature that might be replaced by more
advanced means of active queue management in the future.

Sponsored-by: On-Waves ehf

6 files changed, 95 insertions, 11 deletions

diff --git a/src/bts.h b/src/bts.h
index 2b43736..fae3c5e 100644
--- a/src/bts.h
+++ b/src/bts.h
@@ -118,6 +118,7 @@ struct gprs_rlcmac_bts {
 	uint8_t initial_cs_dl, initial_cs_ul;
 	uint8_t force_cs;	/* 0=use from BTS 1=use from VTY */
 	uint16_t force_llc_lifetime; /* overrides lifetime from SGSN */
+	uint32_t llc_discard_csec;
 	uint8_t t3142;
 	uint8_t t3169;
 	uint8_t t3191;
diff --git a/src/llc.cpp b/src/llc.cpp
index b160961..9c5581f 100644
--- a/src/llc.cpp
+++ b/src/llc.cpp
@@ -160,3 +160,18 @@ bool gprs_llc::is_frame_expired(struct timeval *tv_now, struct timeval *tv)
 
 	return timercmp(tv_now, tv, >);
 }
+
+bool gprs_llc::is_user_data_frame(uint8_t *data, size_t len)
+{
+	if (len < 2)
+		return false;
+
+	if ((data[0] & 0x0f) == 1 /* GPRS_SAPI_GMM */)
+		return false;
+
+	if ((data[0] & 0x0e) != 0xc0 /* LLC UI */)
+		/* It is not an LLC UI frame */
+		return false;
+
+	return true;
+}
diff --git a/src/llc.h b/src/llc.h
index f82c4ed..11a0c7e 100644
--- a/src/llc.h
+++ b/src/llc.h
@@ -29,6 +29,7 @@
 struct gprs_llc {
 	static void calc_pdu_lifetime(BTS *bts, const uint16_t pdu_delay_csec, struct timeval *tv);
 	static bool is_frame_expired(struct timeval *now, struct timeval *tv);
+	static bool is_user_data_frame(uint8_t *data, size_t len);
 
 	void init();
 	void reset();
diff --git a/src/pcu_utils.h b/src/pcu_utils.h
index 84c545b..5ca234a 100644
--- a/src/pcu_utils.h
+++ b/src/pcu_utils.h
@@ -19,3 +19,8 @@
 inline int msecs_to_frames(int msecs) {
 	return (msecs * (1024 * 1000 / 4615)) / 1024;
 }
+
+inline void csecs_to_timeval(unsigned csecs, struct timeval *tv) {
+	tv->tv_sec  = csecs / 100;
+	tv->tv_usec = (csecs % 100) * 10000;
+}
diff --git a/src/pcu_vty.c b/src/pcu_vty.c
index 9b597ad..3d9c79a 100644
--- a/src/pcu_vty.c
+++ b/src/pcu_vty.c
@@ -97,6 +97,9 @@ static int config_write_pcu(struct vty *vty)
 	else if (bts->force_llc_lifetime)
 		vty_out(vty, " queue lifetime %d%s", bts->force_llc_lifetime,
 			VTY_NEWLINE);
+	if (bts->llc_discard_csec)
+		vty_out(vty, " queue hysteresis %d%s", bts->llc_discard_csec,
+			VTY_NEWLINE);
 	if (bts->alloc_algorithm == alloc_algorithm_a)
 		vty_out(vty, " alloc-algorithm a%s", VTY_NEWLINE);
 	if (bts->alloc_algorithm == alloc_algorithm_b)
@@ -209,6 +212,35 @@ DEFUN(cfg_pcu_no_queue_lifetime,
 	return CMD_SUCCESS;
 }
 
+#define QUEUE_HYSTERESIS_STR "Set lifetime hysteresis of LLC frame in centi-seconds " \
+	"(continue discarding until lifetime-hysteresis is reached)\n"
+
+DEFUN(cfg_pcu_queue_hysteresis,
+      cfg_pcu_queue_hysteresis_cmd,
+      "queue hysteresis <1-65534>",
+      QUEUE_STR QUEUE_HYSTERESIS_STR "Hysteresis in centi-seconds")
+{
+	struct gprs_rlcmac_bts *bts = bts_main_data();
+	uint8_t csec = atoi(argv[0]);
+
+	bts->llc_discard_csec = csec;
+
+	return CMD_SUCCESS;
+}
+
+DEFUN(cfg_pcu_no_queue_hysteresis,
+      cfg_pcu_no_queue_hysteresis_cmd,
+      "no queue hysteresis",
+      NO_STR QUEUE_STR QUEUE_HYSTERESIS_STR)
+{
+	struct gprs_rlcmac_bts *bts = bts_main_data();
+
+	bts->llc_discard_csec = 0;
+
+	return CMD_SUCCESS;
+}
+
+
 DEFUN(cfg_pcu_alloc,
       cfg_pcu_alloc_cmd,
       "alloc-algorithm (a|b)",
@@ -366,6 +398,8 @@ int pcu_vty_init(const struct log_info *cat)
 	install_element(PCU_NODE, &cfg_pcu_queue_lifetime_cmd);
 	install_element(PCU_NODE, &cfg_pcu_queue_lifetime_inf_cmd);
 	install_element(PCU_NODE, &cfg_pcu_no_queue_lifetime_cmd);
+	install_element(PCU_NODE, &cfg_pcu_queue_hysteresis_cmd);
+	install_element(PCU_NODE, &cfg_pcu_no_queue_hysteresis_cmd);
 	install_element(PCU_NODE, &cfg_pcu_alloc_cmd);
 	install_element(PCU_NODE, &cfg_pcu_two_phase_cmd);
 	install_element(PCU_NODE, &cfg_pcu_fc_interval_cmd);
diff --git a/src/tbf_dl.cpp b/src/tbf_dl.cpp
index 7e0883b..aeec23f 100644
--- a/src/tbf_dl.cpp
+++ b/src/tbf_dl.cpp
@@ -209,30 +209,58 @@ int gprs_rlcmac_dl_tbf::handle(struct gprs_rlcmac_bts *bts,
 struct msgb *gprs_rlcmac_dl_tbf::llc_dequeue(bssgp_bvc_ctx *bctx)
 {
 	struct msgb *msg;
-	struct timeval *tv, tv_now;
+	struct timeval *tv, tv_now, tv_now2;
 	uint32_t octets = 0, frames = 0;
+	struct timeval hyst_delta = {0, 0};
+	const unsigned keep_small_thresh = 60;
+
+	if (bts_data()->llc_discard_csec)
+		csecs_to_timeval(bts_data()->llc_discard_csec, &hyst_delta);
 
 	gettimeofday(&tv_now, NULL);
+	timeradd(&tv_now, &hyst_delta, &tv_now2);
 
 	while ((msg = m_llc.dequeue())) {
 		tv = (struct timeval *)msg->data;
 		msgb_pull(msg, sizeof(*tv));
 		msgb_pull(msg, sizeof(*tv));
 
-		if (gprs_llc::is_frame_expired(&tv_now, tv)) {
-			LOGP(DRLCMACDL, LOGL_NOTICE, "%s Discarding LLC PDU "
-				"because lifetime limit reached. Queue size %zu\n",
-				tbf_name(this), m_llc.m_queue_size);
-			bts->llc_timedout_frame();
-			frames++;
-			octets += msg->len;
-			msgb_free(msg);
-			continue;
+		/* Is the age below the low water mark? */
+		if (!gprs_llc::is_frame_expired(&tv_now2, tv))
+			break;
+
+		/* Is the age below the high water mark */
+		if (!gprs_llc::is_frame_expired(&tv_now, tv)) {
+			/* Has the previous message not been dropped? */
+			if (frames == 0)
+				break;
+
+			/* Hysteresis mode, try to discard LLC messages until
+			 * the low water mark has been reached */
+
+			/* Check whether to abort the hysteresis mode */
+
+			/* Is the frame small, perhaps only a TCP ACK? */
+			if (msg->len <= keep_small_thresh)
+				break;
+
+			/* Is it a GMM message? */
+			if (!gprs_llc::is_user_data_frame(msg->data, msg->len))
+				break;
 		}
-		break;
+
+		bts->llc_timedout_frame();
+		frames++;
+		octets += msg->len;
+		msgb_free(msg);
+		continue;
 	}
 
 	if (frames) {
+		LOGP(DRLCMACDL, LOGL_NOTICE, "%s Discarding LLC PDU "
+			"because lifetime limit reached, "
+			"count=%u new_queue_size=%zu\n",
+			tbf_name(this), frames, m_llc.m_queue_size);
 		if (frames > 0xff)
 			frames = 0xff;
 		if (octets > 0xffffff)