From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from gandalf.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id CC2435A0283 for ; Mon, 22 May 2023 10:54:02 +0200 (CEST) Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4QPrpb3kDRz4x4N; Mon, 22 May 2023 18:53:59 +1000 (AEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=201602; t=1684745639; bh=W9n/93L7cnRH1xArIE/RDfnclIGAeVWf79el0wykniM=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=SGKtsZM8HfU1pjmoetqWd8UHLHcwwFvhL3OWpErENXBWMBQHQ2SDXGHqiuhu6aHde rGlbDH/8sYAyQKsK8yDKI/tlWKdTRHfFkxoqlXubAhsWdoPzGcS1Ljbt/uGl3D+PFS HS9vFQtxML3BGw556TbaR4V+cawqno/uIS+VUSnU= Date: Mon, 22 May 2023 18:53:35 +1000 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v2 09/10] conf, pasta: With --config-net, copy all addresses by default Message-ID: References: <20230521234224.2770015-1-sbrivio@redhat.com> <20230521234224.2770015-10-sbrivio@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="cn2oTr3koxyaHSz/" Content-Disposition: inline In-Reply-To: <20230521234224.2770015-10-sbrivio@redhat.com> Message-ID-Hash: SFQGL6XHGSXVRVLWG2F6ZWCQ7ZCVSXL6 X-Message-ID-Hash: SFQGL6XHGSXVRVLWG2F6ZWCQ7ZCVSXL6 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, Callum Parsey , me@yawnt.com, lemmi@nerd2nerd.org, Andrea Arcangeli 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: --cn2oTr3koxyaHSz/ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, May 22, 2023 at 01:42:23AM +0200, Stefano Brivio wrote: > Use the newly-introduced NL_DUP mode for nl_addr() to copy all the > addresses associated to the template interface in the outer > namespace, unless --no-copy-addrs (also implied by -a) is given. Again, I'm always concerned by the addition of new command line options. Particularly in this case, where it looks like you can get the same behaviour by using the right -a option. > This is done mostly for consistency with routes. It might partially > cover the issue at: > https://bugs.passt.top/show_bug.cgi?id=3D47 > Support multiple addresses per address family >=20 > for some use cases, but not the originally intended one: we'll still > use a single outbound address (unless the routing table specifies > different preferred source addresses depending on the destination), > regardless of the address used in the target namespace. >=20 > Link: https://bugs.passt.top/show_bug.cgi?id=3D47 > Signed-off-by: Stefano Brivio Logic looks sound though, so Reviewed-by: David Gibson > --- > conf.c | 16 ++++++++++++++-- > passt.1 | 9 +++++++++ > passt.h | 2 ++ > pasta.c | 5 +++-- > 4 files changed, 28 insertions(+), 4 deletions(-) >=20 > diff --git a/conf.c b/conf.c > index 1ffd05c..e6c68e2 100644 > --- a/conf.c > +++ b/conf.c > @@ -901,6 +901,7 @@ pasta_opts: > info( " network namespace is deleted"); > info( " --config-net Configure tap interface in namespace"); > info( " --no-copy-routes Don't copy all routes to namespace"); > + info( " --no-copy-addrs Don't copy all addresses to namespace"); > info( " --ns-mac-addr ADDR Set MAC address on tap interface"); > =20 > exit(EXIT_FAILURE); > @@ -1177,6 +1178,7 @@ void conf(struct ctx *c, int argc, char **argv) > {"outbound-if6", required_argument, NULL, 16 }, > {"config-net", no_argument, NULL, 17 }, > {"no-copy-routes", no_argument, NULL, 18 }, > + {"no-copy-addrs", no_argument, NULL, 19 }, > { 0 }, > }; > struct get_bound_ports_ns_arg ns_ports_arg =3D { .c =3D c }; > @@ -1347,6 +1349,12 @@ void conf(struct ctx *c, int argc, char **argv) > =20 > c->no_copy_routes =3D 1; > break; > + case 19: > + if (c->mode !=3D MODE_PASTA) > + die("--no-copy-addrs is for pasta mode only"); > + > + c->no_copy_addrs =3D 1; > + break; > case 'd': > if (c->debug) > die("Multiple --debug options given"); > @@ -1632,8 +1640,12 @@ void conf(struct ctx *c, int argc, char **argv) > if (*c->sock_path && c->fd_tap >=3D 0) > die("Options --socket and --fd are mutually exclusive"); > =20 > - if (c->mode =3D=3D MODE_PASTA && c->no_copy_routes && !c->pasta_conf_ns) > - die("Option --no-copy-routes needs --config-net"); > + if (c->mode =3D=3D MODE_PASTA && !c->pasta_conf_ns) { > + if (c->no_copy_routes) > + die("Option --no-copy-routes needs --config-net"); > + if (c->no_copy_addrs) > + die("Option --no-copy-addrs needs --config-net"); > + } > =20 > if (!ifi4 && *c->ip4.ifname_out) > ifi4 =3D if_nametoindex(c->ip4.ifname_out); > diff --git a/passt.1 b/passt.1 > index f965c34..87b076d 100644 > --- a/passt.1 > +++ b/passt.1 > @@ -558,6 +558,15 @@ Default is to copy all the routing entries from the = interface in the outer > namespace to the target namespace, translating the output interface attr= ibute to > the outbound interface in the namespace. > =20 > +.TP > +.BR \-\-no-copy-addrs > +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. > + > +Default is to copy all the addresses, except for link-local ones, from t= he > +interface from the outer namespace to the target namespace. > + > .TP > .BR \-\-ns-mac-addr " " \fIaddr > Configure MAC address \fIaddr\fR on the tap interface in the namespace. > diff --git a/passt.h b/passt.h > index d314596..b51a1e5 100644 > --- a/passt.h > +++ b/passt.h > @@ -183,6 +183,7 @@ struct ip6_ctx { > * @pasta_ifn: 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 > @@ -242,6 +243,7 @@ struct ctx { > 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 99ef3fc..4054e9a 100644 > --- a/pasta.c > +++ b/pasta.c > @@ -274,11 +274,12 @@ void pasta_ns_conf(struct ctx *c) > =20 > if (c->pasta_conf_ns) { > enum nl_op op_routes =3D c->no_copy_routes ? NL_SET : NL_DUP; > + enum nl_op op_addrs =3D c->no_copy_addrs ? NL_SET : NL_DUP; > =20 > nl_link(1, c->pasta_ifi, c->mac_guest, 1, c->mtu); > =20 > if (c->ifi4) { > - nl_addr(NL_SET, c->ifi4, c->pasta_ifi, AF_INET, > + nl_addr(op_addrs, c->ifi4, c->pasta_ifi, AF_INET, > &c->ip4.addr, &c->ip4.prefix_len, NULL); > nl_route(op_routes, c->ifi4, c->pasta_ifi, AF_INET, > &c->ip4.gw); > @@ -286,7 +287,7 @@ void pasta_ns_conf(struct ctx *c) > =20 > if (c->ifi6) { > int prefix_len =3D 64; > - nl_addr(NL_SET, c->ifi6, c->pasta_ifi, AF_INET6, > + nl_addr(op_addrs, c->ifi6, c->pasta_ifi, AF_INET6, > &c->ip6.addr, &prefix_len, NULL); > nl_route(op_routes, c->ifi6, c->pasta_ifi, AF_INET6, > &c->ip6.gw); --=20 David Gibson | 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 --cn2oTr3koxyaHSz/ Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmRrLYgACgkQzQJF27ox 2Ge33g//SvEkeGuoHCRwczyKpJm9cL2h8vgutZ6qowax5lYc0GfwnVKk4Nt63ahD Hz6b/9CxBLRzRNBBv2xOgNqNwV5shyhd1jcAxDErbjJgQDtrmvZCTlNf0eXtTPj1 /wNJmsFG78aBkenTfEONQkbY6gznaSUzic82Fu2y4OmO7DILNhsQ17nA4QK4wbP/ iSanmv5qB9zRkWGRMQRHpZcxVaugsUeBwsPB+/TMOQ894PX2L8vsJ2M4spKG5RT0 SJzlMjFnWONXArKTx7ffNQwF2dMJzF7zi5BykpNKh43DYjXe8ggz5SlOjmryJwxb gGH6+oOZFjDC5dK/9RXDrOGIZ6Mkw5UhdJlFAttCy1HUwoQjh9nhI4IVC6VMK/3z HvlgYpTs1LGX31GIt0re1cs9lMfrA33t3WKv84j2dzTJ7S2l6/UUKf9p7aVLtyND zK4Djm1ibvrsX4Vfichpl+m7FYeGLMhlfwszOZJPDgmES2m/BWJhTcKselRdekQJ N15P8XHAQ8S9xtoATLGkOGwHz0WDYIwEpgUdp3Vn3Yvl8HfeajM4WT9pbjILGzR1 AmewO4lvUy36DxgU2xGSCpxiX5SXdXmTfxM/GlAK7/ZzH/qlOj/6yfSrQ7hg8J97 Q30YaDIlMlSOmkM2gXKfA13cZu3hCJykK7FIri7oMyc3sPjgyMc= =p+Hn -----END PGP SIGNATURE----- --cn2oTr3koxyaHSz/--