Improve the way PDCP status PDUs are shown.

For the bitmap, present and missing SNs are both
interesting, but only until the last received SN 
(i.e. the last byte of the bitmap will be padded
out with zero bits).

It would be possible to use existing sequence analysis
info, where available, to decide if any zeros beyond the last
received SN are interesting...

svn path=/trunk/; revision=49959

1 files changed, 63 insertions, 9 deletions

diff --git a/epan/dissectors/packet-pdcp-lte.c b/epan/dissectors/packet-pdcp-lte.c
index 770b51fcd0..286d6b9c32 100644
--- a/epan/dissectors/packet-pdcp-lte.c
+++ b/epan/dissectors/packet-pdcp-lte.c
@@ -90,6 +90,7 @@ static int hf_pdcp_lte_fms = -1;
 static int hf_pdcp_lte_reserved4 = -1;
 static int hf_pdcp_lte_fms2 = -1;
 static int hf_pdcp_lte_bitmap = -1;
+static int hf_pdcp_lte_bitmap_received = -1;
 static int hf_pdcp_lte_bitmap_not_received = -1;
 
 
@@ -1173,7 +1174,6 @@ static void dissect_pdcp_lte(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree
                         {
                             guint16 fms;
                             guint16 modulo;
-                            guint   not_received = 0;
                             guint   sn;
                             proto_tree *bitmap_tree;
                             proto_item *bitmap_ti = NULL;
@@ -1211,33 +1211,81 @@ static void dissect_pdcp_lte(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree
                                 modulo = 32768;
                             }
 
-                            /* Bitmap tree */
+                            /* Any remaining bytes are the bitmap */
                             if (tvb_length_remaining(tvb, offset) > 0) {
+
+                                gint  bitmap_start_offset = offset;
+                                guint bitmap_start_sn = sn;
+                                guint last_seen_sn = 0;
+                                gboolean last_seen_sn_set = FALSE;
+                                gboolean last_seen_sn_added = FALSE;
+                                guint   not_received = 0;
+
+                                /* Create bitmap subtree */
                                 bitmap_ti = proto_tree_add_item(pdcp_tree, hf_pdcp_lte_bitmap, tvb,
                                                                 offset, -1, ENC_NA);
                                 bitmap_tree = proto_item_add_subtree(bitmap_ti, ett_pdcp_report_bitmap);
 
+                                /* We are only bits describing SNs
+                                   *before* the last received one, so need to scan first
+                                   to find the last received one.
+                                   There *should* be at least one bit set, otherwise
+                                   no point in encoding the optional bitmap! */
 
                                 /* For each byte... */
                                 for ( ; tvb_length_remaining(tvb, offset); offset++) {
                                     guint bit_offset = 0;
-                                    /* .. look for error (0) in each bit */
+                                    /* .. look for bits set, in order */
+                                    for ( ; bit_offset < 8; bit_offset++) {
+                                        if ((tvb_get_guint8(tvb, offset) >> (7-bit_offset) & 0x1) == 1) {
+                                            /* (over)write any stored seen value */
+                                            last_seen_sn = sn;
+                                            last_seen_sn_set = TRUE;
+                                            break;
+                                        }
+                                        sn = (sn + 1) % modulo;
+                                    }
+                                }
+
+                                if (!last_seen_sn_set) {
+                                    /* TODO: new-style exert info complaining that no bits were set! */
+                                    return;
+                                }
+
+                                /* Now go through bits again, stopping after last received SN */
+                                sn = bitmap_start_sn;
+                                offset = bitmap_start_offset;
+                                for ( ; !last_seen_sn_added && tvb_length_remaining(tvb, offset); offset++) {
+                                    guint bit_offset = 0;
+                                    /* Show each bit until last received one is done */
                                     for ( ; bit_offset < 8; bit_offset++) {
                                         if ((tvb_get_guint8(tvb, offset) >> (7-bit_offset) & 0x1) == 0) {
                                             proto_tree_add_boolean_bits_format_value(bitmap_tree, hf_pdcp_lte_bitmap_not_received, tvb, offset*8 + bit_offset,
                                                                                      1, 0, " (SN=%u)", sn);
                                             not_received++;
                                         }
+                                        else {
+                                            proto_tree_add_boolean_bits_format_value(bitmap_tree, hf_pdcp_lte_bitmap_received, tvb, offset*8 + bit_offset,
+                                                                                     1, 0, " (SN=%u)", sn);
+                                            if (sn == last_seen_sn) {
+                                                last_seen_sn_added= TRUE;
+                                                break;
+                                            }
+                                        }
                                         sn = (sn + 1) % modulo;
                                     }
                                 }
-                            }
 
-                            if (bitmap_ti != NULL) {
-                                proto_item_append_text(bitmap_ti, " (%u SNs not received)", not_received);
+                                proto_item_append_text(bitmap_ti, " (%u SNs missing before last seen SN %u)",
+                                                       not_received, last_seen_sn);
+
+                                /* Missing PDUs are fms + extra ones seen in bitmap before last seen SN */
+                                write_pdu_label_and_info(root_ti, pinfo, " Status Report (fms=%u, last-seen-sn=%u, %u missing)",
+                                                         fms, last_seen_sn, 1+not_received);
+                            }
+                            else {
+                                write_pdu_label_and_info(root_ti, pinfo, " Status Report (fms=%u)", fms);
                             }
-                            write_pdu_label_and_info(root_ti, pinfo, " Status Report (fms=%u) not-received=%u",
-                                                    fms, not_received);
                         }
                         return;
 
@@ -1611,10 +1659,16 @@ void proto_register_pdcp(void)
               "Status report bitmap (0=error, 1=OK)", HFILL
             }
         },
+        { &hf_pdcp_lte_bitmap_received,
+            { "Received",
+              "pdcp-lte.bitmap.ok", FT_BOOLEAN, BASE_NONE, NULL, 0x0,
+              "Status report received SN", HFILL
+            }
+        },
         { &hf_pdcp_lte_bitmap_not_received,
             { "Not Received",
               "pdcp-lte.bitmap.error", FT_BOOLEAN, BASE_NONE, NULL, 0x0,
-              "Status report PDU error", HFILL
+              "Status report missing SN", HFILL
             }
         },