From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 1166C5A004E for ; Wed, 07 Aug 2024 04:04:23 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1722996251; bh=7utWfs9U7xo3TEsknObnch4Kcs6iJdK69OYScgWCSmk=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=lorvlYUBtjXM8oOrmhsjmRhWnRhCNIeaGAsHijhRmVshCpQYCtQDAtGayW9UYQKi7 7ct8cXsIuNz1il7kIytpabWKhjaB1AzEyDr3G2clxRzl6FDSPe4Edyr5TZ79xJRB6g 30GV5CMP30c8WjCohuK7mi92TMXxsarZuYr2DAizWNqiibheAbRH8ZYqeFruQZpjx9 gwonIjNyUn6QtCB54gE1gZezFRlRGkggarZWMAinfGpCKKs8Ro7K4pn+kTdNSGMvft dYNDdycSovpzn6UwWq1U6eIYzDCsI+uRwYN0pQeY2YcRcVwfVr6ZY2lrdKwOecHv9W LZ2zypw+WWudw== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4WdtlH3gs3z4wd6; Wed, 7 Aug 2024 12:04:11 +1000 (AEST) Date: Wed, 7 Aug 2024 11:35:46 +1000 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH] conf, pasta: Make -g and -a skip route/addresses copy for matching IP version only Message-ID: References: <20240806183822.547868-1-sbrivio@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="3S8ogFb2I0CJwpjM" Content-Disposition: inline In-Reply-To: <20240806183822.547868-1-sbrivio@redhat.com> Message-ID-Hash: 7AK2KZHQQ3MO5ROAC5JSJRIGVFP2KLTO X-Message-ID-Hash: 7AK2KZHQQ3MO5ROAC5JSJRIGVFP2KLTO X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top, Paul Holzinger X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --3S8ogFb2I0CJwpjM Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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. >=20 > That's because, currently, any -g option implies --no-copy-routes > altogether, and any -a implies --no-copy-addrs. >=20 > 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. >=20 > 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. >=20 > Reported-by: Paul Holzinger > Signed-off-by: Stefano Brivio Reviewed-by: David Gibson > --- > conf.c | 32 ++++++++++++++++++++------------ > passt.1 | 4 ++-- > passt.h | 14 ++++++++++---- > pasta.c | 8 ++++---- > 4 files changed, 36 insertions(+), 22 deletions(-) >=20 > 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"); > =20 > warn("--no-copy-routes will be dropped soon"); > - c->no_copy_routes =3D copy_routes_opt =3D true; > + c->ip4.no_copy_routes =3D c->ip6.no_copy_routes =3D true; > + copy_routes_opt =3D true; > break; > case 19: > if (c->mode !=3D MODE_PASTA) > die("--no-copy-addrs is for pasta mode only"); > =20 > warn("--no-copy-addrs will be dropped soon"); > - c->no_copy_addrs =3D copy_addrs_opt =3D true; > + c->ip4.no_copy_addrs =3D c->ip6.no_copy_addrs =3D true; > + copy_addrs_opt =3D true; > break; > case 20: > if (c->mode !=3D MODE_PASTA) > @@ -1465,23 +1467,26 @@ void conf(struct ctx *c, int argc, char **argv) > =20 > break; > case 'a': > - if (c->mode =3D=3D MODE_PASTA) > - c->no_copy_addrs =3D 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 =3D=3D MODE_PASTA) > + c->ip6.no_copy_addrs =3D true; > break; > + } > =20 > 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 =3D=3D MODE_PASTA) > + c->ip4.no_copy_addrs =3D true; > break; > + } > =20 > 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 =3D=3D MODE_PASTA) > - c->no_copy_routes =3D 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 =3D=3D MODE_PASTA) > + c->ip6.no_copy_routes =3D true; > break; > + } > =20 > 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 =3D=3D MODE_PASTA) > + c->ip4.no_copy_routes =3D true; > break; > + } > =20 > 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 interf= ace. > .BR \-\-no-copy-routes " " (DEPRECATED) > With \-\-config-net, do not copy all the routes associated to the interf= ace we > derive addresses and routes from: set up only the default gateway. Impli= ed by > --g, \-\-gateway. > +-g, \-\-gateway, for the corresponding IP version only. > =20 > Default is to copy all the routing entries from the interface in the out= er > namespace to the target namespace, translating the output interface attr= ibute to > @@ -604,7 +604,7 @@ below. > .BR \-\-no-copy-addrs " " (DEPRECATED) > With \-\-config-net, do not copy all the addresses associated to the int= erface > we derive addresses and routes from: set up a single one. Implied by \-a, > -\-\-address. > +\-\-address, for the corresponding IP version only. > =20 > Default is to copy all the addresses, except for link-local ones, from t= he > 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 namesp= ace > + * @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 { > =20 > struct in_addr addr_out; > char ifname_out[IFNAMSIZ]; > + > + bool no_copy_routes; > + bool no_copy_addrs; > }; > =20 > /** > @@ -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 namesp= ace > + * @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 { > =20 > struct in6_addr addr_out; > char ifname_out[IFNAMSIZ]; > + > + bool no_copy_routes; > + bool no_copy_addrs; > }; > =20 > #include > @@ -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 namesp= ace > - * @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; > =20 > 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); > =20 > if (c->ifi4) { > - if (c->no_copy_addrs) { > + if (c->ip4.no_copy_addrs) { > rc =3D 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)); > } > =20 > - if (c->no_copy_routes) { > + if (c->ip4.no_copy_routes) { > rc =3D 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) > } > =20 > if (c->ifi6) { > - if (c->no_copy_addrs) { > + if (c->ip6.no_copy_addrs) { > rc =3D 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)); > } > =20 > - if (c->no_copy_routes) { > + if (c->ip6.no_copy_routes) { > rc =3D nl_route_set_def(nl_sock_ns, c->pasta_ifi, > AF_INET6, &c->ip6.gw); > } else { --=20 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 --3S8ogFb2I0CJwpjM Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmayz2IACgkQzQJF27ox 2GeeDBAAje3Igbm6wqrVzJDDnHKBXvX/98qC/uAnwU+nu8o/v/8UNDkuZ2jJ0Nva ZI6OFyf2C7rycKn/t3e91ys7VRQPuweat8zO5SjBlW4OQcSVnvlqZ2QbWrbCjMWf IlGjBJB00jrUKePXe/am/xNEmcPL5EoDqJ/RHJQ4DBmoVhwZPxG2vB0DZys5mtGF z/pElX+MjGDvP9Bg+XHWkS+GHBYmTNQ3qGlHs35/wugkHafrHRboRkWCn5B2+3JC B1VniA2bEQJBsxors1xCZ+4ZcjMDoVkI/hDNIeMOqAns4HLj0OUYM1BMrsmD5t7K MToZyp7XdmgRSGVIILgLpuA6FH6mRG+n+S96WNhVvkLUCYqdp4KALlLJqZ8+mT2u Gn3QopxM4RI6xd13hUFkL6KgxZVldDkzTe1DI5Cr/ujIKsE5plV/suuPbnTkRyMH Rp42gB5BrtcpKvPO/SAlBNdvRl9yfMK4G1/BrrplEtlBDpyFXoPgraEXCe/jftbh 5J7IRqqhzwEsLui9SYdEiFUbSiuwCVUKa4S73zqjSZ5G8U3oFzWt59mXenpLjVIB oo3S6rCx272laGmuKMt+5X+iUnhv2iAA0YwLMHPI/1rk1AzxKCsy2lS/EhV+agiO QjGZ5j3C8/heKBM96Vc1BgJd4H9jXFbzN22EA3nC4U1aa++JUGY= =IcpV -----END PGP SIGNATURE----- --3S8ogFb2I0CJwpjM--