Wireshark · Ethereal-dev: [Ethereal-dev] [Patch] tcp_graph fixes

["Bill Meier" <wmeier@xxxxxxxxxxx>]

After investigating the time-sequence graphs (Stevens and tcptrace) produced 
using an FTP capture file supplied by Eduardo Segura 
(see http://www.ethereal.com/lists/ethereal-users/200512/msg00153.html ) 
I've identified several problems in tcp_trace.c.

The problems mostly involve incorrect determination of the lower/upper 
sequence number bounds (for the Y axis) in certain cases (e.g. having to do
with 'partial' conversations).

I've reworked the '...get_bounds' code to handle cases such as:

1. out of order data segments (e.g.: the first segment in a captured
conversation has a higher sequence number than a later segment);

2. 'ack' sequence numbers for initial ack segments in a conversation lower
than the sequence numbers of the initial data segments;

3. maximum 'ack + win' sequence number in a conversation greater than the 
max data sequence number;

4. Stevens graph: only use data segment sequence numbers when 
determining bounds;

5. TCP RST packet without 'ack' flag: do not try to use the 'ack' seq num from
the packet in this case. (This was the specific cause of the originally reported
problem).


I've also reworked the tcptrace display code slightly to properly handle 
the initial ack packet of a sequence;

As an example of the some of the fixes the Ethereal tcptrace style graph 
of the following conversation fragment will now be similar to the graph 
produced by Tcptrace.

data:  seq 10000 len 100
data:  seq 10100 len 200
ack:   ack 5000  win 6000
ack:   ack 5400  win 5600 


Please apply the attached patch or if any concerns please let me know.

Thanks

Bill Meier




*** gtk/tcp_graph.c	Fri Dec 16 11:43:54 2005
--- tcp_graph_yy.c	Tue Dec 20 20:17:00 2005
***************
*** 60,65 ****
--- 60,66 ----
  
  #define TCP_SYN(flags)		( flags & TH_SYN )
  #define TCP_ACK(flags)		( flags & TH_ACK )
+ #define TCP_FIN(flags)		( flags & TH_FIN )
  
  #define TXT_WIDTH	850
  #define TXT_HEIGHT	550
***************
*** 442,449 ****
  static gint key_release_event (GtkWidget * , GdkEventKey * );
  static gint leave_notify_event (GtkWidget * , GdkEventCrossing * );
  static gint enter_notify_event (GtkWidget * , GdkEventCrossing * );
! static void tseq_stevens_initialize (struct graph * );
! static void tseq_stevens_get_bounds (struct graph * );
  static void tseq_stevens_read_config (struct graph * );
  static void tseq_stevens_make_elmtlist (struct graph * );
  static void tseq_stevens_toggle_seq_origin (struct graph * );
--- 443,450 ----
  static gint key_release_event (GtkWidget * , GdkEventKey * );
  static gint leave_notify_event (GtkWidget * , GdkEventCrossing * );
  static gint enter_notify_event (GtkWidget * , GdkEventCrossing * );
! static void tseq_initialize (struct graph * );
! static void tseq_get_bounds (struct graph * );
  static void tseq_stevens_read_config (struct graph * );
  static void tseq_stevens_make_elmtlist (struct graph * );
  static void tseq_stevens_toggle_seq_origin (struct graph * );
***************
*** 1688,1694 ****
  	switch (g->type) {
  	case GRAPH_TSEQ_STEVENS:
  	case GRAPH_TSEQ_TCPTRACE:
! 		tseq_stevens_initialize (g);
  		break;
  	case GRAPH_THROUGHPUT:
  		tput_initialize (g);
--- 1689,1695 ----
  	switch (g->type) {
  	case GRAPH_TSEQ_STEVENS:
  	case GRAPH_TSEQ_TCPTRACE:
! 		tseq_initialize (g);
  		break;
  	case GRAPH_THROUGHPUT:
  		tput_initialize (g);
***************
*** 3310,3319 ****
  	g->x_axis->label[1] = NULL;
  }
  
! static void tseq_stevens_initialize (struct graph *g)
  {
! 	debug(DBS_FENTRY) puts ("tseq_stevens_initialize()");
! 	tseq_stevens_get_bounds (g);
  
  	g->x_axis->min = 0;
  	g->y_axis->min = 0;
--- 3311,3321 ----
  	g->x_axis->label[1] = NULL;
  }
  
! /* Used by both 'stevens' and 'tcptrace' */
! static void tseq_initialize (struct graph *g)
  {
! 	debug(DBS_FENTRY) puts ("tseq_initialize()");
! 	tseq_get_bounds (g);
  
  	g->x_axis->min = 0;
  	g->y_axis->min = 0;
***************
*** 3328,3392 ****
  	}
  }
  
! static void tseq_stevens_get_bounds (struct graph *g)
  {
! 	struct segment *tmp, *last, *first;
! 	double t, t0, tmax, ymax;
! 	guint32 seq_base;
! 	guint32 seq_cur;
! 	guint32 ack_base = 0;
  
! 	for (first=g->segments; first->next; first=first->next) {
! 		if(compare_headers(&g->current->ip_src, &g->current->ip_dst,
! 				   g->current->th_sport, g->current->th_dport,
! 				   &first->ip_src, &first->ip_dst,
! 				   first->th_sport, first->th_dport,
! 				   COMPARE_CURR_DIR)) {
! 			break;
! 		}
! 	}
! 	last = NULL;
! 	ymax = 0;
! 	tmax = 0;
! 	
! 	seq_base = first->th_seq;
  	for (tmp=g->segments; tmp; tmp=tmp->next) {
- 		unsigned int highest_byte_num;
- 		last = tmp;
  		if(compare_headers(&g->current->ip_src, &g->current->ip_dst,
  				   g->current->th_sport, g->current->th_dport,
  				   &tmp->ip_src, &tmp->ip_dst,
  				   tmp->th_sport, tmp->th_dport,
  				   COMPARE_CURR_DIR)) {
! 			seq_cur = tmp->th_seq -seq_base;
! 			highest_byte_num = seq_cur + tmp->th_seglen;
! 		}
! 		else {
! 			seq_cur = tmp->th_ack;
! 			if (!ack_base)
! 				ack_base = seq_cur;
! 			highest_byte_num = seq_cur - ack_base;
  		}
- 		if (highest_byte_num > ymax)
- 			ymax = highest_byte_num;
- 		t = tmp->rel_secs + tmp->rel_usecs / 1000000.0;
- 		if (t > tmax)
- 			tmax = t;
  	}
! 	if (!last) {
! 		puts ("tseq_stevens_get_bounds: segment list corrupted!");
! 		return;
  	}
  
- 	t0 = g->segments->rel_secs + g->segments->rel_usecs / 1000000.0;
- 	g->bounds.x0 = t0;
- 	g->bounds.y0 = seq_base;
- 	g->bounds.width = tmax - t0;
- 	g->bounds.height = ymax;
  	g->zoom.x = (g->geom.width - 1) / g->bounds.width;
  	g->zoom.y = (g->geom.height -1) / g->bounds.height;
  }
  
  static void tseq_stevens_make_elmtlist (struct graph *g)
  {
  	struct segment *tmp;
--- 3330,3430 ----
  	}
  }
  
