Re: [PATCH v2 05/10] conf, pasta: With --config-net, copy all routes by default

On Mon, 22 May 2023 18:44:51 +1000 David Gibson wrote:

On Mon, May 22, 2023 at 01:42:19AM +0200, Stefano Brivio wrote:

...

Use the newly-introduced NL_DUP mode for nl_route() to copy all the routes associated to the template interface in the outer namespace, unless --no-copy-routes (also implied by -g) is given.

Otherwise, we can't use default gateways which are not, address-wise, on the same subnet as the container, as reported by Callum.

Reported-by: Callum Parsey Link: https://github.com/containers/podman/issues/18539 Signed-off-by: Stefano Brivio

Reviewed-by: David Gibson

The logic looks sound, although I do have one concern noted below.

...

--- conf.c | 14 ++++++++++++++ passt.1 | 10 ++++++++++ passt.h | 4 +++- pasta.c | 6 ++++-- 4 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/conf.c b/conf.c index 3ee6ae0..7541261 100644 --- a/conf.c +++ b/conf.c @@ -923,6 +923,7 @@ pasta_opts: info( " --no-netns-quit Don't quit if filesystem-bound target"); info( " network namespace is deleted"); info( " --config-net Configure tap interface in namespace"); + info( " --no-copy-routes Don't copy all routes to namespace");

I'm always a bit nervous about adding new options, since it's something we then have to maintain compatibility for. Do we have a confirmed use case where the copy routes behaviour will cause trouble?

Not really, but I wanted to keep around the possibility of having the old behaviour, in case one wants to skip stuff like source routing or fallback routes with different metrics. Compatibility-wise it doesn't look like a huge burden (besides, I think these options could even be dropped at some point). Same as you noticed for 9/10: this could be obtained by passing one or two -g options, but it's not as "immediate" as "just give me one working default gateway". -- Stefano