David Miller wrote:
> From: Ben Woodard <woodard@redhat.com>
> Date: Tue, 03 Oct 2006 11:14:38 -0700
> 
>>> Other issues:
>>>
>>> 1) 2 "u32" in the tcp_sock is a lot of space to devote to this
>>>    new state.  If it can fit in 2 "u16"'s or even less space,
>>>    please use that.
>>>
>>> 2) the expression "(tp->foo ? : sysctl_foo)" is repeated many times
>>>    in the patch, please encapsulate it into an inline function
>>>    or similar
>>>
>> How does this look to you for answering your two complaints above.
> 
> It looks a lot better.  I'd like you to take #2 further,
> put the inline function into net/tcp.h and use it in the
> tcp.c instances too.

Done. I orginally didn't want to grow the header files with two more 
functions that weren't use many places. Anyway this is fixed.

> 
> Also, please don't format code like this:
> 
> +static inline __u16 rto_max(struct tcp_sock *tp){
> +        return tp->rto_max ? : sysctl_tcp_rto_max;
> +}
> +
> +static inline __u16 rto_init(struct tcp_sock *tp){
> +        return tp->rto_init ? : sysctl_tcp_rto_init;
> +}
> 
> The opening brace of each functions belongs on a line by itself,
> thanks.

Fixed.

> 
> Finally, I'm not so sure "seconds" is the best unit for the
> socket option.  In fact you use "seconds" as the unit for
> the socket option and the sysctl is raw jiffies.  That's
> inconsistency is really problematic.
> 
> At the very least, seconds might not be fine enough granularity
> for some circumstances.  Heck, the default RTO_MIN is 1/5 of a
> second. :-)
> 
> I also understand that going to milliseconds or microseconds would
> make the size of the in-socket struct members an issue again.  These
> things are never easy are they? :-/
> 
> Any better ideas?

Here is what I've done. I store the values in the socket structure as ms 
so that they will fit into 16 bits and as jiffies in the global 
variables. Then I populate the global variables using 
proc_doulongvec_ms_jiffies()

How does that look to you?

-ben