! 
! /* Determine "bounds"
!  *  Essentially: look for lowest/highest time and seq in the list of segments
!  *  Note that for tcptrace the "(ack + window) sequence number" would normally be expected 
!  *   to be the upper bound; However, just to be safe, include the data seg sequence numbers
!  *   in the comparison for tcptrace
!  *   (e.g. to handle the case of only data segments).
!  */
! 
! /* ToDo: worry about handling cases such as trying to plot seq of just 1 frame  */
! 
! static void tseq_get_bounds (struct graph *g)
  {
! 	struct segment *tmp;
! 	double   tim;
! 	gboolean data_frame_seen=FALSE;
! 	double   data_tim_low;
! 	double   data_tim_high;
! 	guint32  data_seq_cur;
! 	guint32  data_seq_nxt;
! 	guint32  data_seq_low;
! 	guint32  data_seq_high;
! 	gboolean ack_frame_seen=FALSE;
! 	double   ack_tim_low;
! 	double   ack_tim_high;
! 	guint32  ack_seq_cur;
! 	guint32  ack_seq_low;
! 	guint32  win_seq_cur;
! 	guint32  win_seq_high;
  
! 	/* go thru all segments to determine "bounds" */
  	for (tmp=g->segments; tmp; tmp=tmp->next) {
  		if(compare_headers(&g->current->ip_src, &g->current->ip_dst,
  				   g->current->th_sport, g->current->th_dport,
  				   &tmp->ip_src, &tmp->ip_dst,
  				   tmp->th_sport, tmp->th_dport,
  				   COMPARE_CURR_DIR)) {
! 
! 			/* "data" seg */
! 			tim = tmp->rel_secs + tmp->rel_usecs / 1000000.0;
! 			data_seq_cur = tmp->th_seq;
! 			data_seq_nxt = data_seq_cur + tmp->th_seglen;
! 			if (! data_frame_seen) {
! 				data_tim_low    = data_tim_high = tim;
! 				data_seq_low    = data_seq_cur;
! 				data_seq_high   = data_seq_nxt;
! 				data_frame_seen = TRUE;
! 			}
! 			if (tim          < data_tim_low)  data_tim_low  = tim;
! 			if (tim          > data_tim_high) data_tim_high = tim;
! 			if (data_seq_cur < data_seq_low)  data_seq_low  = data_seq_cur;
! 			if (data_seq_nxt > data_seq_high) data_seq_high = data_seq_nxt;
! 		}
! 		else { /* ack seg */
! 			/* skip ack processing if no ACK (e.g. in RST) */
! 			if (TCP_ACK (tmp->th_flags)) {
! 				tim = tmp->rel_secs + tmp->rel_usecs / 1000000.0;
! 				ack_seq_cur = tmp->th_ack;
! 				win_seq_cur = ack_seq_cur + tmp->th_win;
! 				if (! ack_frame_seen) {
! 					ack_tim_low  = ack_tim_high = tim;
! 					ack_seq_low  = ack_seq_cur;
! 					win_seq_high = win_seq_cur;
! 					ack_frame_seen = TRUE;
! 				}
! 				if (tim         < ack_tim_low)  ack_tim_low  = tim;
! 				if (tim         > ack_tim_high) ack_tim_high = tim;
! 				if (ack_seq_cur < ack_seq_low)  ack_seq_low  = ack_seq_cur;
! 				if (win_seq_cur > win_seq_high) win_seq_high = win_seq_cur;
! 			}
  		}
  	}
! 
!         /* if 'stevens':  use only data segments to determine bounds         */
! 	/* if 'tcptrace': use both data and ack segments to determine bounds */
! 	switch (g->type) {
! 	case GRAPH_TSEQ_STEVENS:
! 		g->bounds.x0     = data_tim_low;
! 		g->bounds.width  = data_tim_high - data_tim_low;
! 		g->bounds.y0     = data_seq_low;
! 		g->bounds.height = data_seq_high - data_seq_low;
! 		break;
! 	case GRAPH_TSEQ_TCPTRACE:
! 		g->bounds.x0     = (data_tim_low <= ack_tim_low)    ? data_tim_low  : ack_tim_low;
! 		g->bounds.width  = ((data_tim_high >= ack_tim_high) ? data_tim_high : ack_tim_high) - g->bounds.x0; ;
! 		g->bounds.y0     = (data_seq_low <= ack_seq_low)    ? data_seq_low  : ack_seq_low;;
! 		g->bounds.height = ((data_seq_high >= win_seq_high) ? data_seq_high : win_seq_high) - g->bounds.y0; ;
! 		break;
  	}
  
  	g->zoom.x = (g->geom.width - 1) / g->bounds.width;
  	g->zoom.y = (g->geom.height -1) / g->bounds.height;
  }
  
