Improve RLC AM sequence analysis by taking MAC retx into account.

svn path=/trunk/; revision=32071

3 files changed, 96 insertions, 29 deletions

diff --git a/epan/dissectors/packet-mac-lte.c b/epan/dissectors/packet-mac-lte.c
index 6dec643e9e..8d973680b7 100644
--- a/epan/dissectors/packet-mac-lte.c
+++ b/epan/dissectors/packet-mac-lte.c
@@ -1304,6 +1304,11 @@ static int DetectIfDLHARQResend(packet_info *pinfo, tvbuff_t *tvb, volatile int
     DLHARQResult *result = NULL;
     proto_item *result_ti;
 
+    /* FDD only for now! */
+    if (p_mac_lte_info->radioType != FDD_RADIO) {
+        return FALSE;
+    }
+
     if (!pinfo->fd->flags.visited) {
         /* First time, so set result and update DL harq table */
         LastFrameData *lastData = NULL;
@@ -1328,6 +1333,7 @@ static int DetectIfDLHARQResend(packet_info *pinfo, tvbuff_t *tvb, volatile int
                     gint nseconds_between_packets =
                           pinfo->fd->abs_ts.nsecs - lastData->received_time.nsecs;
 
+                    /* TODO: want to round to nearest millisecond */
                     gint total_gap = (seconds_between_packets*1000) +
                                      (nseconds_between_packets / 1000000);
 
@@ -1382,6 +1388,22 @@ static int DetectIfDLHARQResend(packet_info *pinfo, tvbuff_t *tvb, volatile int
     return (result != NULL);
 }
 
+
+/* Return TRUE if the given packet is thought to be a retx */
+int is_mac_lte_frame_retx(packet_info *pinfo, guint8 direction)
+{
+    if (direction == DIRECTION_UPLINK) {
+        /* For UL, retx count is stored in per-packet struct */
+        struct mac_lte_info *p_mac_lte_info = p_get_proto_data(pinfo->fd, proto_mac_lte);
+        return ((p_mac_lte_info != NULL) && (p_mac_lte_info->reTxCount > 0));
+    }
+    else {
+        /* For DL, must consult result table */
+        return (g_hash_table_lookup(mac_lte_dl_harq_result_hash, GUINT_TO_POINTER(pinfo->fd->num)) != NULL);
+    }
+}
+
+
 /* Track UL frames, so that when a retx is indicated, we can search for
    the original tx.  We will either find it, and provide a link back to it,
    or flag that we couldn't find as an expert error */
@@ -1391,6 +1413,11 @@ static void TrackReportedULHARQResend(packet_info *pinfo, tvbuff_t *tvb, volatil
 {
     ULHARQResult *result = NULL;
 
+    /* FDD only for now! */
+    if (p_mac_lte_info->radioType != FDD_RADIO) {
+        return;
+    }
+
     if (!pinfo->fd->flags.visited) {
         /* First time, so set result and update UL harq table */
         LastFrameData *lastData = NULL;
diff --git a/epan/dissectors/packet-mac-lte.h b/epan/dissectors/packet-mac-lte.h
index b2859c83ef..b50178c83f 100644
--- a/epan/dissectors/packet-mac-lte.h
+++ b/epan/dissectors/packet-mac-lte.h
@@ -121,6 +121,8 @@ typedef struct mac_lte_tap_info {
 } mac_lte_tap_info;
 
 
+/* Accessor function to check if a frame was considered to be ReTx */
+int is_mac_lte_frame_retx(packet_info *pinfo, guint8 direction);
 
 /*****************************************************************/
 /* UDP framing format                                            */
diff --git a/epan/dissectors/packet-rlc-lte.c b/epan/dissectors/packet-rlc-lte.c
index 82e2360f90..51eed99e35 100644
--- a/epan/dissectors/packet-rlc-lte.c
+++ b/epan/dissectors/packet-rlc-lte.c
@@ -35,6 +35,7 @@
 #include <epan/prefs.h>
 #include <epan/tap.h>
 
+#include "packet-mac-lte.h"
 #include "packet-rlc-lte.h"
 #include "packet-pdcp-lte.h"
 
@@ -128,6 +129,8 @@ static int hf_rlc_lte_sequence_analysis_ok = -1;
 static int hf_rlc_lte_sequence_analysis_previous_frame = -1;
 static int hf_rlc_lte_sequence_analysis_expected_sn = -1;
 static int hf_rlc_lte_sequence_analysis_framing_info_correct = -1;
+
+static int hf_rlc_lte_sequence_analysis_mac_retx = -1;
 static int hf_rlc_lte_sequence_analysis_retx = -1;
 static int hf_rlc_lte_sequence_analysis_repeated = -1;
 static int hf_rlc_lte_sequence_analysis_skipped = -1;
@@ -315,7 +318,7 @@ static int dissect_rlc_lte_extension_header(tvbuff_t *tvb, packet_info *pinfo,
         } else {
             offset++;
         }
- 
+
         s_lengths[s_number_of_extensions++] = (guint16)length;
     }
 
@@ -488,7 +491,7 @@ typedef struct
     guint16   lastSN;
 
     /* AM Only */
-    enum { SN_OK, SN_Repeated, SN_Retx, SN_Missing} amState;
+    enum { SN_OK, SN_Repeated, SN_MAC_Retx, SN_Retx, SN_Missing} amState;
 } state_report_in_frame;
 
 
@@ -513,8 +516,7 @@ static GHashTable *rlc_lte_frame_report_hash = NULL;
 
 /* Add to the tree values associated with sequence analysis for this frame */
 static void addChannelSequenceInfo(state_report_in_frame *p,
-                                   guint16   UEId,
-                                   guint8    rlcMode,
+                                   rlc_lte_info *p_rlc_lte_info,
                                    guint16   sequenceNumber,
                                    gboolean  newSegmentStarted,
                                    rlc_lte_tap_info *tap_info,
@@ -534,7 +536,7 @@ static void addChannelSequenceInfo(state_report_in_frame *p,
                                          ett_rlc_lte_sequence_analysis);
     PROTO_ITEM_SET_GENERATED(seqnum_ti);
 
-    switch (rlcMode) {
+    switch (p_rlc_lte_info->rlcMode) {
         case RLC_AM_MODE:
 
             /********************************************/
@@ -547,7 +549,20 @@ static void addChannelSequenceInfo(state_report_in_frame *p,
                     PROTO_ITEM_SET_GENERATED(ti);
                     proto_item_append_text(seqnum_ti, " - OK");
                     break;
-        
+
+                case SN_MAC_Retx:
+                    ti = proto_tree_add_boolean(seqnum_tree, hf_rlc_lte_sequence_analysis_ok,
+                                                tvb, 0, 0, FALSE);
+                    PROTO_ITEM_SET_GENERATED(ti);
+                    ti = proto_tree_add_boolean(seqnum_tree, hf_rlc_lte_sequence_analysis_mac_retx,
+                                                tvb, 0, 0, TRUE);
+                    PROTO_ITEM_SET_GENERATED(ti);
+                    expert_add_info_format(pinfo, ti, PI_SEQUENCE, PI_WARN,
+                                           "AM Frame retransmitted for %s on UE %u - due to MAC retx!",
+                                           val_to_str(p_rlc_lte_info->direction, direction_vals, "Unknown"),
+                                           p_rlc_lte_info->ueid);
+                    break;
+
                 case SN_Retx:
                     ti = proto_tree_add_boolean(seqnum_tree, hf_rlc_lte_sequence_analysis_ok,
                                                 tvb, 0, 0, FALSE);
@@ -556,8 +571,9 @@ static void addChannelSequenceInfo(state_report_in_frame *p,
                                                 tvb, 0, 0, TRUE);
                     PROTO_ITEM_SET_GENERATED(ti);
                     expert_add_info_format(pinfo, ti, PI_SEQUENCE, PI_WARN,
-                                           "AM Frame retransmitted for UE %u - most likely in response to NACK",
-                                           UEId);
+                                           "AM Frame retransmitted for %s on UE %u - most likely in response to NACK",
+                                           val_to_str(p_rlc_lte_info->direction, direction_vals, "Unknown"),
+                                           p_rlc_lte_info->ueid);
                     proto_item_append_text(seqnum_ti, " - SN %u retransmitted", p->firstSN);
                     break;
 
@@ -569,8 +585,9 @@ static void addChannelSequenceInfo(state_report_in_frame *p,
                                                 tvb, 0, 0, TRUE);
                     PROTO_ITEM_SET_GENERATED(ti);
                     expert_add_info_format(pinfo, ti, PI_SEQUENCE, PI_WARN,
-                                           "AM SN Repeated for UE %u - probably because didn't receive Status PDU, or maybe MAC Retx?",
-                                           UEId);
+                                           "AM SN Repeated for %s for UE %u - probably because didn't receive Status PDU?",
+                                           val_to_str(p_rlc_lte_info->direction, direction_vals, "Unknown"),
+                                           p_rlc_lte_info->ueid);
                     proto_item_append_text(seqnum_ti, "- SN %u Repeated",
                                            p->sequenceExpected);
                     break;
@@ -584,16 +601,18 @@ static void addChannelSequenceInfo(state_report_in_frame *p,
                     PROTO_ITEM_SET_GENERATED(ti);
                     if (p->lastSN != p->firstSN) {
                         expert_add_info_format(pinfo, ti, PI_SEQUENCE, PI_WARN,
-                                               "AM SNs missing for UE %u (%u to %u)",
-                                               UEId, p->firstSN, p->lastSN);
+                                               "AM SNs missing for %s on UE %u (%u to %u)",
+                                               val_to_str(p_rlc_lte_info->direction, direction_vals, "Unknown"),
+                                               p_rlc_lte_info->ueid, p->firstSN, p->lastSN);
                         proto_item_append_text(seqnum_ti, " - SNs missing (%u to %u)",
                                                p->firstSN, p->lastSN);
                         tap_info->missingSNs = ((p->lastSN - p->firstSN) % 1024) + 1;
                     }
                     else {
                         expert_add_info_format(pinfo, ti, PI_SEQUENCE, PI_WARN,
-                                               "AM SN missing for UE %u (%u)",
-                                               UEId, p->firstSN);
+                                               "AM SN missing for %s on UE %u (%u)",
+                                               val_to_str(p_rlc_lte_info->direction, direction_vals, "Unknown"),
+                                               p_rlc_lte_info->ueid, p->firstSN);
                         proto_item_append_text(seqnum_ti, " - SN missing (%u)",
                                                p->firstSN);
                         tap_info->missingSNs = 1;
@@ -613,18 +632,18 @@ static void addChannelSequenceInfo(state_report_in_frame *p,
                 proto_tree_add_uint(seqnum_tree, hf_rlc_lte_sequence_analysis_previous_frame,
                                     tvb, 0, 0, p->previousFrameNum);
             }
-        
+
             /* Expected sequence number */
             ti = proto_tree_add_uint(seqnum_tree, hf_rlc_lte_sequence_analysis_expected_sn,
                                     tvb, 0, 0, p->sequenceExpected);
             PROTO_ITEM_SET_GENERATED(ti);
-        
-        
+
             if (!p->sequenceExpectedCorrect) {
                 /* Incorrect sequence number */
                 expert_add_info_format(pinfo, ti, PI_SEQUENCE, PI_WARN,
-                                       "Wrong Sequence Number for UE %u - got %u, expected %u",
-                                       UEId, sequenceNumber, p->sequenceExpected);
+                                       "Wrong Sequence Number for %s on UE %u - got %u, expected %u",
+                                       val_to_str(p_rlc_lte_info->direction, direction_vals, "Unknown"),
+                                       p_rlc_lte_info->ueid, sequenceNumber, p->sequenceExpected);
             }
             else {
                 /* Correct sequence number, so check frame indication bits consistent */
@@ -636,7 +655,7 @@ static void addChannelSequenceInfo(state_report_in_frame *p,
                         if (!p->sequenceExpectedCorrect) {
                             expert_add_info_format(pinfo, ti, PI_SEQUENCE, PI_WARN,
                                                    "Last segment of previous PDU was not continued for UE %u",
-                                                   UEId);
+                                                   p_rlc_lte_info->ueid);
                         }
                     }
                     else {
@@ -658,7 +677,7 @@ static void addChannelSequenceInfo(state_report_in_frame *p,
                        ti = proto_tree_add_boolean(seqnum_tree, hf_rlc_lte_sequence_analysis_framing_info_correct,
                                                    tvb, 0, 0, TRUE);
                     }
-        
+
                 }
                 PROTO_ITEM_SET_GENERATED(ti);
             }
@@ -686,8 +705,7 @@ static void checkChannelSequenceInfo(packet_info *pinfo, tvbuff_t *tvb,
         p_report_in_frame = (state_report_in_frame*)g_hash_table_lookup(rlc_lte_frame_report_hash,
                                                                         &pinfo->fd->num);
         if (p_report_in_frame != NULL) {
-            addChannelSequenceInfo(p_report_in_frame, p_rlc_lte_info->ueid,
-                                   p_rlc_lte_info->rlcMode,
+            addChannelSequenceInfo(p_report_in_frame, p_rlc_lte_info,
                                    sequenceNumber, first_includes_start,
                                    tap_info, pinfo, tree, tvb);
             return;
@@ -773,8 +791,17 @@ static void checkChannelSequenceInfo(packet_info *pinfo, tvbuff_t *tvb,
                - new SN, but with frames missed out
                Assume window whose front is at expectedSequenceNumber */
 
+            /* First of all, check to see whether frame is judged to be MAC Retx */
+            if (is_mac_lte_frame_retx(pinfo, p_rlc_lte_info->direction)) {
+                /* Just report that this is a MAC Retx */
+                p_report_in_frame->amState = SN_MAC_Retx;
+
+                /* No channel state to update */
+            }
+
+
             /* Expected? */
-            if (sequenceNumber == expectedSequenceNumber) {
+            else if (sequenceNumber == expectedSequenceNumber) {
 
                 /* Set report for this frame */
                 p_report_in_frame->sequenceExpectedCorrect = TRUE;
@@ -844,8 +871,7 @@ static void checkChannelSequenceInfo(packet_info *pinfo, tvbuff_t *tvb,
     g_hash_table_insert(rlc_lte_frame_report_hash, &pinfo->fd->num, p_report_in_frame);
 
     /* Add state report for this frame into tree */
-    addChannelSequenceInfo(p_report_in_frame, p_rlc_lte_info->ueid,
-                           p_rlc_lte_info->rlcMode, sequenceNumber,
+    addChannelSequenceInfo(p_report_in_frame, p_rlc_lte_info, sequenceNumber,
                            first_includes_start, tap_info, pinfo, tree, tvb);
 }
 
@@ -1083,6 +1109,7 @@ static void dissect_rlc_lte_am_status_pdu(tvbuff_t *tvb,
                                           proto_item *status_ti,
                                           int offset,
                                           proto_item *top_ti,
+                                          rlc_lte_info *p_rlc_lte_info,
                                           rlc_lte_tap_info *tap_info)
 {
     guint8     cpt;
@@ -1158,11 +1185,15 @@ static void dissect_rlc_lte_am_status_pdu(tvbuff_t *tvb,
             /* Report as expert info */
             if (e2) {
                 expert_add_info_format(pinfo, nack_ti, PI_SEQUENCE, PI_WARN,
-                                       "Status PDU reports NACK for SN=%u (partial)", (guint16)nack_sn);
+                                       "Status PDU reports NACK (partial) on %s for UE %u",
+                                       val_to_str(p_rlc_lte_info->direction, direction_vals, "Unknown"),
+                                       p_rlc_lte_info->ueid);
             }
             else {
                 expert_add_info_format(pinfo, nack_ti, PI_SEQUENCE, PI_WARN,
-                                       "Status PDU reports NACK for SN=%u", (guint16)nack_sn);
+                                       "Status PDU reports NACK on %s for UE %u",
+                                       val_to_str(p_rlc_lte_info->direction, direction_vals, "Unknown"),
+                                       p_rlc_lte_info->ueid);
             }
 
             bit_offset++;
@@ -1257,7 +1288,8 @@ static void dissect_rlc_lte_am(tvbuff_t *tvb, packet_info *pinfo,
 
         /* Control PDUs are a completely separate format  */
         dissect_rlc_lte_am_status_pdu(tvb, pinfo, am_header_tree, am_header_ti,
-                                      offset, top_ti, tap_info);
+                                      offset, top_ti,
+                                      p_rlc_lte_info, tap_info);
         return;
     }
 
@@ -1981,6 +2013,12 @@ void proto_register_rlc_lte(void)
               NULL, HFILL
             }
         },
+        { &hf_rlc_lte_sequence_analysis_mac_retx,
+            { "Frame retransmitted by MAC",
+              "rlc-lte.sequence-analysis.mac-retx", FT_BOOLEAN, BASE_NONE, 0, 0x0,
+              NULL, HFILL
+            }
+        },
         { &hf_rlc_lte_sequence_analysis_retx,
             { "Retransmitted frame",
               "rlc-lte.sequence-analysis.retx", FT_BOOLEAN, BASE_NONE, 0, 0x0,