Messages in this thread |  | | | Date | Wed, 24 Oct 2007 14:08:36 -0400 | | From | Chuck Lever <> | | Subject | Re: [PATCH 4/5] NET: Make ts_recent_stamp unsigned | |
David Miller wrote:
> From: Chuck Lever <chuck.lever@oracle.com>
> Date: Tue, 23 Oct 2007 11:44:28 -0400
>
>> The get_seconds() function returns an unsigned long. To prevent incorrect
>> comparison results between values saved in ts_recent_stamp and later
>> invocations of get_seconds(), change the type of ts_recent_stamp to match
>> the return type of get_seconds().
>>
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>> Cc: <netdev@vger.kernel.org>
>
> I see two potential problems with this patch:
>
> 1) If you update struct tcp_options_received you should also
> update struct tcp_timewait_sock similarly.
>
> The fact that you missed this suggests that you didn't
> grep the tree to see how else this variable is used and
> this makes me extra concerned about this patch's correctness.
Perhaps the result of wishful thinking on my part. I was hoping for a
small and self-contained change.
> 2) There are computations in the TCP stack using this member that
> probably care about the signedness, such as:
>
> net/ipv4/tcp_ipv4.c: get_seconds() - tcptw->tw_ts_recent_stamp > 1))) {
> include/net/tcp.h: if (get_seconds() >= rx_opt->ts_recent_stamp + TCP_PAWS_24DAYS)
> include/net/tcp.h: if (get_seconds() >= rx_opt->ts_recent_stamp + TCP_PAWS_24DAYS)
>
> We should make sure we understand what is expected here, and
> why it would still be correct after making both ts_recent_stamp
> members unsigned.
Agreed.
I wonder how one could construct a series of mixed case time stamp
comparisons *on purpose* (and without documentation of this assumption)
that produces consistently correct results.
From the invocations of get_seconds() that I sampled, the design of
these comparisons seems to assume that both sides of the comparison are
non-negative. However, they do not seem to account for time crossing zero.
begin:vcard
fn:Chuck Lever
n:Lever;Chuck
org:Oracle Corporation;Corporate Architecture: Linux Projects Group
adr:;;1015 Granger Avenue;Ann Arbor;MI;48104;USA
email;internet:chuck dot lever at nospam oracle dot com
title:Principal Member of Staff
tel;work:+1 248 614 5091
x-mozilla-html:FALSE
version:2.1
end:vcard
|  |