Qt: modify RTT graph (handle GSO, SACK, etc), plus bug fixes

Modifications to RTT graph:
 - change x-axis to time (s) rather than sequence number
    [ avoids sequence number wraparound ambiguity, plus
       easier to correlate RTT changes to tcptrace graph ]
 - change RTT computation to properly handle acks to GSO packets
 - change RTT computation to take SACK blocks into account

Bug fixes:
 - eliminate potential memory leak if some packets are unacked
 - ensure RTT graph is shown if TCPGraph window is opened to it directly

Change-Id: I2bdcab97399ebde0f15c78fa19c882529a814580
Reviewed-on: https://code.wireshark.org/review/19662
Petri-Dish: Alexis La Goutte <alexis.lagoutte@gmail.com>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Alexis La Goutte <alexis.lagoutte@gmail.com>
Reviewed-by: Anders Broman <a.broman58@gmail.com>

1 files changed, 39 insertions, 16 deletions

diff --git a/ui/gtk/tcp_graph.c b/ui/gtk/tcp_graph.c
index 5c57cd2d41..e7658ee878 100644
--- a/ui/gtk/tcp_graph.c
+++ b/ui/gtk/tcp_graph.c
@@ -4230,7 +4230,7 @@ static void rtt_read_config(struct gtk_graph *g)
 static void rtt_initialize(struct gtk_graph *g)
 {
     struct segment *tmp, *first = NULL;
-    struct unack   *unack       = NULL, *u;
+    struct rtt_unack   *unack   = NULL, *u;
     double          rttmax      = 0;
     double          xx0, yy0, ymax;
     guint32         xmax        = 0;
@@ -4255,7 +4255,7 @@ static void rtt_initialize(struct gtk_graph *g)
             seqno -= seq_base;
             if (tmp->th_seglen && !rtt_is_retrans(unack, seqno)) {
                 double time_val = tmp->rel_secs + tmp->rel_usecs / 1000000.0;
-                u = rtt_get_new_unack(time_val, seqno);
+                u = rtt_get_new_unack(time_val, seqno, tmp->th_seglen);
                 if (!u) return;
                 rtt_put_unack_on_list(&unack, u);
             }
@@ -4263,22 +4263,33 @@ static void rtt_initialize(struct gtk_graph *g)
             if (seqno + tmp->th_seglen > xmax)
                 xmax = seqno + tmp->th_seglen;
         } else if (first) {
-            guint32       ackno    = tmp->th_ack -seq_base;
+            guint32       ackno    = tmp->th_ack - seq_base;
             double        time_val = tmp->rel_secs + tmp->rel_usecs / 1000000.0;
-            struct unack *v;
+            struct rtt_unack *v;
 
             for (u=unack; u; u=v) {
-                if (ackno > u->seqno) {
+                if (tcp_seq_after(ackno, u->seqno)) {
                     double rtt = time_val - u->time;
                     if (rtt > rttmax)
                         rttmax = rtt;
+                    if (tcp_seq_eq_or_after(ackno, u->end_seqno)) {
+                        // fully acked segment - can safely be deleted
+                        v = u->next;
+                        rtt_delete_unack_from_list(&unack, u);
+                    } else {
+                        // partial ACK of GSO segment -
+                        //   just modify segment to reflect ACKed bytes
+                        u->seqno = ackno;
+                    }
+                } else {
+                    // nothing was acked in this seg - just move along
                     v = u->next;
-                    rtt_delete_unack_from_list(&unack, u);
-                } else
-                    v = u->next;
+                }
             }
         }
     }
+    // it's possible there's still unacked segs - so be sure to free list!
+    rtt_destroy_unack_list(&unack);
 
     xx0  = seq_base;
     yy0  = 0;
@@ -4295,7 +4306,7 @@ static void rtt_initialize(struct gtk_graph *g)
 static void rtt_make_elmtlist(struct gtk_graph *g)
 {
     struct segment *tmp;
-    struct unack   *unack    = NULL, *u;
+    struct rtt_unack  *unack = NULL, *u;
     struct element *elements, *e;
     guint32         seq_base = (guint32) g->bounds.x0;
 
@@ -4318,17 +4329,17 @@ static void rtt_make_elmtlist(struct gtk_graph *g)
 
             if (tmp->th_seglen && !rtt_is_retrans(unack, seqno)) {
                 double time_val = tmp->rel_secs + tmp->rel_usecs / 1000000.0;
-                u = rtt_get_new_unack(time_val, seqno);
+                u = rtt_get_new_unack(time_val, seqno, tmp->th_seglen);
                 if (!u) return;
                 rtt_put_unack_on_list(&unack, u);
             }
         } else {
-            guint32       ackno    = tmp->th_ack -seq_base;
+            guint32       ackno    = tmp->th_ack - seq_base;
             double        time_val = tmp->rel_secs + tmp->rel_usecs / 1000000.0;
-            struct unack *v;
+            struct rtt_unack *v;
 
             for (u=unack; u; u=v) {
-                if (ackno > u->seqno) {
+                if (tcp_seq_after(ackno, u->seqno)) {
                     double rtt = time_val - u->time;
 
                     e->type   = ELMT_ELLIPSE;
@@ -4339,13 +4350,25 @@ static void rtt_make_elmtlist(struct gtk_graph *g)
                     e->p.ellipse.dim.y      = g->zoom.y * rtt + g->s.rtt.height/2.0;
                     e++;
 
+                    if (tcp_seq_eq_or_after(ackno, u->end_seqno)) {
+                        // fully acked segment - can safely be deleted
+                        v = u->next;
+                        rtt_delete_unack_from_list(&unack, u);
+                    } else {
+                        // partial ACK of GSO segment -
+                        //   just modify segment to reflect ACKed bytes
+                        u->seqno = ackno;
+                    }
+                } else {
+                    // nothing was acked in this seg - just move along
                     v = u->next;
-                    rtt_delete_unack_from_list(&unack, u);
-                } else
-                    v = u->next;
+                }
             }
         }
     }
+    // it's possible there's still unacked segs - so be sure to free list!
+    rtt_destroy_unack_list(&unack);
+
     e->type = ELMT_NONE;
     g->elists->elements = elements;
 }