Allow criu page-server to work over ipv6.

Submitted by Pawel Stradomski on June 7, 2018, 10:32 a.m.

Details

Message ID 2787959.o5BILCkMpb@anselm.c.googlers.com
State Accepted
Series "Allow criu page-server to work over ipv6."
Commit 4001a324cee0e1f48bbb65be46d4c85f9337ab84
Headers show

Commit Message

Pawel Stradomski June 7, 2018, 10:32 a.m.
From 084e03dfb1a3069deebfb50dc3b8daf948477b9e Mon Sep 17 00:00:00 2001
From: Pawel Stradomski <pstradomski@google.com>
Date: Wed, 6 Jun 2018 21:09:13 +0200
Subject: [PATCH] Allow criu page-server to work over ipv6.

Signed-off-by: Pawel Stradomski <pstradomski@google.com>
---
 criu/util.c | 44 ++++++++++++++++++++++++++++++--------------
 1 file changed, 30 insertions(+), 14 deletions(-)

Patch hide | download patch | download mbox

diff --git a/criu/util.c b/criu/util.c
index be892bbb..b46f2964 100644
--- a/criu/util.c
+++ b/criu/util.c
@@ -1268,39 +1268,50 @@  void print_data(unsigned long addr, unsigned char *data, size_t size)
 	}
 }
 
