Subject: Re: [RFC PATCH net-2.6] [TCP]: Congestion control API RTT sampling fix

From: Stephen Hemminger <[email-masked]>
Date: Tue, 12 Jun 2007 08:23:08 -0700

On Tue, 12 Jun 2007 15:06:57 +0300 (EEST)
"Ilpo Järvinen" <[email-masked]> wrote:

> I was thinking something like this to fix the cc module breakage 
> introduced by the API change (haven't tested it besides compile):
> 
> 
> [RFC PATCH net-2.6] [TCP]: Congestion control API RTT sampling fix
> 
> 
> Commit 164891aadf1721fca4dce473bb0e0998181537c6 broke RTT
> sampling of congestion control modules. Inaccurate timestamps
> could be fed to them without providing any way for them to
> identify such cases. Previously RTT sampler was called only if
> FLAG_RETRANS_DATA_ACKED was not set filtering inaccurate
> timestamps nicely. In addition, the new behavior could give an
> invalid timestamp (zero) to RTT sampler if only skbs with
> TCPCB_RETRANS were ACKed. This solves both problems.
> 
> Signed-off-by: Ilpo Järvinen <[email-masked]>
> ---
>  include/linux/ktime.h   |   18 ++++++++++++++++++
>  include/linux/skbuff.h  |    4 ++++
>  net/ipv4/tcp_illinois.c |    3 +++
>  net/ipv4/tcp_input.c    |    6 +++++-
>  net/ipv4/tcp_lp.c       |    3 ++-
>  net/ipv4/tcp_vegas.c    |    3 +++
>  net/ipv4/tcp_veno.c     |    3 +++
>  7 files changed, 38 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/ktime.h b/include/linux/ktime.h
> index c762954..9f7fa3e 100644
> --- a/include/linux/ktime.h
> +++ b/include/linux/ktime.h
> @@ -102,6 +102,12 @@ static inline ktime_t ktime_set(const long secs, const unsigned long nsecs)
>  #define ktime_add_ns(kt, nsval) \
>  		({ (ktime_t){ .tv64 = (kt).tv64 + (nsval) }; })
>  
> +/* Compare two ktime_t variables, returns 1 if equal */
> +static inline int ktime_equal(const ktime_t cmp1, const ktime_t cmp2)
> +{
> +	return cmp1.tv64 == cmp2.tv64;
> +}
> +
>  /* convert a timespec to ktime_t format: */
>  static inline ktime_t timespec_to_ktime(struct timespec ts)
>  {
> @@ -200,6 +206,18 @@ static inline ktime_t ktime_add(const ktime_t add1, const ktime_t add2)
>  extern ktime_t ktime_add_ns(const ktime_t kt, u64 nsec);
>  
>  /**
> + * ktime_equal - Compares two ktime_t variables to see if they are equal
> + * @cmp1:	comparable1
> + * @cmp2:	comparable2
> + *
> + * Compare two ktime_t variables, returns 1 if equal
> + */
> +static inline int ktime_equal(const ktime_t cmp1, const ktime_t cmp2)
> +{
> +	return !((cmp1.tv.sec ^ cmp2.tv.sec) | (cmp1.tv.usec ^ cmp2.tv.usec));
> +}

Since ktime is a union just comparing the two 64bit values should
be simpler.

static inline int ktime_equal(const ktime_t t1, const ktime_t t2)
{
	return t1.s64 == t2.s64;
}