Full-TCP ECN Bug Fixes


January 19, 2005
Symptom: "recvd ecnecho but I am not ECN capable!" error messages
Problem: The ECN bits in the header aren't initialized. So, when TCP agents are re-used, you'll get senders that don't think they are ECN-capable.
Fix: If the bits aren't set to 1, set them to 0.
(See email sent to ns-users list)

FullTcpAgent::sendpacket():

*** 837,842 ****
--- 851,860 ----
  	if ( datalen > 0 && ecn_ ){
  		fh->ect() = ect_;	// on after mutual agreement on ECT
  	}
+ 	else {
+ 		/* Set ect() to 0.  -M. Weigle 1/19/05 */
+ 		fh->ect() = 0;
+ 	}
  
          // fill in CWR and ECE bits which don't actually sit in
          // the tcp_flags but in hdr_flags
***************
*** 848,861 ****
          if ( pflags & TH_CWR ) {
                  fh->cong_action() = 1;
          }
! 
! 	//
! 	// although CWR bit is ordinarily associated with ECN,
! 	// it utility within the simulator for traces.  Thus, set
! 	// it even if we aren't doing ECN
! 	//
! 	if ( datalen > 0 )
! 	 	fh->cwr() =  cong_action_;
  
  	/* actual size is data length plus header length */
  
--- 866,875 ----
          if ( pflags & TH_CWR ) {
                  fh->cong_action() = 1;
          }
! 	else {
! 		/* Set cong_action() to 0  -M. Weigle 1/19/05 */
! 		fh->cong_action() = 0;
! 	}
  
  	/* actual size is data length plus header length */

July 17, 2002
Symptom: <invalid> flags reported when using prpkt()
Problem: flagstr() doesn't include strings for ECE or CWR flags
Fix: Add the cases.

FullTcpAgent::flagstr():

*** 558,565 ****
  		"<ACK>", "<ACK,FIN>", "<ACK,SYN>", "<ACK,SYN,FIN>", // 0x14-0x17
  		"<PSH,ACK>", "<PSH,ACK,FIN>", "<PSH,ACK,SYN>", "<PSH,ACK,SYN,FIN>", // 0x18-0x1b
  	};
! 	if (hflags < 0 || (hflags > 28))
! 		return ("<invalid>");
  	return (flagstrs[hflags]);
  }
  
--- 558,578 ----
  		"<ACK>", "<ACK,FIN>", "<ACK,SYN>", "<ACK,SYN,FIN>", // 0x14-0x17
  		"<PSH,ACK>", "<PSH,ACK,FIN>", "<PSH,ACK,SYN>", "<PSH,ACK,SYN,FIN>", // 0x18-0x1b
  	};
! 	if (hflags < 0 || (hflags > 28)) {
! 		/* Added strings for CWR and ECE  -M. Weigle 6/27/02 */
! 		if (hflags == 72) 
! 	 		return ("<ECE,PSH>");
! 	 	else if (hflags == 80)
! 	 		return ("<ECE,ACK>");
! 	 	else if (hflags == 88) 
! 	 		return ("<ECE,PSH,ACK>");
! 	 	else if (hflags == 152) 
! 	 		return ("<CWR,PSH,ACK>");
! 		else if (hflags == 153)
! 			return ("<CWR,PSH,ACK,FIN>");
! 		else
! 			return ("<invalid>");
! 	}
  	return (flagstrs[hflags]);
  }

June 19, 2002
Problem: CWR bit is set on retransmissions. According to RFC 3168, the CWR bit should be set on the first new data packet after a congestion window reduction and not on retransmissions.
Example: ns tcp-full-bugs.tcl ecn-rexmt (Results)
Fix: Check to make sure the packet isn't a retransmission before setting the CWR bit and before clearing the cong_action_ flag.

(The code for this fix is included in the fix for the next bug.)

June 12, 2002
Symptom: "recvd ecnecho but I am not ECN capable!" error messages
Problem: ECN is not being used even though both sides requested it. When a ECN-enabled SYN is received, the receiver sets ect_. If the ECN-enabled SYN+ACK is dropped and a timeout occurs, cong_action_ is set. Then, when the SYN+ACK is retranmitted, it has the CWR bit set, which is not a valid ECN SYN+ACK.
Example: ns tcp-full-bugs.tcl ecn-drop-synack (Results)
Fix: Don't set CWR if the packet is a SYN+ACK. Move setting CWR from sendpacket() to foutput() so this is done correctly.
(See email sent to ns-users mailing list)