+ 
  static void tseq_stevens_make_elmtlist (struct graph *g)
  {
  	struct segment *tmp;
***************
*** 3412,3417 ****
--- 3450,3456 ----
  				   COMPARE_CURR_DIR)) {
  			continue;
  		}
+ 		/* data seg */
  		seq_cur = tmp->th_seq - seq_base;
  		secs = g->zoom.x * (tmp->rel_secs + tmp->rel_usecs / 1000000.0 - x0);
  		seqno = g->zoom.y * seq_cur;
***************
*** 3537,3542 ****
--- 3576,3582 ----
  	double x0, y0;
  	double p_t; /* ackno, window and time of previous segment */
  	double p_ackno, p_win;
+ 	gboolean ack_seen=FALSE;
  	int toggle=0;
  	guint32 seq_base;
  	guint32 seq_cur;
***************
*** 3558,3578 ****
  	x0 = g->bounds.x0;
  	y0 = g->bounds.y0;
  	seq_base = (guint32) y0;
! 	/* initialize "previous" values */
! 	for (tmp=g->segments; tmp; tmp=tmp->next)
! 		if(!compare_headers(&g->current->ip_src, &g->current->ip_dst,
! 				   g->current->th_sport, g->current->th_dport,
! 				   &tmp->ip_src, &tmp->ip_dst,
! 				   tmp->th_sport, tmp->th_dport,
! 				   COMPARE_CURR_DIR)) {
! 			break;
! 		}
! 	/*
! 	p_ackno = (unsigned int )(g->zoom.y * (tmp->th_ack - y0));
! 	 */
! 	p_ackno = 0;
! 	p_win = g->zoom.y * tmp->th_win;
! 	p_t = g->segments->rel_secs + g->segments->rel_usecs/1000000.0 - x0;
  	for (tmp=g->segments; tmp; tmp=tmp->next) {
  		double secs, data;
  		double x;
--- 3598,3604 ----
  	x0 = g->bounds.x0;
  	y0 = g->bounds.y0;
  	seq_base = (guint32) y0;
! 
  	for (tmp=g->segments; tmp; tmp=tmp->next) {
  		double secs, data;
  		double x;
***************
*** 3588,3595 ****
  			/* forward direction -> we need seqno and amount of data */
  			double y1, y2;
  
! 			seq_cur = tmp->th_seq -seq_base;
! 			if (TCP_SYN (tmp->th_flags))
  				data = 1;
  			else
  				data = tmp->th_seglen;
--- 3614,3621 ----
  			/* forward direction -> we need seqno and amount of data */
  			double y1, y2;
  
! 			seq_cur = tmp->th_seq - seq_base;
! 			if (TCP_SYN (tmp->th_flags) || TCP_FIN (tmp->th_flags))
  				data = 1;
  			else
  				data = tmp->th_seglen;
***************
*** 3619,3626 ****
  			e1++;
  		} else {
  			double ackno, win;
! 			if (TCP_SYN (tmp->th_flags) && ! TCP_ACK (tmp->th_flags))
! 				/* SYN's have ACK==0 and are useless here */
  				continue;
  			/* backward direction -> we need ackno and window */
  			seq_cur = tmp->th_ack - seq_base;
--- 3645,3652 ----
  			e1++;
  		} else {
  			double ackno, win;
! 			if (! TCP_ACK (tmp->th_flags))
! 				/* SYN's and RST's do not necessarily have ACK's*/
  				continue;
  			/* backward direction -> we need ackno and window */
  			seq_cur = tmp->th_ack - seq_base;
***************
*** 3628,3670 ****
  			win = tmp->th_win * g->zoom.y;
  
  			/* ack line */
! 			e0->type = ELMT_LINE;
! 			e0->parent = tmp;
! 			e0->gc = g->s.tseq_tcptrace.gc_ack[toggle];
! 			e0->p.line.dim.x1 = p_t;
! 			e0->p.line.dim.y1 = p_ackno;
! 			e0->p.line.dim.x2 = x;
! 			e0->p.line.dim.y2 = p_ackno;
! 			e0++;
! 			e0->type = ELMT_LINE;
! 			e0->parent = tmp;
! 			e0->gc = g->s.tseq_tcptrace.gc_ack[toggle];
! 			e0->p.line.dim.x1 = x;
! 			e0->p.line.dim.y1 = p_ackno;
! 			e0->p.line.dim.x2 = x;
! 			e0->p.line.dim.y2 = ackno!=p_ackno || ackno<4 ? ackno : ackno-4;
! 			e0++;
! 			/* window line */
! 			e0->type = ELMT_LINE;
! 			e0->parent = tmp;
! 			e0->gc = g->s.tseq_tcptrace.gc_ack[toggle];
! 			e0->p.line.dim.x1 = p_t;
! 			e0->p.line.dim.y1 = p_win + p_ackno;
! 			e0->p.line.dim.x2 = x;
! 			e0->p.line.dim.y2 = p_win + p_ackno;
! 			e0++;
! 			e0->type = ELMT_LINE;
! 			e0->parent = tmp;
! 			e0->gc = g->s.tseq_tcptrace.gc_ack[toggle];
! 			e0->p.line.dim.x1 = x;
! 			e0->p.line.dim.y1 = p_win + p_ackno;
! 			e0->p.line.dim.x2 = x;
! 			e0->p.line.dim.y2 = win + ackno;
! 			e0++;
  			p_ackno = ackno;
  			p_win = win;
  			p_t = x;
- 			toggle = 1^toggle;
  		}
  	}
  	e0->type = ELMT_NONE;
