Bug in tcp_output?

[Rui Paulo]

On 18 Mar 2010, at 20:19, Chris Harrer wrote:

> Hi All,
> 
> 
> 
> In the following block of code, running on a x86_64 platform, I believe that
> cwin should be declared as an int:
> 
>        /*
> 
>         * If snd_nxt == snd_max and we have transmitted a FIN, the
> 
>         * offset will be > 0 even if so_snd.sb_cc is 0, resulting in
> 
>         * a negative length.  This can also occur when TCP opens up
> 
>         * its congestion window while receiving additional duplicate
> 
>         * acks after fast-retransmit because TCP will reset snd_nxt
> 
>         * to snd_max after the fast-retransmit.
> 
>         *
> 
>         * In the normal retransmit-FIN-only case, however, snd_nxt will
> 
>         * be set to snd_una, the offset will be 0, and the length may
> 
>         * wind up 0.
> 
>         *
> 
>         * If sack_rxmit is true we are retransmitting from the scoreboard
> 
>         * in which case len is already set.
> 
>         */
> 
>        if (sack_rxmit == 0) {
> 
>               if (sack_bytes_rxmt == 0)
> 
>                       len = ((long)ulmin(so->so_snd.sb_cc, sendwin) - off);
> 
>               else {
> 
>                       long cwin;  ß-- Should be an int
> 
> 
> 
>                        /*
> 
>                        * We are inside of a SACK recovery episode and are
> 
>                        * sending new data, having retransmitted all the
> 
>                        * data possible in the scoreboard.
> 
>                        */
> 
>                       len = ((long)ulmin(so->so_snd.sb_cc, tp->snd_wnd) 
> 
>                              - off);
> 
>                       /*
> 
>                        * Don't remove this (len > 0) check !
> 
>                        * We explicitly check for len > 0 here (although it 
> 
>                        * isn't really necessary), to work around a gcc 
> 
>                        * optimization issue - to force gcc to compute
> 
>                        * len above. Without this check, the computation
> 
>                        * of len is bungled by the optimizer.
> 
>                        */
> 
>                       if (len > 0) {
> 
>                               cwin = tp->snd_cwnd - 
> 
>                                      (tp->snd_nxt - tp->sack_newdata) -
> 
>                                      sack_bytes_rxmt;
> 
>                               if (cwin < 0)
> 
>                                      cwin = 0;
> 
>                               len = lmin(len, cwin);
> 
>                       }
> 
>               }
> 
>        }
> 
> 
> 
> Consider the case where:
> 
> sack_rxmit = 0
> 
> sack_bytes_rxmt = 0x2238
> 
> off = 0
> 
> len =0xa19c
> 
> tp->snd_cwnd = 0x2238
> 
> tp->snd_nxt = 0xdd6d7974
> 
> tp->sack_newdata = 0xdd6d6858
> 
> In this case cwin evaluates to 0x00000000ffffe37c, which is not <0, but
> instead huge.  This causes the remaining data on the socket’s so->so_snd
> buffer to be sent to the network causing more problems at the receiver which
> is already dropping frames.

I see. This is most likely a bug. Can you send-pr so this doesn't get lost?

Regards,
--
Rui Paulo