[PATCH] conf, pasta: Make -g and -a skip route/addresses copy for matching IP version only

[PATCH] conf, pasta: Make -g and -a skip route/addresses copy for matching IP version only

[Stefano Brivio]

Paul reports that setting IPv4 address and gateway manually, using
--address and --gateway, causes pasta to fail inserting IPv6 routes
in a setup where multiple, inter-dependent IPv6 routes are present
on the host.

That's because, currently, any -g option implies --no-copy-routes
altogether, and any -a implies --no-copy-addrs.

Limit this implication to the matching IP version, instead, by having
two copies of no_copy_routes and no_copy_addrs in the context
structure, separately for IPv4 and IPv6.

While at it, change them to 'bool': we had them as 'int' because
getopt_long() used to set them directly, but it hasn't been the case
for a while already.

Reported-by: Paul Holzinger <pholzing@redhat.com>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 conf.c  | 32 ++++++++++++++++++++------------
 passt.1 |  4 ++--
 passt.h | 14 ++++++++++----
 pasta.c |  8 ++++----
 4 files changed, 36 insertions(+), 22 deletions(-)

diff --git a/conf.c b/conf.c
index 46fcd91..14d8ece 100644
--- a/conf.c
+++ b/conf.c
@@ -1379,14 +1379,16 @@ void conf(struct ctx *c, int argc, char **argv)
 				die("--no-copy-routes is for pasta mode only");
 
 			warn("--no-copy-routes will be dropped soon");
-			c->no_copy_routes = copy_routes_opt = true;
+			c->ip4.no_copy_routes = c->ip6.no_copy_routes = true;
+			copy_routes_opt = true;
 			break;
 		case 19:
 			if (c->mode != MODE_PASTA)
 				die("--no-copy-addrs is for pasta mode only");
 
 			warn("--no-copy-addrs will be dropped soon");
-			c->no_copy_addrs = copy_addrs_opt = true;
+			c->ip4.no_copy_addrs = c->ip6.no_copy_addrs = true;
+			copy_addrs_opt = true;
 			break;
 		case 20:
 			if (c->mode != MODE_PASTA)
@@ -1465,23 +1467,26 @@ void conf(struct ctx *c, int argc, char **argv)
 
 			break;
 		case 'a':
-			if (c->mode == MODE_PASTA)
-				c->no_copy_addrs = 1;
-
 			if (inet_pton(AF_INET6, optarg, &c->ip6.addr)	&&
 			    !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr)	&&
 			    !IN6_IS_ADDR_LOOPBACK(&c->ip6.addr)		&&
 			    !IN6_IS_ADDR_V4MAPPED(&c->ip6.addr)		&&
 			    !IN6_IS_ADDR_V4COMPAT(&c->ip6.addr)		&&
-			    !IN6_IS_ADDR_MULTICAST(&c->ip6.addr))
+			    !IN6_IS_ADDR_MULTICAST(&c->ip6.addr)) {
+				if (c->mode == MODE_PASTA)
+					c->ip6.no_copy_addrs = true;
 				break;
+			}
 
 			if (inet_pton(AF_INET, optarg, &c->ip4.addr)	&&
 			    !IN4_IS_ADDR_UNSPECIFIED(&c->ip4.addr)	&&
 			    !IN4_IS_ADDR_BROADCAST(&c->ip4.addr)	&&
 			    !IN4_IS_ADDR_LOOPBACK(&c->ip4.addr)		&&
-			    !IN4_IS_ADDR_MULTICAST(&c->ip4.addr))
+			    !IN4_IS_ADDR_MULTICAST(&c->ip4.addr)) {
+				if (c->mode == MODE_PASTA)
+					c->ip4.no_copy_addrs = true;
 				break;
+			}
 
 			die("Invalid address: %s", optarg);
 			break;
@@ -1495,19 +1500,22 @@ void conf(struct ctx *c, int argc, char **argv)
 			parse_mac(c->mac, optarg);
 			break;
 		case 'g':
-			if (c->mode == MODE_PASTA)
-				c->no_copy_routes = 1;
-
 			if (inet_pton(AF_INET6, optarg, &c->ip6.gw)     &&
 			    !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.gw)	&&
