Re: [PATCH] Re: Very poor TCP/SACK performance

Savochkin Andrey Vladimirovich (saw@msu.ru)
Thu, 10 Sep 1998 20:19:16 +0400

I can't comment the first two problems but both the explanation
and the fix for the third problem is wrong.
1. I suppose you don't remember the RFC statements about zero window
probing at all. TCP of course isn't obliged to always accept one byte of
data.
2. Problem with window wrapping below zero was fixed ages ago.
3. The suggested fix wastes CPU cycles in a speed critical part.
It has no sense to do extra comparisons when we've just checked
that `TCP_SKB_CB(skb)->seq == tp->rcv_nxt'. And no
sequence check is required for the bulk sender path.
The only thing that may have a sense is to move the check
`!before(TCP_SKB_CB(skb)->seq, tp->rcv_wup + tp->rcv_wnd)'
just before the socket space check.

I've found that the patch was applied for 2.1.121.
I don't think that the patch can be applied as it is.
Let's give the patch some time to be thinked over.

Best wishes
Andrey V.
Savochkin

On Tue, Sep 08, 1998 at 06:34:40PM -0400, Vijay G. Bharadwaj wrote:
[...]
> Problem 3: Zero window probe handling broken
>
> The RFC (1122, IIRC) says that zero window probes should contain one byte
> of garbage data with a stale sequence number. However, both Solaris and
> Windows send a zero-window probe consisting of one byte of new data. Since
> pred_flags match up, Linux happily accepts this byte. Now
> tcp_receive_window() returns -1, so the Linux box advertises a 64K window,
> and things go downhill from there. My fix throws away the extra byte. This
> also enabled me to delete another check a little further down in the code.
>
> This change violates the RFC though, because the RFC (I think it's 1122,
> or maybe it's 793) states that a TCP MUST always be able to accept one
> additional byte of data. However I think this is a bug in the RFC, because
> if a TCP conforms to this, then I can still send it a steady stream of
> 1-byte packets, even if it's advertizing a zero window, and the TCP must
> accept and buffer this data.
>
> I notice that tcp_receive_window() now checks for (win < 0), which also
> fixes the above problem (at least the 64K window is not advertised).
> However, it still means that the remote end can pass us data in 1-byte
> packets, which is terrible for network performance.
[snip]
> @@ -1712,6 +1711,10 @@
> */
>
> if (flg == tp->pred_flags && TCP_SKB_CB(skb)->seq == tp->rcv_nxt) {
> + if (!tcp_sequence(tp, TCP_SKB_CB(skb)->seq, TCP_SKB_CB(skb)->end_seq)) {
> + tcp_send_ack(sk);
> + goto discard;
> + }
> if (len <= th->doff*4) {
> /* Bulk data transfer: sender */
> if (len == th->doff*4) {
> @@ -1726,13 +1729,8 @@
> }
> } else if (TCP_SKB_CB(skb)->ack_seq == tp->snd_una) {
> /* Bulk data transfer: receiver */
> - if (atomic_read(&sk->rmem_alloc) > sk->rcvbuf) {
> - /* We must send an ACK for zero window probes. */
> - if (!before(TCP_SKB_CB(skb)->seq,
> - tp->rcv_wup + tp->rcv_wnd))
> - tcp_send_ack(sk);
> + if (atomic_read(&sk->rmem_alloc) > sk->rcvbuf)
> goto discard;
> - }
>
> __skb_pull(skb,th->doff*4);

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/faq.html