[PATCHv3,4/8] util: Enable hostname resolution of tcp address

Submitted by Radostin Stoyanov on Sept. 4, 2018, 9:26 p.m.

Details

Message ID 20180904212657.8597-5-rstoyanov1@gmail.com
State New
Series "remote: Refactor TCP client/server setup"
Headers show

Commit Message

Radostin Stoyanov Sept. 4, 2018, 9:26 p.m.
Add hostname resolution in setup_tcp_client(). This change allows a
valid hostname to be provided as value for the --address option when
connecting to page server.

Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com>
---
 Documentation/criu.txt |  2 +-
 criu/include/util.h    |  2 +-
 criu/util.c            | 56 +++++++++++++++++++++++++++++++-----------
 3 files changed, 44 insertions(+), 16 deletions(-)

Patch hide | download patch | download mbox

diff --git a/Documentation/criu.txt b/Documentation/criu.txt
index 6bb3f0f5..04e7a610 100644
--- a/Documentation/criu.txt
+++ b/Documentation/criu.txt
@@ -550,7 +550,7 @@  Launches *criu* in page server mode.
     It isn't supposed to use --daemon and --status-fd together.
 
 *--address* 'address'::
-    Page server IP address.
+    Page server IP address or hostname.
 
 *--port* 'number'::
     Page server port number.
diff --git a/criu/include/util.h b/criu/include/util.h
index b286b3e9..aba16818 100644
--- a/criu/include/util.h
+++ b/criu/include/util.h
@@ -293,7 +293,7 @@  void print_data(unsigned long addr, unsigned char *data, size_t size);
 
 int setup_tcp_server(char *type, char *addr, unsigned short *port);
 int run_tcp_server(bool daemon_mode, int *ask, int cfd, int sk);
-int setup_tcp_client(char *addr);
+int setup_tcp_client(char *hostname);
 
 #define LAST_PID_PATH		"sys/kernel/ns_last_pid"
 #define PID_MAX_PATH		"sys/kernel/pid_max"
diff --git a/criu/util.c b/criu/util.c
index 71bb2f6e..5c9fb306 100644
--- a/criu/util.c
+++ b/criu/util.c
@@ -30,6 +30,7 @@ 
 #include <sys/resource.h>
 #include <sys/wait.h>
 #include <sys/socket.h>
+#include <netdb.h>
 #include <netinet/in.h>
 #include <netinet/tcp.h>
 #include <sched.h>
@@ -1384,28 +1385,55 @@  out:
 	return -1;
 }
 