-			    !IN6_IS_ADDR_LOOPBACK(&c->ip6.gw))
+			    !IN6_IS_ADDR_LOOPBACK(&c->ip6.gw)) {
+				if (c->mode == MODE_PASTA)
+					c->ip6.no_copy_routes = true;
 				break;
+			}
 
 			if (inet_pton(AF_INET, optarg, &c->ip4.gw)	&&
 			    !IN4_IS_ADDR_UNSPECIFIED(&c->ip4.gw)	&&
 			    !IN4_IS_ADDR_BROADCAST(&c->ip4.gw)		&&
-			    !IN4_IS_ADDR_LOOPBACK(&c->ip4.gw))
+			    !IN4_IS_ADDR_LOOPBACK(&c->ip4.gw)) {
+				if (c->mode == MODE_PASTA)
+					c->ip4.no_copy_routes = true;
 				break;
+			}
 
 			die("Invalid gateway address: %s", optarg);
 			break;
diff --git a/passt.1 b/passt.1
index 81789cc..3062b71 100644
--- a/passt.1
+++ b/passt.1
@@ -589,7 +589,7 @@ or sourced from the host, and bring up the tap interface.
 .BR \-\-no-copy-routes " " (DEPRECATED)
 With \-\-config-net, do not copy all the routes associated to the interface we
 derive addresses and routes from: set up only the default gateway. Implied by
--g, \-\-gateway.
+-g, \-\-gateway, for the corresponding IP version only.
 
 Default is to copy all the routing entries from the interface in the outer
 namespace to the target namespace, translating the output interface attribute to
@@ -604,7 +604,7 @@ below.
 .BR \-\-no-copy-addrs " " (DEPRECATED)
 With \-\-config-net, do not copy all the addresses associated to the interface
 we derive addresses and routes from: set up a single one. Implied by \-a,
-\-\-address.
+\-\-address, for the corresponding IP version only.
 
 Default is to copy all the addresses, except for link-local ones, from the
 interface from the outer namespace to the target namespace.
