[lwip-users] [lwip] Congestion window bug

[Kieran Mansley]

Just found a non-critical bug that puts the stack into slow-start more
often than it should.  When the pcb->cwnd is incremented, it can overflow
leading it to take on a very small rather than very large number.  To fix
it I changed (in tcp_receive()):

          if(pcb->cwnd + pcb->mss * pcb->mss / pcb->cwnd > pcb->cwnd) {
            pcb->cwnd += pcb->mss * pcb->mss / pcb->cwnd;
          }
          DEBUGF(TCP_CWND_DEBUG, ("tcp_receive: congestion avoidance cwnd
%u\n", pcb->cwnd));

to be:

          u16_t new_cwnd = (pcb->cwnd + pcb->mss * pcb->mss / pcb->cwnd);
          if(new_cwnd > pcb->cwnd) {
            pcb->cwnd = new_cwnd;
          }
          DEBUGF(TCP_CWND_DEBUG, ("tcp_receive: congestion avoidance cwnd
%u\n", pcb->cwnd));

I think gcc was comparing the two values in the "if" statment as 32 bit
integers, rather than as 16 bit integers, so putting a simple cast would
probably solve it too, but the modification also reduces the duplicated
calculation.  (The compiler should optimise that out anyway I suppose, but
doesn't do any harm to be explicit).

There is another potential source of the bug where it does something
similar when *not* transmitting a fast retransmit (in tcp_receive()):

            /* Inflate the congestion window, but not if it means that
               the value overflows. */
            if(pcb->cwnd + pcb->mss > pcb->cwnd) {
              pcb->cwnd += pcb->mss;
            }

Fixed in exactly the same way.

I can supply patches against 0.5.3 if anyone needs them, but it's a
trivial change.

Kieran

[This message was sent through the lwip discussion list.]