Improved HDLC abort handling

libs/spandsp/src/hdlc.c

@@ -126,7 +126,7 @@ static __inline__ void octet_count(hdlc_rx_state_t *s)
 
 static void rx_flag_or_abort(hdlc_rx_state_t *s)
 {
-    if ((s->raw_bit_stream & 0x8000))
+    if ((s->raw_bit_stream & 0x0100))
     {
         /* Hit HDLC abort */
         s->rx_aborts++;
@@ -196,7 +196,10 @@ static void rx_flag_or_abort(hdlc_rx_state_t *s)
         {
             /* Check the flags are back-to-back when testing for valid preamble. This
                greatly reduces the chances of false preamble detection, and anything
-               which doesn't send them back-to-back is badly broken. */
+               which doesn't send them back-to-back is badly broken. When we are one
+               flag away from OK we should not apply the back-to-back consition, as
+               between an abort and the following start of frame things might not be
+               octet aligned. */
             if (s->flags_seen != s->framing_ok_threshold - 1  &&  s->num_bits != 7)
             {
                 /* Don't set the flags seen indicator back to zero too aggressively.
@@ -222,14 +225,23 @@ static void rx_flag_or_abort(hdlc_rx_state_t *s)
 
 static __inline__ void hdlc_rx_put_bit_core(hdlc_rx_state_t *s)
 {
-    if ((s->raw_bit_stream & 0x3F00) == 0x3E00)
+    if ((s->raw_bit_stream & 0x3E00) == 0x3E00)
+    {
+        /* There are at least 5 ones in a row. We could be at a:
+            - point where stuffing occurs
+            - a flag
+            - an abort
+            - the result of bit errors */
+        /* Is this a bit to be skipped for destuffing? */
+        if ((s->raw_bit_stream & 0x4100) == 0)
+            return;
+        /* Is this a flag or abort? */
+        if ((s->raw_bit_stream & 0xFE00) == 0x7E00)
         {
-        /* Its time to either skip a bit, for stuffing, or process a
-           flag or abort */
-        if ((s->raw_bit_stream & 0x4000))
             rx_flag_or_abort(s);
             return;
         }
+    }
     s->num_bits++;
     if (s->flags_seen < s->framing_ok_threshold)
     {

libs/spandsp/src/t38_gateway.c

@@ -152,6 +152,8 @@
 /*! The number of consecutive flags to declare HDLC framing is OK. */
 #define HDLC_FRAMING_OK_THRESHOLD               5
 
+#define HDLC_TRAMISSION_LAG_OCTETS              2
+
 enum
 {
     DISBIT1 = 0x01,
@@ -1765,7 +1767,7 @@ static void rx_flag_or_abort(hdlc_rx_state_t *t)
 
     s = (t38_gateway_state_t *) t->frame_user_data;
     u = &s->core.to_t38;
-    if ((t->raw_bit_stream & 0x80))
+    if ((t->raw_bit_stream & 0x01))
     {
         /* Hit HDLC abort */
         t->rx_aborts++;
@@ -1774,6 +1776,23 @@ static void rx_flag_or_abort(hdlc_rx_state_t *t)
         else
             t->flags_seen = t->framing_ok_threshold - 1;
         /*endif*/
+        if (t->len > 0)
+        {
+            span_log(&s->logging, SPAN_LOG_FLOW, "HDLC frame aborted at %d\n", t->len);
+            /* If we have sent some of this frame we need to treat the abort as though it is a CRC error,
+               as reporting a bad CRC is the only way T.38 allows us to scrub a frame in progress. */
+            /* It seems some boxes may not like us sending a _SIG_END here, and then another
+               when the carrier actually drops. Lets just send T38_FIELD_HDLC_FCS_OK here. */
+            if (t->len > 2)
+            {
+                category = (s->t38x.current_tx_data_type == T38_DATA_V21)  ?  T38_PACKET_CATEGORY_CONTROL_DATA  :  T38_PACKET_CATEGORY_IMAGE_DATA;
+                if (t38_core_send_data(&s->t38x.t38, s->t38x.current_tx_data_type, T38_FIELD_HDLC_FCS_BAD, NULL, 0, category) < 0)
+                    span_log(&s->logging, SPAN_LOG_WARNING, "T.38 send failed\n");
+                /*endif*/
+            }
+            /*endif*/
+        }
+        /*endif*/
     }
     else
     {
@@ -1863,7 +1882,10 @@ static void rx_flag_or_abort(hdlc_rx_state_t *t)
         {
             /* Check the flags are back-to-back when testing for valid preamble. This
                greatly reduces the chances of false preamble detection, and anything
-               which doesn't send them back-to-back is badly broken. */
+               which doesn't send them back-to-back is badly broken. When we are one
+               flag away from OK we should not apply the back-to-back consition, as
+               between an abort and the following start of frame things might not be
+               octet aligned. */
             if (t->flags_seen != t->framing_ok_threshold - 1  &&  t->num_bits != 7)
                 t->flags_seen = 0;
             /*endif*/
@@ -1906,14 +1928,26 @@ static void t38_hdlc_rx_put_bit(hdlc_rx_state_t *t, int new_bit)
     }
     /*endif*/
     t->raw_bit_stream = (t->raw_bit_stream << 1) | (new_bit & 1);
-    if ((t->raw_bit_stream & 0x3F) == 0x3E)
+    if ((t->raw_bit_stream & 0x3E) == 0x3E)
+    {
+        /* There are at least 5 ones in a row. We could be at a:
+            - point where stuffing occurs
+            - a flag
+            - an abort
+            - the result of bit errors */
+        /* Is this a bit to be skipped for destuffing? */
+        if ((t->raw_bit_stream & 0x41) == 0)
+            return;
+        /*endif*/
+        /* Is this a flag or abort? */
+        if ((t->raw_bit_stream & 0xFE) == 0x7E)
         {
-        /* Its time to either skip a bit, for stuffing, or process a flag or abort */
-        if ((t->raw_bit_stream & 0x40))
             rx_flag_or_abort(t);
             return;
         }
         /*endif*/
+    }
+    /*endif*/
     t->num_bits++;
     if (t->flags_seen < t->framing_ok_threshold)
         return;
@@ -1928,7 +1962,7 @@ static void t38_hdlc_rx_put_bit(hdlc_rx_state_t *t, int new_bit)
     if (t->len >= (int) sizeof(t->buffer))
     {
         /* This is too long. Abandon the frame, and wait for the next flag octet. */
-        if ((t->len + 2) >= u->octets_per_data_packet)
+        if ((t->len + HDLC_TRAMISSION_LAG_OCTETS) >= u->octets_per_data_packet)
         {
             /* We will have sent some of this frame already, so we need to send termination of this bad HDLC frame. */
             category = (s->t38x.current_tx_data_type == T38_DATA_V21)  ?  T38_PACKET_CATEGORY_CONTROL_DATA  :  T38_PACKET_CATEGORY_IMAGE_DATA;
@@ -1968,7 +2002,7 @@ static void t38_hdlc_rx_put_bit(hdlc_rx_state_t *t, int new_bit)
     u->crc = crc_itu16_calc(&t->buffer[t->len], 1, u->crc);
     /* Make the transmission lag by two octets, so we do not send the CRC, and
        do not report the CRC result too late. */
-    if (++t->len <= 2)
+    if (++t->len <= HDLC_TRAMISSION_LAG_OCTETS)
         return;
     /*endif*/
     if (s->t38x.current_tx_data_type == T38_DATA_V21)
@@ -1979,7 +2013,7 @@ static void t38_hdlc_rx_put_bit(hdlc_rx_state_t *t, int new_bit)
     }
     if (++u->data_ptr >= u->octets_per_data_packet)
     {
-        bit_reverse(u->data, &t->buffer[t->len - 2 - u->data_ptr], u->data_ptr);
+        bit_reverse(u->data, &t->buffer[t->len - HDLC_TRAMISSION_LAG_OCTETS - u->data_ptr], u->data_ptr);
         category = (s->t38x.current_tx_data_type == T38_DATA_V21)  ?  T38_PACKET_CATEGORY_CONTROL_DATA  :  T38_PACKET_CATEGORY_IMAGE_DATA;
         if (t38_core_send_data(&s->t38x.t38, s->t38x.current_tx_data_type, T38_FIELD_HDLC_DATA, u->data, u->data_ptr, category) < 0)
             span_log(&s->logging, SPAN_LOG_WARNING, "T.38 send failed\n");