Page MenuHomeFreeBSD
Differential D23364 Diff 67291 sys/netinet/tcp_output.c

Changeset View

Standalone View

View Options

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)) {
rscheffAuthorUnsubmitted
Done
Inline Actions

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...

rscheff: I don't quite understand this conditional; I assume this is to prevent window probes (which may…
rgrimesUnsubmitted
Done
Inline Actions

Perhaps 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…
rscheffAuthorUnsubmitted
Done
Inline Actions

I'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…
rscheffAuthorUnsubmitted
Done
Inline Actions

So, 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