diff --git a/passt.h b/passt.h
index d0f31a2..12ae1b9 100644
--- a/passt.h
+++ b/passt.h
@@ -100,6 +100,8 @@ enum passt_modes {
  * @dns_host:		Use this DNS on the host for forwarding
  * @addr_out:		Optional source address for outbound traffic
  * @ifname_out:		Optional interface name to bind outbound sockets to
+ * @no_copy_routes:	Don't copy all routes when configuring target namespace
+ * @no_copy_addrs:	Don't copy all addresses when configuring namespace
  */
 struct ip4_ctx {
 	struct in_addr addr;
@@ -112,6 +114,9 @@ struct ip4_ctx {
 
 	struct in_addr addr_out;
 	char ifname_out[IFNAMSIZ];
+
+	bool no_copy_routes;
+	bool no_copy_addrs;
 };
 
 /**
@@ -126,6 +131,8 @@ struct ip4_ctx {
  * @dns_host:		Use this DNS on the host for forwarding
  * @addr_out:		Optional source address for outbound traffic
  * @ifname_out:		Optional interface name to bind outbound sockets to
+ * @no_copy_routes:	Don't copy all routes when configuring target namespace
+ * @no_copy_addrs:	Don't copy all addresses when configuring namespace
  */
 struct ip6_ctx {
 	struct in6_addr addr;
@@ -139,6 +146,9 @@ struct ip6_ctx {
 
 	struct in6_addr addr_out;
 	char ifname_out[IFNAMSIZ];
+
+	bool no_copy_routes;
+	bool no_copy_addrs;
 };
 
 #include <netinet/if_ether.h>
@@ -173,8 +183,6 @@ struct ip6_ctx {
  * @pasta_ifn:		Name of namespace interface for pasta
  * @pasta_ifi:		Index of namespace interface for pasta
  * @pasta_conf_ns:	Configure namespace after creating it
- * @no_copy_routes:	Don't copy all routes when configuring target namespace
- * @no_copy_addrs:	Don't copy all addresses when configuring namespace
  * @no_tcp:		Disable TCP operation
  * @tcp:		Context for TCP protocol handler
  * @no_tcp:		Disable UDP operation
@@ -233,8 +241,6 @@ struct ctx {
 	char pasta_ifn[IF_NAMESIZE];
 	unsigned int pasta_ifi;
 	int pasta_conf_ns;
-	int no_copy_routes;
-	int no_copy_addrs;
 
 	int no_tcp;
 	struct tcp_ctx tcp;
diff --git a/pasta.c b/pasta.c
index b4a3d99..615ff7b 100644
--- a/pasta.c
+++ b/pasta.c
@@ -306,7 +306,7 @@ void pasta_ns_conf(struct ctx *c)
 		nl_link_up(nl_sock_ns, c->pasta_ifi, c->mtu);
 
 		if (c->ifi4) {
-			if (c->no_copy_addrs) {
+			if (c->ip4.no_copy_addrs) {
 				rc = nl_addr_set(nl_sock_ns, c->pasta_ifi,
 						 AF_INET,
 						 &c->ip4.addr,
@@ -322,7 +322,7 @@ void pasta_ns_conf(struct ctx *c)
 				    strerror(-rc));
 			}
 
-			if (c->no_copy_routes) {
+			if (c->ip4.no_copy_routes) {
 				rc = nl_route_set_def(nl_sock_ns, c->pasta_ifi,
 						      AF_INET, &c->ip4.gw);
 			} else {
@@ -337,7 +337,7 @@ void pasta_ns_conf(struct ctx *c)
 		}
 
 		if (c->ifi6) {
-			if (c->no_copy_addrs) {
+			if (c->ip6.no_copy_addrs) {
 				rc = nl_addr_set(nl_sock_ns, c->pasta_ifi,
 						 AF_INET6, &c->ip6.addr, 64);
 			} else {
@@ -351,7 +351,7 @@ void pasta_ns_conf(struct ctx *c)
 				    strerror(-rc));
 			}
 
-			if (c->no_copy_routes) {
+			if (c->ip6.no_copy_routes) {
 				rc = nl_route_set_def(nl_sock_ns, c->pasta_ifi,
 						      AF_INET6, &c->ip6.gw);
 			} else {
-- 
@@ -306,7 +306,7 @@ void pasta_ns_conf(struct ctx *c)
 		nl_link_up(nl_sock_ns, c->pasta_ifi, c->mtu);
 
 		if (c->ifi4) {
-			if (c->no_copy_addrs) {
+			if (c->ip4.no_copy_addrs) {
 				rc = nl_addr_set(nl_sock_ns, c->pasta_ifi,
 						 AF_INET,
 						 &c->ip4.addr,
@@ -322,7 +322,7 @@ void pasta_ns_conf(struct ctx *c)
 				    strerror(-rc));
 			}
 
-			if (c->no_copy_routes) {
+			if (c->ip4.no_copy_routes) {
 				rc = nl_route_set_def(nl_sock_ns, c->pasta_ifi,
 						      AF_INET, &c->ip4.gw);
 			} else {
@@ -337,7 +337,7 @@ void pasta_ns_conf(struct ctx *c)
 		}
 
 		if (c->ifi6) {
-			if (c->no_copy_addrs) {
+			if (c->ip6.no_copy_addrs) {
 				rc = nl_addr_set(nl_sock_ns, c->pasta_ifi,
 						 AF_INET6, &c->ip6.addr, 64);
 			} else {
@@ -351,7 +351,7 @@ void pasta_ns_conf(struct ctx *c)
 				    strerror(-rc));
 			}
 
-			if (c->no_copy_routes) {
+			if (c->ip6.no_copy_routes) {
 				rc = nl_route_set_def(nl_sock_ns, c->pasta_ifi,
 						      AF_INET6, &c->ip6.gw);
 			} else {
-- 
2.43.0

Re: [PATCH] conf, pasta: Make -g and -a skip route/addresses copy for matching IP version only

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 8605 bytes --]

On Tue, Aug 06, 2024 at 08:38:22PM +0200, Stefano Brivio wrote:
> Paul reports that setting IPv4 address and gateway manually, using
> --address and --gateway, causes pasta to fail inserting IPv6 routes
> in a setup where multiple, inter-dependent IPv6 routes are present
> on the host.
> 
> That's because, currently, any -g option implies --no-copy-routes
> altogether, and any -a implies --no-copy-addrs.
> 
> Limit this implication to the matching IP version, instead, by having
> two copies of no_copy_routes and no_copy_addrs in the context
> structure, separately for IPv4 and IPv6.
> 
> While at it, change them to 'bool': we had them as 'int' because
> getopt_long() used to set them directly, but it hasn't been the case
> for a while already.
> 
> Reported-by: Paul Holzinger <pholzing@redhat.com>
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  conf.c  | 32 ++++++++++++++++++++------------
>  passt.1 |  4 ++--
>  passt.h | 14 ++++++++++----
>  pasta.c |  8 ++++----
>  4 files changed, 36 insertions(+), 22 deletions(-)
> 
> diff --git a/conf.c b/conf.c
> index 46fcd91..14d8ece 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -1379,14 +1379,16 @@ void conf(struct ctx *c, int argc, char **argv)
>  				die("--no-copy-routes is for pasta mode only");
>  
>  			warn("--no-copy-routes will be dropped soon");
> -			c->no_copy_routes = copy_routes_opt = true;
> +			c->ip4.no_copy_routes = c->ip6.no_copy_routes = true;
> +			copy_routes_opt = true;
>  			break;
>  		case 19:
>  			if (c->mode != MODE_PASTA)
>  				die("--no-copy-addrs is for pasta mode only");
>  
>  			warn("--no-copy-addrs will be dropped soon");
> -			c->no_copy_addrs = copy_addrs_opt = true;
> +			c->ip4.no_copy_addrs = c->ip6.no_copy_addrs = true;
> +			copy_addrs_opt = true;
>  			break;
>  		case 20:
>  			if (c->mode != MODE_PASTA)
> @@ -1465,23 +1467,26 @@ void conf(struct ctx *c, int argc, char **argv)
>  
>  			break;
>  		case 'a':
> -			if (c->mode == MODE_PASTA)
> -				c->no_copy_addrs = 1;
> -
>  			if (inet_pton(AF_INET6, optarg, &c->ip6.addr)	&&
>  			    !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr)	&&
>  			    !IN6_IS_ADDR_LOOPBACK(&c->ip6.addr)		&&
>  			    !IN6_IS_ADDR_V4MAPPED(&c->ip6.addr)		&&
>  			    !IN6_IS_ADDR_V4COMPAT(&c->ip6.addr)		&&
> -			    !IN6_IS_ADDR_MULTICAST(&c->ip6.addr))
> +			    !IN6_IS_ADDR_MULTICAST(&c->ip6.addr)) {
> +				if (c->mode == MODE_PASTA)
> +					c->ip6.no_copy_addrs = true;
>  				break;
> +			}
>  
>  			if (inet_pton(AF_INET, optarg, &c->ip4.addr)	&&
>  			    !IN4_IS_ADDR_UNSPECIFIED(&c->ip4.addr)	&&
>  			    !IN4_IS_ADDR_BROADCAST(&c->ip4.addr)	&&
>  			    !IN4_IS_ADDR_LOOPBACK(&c->ip4.addr)		&&
> -			    !IN4_IS_ADDR_MULTICAST(&c->ip4.addr))
> +			    !IN4_IS_ADDR_MULTICAST(&c->ip4.addr)) {
> +				if (c->mode == MODE_PASTA)
> +					c->ip4.no_copy_addrs = true;
>  				break;
> +			}
>  
>  			die("Invalid address: %s", optarg);
>  			break;
> @@ -1495,19 +1500,22 @@ void conf(struct ctx *c, int argc, char **argv)
>  			parse_mac(c->mac, optarg);
>  			break;
>  		case 'g':
> -			if (c->mode == MODE_PASTA)
> -				c->no_copy_routes = 1;
> -
>  			if (inet_pton(AF_INET6, optarg, &c->ip6.gw)     &&
>  			    !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.gw)	&&
> -			    !IN6_IS_ADDR_LOOPBACK(&c->ip6.gw))
> +			    !IN6_IS_ADDR_LOOPBACK(&c->ip6.gw)) {
> +				if (c->mode == MODE_PASTA)
> +					c->ip6.no_copy_routes = true;
>  				break;
> +			}
>  
>  			if (inet_pton(AF_INET, optarg, &c->ip4.gw)	&&
>  			    !IN4_IS_ADDR_UNSPECIFIED(&c->ip4.gw)	&&
>  			    !IN4_IS_ADDR_BROADCAST(&c->ip4.gw)		&&
> -			    !IN4_IS_ADDR_LOOPBACK(&c->ip4.gw))
> +			    !IN4_IS_ADDR_LOOPBACK(&c->ip4.gw)) {
> +				if (c->mode == MODE_PASTA)
> +					c->ip4.no_copy_routes = true;
>  				break;
> +			}
>  
>  			die("Invalid gateway address: %s", optarg);
>  			break;
> diff --git a/passt.1 b/passt.1
> index 81789cc..3062b71 100644
> --- a/passt.1
> +++ b/passt.1
> @@ -589,7 +589,7 @@ or sourced from the host, and bring up the tap interface.
>  .BR \-\-no-copy-routes " " (DEPRECATED)
>  With \-\-config-net, do not copy all the routes associated to the interface we
>  derive addresses and routes from: set up only the default gateway. Implied by
> --g, \-\-gateway.
> +-g, \-\-gateway, for the corresponding IP version only.
>  
>  Default is to copy all the routing entries from the interface in the outer
>  namespace to the target namespace, translating the output interface attribute to
> @@ -604,7 +604,7 @@ below.
>  .BR \-\-no-copy-addrs " " (DEPRECATED)
>  With \-\-config-net, do not copy all the addresses associated to the interface
>  we derive addresses and routes from: set up a single one. Implied by \-a,
> -\-\-address.
> +\-\-address, for the corresponding IP version only.
>  
>  Default is to copy all the addresses, except for link-local ones, from the
>  interface from the outer namespace to the target namespace.
> diff --git a/passt.h b/passt.h
> index d0f31a2..12ae1b9 100644
> --- a/passt.h
> +++ b/passt.h
> @@ -100,6 +100,8 @@ enum passt_modes {
>   * @dns_host:		Use this DNS on the host for forwarding
>   * @addr_out:		Optional source address for outbound traffic
>   * @ifname_out:		Optional interface name to bind outbound sockets to
> + * @no_copy_routes:	Don't copy all routes when configuring target namespace
> + * @no_copy_addrs:	Don't copy all addresses when configuring namespace
>   */
>  struct ip4_ctx {
>  	struct in_addr addr;
> @@ -112,6 +114,9 @@ struct ip4_ctx {
>  
>  	struct in_addr addr_out;
>  	char ifname_out[IFNAMSIZ];
> +
> +	bool no_copy_routes;
> +	bool no_copy_addrs;
>  };
>  
>  /**
> @@ -126,6 +131,8 @@ struct ip4_ctx {
>   * @dns_host:		Use this DNS on the host for forwarding
>   * @addr_out:		Optional source address for outbound traffic
>   * @ifname_out:		Optional interface name to bind outbound sockets to
> + * @no_copy_routes:	Don't copy all routes when configuring target namespace
> + * @no_copy_addrs:	Don't copy all addresses when configuring namespace
>   */
>  struct ip6_ctx {
>  	struct in6_addr addr;
> @@ -139,6 +146,9 @@ struct ip6_ctx {
>  
>  	struct in6_addr addr_out;
>  	char ifname_out[IFNAMSIZ];
> +
> +	bool no_copy_routes;
> +	bool no_copy_addrs;
>  };
>  
>  #include <netinet/if_ether.h>
> @@ -173,8 +183,6 @@ struct ip6_ctx {
>   * @pasta_ifn:		Name of namespace interface for pasta
>   * @pasta_ifi:		Index of namespace interface for pasta
>   * @pasta_conf_ns:	Configure namespace after creating it
> - * @no_copy_routes:	Don't copy all routes when configuring target namespace
> - * @no_copy_addrs:	Don't copy all addresses when configuring namespace
>   * @no_tcp:		Disable TCP operation
>   * @tcp:		Context for TCP protocol handler
>   * @no_tcp:		Disable UDP operation
> @@ -233,8 +241,6 @@ struct ctx {
>  	char pasta_ifn[IF_NAMESIZE];
>  	unsigned int pasta_ifi;
>  	int pasta_conf_ns;
> -	int no_copy_routes;
> -	int no_copy_addrs;
>  
>  	int no_tcp;
>  	struct tcp_ctx tcp;
> diff --git a/pasta.c b/pasta.c
> index b4a3d99..615ff7b 100644
> --- a/pasta.c
> +++ b/pasta.c
> @@ -306,7 +306,7 @@ void pasta_ns_conf(struct ctx *c)
>  		nl_link_up(nl_sock_ns, c->pasta_ifi, c->mtu);
>  
>  		if (c->ifi4) {
> -			if (c->no_copy_addrs) {
> +			if (c->ip4.no_copy_addrs) {
>  				rc = nl_addr_set(nl_sock_ns, c->pasta_ifi,
>  						 AF_INET,
>  						 &c->ip4.addr,
> @@ -322,7 +322,7 @@ void pasta_ns_conf(struct ctx *c)
>  				    strerror(-rc));
>  			}
>  
> -			if (c->no_copy_routes) {
> +			if (c->ip4.no_copy_routes) {
>  				rc = nl_route_set_def(nl_sock_ns, c->pasta_ifi,
>  						      AF_INET, &c->ip4.gw);
>  			} else {
> @@ -337,7 +337,7 @@ void pasta_ns_conf(struct ctx *c)
>  		}
>  
>  		if (c->ifi6) {
> -			if (c->no_copy_addrs) {
> +			if (c->ip6.no_copy_addrs) {
>  				rc = nl_addr_set(nl_sock_ns, c->pasta_ifi,
>  						 AF_INET6, &c->ip6.addr, 64);
>  			} else {
> @@ -351,7 +351,7 @@ void pasta_ns_conf(struct ctx *c)
>  				    strerror(-rc));
>  			}
>  
> -			if (c->no_copy_routes) {
> +			if (c->ip6.no_copy_routes) {
>  				rc = nl_route_set_def(nl_sock_ns, c->pasta_ifi,
>  						      AF_INET6, &c->ip6.gw);
>  			} else {

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] conf, pasta: Make -g and -a skip route/addresses copy for matching IP version only

[Paul Holzinger]

On 06/08/2024 20:38, Stefano Brivio wrote:
> Paul reports that setting IPv4 address and gateway manually, using
> --address and --gateway, causes pasta to fail inserting IPv6 routes
> in a setup where multiple, inter-dependent IPv6 routes are present
> on the host.
>
> That's because, currently, any -g option implies --no-copy-routes
> altogether, and any -a implies --no-copy-addrs.
>
> Limit this implication to the matching IP version, instead, by having
> two copies of no_copy_routes and no_copy_addrs in the context
> structure, separately for IPv4 and IPv6.
>
> While at it, change them to 'bool': we had them as 'int' because
> getopt_long() used to set them directly, but it hasn't been the case
> for a while already.
>
> Reported-by: Paul Holzinger <pholzing@redhat.com>
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

Tested-by: Paul Holzinger <pholzing@redhat.com>

I got confused for a moment because `pasta --config-net ip -6 route` did not
show any routes, well turns out the -6 was parsed by pasta so I had to do
`pasta --config-net -- ip -6 route` and it works as expected now.

> ---
>   conf.c  | 32 ++++++++++++++++++++------------
>   passt.1 |  4 ++--
>   passt.h | 14 ++++++++++----
>   pasta.c |  8 ++++----
>   4 files changed, 36 insertions(+), 22 deletions(-)
>
-- 
Paul Holzinger