Used TBIT (www.icir.org/tbit/) to verify the RFC3168 compliance of the FreeBSD ECN implementiation. First, the stock IP stack filers out the CE and ECT(1) codepoints - even when TCP negotiates successfully for ECN. This can be seen by "netstat -sp tcp": Fix: rsFreeBSD# diff -U 10 netinet.orig/tcp_input.c netinet/tcp_input.c --- netinet.orig/tcp_input.c 2009-10-25 02:10:29.000000000 +0100 +++ netinet/tcp_input.c 2010-04-10 10:30:12.000000000 +0200 @@ -1127,36 +1127,37 @@ /* * Unscale the window into a 32-bit value. * For the SYN_SENT state the scale is zero. */ tiwin = th->th_win << tp->snd_scale; /* * TCP ECN processing. */ if (tp->t_flags & TF_ECN_PERMIT) { + + if (thflags & TH_CWR) + tp->t_flags &= ~TF_ECN_SND_ECE; + switch (iptos & IPTOS_ECN_MASK) { case IPTOS_ECN_CE: tp->t_flags |= TF_ECN_SND_ECE; TCPSTAT_INC(tcps_ecn_ce); break; case IPTOS_ECN_ECT0: TCPSTAT_INC(tcps_ecn_ect0); break; case IPTOS_ECN_ECT1: TCPSTAT_INC(tcps_ecn_ect1); break; } - if (thflags & TH_CWR) - tp->t_flags &= ~TF_ECN_SND_ECE; - /* * Congestion experienced. * Ignore if we are already trying to recover. */ if ((thflags & TH_ECE) && SEQ_LEQ(th->th_ack, tp->snd_recover)) { TCPSTAT_INC(tcps_ecn_rcwnd); tcp_congestion_exp(tp); } } Patch attached with submission follows: How-To-Repeat: Run NewECN test of TBIT against an ECN activated freebsd stack (with deactivated CE/ECT(1) filtering in the IP stack). SImilar, injecting a CWR/CE data segment into an ECN-enabled TCP stream can demonstrate the problem.
Author: rpaulo Date: Sat Apr 10 12:47:06 2010 New Revision: 206456 URL: http://svn.freebsd.org/changeset/base/206456 Log: Honor the CE bit even when the CWR bit is set. PR: 145600 Submitted by: Richard Scheffenegger <rs at netapp.com> MFC after: 1 week Modified: head/sys/netinet/tcp_input.c Modified: head/sys/netinet/tcp_input.c ============================================================================== --- head/sys/netinet/tcp_input.c Sat Apr 10 12:29:09 2010 (r206455) +++ head/sys/netinet/tcp_input.c Sat Apr 10 12:47:06 2010 (r206456) @@ -1134,6 +1134,8 @@ tcp_do_segment(struct mbuf *m, struct tc * TCP ECN processing. */ if (tp->t_flags & TF_ECN_PERMIT) { + if (thflags & TH_CWR) + tp->t_flags &= ~TF_ECN_SND_ECE; switch (iptos & IPTOS_ECN_MASK) { case IPTOS_ECN_CE: tp->t_flags |= TF_ECN_SND_ECE; @@ -1146,10 +1148,6 @@ tcp_do_segment(struct mbuf *m, struct tc TCPSTAT_INC(tcps_ecn_ect1); break; } - - if (thflags & TH_CWR) - tp->t_flags &= ~TF_ECN_SND_ECE; - /* * Congestion experienced. * Ignore if we are already trying to recover. _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
State Changed From-To: open->patched Fixed in HEAD. MFC pending. Thanks!
Responsible Changed From-To: freebsd-bugs->rpaulo Fixed in HEAD. MFC pending. Thanks!
Author: rpaulo Date: Sat Apr 17 17:40:12 2010 New Revision: 206762 URL: http://svn.freebsd.org/changeset/base/206762 Log: MFC r206456: Honor the CE bit even when the CWR bit is set. PR: 145600 Submitted by: Richard Scheffenegger <rs at netapp.com> Modified: stable/8/sys/netinet/tcp_input.c Directory Properties: stable/8/sys/ (props changed) stable/8/sys/amd64/include/xen/ (props changed) stable/8/sys/cddl/contrib/opensolaris/ (props changed) stable/8/sys/contrib/dev/acpica/ (props changed) stable/8/sys/contrib/pf/ (props changed) stable/8/sys/dev/xen/xenpci/ (props changed) Modified: stable/8/sys/netinet/tcp_input.c ============================================================================== --- stable/8/sys/netinet/tcp_input.c Sat Apr 17 17:02:17 2010 (r206761) +++ stable/8/sys/netinet/tcp_input.c Sat Apr 17 17:40:12 2010 (r206762) @@ -1134,6 +1134,8 @@ tcp_do_segment(struct mbuf *m, struct tc * TCP ECN processing. */ if (tp->t_flags & TF_ECN_PERMIT) { + if (thflags & TH_CWR) + tp->t_flags &= ~TF_ECN_SND_ECE; switch (iptos & IPTOS_ECN_MASK) { case IPTOS_ECN_CE: tp->t_flags |= TF_ECN_SND_ECE; @@ -1146,10 +1148,6 @@ tcp_do_segment(struct mbuf *m, struct tc TCPSTAT_INC(tcps_ecn_ect1); break; } - - if (thflags & TH_CWR) - tp->t_flags &= ~TF_ECN_SND_ECE; - /* * Congestion experienced. * Ignore if we are already trying to recover. _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Responsible Changed From-To: rpaulo->freebsd-bugs rpaulo has return his commit bit for safekeeing.
Responsible Changed From-To: freebsd-bugs->freebsd-net Reassign to networking team. Network people, this already had been committed and might be interesting to commit to 7-stable as well. If not please close the ticket.