-static int get_sockaddr_in(struct sockaddr_in *addr, char *host)
+static int get_sockaddr_in(struct sockaddr_storage *addr, char *host)
 {
 	memset(addr, 0, sizeof(*addr));
-	addr->sin_family = AF_INET;
 
-	if (!host)
-		addr->sin_addr.s_addr = INADDR_ANY;
-	else if (!inet_aton(host, &addr->sin_addr)) {
+	if (!host) {
+ 		((struct sockaddr_in *)addr)->sin_addr.s_addr = INADDR_ANY;
+		addr->ss_family = AF_INET;
+	} else if (inet_pton(AF_INET, host, &((struct sockaddr_in *)addr)->sin_addr)) {
+		addr->ss_family = AF_INET;
+	} else if (inet_pton(AF_INET6, host, &((struct sockaddr_in6 *)addr)->sin6_addr)) {
+		addr->ss_family = AF_INET6;
+	} else {
 		pr_perror("Bad server address");
 		return -1;
 	}
 
-	addr->sin_port = htons(opts.port);
+	if (addr->ss_family == AF_INET6) {
+		((struct sockaddr_in6 *)addr)->sin6_port = htons(opts.port);
+	} else if (addr->ss_family == AF_INET) {
+		((struct sockaddr_in *)addr)->sin_port = htons(opts.port);
+	}
+
 	return 0;
 }
 
 int setup_tcp_server(char *type)
 {
 	int sk = -1;
-	struct sockaddr_in saddr;
+	struct sockaddr_storage saddr;
 	socklen_t slen = sizeof(saddr);
 
+	if (get_sockaddr_in(&saddr, opts.addr)) {
+		return -1;
+	}
+
 	pr_info("Starting %s server on port %u\n", type, opts.port);
 
-	sk = socket(PF_INET, SOCK_STREAM, IPPROTO_TCP);
+	sk = socket(saddr.ss_family, SOCK_STREAM, IPPROTO_TCP);
+
 	if (sk < 0) {
 		pr_perror("Can't init %s server", type);
 		return -1;
 	}
 
-	if (get_sockaddr_in(&saddr, opts.addr))
-		goto out;
-
 	if (bind(sk, (struct sockaddr *)&saddr, slen)) {
 		pr_perror("Can't bind %s server", type);
 		goto out;
@@ -1318,7 +1329,12 @@  int setup_tcp_server(char *type)
 			goto out;
 		}
 
-		opts.port = ntohs(saddr.sin_port);
+		if (saddr.ss_family == AF_INET6) {
+			opts.port = ntohs(((struct sockaddr_in *)&saddr)->sin_port);
+		} else if (saddr.ss_family == AF_INET) {
+			opts.port = ntohs(((struct sockaddr_in6 *)&saddr)->sin6_port);
+		}
+
 		pr_info("Using %u port\n", opts.port);
 	}
 
@@ -1377,7 +1393,7 @@  out:
 
 int setup_tcp_client(char *addr)
 {
-	struct sockaddr_in saddr;
+	struct sockaddr_storage saddr;
 	int sk;
 
 	pr_info("Connecting to server %s:%u\n", addr, opts.port);
@@ -1385,7 +1401,7 @@  int setup_tcp_client(char *addr)
 	if (get_sockaddr_in(&saddr, addr))
 		return -1;
 
-	sk = socket(PF_INET, SOCK_STREAM, IPPROTO_TCP);
+	sk = socket(saddr.ss_family, SOCK_STREAM, IPPROTO_TCP);
 	if (sk < 0) {
 		pr_perror("Can't create socket");
 		return -1;

Comments

Andrey Vagin June 11, 2018, 10:49 p.m.
Applied, thanks!

On Thu, Jun 07, 2018 at 12:32:49PM +0200, Pawel Stradomski wrote:
> From 084e03dfb1a3069deebfb50dc3b8daf948477b9e Mon Sep 17 00:00:00 2001
> From: Pawel Stradomski <pstradomski@google.com>
> Date: Wed, 6 Jun 2018 21:09:13 +0200
> Subject: [PATCH] Allow criu page-server to work over ipv6.
> 
> Signed-off-by: Pawel Stradomski <pstradomski@google.com>
> ---
>  criu/util.c | 44 ++++++++++++++++++++++++++++++--------------
>  1 file changed, 30 insertions(+), 14 deletions(-)
> 
> diff --git a/criu/util.c b/criu/util.c
> index be892bbb..b46f2964 100644
> --- a/criu/util.c
> +++ b/criu/util.c
> @@ -1268,39 +1268,50 @@ void print_data(unsigned long addr, unsigned char *data, size_t size)
>  	}
>  }
>  
> -static int get_sockaddr_in(struct sockaddr_in *addr, char *host)
> +static int get_sockaddr_in(struct sockaddr_storage *addr, char *host)
>  {
>  	memset(addr, 0, sizeof(*addr));
> -	addr->sin_family = AF_INET;
>  
> -	if (!host)
> -		addr->sin_addr.s_addr = INADDR_ANY;
> -	else if (!inet_aton(host, &addr->sin_addr)) {
> +	if (!host) {
> + 		((struct sockaddr_in *)addr)->sin_addr.s_addr = INADDR_ANY;
> +		addr->ss_family = AF_INET;
> +	} else if (inet_pton(AF_INET, host, &((struct sockaddr_in *)addr)->sin_addr)) {
> +		addr->ss_family = AF_INET;
> +	} else if (inet_pton(AF_INET6, host, &((struct sockaddr_in6 *)addr)->sin6_addr)) {
> +		addr->ss_family = AF_INET6;
> +	} else {
>  		pr_perror("Bad server address");
>  		return -1;
>  	}
>  
> -	addr->sin_port = htons(opts.port);
> +	if (addr->ss_family == AF_INET6) {
> +		((struct sockaddr_in6 *)addr)->sin6_port = htons(opts.port);
> +	} else if (addr->ss_family == AF_INET) {
> +		((struct sockaddr_in *)addr)->sin_port = htons(opts.port);
> +	}
> +
>  	return 0;
>  }
>  
>  int setup_tcp_server(char *type)
>  {
>  	int sk = -1;
> -	struct sockaddr_in saddr;
> +	struct sockaddr_storage saddr;
>  	socklen_t slen = sizeof(saddr);
>  
> +	if (get_sockaddr_in(&saddr, opts.addr)) {
> +		return -1;
> +	}
> +
>  	pr_info("Starting %s server on port %u\n", type, opts.port);
>  
> -	sk = socket(PF_INET, SOCK_STREAM, IPPROTO_TCP);
> +	sk = socket(saddr.ss_family, SOCK_STREAM, IPPROTO_TCP);
> +
>  	if (sk < 0) {
>  		pr_perror("Can't init %s server", type);
>  		return -1;
>  	}
>  
> -	if (get_sockaddr_in(&saddr, opts.addr))
> -		goto out;
> -
>  	if (bind(sk, (struct sockaddr *)&saddr, slen)) {
>  		pr_perror("Can't bind %s server", type);
>  		goto out;
> @@ -1318,7 +1329,12 @@ int setup_tcp_server(char *type)
>  			goto out;
>  		}
>  
> -		opts.port = ntohs(saddr.sin_port);
> +		if (saddr.ss_family == AF_INET6) {
> +			opts.port = ntohs(((struct sockaddr_in *)&saddr)->sin_port);
> +		} else if (saddr.ss_family == AF_INET) {
> +			opts.port = ntohs(((struct sockaddr_in6 *)&saddr)->sin6_port);
> +		}
> +
>  		pr_info("Using %u port\n", opts.port);
>  	}
>  
> @@ -1377,7 +1393,7 @@ out:
>  
>  int setup_tcp_client(char *addr)
>  {
> -	struct sockaddr_in saddr;
> +	struct sockaddr_storage saddr;
>  	int sk;
>  
>  	pr_info("Connecting to server %s:%u\n", addr, opts.port);
> @@ -1385,7 +1401,7 @@ int setup_tcp_client(char *addr)
>  	if (get_sockaddr_in(&saddr, addr))
>  		return -1;
>  
> -	sk = socket(PF_INET, SOCK_STREAM, IPPROTO_TCP);
> +	sk = socket(saddr.ss_family, SOCK_STREAM, IPPROTO_TCP);
>  	if (sk < 0) {
>  		pr_perror("Can't create socket");
>  		return -1;
> -- 
> 2.18.0.rc1.242.g61856ae69a-goog
> 
> 
> -- 
> Pawel Stradomski
> _______________________________________________
> CRIU mailing list
> CRIU@openvz.org
> https://lists.openvz.org/mailman/listinfo/criu
Pawel Stradomski June 12, 2018, 9:07 a.m.
Thanks!
wt., 12 cze 2018 o 00:49 Andrei Vagin <avagin@virtuozzo.com> napisał(a):

> Applied, thanks!
>
> On Thu, Jun 07, 2018 at 12:32:49PM +0200, Pawel Stradomski wrote:
> > From 084e03dfb1a3069deebfb50dc3b8daf948477b9e Mon Sep 17 00:00:00 2001
> > From: Pawel Stradomski <pstradomski@google.com>
> > Date: Wed, 6 Jun 2018 21:09:13 +0200
> > Subject: [PATCH] Allow criu page-server to work over ipv6.
> >
> > Signed-off-by: Pawel Stradomski <pstradomski@google.com>
> > ---
> >  criu/util.c | 44 ++++++++++++++++++++++++++++++--------------
> >  1 file changed, 30 insertions(+), 14 deletions(-)
> >
> > diff --git a/criu/util.c b/criu/util.c
> > index be892bbb..b46f2964 100644
> > --- a/criu/util.c
> > +++ b/criu/util.c
> > @@ -1268,39 +1268,50 @@ void print_data(unsigned long addr, unsigned
> char *data, size_t size)
> >       }
> >  }
> >
> > -static int get_sockaddr_in(struct sockaddr_in *addr, char *host)
> > +static int get_sockaddr_in(struct sockaddr_storage *addr, char *host)
> >  {
> >       memset(addr, 0, sizeof(*addr));
> > -     addr->sin_family = AF_INET;
> >
> > -     if (!host)
> > -             addr->sin_addr.s_addr = INADDR_ANY;
> > -     else if (!inet_aton(host, &addr->sin_addr)) {
> > +     if (!host) {
> > +             ((struct sockaddr_in *)addr)->sin_addr.s_addr = INADDR_ANY;
> > +             addr->ss_family = AF_INET;
> > +     } else if (inet_pton(AF_INET, host, &((struct sockaddr_in
> *)addr)->sin_addr)) {
> > +             addr->ss_family = AF_INET;
> > +     } else if (inet_pton(AF_INET6, host, &((struct sockaddr_in6
> *)addr)->sin6_addr)) {
> > +             addr->ss_family = AF_INET6;
> > +     } else {
> >               pr_perror("Bad server address");
> >               return -1;
> >       }
> >
> > -     addr->sin_port = htons(opts.port);
> > +     if (addr->ss_family == AF_INET6) {
> > +             ((struct sockaddr_in6 *)addr)->sin6_port =
> htons(opts.port);
> > +     } else if (addr->ss_family == AF_INET) {
> > +             ((struct sockaddr_in *)addr)->sin_port = htons(opts.port);
> > +     }
> > +
> >       return 0;
> >  }
> >
> >  int setup_tcp_server(char *type)
> >  {
> >       int sk = -1;
> > -     struct sockaddr_in saddr;
> > +     struct sockaddr_storage saddr;
> >       socklen_t slen = sizeof(saddr);
> >
> > +     if (get_sockaddr_in(&saddr, opts.addr)) {
> > +             return -1;
> > +     }
> > +
> >       pr_info("Starting %s server on port %u\n", type, opts.port);
> >
> > -     sk = socket(PF_INET, SOCK_STREAM, IPPROTO_TCP);
> > +     sk = socket(saddr.ss_family, SOCK_STREAM, IPPROTO_TCP);
> > +
> >       if (sk < 0) {
> >               pr_perror("Can't init %s server", type);
> >               return -1;
> >       }
> >
> > -     if (get_sockaddr_in(&saddr, opts.addr))
> > -             goto out;
> > -
> >       if (bind(sk, (struct sockaddr *)&saddr, slen)) {
> >               pr_perror("Can't bind %s server", type);
> >               goto out;
> > @@ -1318,7 +1329,12 @@ int setup_tcp_server(char *type)
> >                       goto out;
> >               }
> >
> > -             opts.port = ntohs(saddr.sin_port);
> > +             if (saddr.ss_family == AF_INET6) {
> > +                     opts.port = ntohs(((struct sockaddr_in
> *)&saddr)->sin_port);
> > +             } else if (saddr.ss_family == AF_INET) {
> > +                     opts.port = ntohs(((struct sockaddr_in6
> *)&saddr)->sin6_port);
> > +             }
> > +
> >               pr_info("Using %u port\n", opts.port);
> >       }
> >
> > @@ -1377,7 +1393,7 @@ out:
> >
> >  int setup_tcp_client(char *addr)
> >  {
> > -     struct sockaddr_in saddr;
> > +     struct sockaddr_storage saddr;
> >       int sk;
> >
> >       pr_info("Connecting to server %s:%u\n", addr, opts.port);
> > @@ -1385,7 +1401,7 @@ int setup_tcp_client(char *addr)
> >       if (get_sockaddr_in(&saddr, addr))
> >               return -1;
> >
> > -     sk = socket(PF_INET, SOCK_STREAM, IPPROTO_TCP);
> > +     sk = socket(saddr.ss_family, SOCK_STREAM, IPPROTO_TCP);
> >       if (sk < 0) {
> >               pr_perror("Can't create socket");
> >               return -1;
> > --
> > 2.18.0.rc1.242.g61856ae69a-goog
> >
> >
> > --
> > Pawel Stradomski
> > _______________________________________________
> > CRIU mailing list
> > CRIU@openvz.org
> > https://lists.openvz.org/mailman/listinfo/criu
>
Radostin Stoyanov Aug. 30, 2018, 1:03 p.m.
On 07/06/18 11:32, Pawel Stradomski wrote:
> +		if (saddr.ss_family == AF_INET6) {
> +			opts.port = ntohs(((struct sockaddr_in *)&saddr)->sin_port);
> +		} else if (saddr.ss_family == AF_INET) {
> +			opts.port = ntohs(((struct sockaddr_in6 *)&saddr)->sin6_port);
> +		}
> +

This patch has been merged into master but I noticed this part from
setup_tcp_server() in util.c
and I was wondering if the logic in this condition should be reversed?

For example:
       
  if (saddr.ss_family == AF_INET6) {
    opts.port = ntohs(((struct sockaddr_in6 *)&saddr)->sin6_port);
  } else if (saddr.ss_family == AF_INET) {
    opts.port = ntohs(((struct sockaddr_in *)&saddr)->sin_port);
  }

Thanks,
Radostin
Pawel Stradomski Aug. 30, 2018, 1:08 p.m.
Indeed, looks wrong. It works merely by accident - both structs have port
field at the same offset.

czw., 30 sie 2018 o 15:03 Radostin Stoyanov <rstoyanov1@gmail.com>
napisał(a):

> On 07/06/18 11:32, Pawel Stradomski wrote:
> > +             if (saddr.ss_family == AF_INET6) {
> > +                     opts.port = ntohs(((struct sockaddr_in
> *)&saddr)->sin_port);
> > +             } else if (saddr.ss_family == AF_INET) {
> > +                     opts.port = ntohs(((struct sockaddr_in6
> *)&saddr)->sin6_port);
> > +             }
> > +
>
> This patch has been merged into master but I noticed this part from
> setup_tcp_server() in util.c
> and I was wondering if the logic in this condition should be reversed?
>
> For example:
>
>   if (saddr.ss_family == AF_INET6) {
>     opts.port = ntohs(((struct sockaddr_in6 *)&saddr)->sin6_port);
>   } else if (saddr.ss_family == AF_INET) {
>     opts.port = ntohs(((struct sockaddr_in *)&saddr)->sin_port);
>   }
>
> Thanks,
> Radostin
>