Re: [PATCH 1/2] tcp: make tcp_timestamp dependent on tcp_sock

[Andrew Vagin]

On Wed, Jan 23, 2013 at 08:25:17AM -0800, Eric Dumazet wrote:
> > @@ -1142,9 +1145,11 @@ static inline void tcp_openreq_init(struct request_sock *req,
> >  static inline void tcp_synack_rtt_meas(struct sock *sk,
> >  				       struct request_sock *req)
> >  {
> > +	const struct tcp_sock *tp = tcp_sk(sk);
> > +
> >  	if (tcp_rsk(req)->snt_synack)
> >  		tcp_valid_rtt_meas(sk,
> > -		    tcp_time_stamp - tcp_rsk(req)->snt_synack);
> > +		    tcp_time_stamp(tp) - tcp_rsk(req)->snt_synack);
> >  }
> >  
> 
> This first chunk looks not needed.
> 
> Can SYN_RECV sockets be live-migrated ? Probably not.
> 
> Quite frankly I am sure there is an issue for timewait (not real
> sockets).
> 
> Have you really tested this patch, or is it an RFC ?

I set RFC, when I'm not sure, that this idea is right and want to get
opinions and comments before spend a lot of time.

I have tested this patch. I have a virtuall machine, where I test
kernels. SSH is used for accessing this VM. My test dumps an outside
TCP connection, then reboots the VM and restores the TCP connection.
Without this patch the tcp connection are not restored correctly, but
with this patch it works properly.

A timestamp offset is zero by default, so I did not catch any bugs.
I will test my patches more carefully.

Thank you all for the comments and sorry for noise in the ML.

> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/