(The code below shows fixes for both of these bugs)

FullTcpAgent::sendpacket():

- //
- // although CWR bit is ordinarily associated with ECN,
- // it utility within the simulator for traces.  Thus, set
- // it even if we aren't doing ECN
- //
- if ( datalen > 0 )
-	fh->cwr() = cong_action_;
----

FullTcpAgent::foutput():

*** 1068,1076 ****
                  pflags |= TH_ECE;
                  pflags &= ~TH_CWR;
          }
!   
!         /* set CWR if necessary */
!         if (ecn_ && ect_ && cong_action_) pflags |= TH_CWR;
    
          /* set ECE if necessary */
          if (ecn_ && ect_ && recent_ce_ ) pflags |= TH_ECE;
--- 1082,1105 ----
                  pflags |= TH_ECE;
                  pflags &= ~TH_CWR;
          }
! 	else if (ecn_ && ect_ && cong_action_ && !is_retransmit) 
! 		/* 
! 		 * Don't set CWR for a retranmitted SYN+ACK (has ecn_ 
! 		 * and cong_action_ set) or on any retransmits.
! 		 * -M. Weigle 6/19/02
! 		 */
! 		/* set CWR if necessary */
! 		pflags |= TH_CWR;
! 
! 	/* moved from sendpacket()  -M. Weigle 6/19/02 */
! 	//
! 	// although CWR bit is ordinarily associated with ECN,
! 	// it has utility within the simulator for traces. Thus, set
! 	// it even if we aren't doing ECN
! 	//
! 	if (datalen > 0 && cong_action_ && !is_retransmit) {
! 		pflags |= TH_CWR;
! 	}
    
          /* set ECE if necessary */
          if (ecn_ && ect_ && recent_ce_ ) pflags |= TH_ECE;

FullTcpAgent::foutput():

*** 1107,1113 ****
  	 */
  
  	int reliable = datalen + syn + fin; // seq #'s reliably sent
! 	if (cong_action_ && reliable > 0)
  		cong_action_ = FALSE;
  
  	// highest: greatest sequence number sent + 1
--- 1136,1146 ----
  	 */
  
  	int reliable = datalen + syn + fin; // seq #'s reliably sent
! 	/* 
! 	 * Don't reset cong_action_ until we send new data.
! 	 * -M. Weigle 6/19/02
! 	 */
! 	if (cong_action_ && reliable > 0 && !is_retransmit)
  		cong_action_ = FALSE;
  
  	// highest: greatest sequence number sent + 1

June 12, 2002
Problem: ect_ is defined in both FullTcpAgent and its parent TcpAgent.
Fix: Remove the extra definition from FullTcpAgent.
(See email sent to ns-users list)

FullTcpAgent definition in tcp/tcp-full.h

*** 119,125 ****
  		closed_(0), pipe_(-1), rtxbytes_(0), fastrecov_(FALSE),
          	last_send_time_(-1.0), infinite_send_(FALSE), irs_(-1),
          	delack_timer_(this), flags_(0),
!         	state_(TCPS_CLOSED), ect_(FALSE), recent_ce_(FALSE),
          	last_state_(TCPS_CLOSED), rq_(rcv_nxt_), last_ack_sent_(-1) { }
  
  	~FullTcpAgent() { cancel_timers(); rq_.clear(); }
--- 119,125 ----
  		closed_(0), pipe_(-1), rtxbytes_(0), fastrecov_(FALSE),
          	last_send_time_(-1.0), infinite_send_(FALSE), irs_(-1),
          	delack_timer_(this), flags_(0),
!         	state_(TCPS_CLOSED), recent_ce_(FALSE),
          	last_state_(TCPS_CLOSED), rq_(rcv_nxt_), last_ack_sent_(-1) { }
  
  	~FullTcpAgent() { cancel_timers(); rq_.clear(); }
***************
*** 225,231 ****
  	int maxseg_;        /* MSS */
  	int flags_;     /* controls next output() call */
  	int state_;     /* enumerated type: FSM state */
- 	int ect_;	/* turn on ect bit now? */
  	int recent_ce_;	/* last ce bit we saw */
  	int last_state_; /* FSM state at last pkt recv */
  	int rcv_nxt_;       /* next sequence number expected */

Michele C. Weigle