[CRIU] [PATCH 2/2] restore: don't desable tcp repair mode twice

[Pavel Emelyanov]

On 01/14/2013 02:49 PM, Andrey Vagin wrote:
> TCP repair mode should be disabled after unlocking connections.
> Disabling of repair mode drops SO_REUSEADDR, so it should be restored.
> 
> Signed-off-by: Andrey Vagin <avagin at openvz.org>
> ---
>  include/restorer.h |  2 +-
>  include/sk-inet.h  |  5 +++++
>  pie/restorer.c     | 17 ++++++++++++++---
>  sk-inet.c          | 13 ++++++++-----
>  sk-tcp.c           | 14 ++++++++------
>  5 files changed, 36 insertions(+), 15 deletions(-)
> 
> diff --git a/include/restorer.h b/include/restorer.h
> index 097cba1..7366ad4 100644
> --- a/include/restorer.h
> +++ b/include/restorer.h
> @@ -126,7 +126,7 @@ struct task_restore_core_args {
>  	int				nr_rlim;
>  	struct rlimit			rlims[RLIM_NLIMITS];
>  
> -	int				*rst_tcp_socks;
> +	struct rst_tcp_sock		*rst_tcp_socks;
>  	int				rst_tcp_socks_size;
>  } __aligned(sizeof(long));
>  
> diff --git a/include/sk-inet.h b/include/sk-inet.h
> index 522f549..6aa1b70 100644
> --- a/include/sk-inet.h
> +++ b/include/sk-inet.h
> @@ -45,6 +45,11 @@ struct inet_sk_info {
>  int inet_bind(int sk, struct inet_sk_info *);
>  int inet_connect(int sk, struct inet_sk_info *);
>  
> +struct rst_tcp_sock {
> +	int	sk;
> +	bool	reuseaddr;
> +};
> +
>  static inline void tcp_repair_off(int fd)
>  {
>  	int aux = 0;
> diff --git a/pie/restorer.c b/pie/restorer.c
> index 2034fb5..3f97d6f 100644
> --- a/pie/restorer.c
> +++ b/pie/restorer.c
> @@ -274,15 +274,26 @@ static u64 restore_mapping(const VmaEntry *vma_entry)
>  	return addr;
>  }
>  
> -static void rst_tcp_socks_all(int *arr, int size)
> +static void rst_tcp_repair_off(struct rst_tcp_sock *rts)
> +{
> +	int aux;
> +
> +	tcp_repair_off(rts->sk);
> +
> +	aux = rts->reuseaddr;
> +	if (sys_setsockopt(rts->sk, SOL_SOCKET, SO_REUSEADDR, &aux, sizeof(aux)) < 0)
> +		pr_perror("Failed to restore of SO_REUSEADDR on socket");
> +}
> +
> +static void rst_tcp_socks_all(struct rst_tcp_sock *arr, int size)
>  {
>  	int i;
>  
>  	if (size == 0)
>  		return;
>  
> -	for (i =0; arr[i] >= 0; i++)
> -		tcp_repair_off(arr[i]);
> +	for (i =0; arr[i].sk >= 0; i++)
> +		rst_tcp_repair_off(arr + i);
>  
>  	sys_munmap(arr, size);
>  }
> diff --git a/sk-inet.c b/sk-inet.c
> index b411c28..f8b47c0 100644
> --- a/sk-inet.c
> +++ b/sk-inet.c
> @@ -439,16 +439,19 @@ static int post_open_inet_sk(struct file_desc *d, int sk)
>  
>  	ii = container_of(d, struct inet_sk_info, d);
>  
> +	/*
> +	 * TCP sockets are handled at the last moment
> +	 * after unlocking connections.
> +	 */
> +	if (tcp_connection(ii->ie))
> +		return 0;
> +
>  	/* SO_REUSEADDR is set for all sockets */
> -	if (!tcp_connection(ii->ie) && ii->ie->opts->reuseaddr)
> +	if (ii->ie->opts->reuseaddr)
>  		return 0;
>  
>  	futex_wait_until(&ii->port->users, 0);

This cal deadlock. Consider you have a socket that "shares" port with
tcp socket. It will wait for tcp socket to "finish" reusing address, while
tcp socket owner will wait for this task to restore.

>  
> -	/* Disabling repair mode drops SO_REUSEADDR */
> -	if (tcp_connection(ii->ie))
> -		tcp_repair_off(sk);
> -
>  	val = ii->ie->opts->reuseaddr;
>  	if (restore_opt(sk, SOL_SOCKET, SO_REUSEADDR, &val))
>  		return -1;
> diff --git a/sk-tcp.c b/sk-tcp.c
> index ebf9815..825f8b8 100644
> --- a/sk-tcp.c
> +++ b/sk-tcp.c
> @@ -520,7 +520,7 @@ err:
>   * rst_tcp_socks contains sockets in repair mode,
>   * which will be off in restorer before resuming.
>   */
> -static int *rst_tcp_socks = NULL;
> +static struct rst_tcp_sock *rst_tcp_socks = NULL;
>  static int rst_tcp_socks_num = 0;
>  int rst_tcp_socks_size = 0;
>  
> @@ -532,7 +532,7 @@ int rst_tcp_socks_remap(void *addr)
>  		return 0;
>  	}
>  
> -	rst_tcp_socks[rst_tcp_socks_num] = -1;
> +	rst_tcp_socks[rst_tcp_socks_num].sk = -1;
>  
>  	ret = mmap(addr, rst_tcp_socks_size, PROT_READ | PROT_WRITE,
>  			MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0);
> @@ -547,17 +547,19 @@ int rst_tcp_socks_remap(void *addr)
>  	return 0;
>  }
>  
> -static int rst_tcp_socks_add(int fd)
> +static int rst_tcp_socks_add(int fd, bool reuseaddr)
>  {
>  	/* + 2 = ( new one + guard (-1) ) */
> -	if ((rst_tcp_socks_num + 2) * sizeof(int) > rst_tcp_socks_size) {
> +	if ((rst_tcp_socks_num + 2) * sizeof(struct rst_tcp_sock) > rst_tcp_socks_size) {
>  		rst_tcp_socks_size += PAGE_SIZE;
>  		rst_tcp_socks = xrealloc(rst_tcp_socks, rst_tcp_socks_size);
>  		if (rst_tcp_socks == NULL)
>  			return -1;
>  	}
>  
> -	rst_tcp_socks[rst_tcp_socks_num++] = fd;
> +	rst_tcp_socks[rst_tcp_socks_num].sk = fd;
> +	rst_tcp_socks[rst_tcp_socks_num].reuseaddr = reuseaddr;
> +	rst_tcp_socks_num++;
>  	return 0;
>  }
>  
> @@ -568,7 +570,7 @@ int restore_one_tcp(int fd, struct inet_sk_info *ii)
>  	if (tcp_repair_on(fd))
>  		return -1;
>  
> -	if (rst_tcp_socks_add(fd))
> +	if (rst_tcp_socks_add(fd, ii->ie->opts->reuseaddr))
>  		return -1;
>  
>  	if (restore_tcp_conn_state(fd, ii))
>