-int setup_tcp_client(char *addr)
+int setup_tcp_client(char *hostname)
 {
 	struct sockaddr_storage saddr;
-	int sk;
-
-	pr_info("Connecting to server %s:%u\n", addr, opts.port);
+	struct addrinfo addr_criteria, *addr_list, *p;
+	char ipstr[INET6_ADDRSTRLEN];
+	int sk = -1;
+	void *ip;
 
-	if (get_sockaddr_in(&saddr, addr, opts.port))
-		return -1;
+	memset(&addr_criteria, 0, sizeof(addr_criteria));
+	addr_criteria.ai_family = AF_UNSPEC;
+	addr_criteria.ai_socktype = SOCK_STREAM;
+	addr_criteria.ai_protocol = IPPROTO_TCP;
 
-	sk = socket(saddr.ss_family, SOCK_STREAM, IPPROTO_TCP);
-	if (sk < 0) {
-		pr_perror("Can't create socket");
-		return -1;
+	if (getaddrinfo(hostname, NULL, &addr_criteria, &addr_list)) {
+		pr_perror("Failed to resolve hostname: %s", hostname);
+		goto out;
 	}
 
-	if (connect(sk, (struct sockaddr *)&saddr, sizeof(saddr)) < 0) {
-		pr_perror("Can't connect to server");
-		close(sk);
-		return -1;
+	for(p = addr_list; p != NULL; p = p->ai_next) {
+
+		if (p->ai_family == AF_INET) {
+			struct sockaddr_in *ipv4 = (struct sockaddr_in *)p->ai_addr;
+			ip = &(ipv4->sin_addr);
+		} else {
+			struct sockaddr_in6 *ipv6 = (struct sockaddr_in6 *)p->ai_addr;
+			ip = &(ipv6->sin6_addr);
+		}
+
+		inet_ntop(p->ai_family, ip, ipstr, sizeof(ipstr));
+		pr_info("Connecting to server %s:%u\n", ipstr, opts.port);
+
+		if (get_sockaddr_in(&saddr, ipstr, opts.port))
+			goto out;
+
+		sk = socket(saddr.ss_family, SOCK_STREAM, IPPROTO_TCP);
+		if (sk < 0) {
+			pr_perror("Can't create socket");
+			goto out;
+		}
+
+		if (connect(sk, (struct sockaddr *)&saddr, sizeof(saddr)) < 0) {
+			pr_info("Can't connect to server %s:%u\n", ipstr, opts.port);
+			close(sk);
+			sk = -1;
+		}
 	}
 
+out:
+	freeaddrinfo(addr_list);
 	return sk;
 }
 

Comments

Andrey Vagin Sept. 10, 2018, 7:04 p.m.
On Tue, Sep 04, 2018 at 10:26:53PM +0100, Radostin Stoyanov wrote:
> Add hostname resolution in setup_tcp_client(). This change allows a
> valid hostname to be provided as value for the --address option when
> connecting to page server.
> 
> Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com>
> ---
>  Documentation/criu.txt |  2 +-
>  criu/include/util.h    |  2 +-
>  criu/util.c            | 56 +++++++++++++++++++++++++++++++-----------
>  3 files changed, 44 insertions(+), 16 deletions(-)
> 
> diff --git a/Documentation/criu.txt b/Documentation/criu.txt
> index 6bb3f0f5..04e7a610 100644
> --- a/Documentation/criu.txt
> +++ b/Documentation/criu.txt
> @@ -550,7 +550,7 @@ Launches *criu* in page server mode.
>      It isn't supposed to use --daemon and --status-fd together.
>  
>  *--address* 'address'::
> -    Page server IP address.
> +    Page server IP address or hostname.
>  
>  *--port* 'number'::
>      Page server port number.
> diff --git a/criu/include/util.h b/criu/include/util.h
> index b286b3e9..aba16818 100644
> --- a/criu/include/util.h
> +++ b/criu/include/util.h
> @@ -293,7 +293,7 @@ void print_data(unsigned long addr, unsigned char *data, size_t size);
>  
>  int setup_tcp_server(char *type, char *addr, unsigned short *port);
>  int run_tcp_server(bool daemon_mode, int *ask, int cfd, int sk);
> -int setup_tcp_client(char *addr);
> +int setup_tcp_client(char *hostname);
>  
>  #define LAST_PID_PATH		"sys/kernel/ns_last_pid"
>  #define PID_MAX_PATH		"sys/kernel/pid_max"
> diff --git a/criu/util.c b/criu/util.c
> index 71bb2f6e..5c9fb306 100644
> --- a/criu/util.c
> +++ b/criu/util.c
> @@ -30,6 +30,7 @@
>  #include <sys/resource.h>
>  #include <sys/wait.h>
>  #include <sys/socket.h>
> +#include <netdb.h>
>  #include <netinet/in.h>
>  #include <netinet/tcp.h>
>  #include <sched.h>
> @@ -1384,28 +1385,55 @@ out:
>  	return -1;
>  }
>  
> -int setup_tcp_client(char *addr)
> +int setup_tcp_client(char *hostname)
>  {
>  	struct sockaddr_storage saddr;
> -	int sk;
> -
> -	pr_info("Connecting to server %s:%u\n", addr, opts.port);
> +	struct addrinfo addr_criteria, *addr_list, *p;
> +	char ipstr[INET6_ADDRSTRLEN];
> +	int sk = -1;
> +	void *ip;
>  
> -	if (get_sockaddr_in(&saddr, addr, opts.port))
> -		return -1;
> +	memset(&addr_criteria, 0, sizeof(addr_criteria));
> +	addr_criteria.ai_family = AF_UNSPEC;
> +	addr_criteria.ai_socktype = SOCK_STREAM;
> +	addr_criteria.ai_protocol = IPPROTO_TCP;
>  
> -	sk = socket(saddr.ss_family, SOCK_STREAM, IPPROTO_TCP);
> -	if (sk < 0) {
> -		pr_perror("Can't create socket");
> -		return -1;
> +	if (getaddrinfo(hostname, NULL, &addr_criteria, &addr_list)) {
> +		pr_perror("Failed to resolve hostname: %s", hostname);
> +		goto out;
>  	}
>  
> -	if (connect(sk, (struct sockaddr *)&saddr, sizeof(saddr)) < 0) {
> -		pr_perror("Can't connect to server");
> -		close(sk);
> -		return -1;
> +	for(p = addr_list; p != NULL; p = p->ai_next) {
> +
> +		if (p->ai_family == AF_INET) {
> +			struct sockaddr_in *ipv4 = (struct sockaddr_in *)p->ai_addr;
> +			ip = &(ipv4->sin_addr);
> +		} else {
> +			struct sockaddr_in6 *ipv6 = (struct sockaddr_in6 *)p->ai_addr;
> +			ip = &(ipv6->sin6_addr);
> +		}
> +
> +		inet_ntop(p->ai_family, ip, ipstr, sizeof(ipstr));
> +		pr_info("Connecting to server %s:%u\n", ipstr, opts.port);
> +
> +		if (get_sockaddr_in(&saddr, ipstr, opts.port))
> +			goto out;
> +
> +		sk = socket(saddr.ss_family, SOCK_STREAM, IPPROTO_TCP);
> +		if (sk < 0) {
> +			pr_perror("Can't create socket");
> +			goto out;
> +		}
> +
> +		if (connect(sk, (struct sockaddr *)&saddr, sizeof(saddr)) < 0) {
> +			pr_info("Can't connect to server %s:%u\n", ipstr, opts.port);
> +			close(sk);
> +			sk = -1;
> +		}

How many sockets are we going to create? Why do we need this loop?

>  	}
>  
> +out:
> +	freeaddrinfo(addr_list);
>  	return sk;
>  }
>  
> -- 
> 2.17.1
> 
> _______________________________________________
> CRIU mailing list
> CRIU@openvz.org
> https://lists.openvz.org/mailman/listinfo/criu
Radostin Stoyanov Sept. 10, 2018, 11:37 p.m.
On 10/09/18 20:04, Andrei Vagin wrote:
> On Tue, Sep 04, 2018 at 10:26:53PM +0100, Radostin Stoyanov wrote:
>> Add hostname resolution in setup_tcp_client(). This change allows a
>> valid hostname to be provided as value for the --address option when
>> connecting to page server.
>>
>> Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com>
>> ---
>>  Documentation/criu.txt |  2 +-
>>  criu/include/util.h    |  2 +-
>>  criu/util.c            | 56 +++++++++++++++++++++++++++++++-----------
>>  3 files changed, 44 insertions(+), 16 deletions(-)
>>
>> diff --git a/Documentation/criu.txt b/Documentation/criu.txt
>> index 6bb3f0f5..04e7a610 100644
>> --- a/Documentation/criu.txt
>> +++ b/Documentation/criu.txt
>> @@ -550,7 +550,7 @@ Launches *criu* in page server mode.
>>      It isn't supposed to use --daemon and --status-fd together.
>>  
>>  *--address* 'address'::
>> -    Page server IP address.
>> +    Page server IP address or hostname.
>>  
>>  *--port* 'number'::
>>      Page server port number.
>> diff --git a/criu/include/util.h b/criu/include/util.h
>> index b286b3e9..aba16818 100644
>> --- a/criu/include/util.h
>> +++ b/criu/include/util.h
>> @@ -293,7 +293,7 @@ void print_data(unsigned long addr, unsigned char *data, size_t size);
>>  
>>  int setup_tcp_server(char *type, char *addr, unsigned short *port);
>>  int run_tcp_server(bool daemon_mode, int *ask, int cfd, int sk);
>> -int setup_tcp_client(char *addr);
>> +int setup_tcp_client(char *hostname);
>>  
>>  #define LAST_PID_PATH		"sys/kernel/ns_last_pid"
>>  #define PID_MAX_PATH		"sys/kernel/pid_max"
>> diff --git a/criu/util.c b/criu/util.c
>> index 71bb2f6e..5c9fb306 100644
>> --- a/criu/util.c
>> +++ b/criu/util.c
>> @@ -30,6 +30,7 @@
>>  #include <sys/resource.h>
>>  #include <sys/wait.h>
>>  #include <sys/socket.h>
>> +#include <netdb.h>
>>  #include <netinet/in.h>
>>  #include <netinet/tcp.h>
>>  #include <sched.h>
>> @@ -1384,28 +1385,55 @@ out:
>>  	return -1;
>>  }
>>  
>> -int setup_tcp_client(char *addr)
>> +int setup_tcp_client(char *hostname)
>>  {
>>  	struct sockaddr_storage saddr;
>> -	int sk;
>> -
>> -	pr_info("Connecting to server %s:%u\n", addr, opts.port);
>> +	struct addrinfo addr_criteria, *addr_list, *p;
>> +	char ipstr[INET6_ADDRSTRLEN];
>> +	int sk = -1;
>> +	void *ip;
>>  
>> -	if (get_sockaddr_in(&saddr, addr, opts.port))
>> -		return -1;
>> +	memset(&addr_criteria, 0, sizeof(addr_criteria));
>> +	addr_criteria.ai_family = AF_UNSPEC;
>> +	addr_criteria.ai_socktype = SOCK_STREAM;
>> +	addr_criteria.ai_protocol = IPPROTO_TCP;
>>  
>> -	sk = socket(saddr.ss_family, SOCK_STREAM, IPPROTO_TCP);
>> -	if (sk < 0) {
>> -		pr_perror("Can't create socket");
>> -		return -1;
>> +	if (getaddrinfo(hostname, NULL, &addr_criteria, &addr_list)) {
>> +		pr_perror("Failed to resolve hostname: %s", hostname);
>> +		goto out;
>>  	}
>>  
>> -	if (connect(sk, (struct sockaddr *)&saddr, sizeof(saddr)) < 0) {
>> -		pr_perror("Can't connect to server");
>> -		close(sk);
>> -		return -1;
>> +	for(p = addr_list; p != NULL; p = p->ai_next) {
>> +
>> +		if (p->ai_family == AF_INET) {
>> +			struct sockaddr_in *ipv4 = (struct sockaddr_in *)p->ai_addr;
>> +			ip = &(ipv4->sin_addr);
>> +		} else {
>> +			struct sockaddr_in6 *ipv6 = (struct sockaddr_in6 *)p->ai_addr;
>> +			ip = &(ipv6->sin6_addr);
>> +		}
>> +
>> +		inet_ntop(p->ai_family, ip, ipstr, sizeof(ipstr));
>> +		pr_info("Connecting to server %s:%u\n", ipstr, opts.port);
>> +
>> +		if (get_sockaddr_in(&saddr, ipstr, opts.port))
>> +			goto out;
>> +
>> +		sk = socket(saddr.ss_family, SOCK_STREAM, IPPROTO_TCP);
>> +		if (sk < 0) {
>> +			pr_perror("Can't create socket");
>> +			goto out;
>> +		}
>> +
>> +		if (connect(sk, (struct sockaddr *)&saddr, sizeof(saddr)) < 0) {
>> +			pr_info("Can't connect to server %s:%u\n", ipstr, opts.port);
>> +			close(sk);
>> +			sk = -1;
>> +		}
> How many sockets are we going to create? Why do we need this loop?
Hi Andrei,

Thank you for the code review!

getaddrinfo() returns a list of socket address structures, each of which
contains an IP address and port number.

The `addr_list` argument returns a list of structures, rather than a
single structure,
because there may be multiple combinations of host and service
corresponding to
the criteria specified in hostname and addr_criteria.

For example, multiple address structures could be returned for a host
with more
than one network interface.

Here is a simple demo:

$ cat resolve_hostname.c
#include <stdio.h>
#include <string.h>
#include <netdb.h>
#include <arpa/inet.h>

int main(int argc, char **argv)
{
        char *addrString = argv[1];
        char buff[INET6_ADDRSTRLEN];

        struct addrinfo addrCriteria;
        memset(&addrCriteria, 0, sizeof(addrCriteria));
        addrCriteria.ai_family = AF_UNSPEC;
        addrCriteria.ai_socktype = SOCK_STREAM;
        addrCriteria.ai_protocol = IPPROTO_TCP;

        struct addrinfo *addrList;
        int ret = getaddrinfo(addrString, NULL, &addrCriteria, &addrList);
        if(ret != 0) {
                fprintf(stderr, "failed to get address list\n");
                return -1;
        }

        for(struct addrinfo *p = addrList; p != NULL; p = p->ai_next) {
                void *addr;
                char *ipver;

                if (p->ai_family == AF_INET)
                {
                        struct sockaddr_in *ipv4 = (struct sockaddr_in
*)p->ai_addr;
                        addr = &(ipv4->sin_addr);
                        ipver = "IPv4";
                }
                else
                {
                        struct sockaddr_in6 *ipv6 = (struct sockaddr_in6
*)p->ai_addr;
                        addr = &(ipv6->sin6_addr);
                        ipver = "IPv6";
                }

                inet_ntop(p->ai_family, addr, buff, sizeof(buff));
                printf("%s: %s\n", ipver, buff);
        }

        freeaddrinfo(addrList);
        return 0;
}

$ gcc resolve_hostname.c

$ ./a.out google.com
IPv6: 2a00:1450:4009:809::200e
IPv4: 216.58.214.14

In our case, setup_tcp_client() resolves the hostname to a list of ip
addresses, then it
loops through each address and tries to connect. The loop stops if the
connection is
successful, otherwise it continues. If the loop has finished and no
successful connection,
has been established, setup_tcp_client() returns -1.

The reason I decided to use `getaddrinfo` instead of `gethostbyname`
(this is what is used in setup_TCP_client_socket())
was because of the note below (copied from the man page gethostbyname(3)).

       The  gethostbyname*(),  gethostbyaddr*(),  herror(),  and
       hstrerror() functions are obsolete.  Applications  should
       use  getaddrinfo(3),  getnameinfo(3), and gai_strerror(3)
       instead.

Perhaps I should explain this in the commit message.

Radostin
Radostin Stoyanov Sept. 10, 2018, 11:54 p.m.
On 10/09/18 20:04, Andrei Vagin wrote:
> On Tue, Sep 04, 2018 at 10:26:53PM +0100, Radostin Stoyanov wrote:
>> Add hostname resolution in setup_tcp_client(). This change allows a
>> valid hostname to be provided as value for the --address option when
>> connecting to page server.
>>
>> Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com>
>> ---
>>  Documentation/criu.txt |  2 +-
>>  criu/include/util.h    |  2 +-
>>  criu/util.c            | 56 +++++++++++++++++++++++++++++++-----------
>>  3 files changed, 44 insertions(+), 16 deletions(-)
>>
>> diff --git a/Documentation/criu.txt b/Documentation/criu.txt
>> index 6bb3f0f5..04e7a610 100644
>> --- a/Documentation/criu.txt
>> +++ b/Documentation/criu.txt
>> @@ -550,7 +550,7 @@ Launches *criu* in page server mode.
>>      It isn't supposed to use --daemon and --status-fd together.
>>  
>>  *--address* 'address'::
>> -    Page server IP address.
>> +    Page server IP address or hostname.
>>  
>>  *--port* 'number'::
>>      Page server port number.
>> diff --git a/criu/include/util.h b/criu/include/util.h
>> index b286b3e9..aba16818 100644
>> --- a/criu/include/util.h
>> +++ b/criu/include/util.h
>> @@ -293,7 +293,7 @@ void print_data(unsigned long addr, unsigned char *data, size_t size);
>>  
>>  int setup_tcp_server(char *type, char *addr, unsigned short *port);
>>  int run_tcp_server(bool daemon_mode, int *ask, int cfd, int sk);
>> -int setup_tcp_client(char *addr);
>> +int setup_tcp_client(char *hostname);
>>  
>>  #define LAST_PID_PATH		"sys/kernel/ns_last_pid"
>>  #define PID_MAX_PATH		"sys/kernel/pid_max"
>> diff --git a/criu/util.c b/criu/util.c
>> index 71bb2f6e..5c9fb306 100644
>> --- a/criu/util.c
>> +++ b/criu/util.c
>> @@ -30,6 +30,7 @@
>>  #include <sys/resource.h>
>>  #include <sys/wait.h>
>>  #include <sys/socket.h>
>> +#include <netdb.h>
>>  #include <netinet/in.h>
>>  #include <netinet/tcp.h>
>>  #include <sched.h>
>> @@ -1384,28 +1385,55 @@ out:
>>  	return -1;
>>  }
>>  
>> -int setup_tcp_client(char *addr)
>> +int setup_tcp_client(char *hostname)
>>  {
>>  	struct sockaddr_storage saddr;
>> -	int sk;
>> -
>> -	pr_info("Connecting to server %s:%u\n", addr, opts.port);
>> +	struct addrinfo addr_criteria, *addr_list, *p;
>> +	char ipstr[INET6_ADDRSTRLEN];
>> +	int sk = -1;
>> +	void *ip;
>>  
>> -	if (get_sockaddr_in(&saddr, addr, opts.port))
>> -		return -1;
>> +	memset(&addr_criteria, 0, sizeof(addr_criteria));
>> +	addr_criteria.ai_family = AF_UNSPEC;
>> +	addr_criteria.ai_socktype = SOCK_STREAM;
>> +	addr_criteria.ai_protocol = IPPROTO_TCP;
>>  
>> -	sk = socket(saddr.ss_family, SOCK_STREAM, IPPROTO_TCP);
>> -	if (sk < 0) {
>> -		pr_perror("Can't create socket");
>> -		return -1;
>> +	if (getaddrinfo(hostname, NULL, &addr_criteria, &addr_list)) {
>> +		pr_perror("Failed to resolve hostname: %s", hostname);
>> +		goto out;
>>  	}
>>  
>> -	if (connect(sk, (struct sockaddr *)&saddr, sizeof(saddr)) < 0) {
>> -		pr_perror("Can't connect to server");
>> -		close(sk);
>> -		return -1;
>> +	for(p = addr_list; p != NULL; p = p->ai_next) {
>> +
>> +		if (p->ai_family == AF_INET) {
>> +			struct sockaddr_in *ipv4 = (struct sockaddr_in *)p->ai_addr;
>> +			ip = &(ipv4->sin_addr);
>> +		} else {
>> +			struct sockaddr_in6 *ipv6 = (struct sockaddr_in6 *)p->ai_addr;
>> +			ip = &(ipv6->sin6_addr);
>> +		}
>> +
>> +		inet_ntop(p->ai_family, ip, ipstr, sizeof(ipstr));
>> +		pr_info("Connecting to server %s:%u\n", ipstr, opts.port);
>> +
>> +		if (get_sockaddr_in(&saddr, ipstr, opts.port))
>> +			goto out;
>> +
>> +		sk = socket(saddr.ss_family, SOCK_STREAM, IPPROTO_TCP);
>> +		if (sk < 0) {
>> +			pr_perror("Can't create socket");
>> +			goto out;
>> +		}
>> +
>> +		if (connect(sk, (struct sockaddr *)&saddr, sizeof(saddr)) < 0) {
>> +			pr_info("Can't connect to server %s:%u\n", ipstr, opts.port);
>> +			close(sk);
>> +			sk = -1;
>> +		}
> How many sockets are we going to create? Why do we need this loop?
Oh, I just realised that I forgot to add break here (when the connection
is successful).

Thank you for pointing this out. I will resend the series.

Radostin
>
>>  	}
>>  
>> +out:
>> +	freeaddrinfo(addr_list);
>>  	return sk;
>>  }
>>  
>> -- 
>> 2.17.1
>>
>> _______________________________________________
>> CRIU mailing list
>> CRIU@openvz.org
>> https://lists.openvz.org/mailman/listinfo/criu