Hi,
I'm currently verifying different ECN implementations against full
RC3168 compliance, using Sally Floyd's TBIT tool.
It seems that the OpenBSD ECN implementation has a day-1 bug, w.r.t the
handling of CE/CWR frames.
The reference implementation in ns-2 first checks the CWR bit, and
reset's ECE, before going on and checking the CE codepoint in the IP
header.
However, similar to FreeBSD, OpenBSD does it the other way round - a
CE-marked frame with CWR set will *NOT* result in the ECE bit staying
set...
This deviation can result in excessive packet drops at the ECN-capable
bottleneck router, and also does not work very well with enhanced
CC-algorighms requiring the best possible knowledge about impeding
congestion.
Reference implementation (from tcp-sink.cc in ns2):
if ( (sf != 0 && sf->cong_action()) || of->cong_action() )
// Sender has responsed to congestion.
acker_->update_ecn_unacked(0);
if ( (sf != 0 && sf->ect() && sf->ce()) ||
(of->ect() && of->ce()) )
// New report of congestion.
acker_->update_ecn_unacked(1);
if ( (sf != 0 && sf->ect()) || of->ect() )
// Set EcnEcho bit.
nf->ecnecho() = acker_->ecn_unacked();
Basically, CWR is checked first, and ECE cleared; if that segment also
contains the CE codepoint again, ECE is set anew.
Implementation in tcp_input.c (1.111 - current):
#ifdef TCP_ECN
/* if congestion experienced, set ECE bit in
subsequent packets. */
if ((iptos & IPTOS_ECN_MASK) == IPTOS_ECN_CE) {
tp->t_flags |= TF_RCVD_CE;
tcpstat.tcps_ecn_rcvce++;
}
#endif
[...]
/*
* if we receive CWR, we know that the peer
has reduced
* its congestion window. stop sending
ecn-echo.
*/
if ((tiflags & TH_CWR)) {
tp->t_flags &= ~TF_RCVD_CE;
tcpstat.tcps_ecn_rcvcwr++;
}
#endif /* TCP_ECN */
A simple fix would be to double-check in the "established" state, before
clearing ECE:
/*
* if we receive CWR, we know that the peer
has reduced
* its congestion window. stop sending
ecn-echo.
*/
if ((tiflags & TH_CWR)) {
if ((iptos & IPTOS_ECN_MASK) !=
IPTOS_ECN_CE)
tp->t_flags &= ~TF_RCVD_CE;
tcpstat.tcps_ecn_rcvcwr++;
}
#endif /* TCP_ECN */
(Similar bug in FreeBSD was fixed a few days ago..)
Best regards,
Richard Scheffenegger
