Changeset View
Standalone View
sys/netinet/tcp_output.c
Show First 20 Lines • Show All 1,157 Lines • ▼ Show 20 Lines | if (tp->t_state == TCPS_ESTABLISHED && | ||||
(tp->t_flags2 & TF2_ECN_PERMIT)) { | (tp->t_flags2 & TF2_ECN_PERMIT)) { | ||||
/* | /* | ||||
* If the peer has ECN, mark data packets with | * If the peer has ECN, mark data packets with | ||||
* ECN capable transmission (ECT). | * ECN capable transmission (ECT). | ||||
* Ignore pure ack packets, retransmissions and window probes. | * Ignore pure ack packets, retransmissions and window probes. | ||||
*/ | */ | ||||
if (len > 0 && SEQ_GEQ(tp->snd_nxt, tp->snd_max) && | if (len > 0 && SEQ_GEQ(tp->snd_nxt, tp->snd_max) && | ||||
(sack_rxmit == 0) && | (sack_rxmit == 0) && | ||||
!((tp->t_flags & TF_FORCEDATA) && len == 1)) { | !((tp->t_flags & TF_FORCEDATA) && len == 1)) { | ||||
rscheff: I don't quite understand this conditional; I assume this is to prevent window probes (which may… | |||||
Done Inline ActionsPerhaps the block comment above needs to be expanded, but that should be in a seperate change, IMHO. rgrimes: Perhaps the block comment above needs to be expanded, but that should be in a seperate change… | |||||
Done Inline ActionsI'm trying to get a commit log from Rui Paulo, or some other validation as to what the intent has been around this condition. rscheff: I'm trying to get a commit log from Rui Paulo, or some other validation as to what the intent… | |||||
Done Inline ActionsSo, after some pondering about this: If (for whatever reason) cwnd ends up at zero, and the (persist) timer is clocking out new 1-byte payload segments, those should still be ECT-marked (and allowed to carry a CWR). I propose to add a check like ((tp->t_flags & TF_FORCEDATA) && len == 1 && SEQ_LT(tp->snd_una, tp->snd_max) on the basis that zero window probes don't get ACKed until the receiver window can hold them (and then, the next window probe would be a new data segment). For a normal window probe (when there is additional data to send in the socket), only the first probe would be ECT0 marked; the byte would not get acked, and all subsequent probes would be retransmissions. rscheff: So, after some pondering about this: If (for whatever reason) cwnd ends up at zero, and the… | |||||
#ifdef INET6 | #ifdef INET6 | ||||
if (isipv6) | if (isipv6) | ||||
ip6->ip6_flow |= htonl(IPTOS_ECN_ECT0 << 20); | ip6->ip6_flow |= htonl(IPTOS_ECN_ECT0 << 20); | ||||
else | else | ||||
#endif | #endif | ||||
ip->ip_tos |= IPTOS_ECN_ECT0; | ip->ip_tos |= IPTOS_ECN_ECT0; | ||||
TCPSTAT_INC(tcps_ecn_ect0); | TCPSTAT_INC(tcps_ecn_ect0); | ||||
} | |||||
/* | /* | ||||
* Reply with proper ECN notifications. | * Reply with proper ECN notifications. | ||||
* Only set CWR on new data segments | |||||
*/ | */ | ||||
if (tp->t_flags2 & TF2_ECN_SND_CWR) { | if (tp->t_flags2 & TF2_ECN_SND_CWR) { | ||||
flags |= TH_CWR; | flags |= TH_CWR; | ||||
tp->t_flags2 &= ~TF2_ECN_SND_CWR; | tp->t_flags2 &= ~TF2_ECN_SND_CWR; | ||||
} | |||||
} | } | ||||
if (tp->t_flags2 & TF2_ECN_SND_ECE) | if (tp->t_flags2 & TF2_ECN_SND_ECE) | ||||
flags |= TH_ECE; | flags |= TH_ECE; | ||||
} | } | ||||
/* | /* | ||||
* If we are doing retransmissions, then snd_nxt will | * If we are doing retransmissions, then snd_nxt will | ||||
* not reflect the first unsent octet. For ACK only | * not reflect the first unsent octet. For ACK only | ||||
* packets, we do not want the sequence number of the | * packets, we do not want the sequence number of the | ||||
▲ Show 20 Lines • Show All 914 Lines • Show Last 20 Lines |
I don't quite understand this conditional; I assume this is to prevent window probes (which may be retransmissions) to be ECT-marked. However, this also affects persist-timer triggered 1-byte data segments which never have been sent before.
While strictly speaking, this is a potential bug (delaying CWR erraneously, running into more than 1 window of ECE and an additional cwnd reduction), the impact should be fairly limited and may not warrant additional checks...