kern/75122: [PATCH] Incorrect inflight bandwidth calculation on first packet

[Dan Nelson]

In the last episode (Dec 18), Uwe Doering said:
> Dan Nelson wrote:
> >Updated patch including Matt's recommended fix:
> >
> >+	/*
> >+	 * Sanity check, plus ignore pure window update acks.
> >+	 */
> >+	if ((int)(ack_seq - tp->t_bw_rtseq) <= 0)
> >+		return;
> 
> I wonder, isn't there a flaw in the logic with regard to the sequence
> number handling?  If the sequence number wraps around 't_bw_rtseq' no
> longer gets set and therefore the bandwidth calculation stops until
> 'ack_seq' either catches up with 't_bw_rtseq' again (which would take
> quite a while), or 'ticks' wraps around as well, or there is
> inactivity for more than 10 seconds.  This is probably not the
> intended behavior.

I think the code works as-is.  ack_seq and tp->t_bw_rtseq are both of
type "tcp_seq" which is a u_int32_t.  Wrap-around is handled
transparently when your variables are unsigned and your sequence space
covers all possible values.  It's the magic of mod(2^32) arithmetic :)
The (int) cast just makes the if simpler.  Without the cast it would
read

 if (ack_seq - tp->t_bw_rtseq > 2147483648U || ack_seq == tp->t_bw_rtseq)

The sanity check is probably not even necessary, as any really invalid
sequence numbers would have caused the packet to be dropped before it
got this far.


-- 
	Dan Nelson
	dnelson at allantgroup.com