--- 3654,3699 ----
  			win = tmp->th_win * g->zoom.y;
  
  			/* ack line */
! 			if (ack_seen == TRUE) { /* don't plot the first ack */
! 				e0->type = ELMT_LINE;
! 				e0->parent = tmp;
! 				e0->gc = g->s.tseq_tcptrace.gc_ack[toggle];
! 				e0->p.line.dim.x1 = p_t;
! 				e0->p.line.dim.y1 = p_ackno;
! 				e0->p.line.dim.x2 = x;
! 				e0->p.line.dim.y2 = p_ackno;
! 				e0++;
! 				e0->type = ELMT_LINE;
! 				e0->parent = tmp;
! 				e0->gc = g->s.tseq_tcptrace.gc_ack[toggle];
! 				e0->p.line.dim.x1 = x;
! 				e0->p.line.dim.y1 = p_ackno;
! 				e0->p.line.dim.x2 = x;
! 				e0->p.line.dim.y2 = ackno!=p_ackno || ackno<4 ? ackno : ackno-4;
! 				e0++;
! 				/* window line */
! 				e0->type = ELMT_LINE;
! 				e0->parent = tmp;
! 				e0->gc = g->s.tseq_tcptrace.gc_ack[toggle];
! 				e0->p.line.dim.x1 = p_t;
! 				e0->p.line.dim.y1 = p_win + p_ackno;
! 				e0->p.line.dim.x2 = x;
! 				e0->p.line.dim.y2 = p_win + p_ackno;
! 				e0++;
! 				e0->type = ELMT_LINE;
! 				e0->parent = tmp;
! 				e0->gc = g->s.tseq_tcptrace.gc_ack[toggle];
! 				e0->p.line.dim.x1 = x;
! 				e0->p.line.dim.y1 = p_win + p_ackno;
! 				e0->p.line.dim.x2 = x;
! 				e0->p.line.dim.y2 = win + ackno;
! 				e0++;
! 				toggle = 1^toggle;
! 			}
! 			ack_seen = TRUE;
  			p_ackno = ackno;
  			p_win = win;
  			p_t = x;
  		}
  	}
  	e0->type = ELMT_NONE;