Re: [PATCH] netlink: Fix selection of template interface

On Wed, Mar 20, 2024 at 12:34:54PM +0100, Paul Holzinger wrote:

On 20/03/2024 12:02, Stefano Brivio wrote:

...

On Wed, 20 Mar 2024 11:51:59 +0100 Paul Holzinger wrote:

[snip]

...

...

...

+ if (anyifi) { + if (nany == 1) + return anyifi; + + warn("Multiple interfaces with %s routes, use -i to select one", + af == AF_INET ? "IPv4" : "IPv6"); This should not be a warning, for me this always triggers because I have two interfaces with link local addresses and no global ipv6 route as I do not have any ipv6 connection. I was about to reply as I just applied this with s/warn/info/ here :) The more I think about this I think this suggestion is just wrong. I have a valid ipv4 connection and no ipv6, offering me to use -i is just wrong as it will not help me, especially if one uses a different interface for ipv4/ipv6 connections.

Yeah, I think you're right. We should ignore link-local routes when doin this scan. Both for IPv6 and IPv4, although IPv4 link-local is much rarer. I'll have a look at that tomorrow, in the meantime this is still better than what we had.

...

...

Or maybe the correct fix is to never consider ipv6 link local routes for this logic? At least I cannot see the purpose of using a interface with only a link local route.

...

+ } + + if (!nany) + warn("No interfaces with %s routes", af == AF_INET ? "IPv4" : "IPv6"); ...and here, because if one has no IPv6 routes we would reintroduce the issue we just fixed in 338b6321ac0d ("conf: No routable interface for IPv4 or IPv6 is informational, not a warning").

I think the purpose of picking interfaces based on routes for link-local destinations is for practical test setups like the one described in https://github.com/containers/podman/issues/21896.

Functionally it doesn't make sense, but it shouldn't harm either (right?).

I am not sure how the issue is related to link local addresses, for ipv6 a link local route must explicitly never be routed anywhere else besides the current interface so picking this seems incorrect as it will be unusable not matter what.

...

...

...

+ + return 0; } /** I reproduced by having a second interface and confirm this patch fixes it. Thanks for checking! Let's hope that was the case in the failing test.

-- 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